[PATCH 00/10] board/ti: k3 boards: Stop using findfdt
This is a wide cleanup to switch to setting fdtfile using env_set instead of scripted magic. 'fdtfile' is expected to be set by default. This allows the stdboot triggered efi loaders to find the correct OS device tree file.
This is a refresh of https://lore.kernel.org/all/86le9dwz4d.fsf@udb0321960.dhcp.ti.com/ which was the wrong approach.
Bootlogs: https://gist.github.com/nmenon/2f4a142c1bcaa09d544b1f2e206ea134
NOTE: There are a couple of checkpatch WARN (around LATE_INIT) and CHECK (fdt_ops #ifdeffery) that on closer inspection looks fine and consistent with other similar usage.
Based on next branch at: c2c598e87cfe Merge branch 'staging' of https://source.denx.de/u-boot/custodians/u-boot-tegra into next
Nishanth Menon (10): board: ti: common: Introduce a common fdt ops library board: ti: am62ax: Set fdtfile from C code instead of findfdt script board: ti: am62x: Set fdtfile from C code instead of findfdt script board: ti: am64x: Set fdtfile from C code instead of findfdt script board: ti: am65x: Set fdtfile from C code instead of findfdt script board: ti: j721e: Set fdtfile from C code instead of findfdt script board: ti: j721s2: Set fdtfile from C code instead of findfdt script board: beagle: beagleboneai64: Set fdtfile from C code instead of findfdt script board: beagle: beagleplay: Set fdtfile from C code instead of findfdt script include: env: ti: Drop default_findfdt
board/beagle/beagleboneai64/beagleboneai64.c | 14 ++++ .../beagle/beagleboneai64/beagleboneai64.env | 1 - board/beagle/beagleplay/beagleplay.c | 14 ++++ board/beagle/beagleplay/beagleplay.env | 1 - board/ti/am62ax/am62ax.env | 1 - board/ti/am62ax/evm.c | 10 +++ board/ti/am62x/am62x.env | 1 - board/ti/am62x/evm.c | 8 +++ board/ti/am64x/am64x.env | 9 --- board/ti/am64x/evm.c | 8 +++ board/ti/am65x/am65x.env | 3 - board/ti/am65x/evm.c | 2 + board/ti/common/Kconfig | 12 ++++ board/ti/common/Makefile | 1 + board/ti/common/fdt_ops.c | 65 +++++++++++++++++++ board/ti/common/fdt_ops.h | 41 ++++++++++++ board/ti/j721e/evm.c | 8 +++ board/ti/j721e/j721e.env | 10 --- board/ti/j721s2/evm.c | 8 +++ board/ti/j721s2/j721s2.env | 8 --- configs/am62ax_evm_a53_defconfig | 1 + configs/am62x_beagleplay_a53_defconfig | 3 +- configs/am62x_evm_a53_defconfig | 1 + configs/j721e_beagleboneai64_a72_defconfig | 3 +- include/env/ti/default_findfdt.env | 12 ---- 25 files changed, 197 insertions(+), 48 deletions(-) create mode 100644 board/ti/common/fdt_ops.c create mode 100644 board/ti/common/fdt_ops.h delete mode 100644 include/env/ti/default_findfdt.env
Introduce a common fdt operations library for basic device tree operations that are common between various boards.
The first library to introduce here is the capability to set up fdtfile as a standard variable as part of board identification rather than depend on scripted ifdeffery.
Signed-off-by: Nishanth Menon nm@ti.com --- board/ti/common/Kconfig | 12 ++++++++ board/ti/common/Makefile | 1 + board/ti/common/fdt_ops.c | 65 +++++++++++++++++++++++++++++++++++++++ board/ti/common/fdt_ops.h | 41 ++++++++++++++++++++++++ 4 files changed, 119 insertions(+) create mode 100644 board/ti/common/fdt_ops.c create mode 100644 board/ti/common/fdt_ops.h
diff --git a/board/ti/common/Kconfig b/board/ti/common/Kconfig index 49edd98014ab..06a8a36aa1cd 100644 --- a/board/ti/common/Kconfig +++ b/board/ti/common/Kconfig @@ -49,3 +49,15 @@ config TI_COMMON_CMD_OPTIONS imply CMD_SPI imply CMD_TIME imply CMD_USB if USB + +config TI_EVM_FDT_FOLDER_PATH + string "Location of Folder path where dtb is present" + default "ti/davinci" if ARCH_DAVINCI + default "ti/keystone" if ARCH_KEYSTONE + default "ti/omap" if ARCH_OMAP2PLUS + default "ti" if ARCH_K3 + depends on ARCH_DAVINCI || ARCH_KEYSTONE || ARCH_OMAP2PLUS || ARCH_K3 + help + Folder path for kernel device tree default. + This is used along with fdtfile path to locate the kernel + device tree blob. diff --git a/board/ti/common/Makefile b/board/ti/common/Makefile index 26bf12e2e6d5..5ac361ba7fcf 100644 --- a/board/ti/common/Makefile +++ b/board/ti/common/Makefile @@ -3,3 +3,4 @@
obj-${CONFIG_TI_I2C_BOARD_DETECT} += board_detect.o obj-${CONFIG_CMD_EXTENSION} += cape_detect.o +obj-${CONFIG_OF_LIBFDT} += fdt_ops.o diff --git a/board/ti/common/fdt_ops.c b/board/ti/common/fdt_ops.c new file mode 100644 index 000000000000..f8770cae4a54 --- /dev/null +++ b/board/ti/common/fdt_ops.c @@ -0,0 +1,65 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Library to support FDT file operations which are common + * + * Copyright (C) 2024 Texas Instruments Incorporated - https://www.ti.com/ + */ + +#include <env.h> +#include <vsprintf.h> +#include "fdt_ops.h" + +void ti_set_fdt_env(const char *name_fdt, struct ti_fdt_map *fdt_map) +{ + char *fdt_file_name = NULL; + char fdtfile[TI_FDT_FILE_MAX]; + + if (name_fdt) { + while (fdt_map) { + /* Check for NULL terminator in the list */ + if (!fdt_map->name_fdt) + break; + if (!strncmp(fdt_map->name_fdt, name_fdt, TI_NAME_FDT_MAX)) { + fdt_file_name = fdt_map->fdt_file_name; + break; + } + fdt_map++; + } + } + + /* match not found OR null name_fdt */ + if (!fdt_file_name) { + /* + * Prioritize CONFIG_DEFAULT_FDT_FILE - if that is not defined, + * or is empty, then use CONFIG_DEFAULT_DEVICE_TREE + */ +#ifdef CONFIG_DEFAULT_FDT_FILE + if (strlen(CONFIG_DEFAULT_FDT_FILE)) { + snprintf(fdtfile, sizeof(fdtfile), "%s/%s", + CONFIG_TI_EVM_FDT_FOLDER_PATH, CONFIG_DEFAULT_FDT_FILE); + } else +#endif + { + snprintf(fdtfile, sizeof(fdtfile), "%s/%s.dtb", + CONFIG_TI_EVM_FDT_FOLDER_PATH, + CONFIG_DEFAULT_DEVICE_TREE); + } + } else { + snprintf(fdtfile, sizeof(fdtfile), "%s/%s", CONFIG_TI_EVM_FDT_FOLDER_PATH, + fdt_file_name); + } + + env_set("fdtfile", fdtfile); + + /* + * XXX: DEPRECATION WARNING: 2 u-boot versions. + * + * Maintain compatibility with downstream scripts that may be using + * name_fdt + */ + if (name_fdt) + env_set("name_fdt", name_fdt); + /* Also set the findfdt legacy script to warn users to stop using this */ + env_set("findfdt", + "echo WARN: fdtfile already set. Stop using findfdt in script"); +} diff --git a/board/ti/common/fdt_ops.h b/board/ti/common/fdt_ops.h new file mode 100644 index 000000000000..c01697bed28f --- /dev/null +++ b/board/ti/common/fdt_ops.h @@ -0,0 +1,41 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Library to support common device tree manipulation for TI EVMs + * + * Copyright (C) 2024 Texas Instruments Incorporated - https://www.ti.com + */ + +#ifndef __FDT_OPS_H +#define __FDT_OPS_H + +#define TI_NAME_FDT_MAX 20 +#define TI_FDT_FILE_MAX 200 + +/** + * struct ti_fdt_map - mapping of device tree blob name to board name + * @name_fdt: board_name up to TI_NAME_FDT_MAX long + * @fdt_file_name: device tree blob name as described by kernel + */ +struct ti_fdt_map { + const char *name_fdt; + char *fdt_file_name; +}; + +/** + * ti_set_fdt_env - Find the correct device tree file name and set 'fdtfile' + * env variable with correct folder structure appropriate to the architecture + * and kernel conventions. This function is invoked typically as part of + * board_late_init + * + * fdt name is picked by: + * a) If a match is found, use the match + * b) If not, CONFIG_DEFAULT_FDT_FILE (Boot OS device tree) if that is defined + * and not null + * c) If not, Use CONFIG_DEFAULT_DEVICE_TREE (DT control for bootloader) + * + * @name_fdt: match to search with (max of TI_NAME_FDT_MAX chars) + * @fdt_map: NULL terminated array of device tree file name matches. + */ +void ti_set_fdt_env(const char *name_fdt, struct ti_fdt_map *fdt_map); + +#endif /* __FDT_OPS_H */
On 1/8/24 11:32 AM, Nishanth Menon wrote:
Introduce a common fdt operations library for basic device tree operations that are common between various boards.
The first library to introduce here is the capability to set up fdtfile as a standard variable as part of board identification rather than depend on scripted ifdeffery.
Signed-off-by: Nishanth Menon nm@ti.com
board/ti/common/Kconfig | 12 ++++++++ board/ti/common/Makefile | 1 + board/ti/common/fdt_ops.c | 65 +++++++++++++++++++++++++++++++++++++++ board/ti/common/fdt_ops.h | 41 ++++++++++++++++++++++++ 4 files changed, 119 insertions(+) create mode 100644 board/ti/common/fdt_ops.c create mode 100644 board/ti/common/fdt_ops.h
diff --git a/board/ti/common/Kconfig b/board/ti/common/Kconfig index 49edd98014ab..06a8a36aa1cd 100644 --- a/board/ti/common/Kconfig +++ b/board/ti/common/Kconfig @@ -49,3 +49,15 @@ config TI_COMMON_CMD_OPTIONS imply CMD_SPI imply CMD_TIME imply CMD_USB if USB
+config TI_EVM_FDT_FOLDER_PATH
- string "Location of Folder path where dtb is present"
- default "ti/davinci" if ARCH_DAVINCI
- default "ti/keystone" if ARCH_KEYSTONE
- default "ti/omap" if ARCH_OMAP2PLUS
- default "ti" if ARCH_K3
- depends on ARCH_DAVINCI || ARCH_KEYSTONE || ARCH_OMAP2PLUS || ARCH_K3
- help
Folder path for kernel device tree default.This is used along with fdtfile path to locate the kerneldevice tree blob.diff --git a/board/ti/common/Makefile b/board/ti/common/Makefile index 26bf12e2e6d5..5ac361ba7fcf 100644 --- a/board/ti/common/Makefile +++ b/board/ti/common/Makefile @@ -3,3 +3,4 @@
obj-${CONFIG_TI_I2C_BOARD_DETECT} += board_detect.o obj-${CONFIG_CMD_EXTENSION} += cape_detect.o +obj-${CONFIG_OF_LIBFDT} += fdt_ops.o diff --git a/board/ti/common/fdt_ops.c b/board/ti/common/fdt_ops.c new file mode 100644 index 000000000000..f8770cae4a54 --- /dev/null +++ b/board/ti/common/fdt_ops.c @@ -0,0 +1,65 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/*
- Library to support FDT file operations which are common
- Copyright (C) 2024 Texas Instruments Incorporated - https://www.ti.com/
- */
+#include <env.h> +#include <vsprintf.h> +#include "fdt_ops.h"
+void ti_set_fdt_env(const char *name_fdt, struct ti_fdt_map *fdt_map) +{
- char *fdt_file_name = NULL;
- char fdtfile[TI_FDT_FILE_MAX];
- if (name_fdt) {
while (fdt_map) {/* Check for NULL terminator in the list */if (!fdt_map->name_fdt)break;if (!strncmp(fdt_map->name_fdt, name_fdt, TI_NAME_FDT_MAX)) {fdt_file_name = fdt_map->fdt_file_name;break;}fdt_map++;}- }
- /* match not found OR null name_fdt */
- if (!fdt_file_name) {
/** Prioritize CONFIG_DEFAULT_FDT_FILE - if that is not defined,* or is empty, then use CONFIG_DEFAULT_DEVICE_TREE*/+#ifdef CONFIG_DEFAULT_FDT_FILE
if (strlen(CONFIG_DEFAULT_FDT_FILE)) {snprintf(fdtfile, sizeof(fdtfile), "%s/%s",CONFIG_TI_EVM_FDT_FOLDER_PATH, CONFIG_DEFAULT_FDT_FILE);} else+#endif
{snprintf(fdtfile, sizeof(fdtfile), "%s/%s.dtb",CONFIG_TI_EVM_FDT_FOLDER_PATH,CONFIG_DEFAULT_DEVICE_TREE);}- } else {
snprintf(fdtfile, sizeof(fdtfile), "%s/%s", CONFIG_TI_EVM_FDT_FOLDER_PATH,fdt_file_name);- }
- env_set("fdtfile", fdtfile);
- /*
* XXX: DEPRECATION WARNING: 2 u-boot versions.** Maintain compatibility with downstream scripts that may be using* name_fdt*/- if (name_fdt)
env_set("name_fdt", name_fdt);
"name_fdt" should match "fdtfile", you should have just:
env_set("name_fdt", fdtfile);
are you mixing this up with "board_name"?
Andrew
- /* Also set the findfdt legacy script to warn users to stop using this */
- env_set("findfdt",
"echo WARN: fdtfile already set. Stop using findfdt in script");+} diff --git a/board/ti/common/fdt_ops.h b/board/ti/common/fdt_ops.h new file mode 100644 index 000000000000..c01697bed28f --- /dev/null +++ b/board/ti/common/fdt_ops.h @@ -0,0 +1,41 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/*
- Library to support common device tree manipulation for TI EVMs
- Copyright (C) 2024 Texas Instruments Incorporated - https://www.ti.com
- */
+#ifndef __FDT_OPS_H +#define __FDT_OPS_H
+#define TI_NAME_FDT_MAX 20 +#define TI_FDT_FILE_MAX 200
+/**
- struct ti_fdt_map - mapping of device tree blob name to board name
- @name_fdt: board_name up to TI_NAME_FDT_MAX long
- @fdt_file_name: device tree blob name as described by kernel
- */
+struct ti_fdt_map {
- const char *name_fdt;
- char *fdt_file_name;
+};
+/**
- ti_set_fdt_env - Find the correct device tree file name and set 'fdtfile'
- env variable with correct folder structure appropriate to the architecture
- and kernel conventions. This function is invoked typically as part of
- board_late_init
- fdt name is picked by:
- a) If a match is found, use the match
- b) If not, CONFIG_DEFAULT_FDT_FILE (Boot OS device tree) if that is defined
- and not null
- c) If not, Use CONFIG_DEFAULT_DEVICE_TREE (DT control for bootloader)
- @name_fdt: match to search with (max of TI_NAME_FDT_MAX chars)
- @fdt_map: NULL terminated array of device tree file name matches.
- */
+void ti_set_fdt_env(const char *name_fdt, struct ti_fdt_map *fdt_map);
+#endif /* __FDT_OPS_H */
On 12:50-20240108, Andrew Davis wrote:
On 1/8/24 11:32 AM, Nishanth Menon wrote:
Introduce a common fdt operations library for basic device tree operations that are common between various boards.
The first library to introduce here is the capability to set up fdtfile as a standard variable as part of board identification rather than depend on scripted ifdeffery.
Signed-off-by: Nishanth Menon nm@ti.com
board/ti/common/Kconfig | 12 ++++++++ board/ti/common/Makefile | 1 + board/ti/common/fdt_ops.c | 65 +++++++++++++++++++++++++++++++++++++++ board/ti/common/fdt_ops.h | 41 ++++++++++++++++++++++++ 4 files changed, 119 insertions(+) create mode 100644 board/ti/common/fdt_ops.c create mode 100644 board/ti/common/fdt_ops.h
diff --git a/board/ti/common/Kconfig b/board/ti/common/Kconfig index 49edd98014ab..06a8a36aa1cd 100644 --- a/board/ti/common/Kconfig +++ b/board/ti/common/Kconfig @@ -49,3 +49,15 @@ config TI_COMMON_CMD_OPTIONS imply CMD_SPI imply CMD_TIME imply CMD_USB if USB
+config TI_EVM_FDT_FOLDER_PATH
- string "Location of Folder path where dtb is present"
- default "ti/davinci" if ARCH_DAVINCI
- default "ti/keystone" if ARCH_KEYSTONE
- default "ti/omap" if ARCH_OMAP2PLUS
- default "ti" if ARCH_K3
- depends on ARCH_DAVINCI || ARCH_KEYSTONE || ARCH_OMAP2PLUS || ARCH_K3
- help
Folder path for kernel device tree default.This is used along with fdtfile path to locate the kerneldevice tree blob.diff --git a/board/ti/common/Makefile b/board/ti/common/Makefile index 26bf12e2e6d5..5ac361ba7fcf 100644 --- a/board/ti/common/Makefile +++ b/board/ti/common/Makefile @@ -3,3 +3,4 @@ obj-${CONFIG_TI_I2C_BOARD_DETECT} += board_detect.o obj-${CONFIG_CMD_EXTENSION} += cape_detect.o +obj-${CONFIG_OF_LIBFDT} += fdt_ops.o diff --git a/board/ti/common/fdt_ops.c b/board/ti/common/fdt_ops.c new file mode 100644 index 000000000000..f8770cae4a54 --- /dev/null +++ b/board/ti/common/fdt_ops.c @@ -0,0 +1,65 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/*
- Library to support FDT file operations which are common
- Copyright (C) 2024 Texas Instruments Incorporated - https://www.ti.com/
- */
+#include <env.h> +#include <vsprintf.h> +#include "fdt_ops.h"
+void ti_set_fdt_env(const char *name_fdt, struct ti_fdt_map *fdt_map) +{
- char *fdt_file_name = NULL;
- char fdtfile[TI_FDT_FILE_MAX];
- if (name_fdt) {
while (fdt_map) {/* Check for NULL terminator in the list */if (!fdt_map->name_fdt)break;if (!strncmp(fdt_map->name_fdt, name_fdt, TI_NAME_FDT_MAX)) {fdt_file_name = fdt_map->fdt_file_name;break;}fdt_map++;}- }
- /* match not found OR null name_fdt */
- if (!fdt_file_name) {
/** Prioritize CONFIG_DEFAULT_FDT_FILE - if that is not defined,* or is empty, then use CONFIG_DEFAULT_DEVICE_TREE*/+#ifdef CONFIG_DEFAULT_FDT_FILE
if (strlen(CONFIG_DEFAULT_FDT_FILE)) {snprintf(fdtfile, sizeof(fdtfile), "%s/%s",CONFIG_TI_EVM_FDT_FOLDER_PATH, CONFIG_DEFAULT_FDT_FILE);} else+#endif
{snprintf(fdtfile, sizeof(fdtfile), "%s/%s.dtb",CONFIG_TI_EVM_FDT_FOLDER_PATH,CONFIG_DEFAULT_DEVICE_TREE);}- } else {
snprintf(fdtfile, sizeof(fdtfile), "%s/%s", CONFIG_TI_EVM_FDT_FOLDER_PATH,fdt_file_name);- }
- env_set("fdtfile", fdtfile);
- /*
* XXX: DEPRECATION WARNING: 2 u-boot versions.** Maintain compatibility with downstream scripts that may be using* name_fdt*/- if (name_fdt)
env_set("name_fdt", name_fdt);"name_fdt" should match "fdtfile", you should have just:
env_set("name_fdt", fdtfile);
are you mixing this up with "board_name"?
It should have been fdtfile. Thanks for catching it.
Nishanth Menon nm@ti.com writes:
Introduce a common fdt operations library for basic device tree operations that are common between various boards.
The first library to introduce here is the capability to set up fdtfile as a standard variable as part of board identification rather than depend on scripted ifdeffery.
Signed-off-by: Nishanth Menon nm@ti.com
board/ti/common/Kconfig | 12 ++++++++ board/ti/common/Makefile | 1 + board/ti/common/fdt_ops.c | 65 +++++++++++++++++++++++++++++++++++++++ board/ti/common/fdt_ops.h | 41 ++++++++++++++++++++++++ 4 files changed, 119 insertions(+) create mode 100644 board/ti/common/fdt_ops.c create mode 100644 board/ti/common/fdt_ops.h
diff --git a/board/ti/common/Kconfig b/board/ti/common/Kconfig index 49edd98014ab..06a8a36aa1cd 100644 --- a/board/ti/common/Kconfig +++ b/board/ti/common/Kconfig @@ -49,3 +49,15 @@ config TI_COMMON_CMD_OPTIONS imply CMD_SPI imply CMD_TIME imply CMD_USB if USB
+config TI_EVM_FDT_FOLDER_PATH
- string "Location of Folder path where dtb is present"
- default "ti/davinci" if ARCH_DAVINCI
- default "ti/keystone" if ARCH_KEYSTONE
- default "ti/omap" if ARCH_OMAP2PLUS
- default "ti" if ARCH_K3
- depends on ARCH_DAVINCI || ARCH_KEYSTONE || ARCH_OMAP2PLUS || ARCH_K3
- help
Folder path for kernel device tree default.This is used along with fdtfile path to locate the kerneldevice tree blob.
It's not clear to me why we need the flexibility of specifying a FDT filename per board independently of the FDT folder path. Why can't the path be part of the fdt_map?
diff --git a/board/ti/common/Makefile b/board/ti/common/Makefile index 26bf12e2e6d5..5ac361ba7fcf 100644 --- a/board/ti/common/Makefile +++ b/board/ti/common/Makefile @@ -3,3 +3,4 @@
obj-${CONFIG_TI_I2C_BOARD_DETECT} += board_detect.o obj-${CONFIG_CMD_EXTENSION} += cape_detect.o +obj-${CONFIG_OF_LIBFDT} += fdt_ops.o diff --git a/board/ti/common/fdt_ops.c b/board/ti/common/fdt_ops.c new file mode 100644 index 000000000000..f8770cae4a54 --- /dev/null +++ b/board/ti/common/fdt_ops.c @@ -0,0 +1,65 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/*
- Library to support FDT file operations which are common
- Copyright (C) 2024 Texas Instruments Incorporated - https://www.ti.com/
- */
+#include <env.h> +#include <vsprintf.h> +#include "fdt_ops.h"
+void ti_set_fdt_env(const char *name_fdt, struct ti_fdt_map *fdt_map)
This function takes a board name and sets the FDT name, so why isn't the first parameter called 'board_name' or similar?
+{
- char *fdt_file_name = NULL;
- char fdtfile[TI_FDT_FILE_MAX];
- if (name_fdt) {
while (fdt_map) {/* Check for NULL terminator in the list */if (!fdt_map->name_fdt)break;if (!strncmp(fdt_map->name_fdt, name_fdt, TI_NAME_FDT_MAX)) {
Why do we need a max length? Shouldn't strcmp() be fine given the name_fdt member of the fdt_map is set in code (ie, not read in)?
fdt_file_name = fdt_map->fdt_file_name;break;}fdt_map++;}- }
- /* match not found OR null name_fdt */
- if (!fdt_file_name) {
/** Prioritize CONFIG_DEFAULT_FDT_FILE - if that is not defined,* or is empty, then use CONFIG_DEFAULT_DEVICE_TREE*/+#ifdef CONFIG_DEFAULT_FDT_FILE
if (strlen(CONFIG_DEFAULT_FDT_FILE)) {snprintf(fdtfile, sizeof(fdtfile), "%s/%s",CONFIG_TI_EVM_FDT_FOLDER_PATH, CONFIG_DEFAULT_FDT_FILE);
I do not see where any TI platforms set CONFIG_DEFAULT_FDT_FILE, so why have logic that checks for it? We don't use it. With this patch (fdt_map) I don't see why we would start needing it in the future.
} else+#endif
{snprintf(fdtfile, sizeof(fdtfile), "%s/%s.dtb",CONFIG_TI_EVM_FDT_FOLDER_PATH,CONFIG_DEFAULT_DEVICE_TREE);
If fdtfile isn't set, EFI bootmeth falls back to the control DT anyway, so this is unnecessary duplication of logic.
}- } else {
snprintf(fdtfile, sizeof(fdtfile), "%s/%s", CONFIG_TI_EVM_FDT_FOLDER_PATH,fdt_file_name);- }
- env_set("fdtfile", fdtfile);
- /*
* XXX: DEPRECATION WARNING: 2 u-boot versions.** Maintain compatibility with downstream scripts that may be using* name_fdt*/- if (name_fdt)
env_set("name_fdt", name_fdt);- /* Also set the findfdt legacy script to warn users to stop using this */
- env_set("findfdt",
"echo WARN: fdtfile already set. Stop using findfdt in script");+} diff --git a/board/ti/common/fdt_ops.h b/board/ti/common/fdt_ops.h new file mode 100644 index 000000000000..c01697bed28f --- /dev/null +++ b/board/ti/common/fdt_ops.h @@ -0,0 +1,41 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/*
- Library to support common device tree manipulation for TI EVMs
- Copyright (C) 2024 Texas Instruments Incorporated - https://www.ti.com
- */
+#ifndef __FDT_OPS_H +#define __FDT_OPS_H
+#define TI_NAME_FDT_MAX 20
TI_BOARD_NAME_MAX??
+#define TI_FDT_FILE_MAX 200
+/**
- struct ti_fdt_map - mapping of device tree blob name to board name
- @name_fdt: board_name up to TI_NAME_FDT_MAX long
If this is the board_name, why call it name_fdt? Why not board_name?
- @fdt_file_name: device tree blob name as described by kernel
- */
+struct ti_fdt_map {
- const char *name_fdt;
- char *fdt_file_name;
+};
+/**
- ti_set_fdt_env - Find the correct device tree file name and set 'fdtfile'
"Find the correct device tree file name based on the board name and "...
- env variable with correct folder structure appropriate to the architecture
- and kernel conventions. This function is invoked typically as part of
- board_late_init
- fdt name is picked by:
- a) If a match is found, use the match
"a) If a board name match is found, use the match"
- b) If not, CONFIG_DEFAULT_FDT_FILE (Boot OS device tree) if that is defined
- and not null
- c) If not, Use CONFIG_DEFAULT_DEVICE_TREE (DT control for bootloader)
- @name_fdt: match to search with (max of TI_NAME_FDT_MAX chars)
- @fdt_map: NULL terminated array of device tree file name matches.
- */
+void ti_set_fdt_env(const char *name_fdt, struct ti_fdt_map *fdt_map);
+#endif /* __FDT_OPS_H */
2.43.0
On 20:20-20240108, Jon Humphreys wrote: [...]
+config TI_EVM_FDT_FOLDER_PATH
- string "Location of Folder path where dtb is present"
- default "ti/davinci" if ARCH_DAVINCI
- default "ti/keystone" if ARCH_KEYSTONE
- default "ti/omap" if ARCH_OMAP2PLUS
- default "ti" if ARCH_K3
- depends on ARCH_DAVINCI || ARCH_KEYSTONE || ARCH_OMAP2PLUS || ARCH_K3
- help
Folder path for kernel device tree default.This is used along with fdtfile path to locate the kerneldevice tree blob.It's not clear to me why we need the flexibility of specifying a FDT filename per board independently of the FDT folder path. Why can't the path be part of the fdt_map?
Because of https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
This does not happen too often, but the folder paths are irritating and impact multiple platforms at the same time.
[...]
- Copyright (C) 2024 Texas Instruments Incorporated - https://www.ti.com/
- */
+#include <env.h> +#include <vsprintf.h> +#include "fdt_ops.h"
+void ti_set_fdt_env(const char *name_fdt, struct ti_fdt_map *fdt_map)
This function takes a board name and sets the FDT name, so why isn't the first parameter called 'board_name' or similar?
Fair enough - board_name is more appropriate.
+{
- char *fdt_file_name = NULL;
- char fdtfile[TI_FDT_FILE_MAX];
- if (name_fdt) {
while (fdt_map) {/* Check for NULL terminator in the list */if (!fdt_map->name_fdt)break;if (!strncmp(fdt_map->name_fdt, name_fdt, TI_NAME_FDT_MAX)) {Why do we need a max length? Shouldn't strcmp() be fine given the name_fdt member of the fdt_map is set in code (ie, not read in)?
prefer strncmp to strcmp where possible. it isn't a matter of set in code or not, it is a possibility of making a mistake - hence the constant sized string.
fdt_file_name = fdt_map->fdt_file_name;break;}fdt_map++;}- }
- /* match not found OR null name_fdt */
- if (!fdt_file_name) {
/** Prioritize CONFIG_DEFAULT_FDT_FILE - if that is not defined,* or is empty, then use CONFIG_DEFAULT_DEVICE_TREE*/+#ifdef CONFIG_DEFAULT_FDT_FILE
if (strlen(CONFIG_DEFAULT_FDT_FILE)) {snprintf(fdtfile, sizeof(fdtfile), "%s/%s",CONFIG_TI_EVM_FDT_FOLDER_PATH, CONFIG_DEFAULT_FDT_FILE);I do not see where any TI platforms set CONFIG_DEFAULT_FDT_FILE, so why have logic that checks for it? We don't use it. With this patch (fdt_map) I don't see why we would start needing it in the future.
You don't need to explicitly set since it is already set by default - check the .config.
} else+#endif
{snprintf(fdtfile, sizeof(fdtfile), "%s/%s.dtb",CONFIG_TI_EVM_FDT_FOLDER_PATH,CONFIG_DEFAULT_DEVICE_TREE);If fdtfile isn't set, EFI bootmeth falls back to the control DT anyway, so this is unnecessary duplication of logic.
Wrong translation of what is going on here - it is assuming the fdtfile _name_ is the same as the board file used for u-boot not the control DT content(which is the fall back for bootmeth). It is possible that the content is the same as what U-Boot is using, but the file name is what is being selected here.
[...]
+#define TI_NAME_FDT_MAX 20
TI_BOARD_NAME_MAX??
OK - will fix.
+#define TI_FDT_FILE_MAX 200
+/**
- struct ti_fdt_map - mapping of device tree blob name to board name
- @name_fdt: board_name up to TI_NAME_FDT_MAX long
If this is the board_name, why call it name_fdt? Why not board_name?
will replace with board_name
- @fdt_file_name: device tree blob name as described by kernel
- */
+struct ti_fdt_map {
- const char *name_fdt;
- char *fdt_file_name;
+};
+/**
- ti_set_fdt_env - Find the correct device tree file name and set 'fdtfile'
"Find the correct device tree file name based on the board name and "...
OK.
- env variable with correct folder structure appropriate to the architecture
- and kernel conventions. This function is invoked typically as part of
- board_late_init
- fdt name is picked by:
- a) If a match is found, use the match
"a) If a board name match is found, use the match"
OK.
[...]
On 08/01/2024 19:32, Nishanth Menon wrote:
Introduce a common fdt operations library for basic device tree operations that are common between various boards.
The first library to introduce here is the capability to set up fdtfile as a standard variable as part of board identification rather than depend on scripted ifdeffery.
Signed-off-by: Nishanth Menon nm@ti.com
board/ti/common/Kconfig | 12 ++++++++ board/ti/common/Makefile | 1 + board/ti/common/fdt_ops.c | 65 +++++++++++++++++++++++++++++++++++++++ board/ti/common/fdt_ops.h | 41 ++++++++++++++++++++++++ 4 files changed, 119 insertions(+) create mode 100644 board/ti/common/fdt_ops.c create mode 100644 board/ti/common/fdt_ops.h
diff --git a/board/ti/common/Kconfig b/board/ti/common/Kconfig index 49edd98014ab..06a8a36aa1cd 100644 --- a/board/ti/common/Kconfig +++ b/board/ti/common/Kconfig @@ -49,3 +49,15 @@ config TI_COMMON_CMD_OPTIONS imply CMD_SPI imply CMD_TIME imply CMD_USB if USB
+config TI_EVM_FDT_FOLDER_PATH
- string "Location of Folder path where dtb is present"
- default "ti/davinci" if ARCH_DAVINCI
- default "ti/keystone" if ARCH_KEYSTONE
- default "ti/omap" if ARCH_OMAP2PLUS
- default "ti" if ARCH_K3
- depends on ARCH_DAVINCI || ARCH_KEYSTONE || ARCH_OMAP2PLUS || ARCH_K3
- help
Folder path for kernel device tree default.This is used along with fdtfile path to locate the kerneldevice tree blob.diff --git a/board/ti/common/Makefile b/board/ti/common/Makefile index 26bf12e2e6d5..5ac361ba7fcf 100644 --- a/board/ti/common/Makefile +++ b/board/ti/common/Makefile @@ -3,3 +3,4 @@
obj-${CONFIG_TI_I2C_BOARD_DETECT} += board_detect.o obj-${CONFIG_CMD_EXTENSION} += cape_detect.o +obj-${CONFIG_OF_LIBFDT} += fdt_ops.o diff --git a/board/ti/common/fdt_ops.c b/board/ti/common/fdt_ops.c new file mode 100644 index 000000000000..f8770cae4a54 --- /dev/null +++ b/board/ti/common/fdt_ops.c @@ -0,0 +1,65 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/*
- Library to support FDT file operations which are common
- Copyright (C) 2024 Texas Instruments Incorporated - https://www.ti.com/
- */
+#include <env.h> +#include <vsprintf.h> +#include "fdt_ops.h"
+void ti_set_fdt_env(const char *name_fdt, struct ti_fdt_map *fdt_map) +{
- char *fdt_file_name = NULL;
- char fdtfile[TI_FDT_FILE_MAX];
- if (name_fdt) {
while (fdt_map) {/* Check for NULL terminator in the list */if (!fdt_map->name_fdt)break;if (!strncmp(fdt_map->name_fdt, name_fdt, TI_NAME_FDT_MAX)) {fdt_file_name = fdt_map->fdt_file_name;break;}fdt_map++;}- }
- /* match not found OR null name_fdt */
- if (!fdt_file_name) {
/** Prioritize CONFIG_DEFAULT_FDT_FILE - if that is not defined,* or is empty, then use CONFIG_DEFAULT_DEVICE_TREE*/+#ifdef CONFIG_DEFAULT_FDT_FILE
if (strlen(CONFIG_DEFAULT_FDT_FILE)) {snprintf(fdtfile, sizeof(fdtfile), "%s/%s",CONFIG_TI_EVM_FDT_FOLDER_PATH, CONFIG_DEFAULT_FDT_FILE);} else+#endif
{snprintf(fdtfile, sizeof(fdtfile), "%s/%s.dtb",CONFIG_TI_EVM_FDT_FOLDER_PATH,CONFIG_DEFAULT_DEVICE_TREE);}- } else {
snprintf(fdtfile, sizeof(fdtfile), "%s/%s", CONFIG_TI_EVM_FDT_FOLDER_PATH,fdt_file_name);- }
- env_set("fdtfile", fdtfile);
- /*
* XXX: DEPRECATION WARNING: 2 u-boot versions.** Maintain compatibility with downstream scripts that may be using* name_fdt*/- if (name_fdt)
env_set("name_fdt", name_fdt);- /* Also set the findfdt legacy script to warn users to stop using this */
- env_set("findfdt",
"echo WARN: fdtfile already set. Stop using findfdt in script");+} diff --git a/board/ti/common/fdt_ops.h b/board/ti/common/fdt_ops.h new file mode 100644 index 000000000000..c01697bed28f --- /dev/null +++ b/board/ti/common/fdt_ops.h @@ -0,0 +1,41 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/*
- Library to support common device tree manipulation for TI EVMs
- Copyright (C) 2024 Texas Instruments Incorporated - https://www.ti.com
- */
+#ifndef __FDT_OPS_H +#define __FDT_OPS_H
+#define TI_NAME_FDT_MAX 20 +#define TI_FDT_FILE_MAX 200
+/**
- struct ti_fdt_map - mapping of device tree blob name to board name
- @name_fdt: board_name up to TI_NAME_FDT_MAX long
- @fdt_file_name: device tree blob name as described by kernel
- */
+struct ti_fdt_map {
- const char *name_fdt;
Can we call this board_name? as name_fdt corresponds to device tree name.
- char *fdt_file_name;
+};
+/**
- ti_set_fdt_env - Find the correct device tree file name and set 'fdtfile'
- env variable with correct folder structure appropriate to the architecture
- and kernel conventions. This function is invoked typically as part of
- board_late_init
- fdt name is picked by:
- a) If a match is found, use the match
- b) If not, CONFIG_DEFAULT_FDT_FILE (Boot OS device tree) if that is defined
- and not null
- c) If not, Use CONFIG_DEFAULT_DEVICE_TREE (DT control for bootloader)
- @name_fdt: match to search with (max of TI_NAME_FDT_MAX chars)
- @fdt_map: NULL terminated array of device tree file name matches.
- */
+void ti_set_fdt_env(const char *name_fdt, struct ti_fdt_map *fdt_map);
+#endif /* __FDT_OPS_H */
On 15:08-20240109, Roger Quadros wrote: [..]
+/**
- struct ti_fdt_map - mapping of device tree blob name to board name
- @name_fdt: board_name up to TI_NAME_FDT_MAX long
- @fdt_file_name: device tree blob name as described by kernel
- */
+struct ti_fdt_map {
- const char *name_fdt;
Can we call this board_name? as name_fdt corresponds to device tree name.
Done.
Stop using the findfdt script and switch to setting the fdtfile from C code.
While at this, replace findfdt in environment with a warning as it is no longer needed
Signed-off-by: Nishanth Menon nm@ti.com --- board/ti/am62ax/am62ax.env | 1 - board/ti/am62ax/evm.c | 10 ++++++++++ configs/am62ax_evm_a53_defconfig | 1 + 3 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/board/ti/am62ax/am62ax.env b/board/ti/am62ax/am62ax.env index a6d967e982d4..334374abb73e 100644 --- a/board/ti/am62ax/am62ax.env +++ b/board/ti/am62ax/am62ax.env @@ -1,5 +1,4 @@ #include <env/ti/ti_common.env> -#include <env/ti/default_findfdt.env> #include <env/ti/mmc.env>
name_kern=Image diff --git a/board/ti/am62ax/evm.c b/board/ti/am62ax/evm.c index cd3360a43029..62d3664936e7 100644 --- a/board/ti/am62ax/evm.c +++ b/board/ti/am62ax/evm.c @@ -13,6 +13,8 @@ #include <fdt_support.h> #include <spl.h>
+#include "../common/fdt_ops.h" + int board_init(void) { return 0; @@ -27,3 +29,11 @@ int dram_init_banksize(void) { return fdtdec_setup_memory_banksize(); } + +#ifdef CONFIG_BOARD_LATE_INIT +int board_late_init(void) +{ + ti_set_fdt_env(NULL, NULL); + return 0; +} +#endif diff --git a/configs/am62ax_evm_a53_defconfig b/configs/am62ax_evm_a53_defconfig index 38083586a3ec..e5fcd8cc5b6f 100644 --- a/configs/am62ax_evm_a53_defconfig +++ b/configs/am62ax_evm_a53_defconfig @@ -24,6 +24,7 @@ CONFIG_SPL_LOAD_FIT_ADDRESS=0x81000000 CONFIG_BOOTSTD_FULL=y CONFIG_BOOTSTD_DEFAULTS=y CONFIG_BOOTCOMMAND="run envboot; bootflow scan -lb" +CONFIG_BOARD_LATE_INIT=y CONFIG_SPL_MAX_SIZE=0x58000 CONFIG_SPL_PAD_TO=0x0 CONFIG_SPL_HAS_BSS_LINKER_SECTION=y
Nishanth Menon nm@ti.com writes:
Stop using the findfdt script and switch to setting the fdtfile from C code.
While at this, replace findfdt in environment with a warning as it is no longer needed
Signed-off-by: Nishanth Menon nm@ti.com
board/ti/am62ax/am62ax.env | 1 - board/ti/am62ax/evm.c | 10 ++++++++++ configs/am62ax_evm_a53_defconfig | 1 + 3 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/board/ti/am62ax/am62ax.env b/board/ti/am62ax/am62ax.env index a6d967e982d4..334374abb73e 100644 --- a/board/ti/am62ax/am62ax.env +++ b/board/ti/am62ax/am62ax.env @@ -1,5 +1,4 @@ #include <env/ti/ti_common.env> -#include <env/ti/default_findfdt.env> #include <env/ti/mmc.env>
name_kern=Image diff --git a/board/ti/am62ax/evm.c b/board/ti/am62ax/evm.c index cd3360a43029..62d3664936e7 100644 --- a/board/ti/am62ax/evm.c +++ b/board/ti/am62ax/evm.c @@ -13,6 +13,8 @@ #include <fdt_support.h> #include <spl.h>
+#include "../common/fdt_ops.h"
int board_init(void) { return 0; @@ -27,3 +29,11 @@ int dram_init_banksize(void) { return fdtdec_setup_memory_banksize(); }
+#ifdef CONFIG_BOARD_LATE_INIT +int board_late_init(void) +{
- ti_set_fdt_env(NULL, NULL);
If a board only has one FDT possible, why not just hard code the path here, rather than use the set_fdt_env() logic with NULLs and create/set TI_EVM_FDT_FOLDER_PATH config that is only used here anyway?
If you think we might add additional board types / FDTs later, then instead let's formalize the fdt_map concept and create a CONFIG_USES_FDT_MAP that will then rely on the fdt_map and call the ti_set_fdt_env(), and if not set, then it just returns a hard coded value, which could be based on the CONFIG_DEFAULT_DEVICE_TREE config.
- return 0;
+} +#endif diff --git a/configs/am62ax_evm_a53_defconfig b/configs/am62ax_evm_a53_defconfig index 38083586a3ec..e5fcd8cc5b6f 100644 --- a/configs/am62ax_evm_a53_defconfig +++ b/configs/am62ax_evm_a53_defconfig @@ -24,6 +24,7 @@ CONFIG_SPL_LOAD_FIT_ADDRESS=0x81000000 CONFIG_BOOTSTD_FULL=y CONFIG_BOOTSTD_DEFAULTS=y CONFIG_BOOTCOMMAND="run envboot; bootflow scan -lb" +CONFIG_BOARD_LATE_INIT=y CONFIG_SPL_MAX_SIZE=0x58000 CONFIG_SPL_PAD_TO=0x0 CONFIG_SPL_HAS_BSS_LINKER_SECTION=y -- 2.43.0
On 20:48-20240108, Jon Humphreys wrote: [..]
+#ifdef CONFIG_BOARD_LATE_INIT +int board_late_init(void) +{
- ti_set_fdt_env(NULL, NULL);
If a board only has one FDT possible, why not just hard code the path here, rather than use the set_fdt_env() logic with NULLs and create/set TI_EVM_FDT_FOLDER_PATH config that is only used here anyway?
Centralizing the logic has the additional benefit of standardizing fall back when maps are not matched. It is better than having each of the board file introduce random way of doing things.
If you think we might add additional board types / FDTs later, then instead let's formalize the fdt_map concept and create a CONFIG_USES_FDT_MAP that will then rely on the fdt_map and call the ti_set_fdt_env(), and if not set, then it just returns a hard coded value, which could be based on the CONFIG_DEFAULT_DEVICE_TREE config.
Ends up being superflous - there is already a push to reduce the config options.
[..]
Stop using the findfdt script and switch to setting the fdtfile from C code.
While at this, replace findfdt in environment with a warning as it is no longer needed
Signed-off-by: Nishanth Menon nm@ti.com --- board/ti/am62x/am62x.env | 1 - board/ti/am62x/evm.c | 8 ++++++++ configs/am62x_evm_a53_defconfig | 1 + 3 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/board/ti/am62x/am62x.env b/board/ti/am62x/am62x.env index e53a55c38fbb..9cb186c2a03c 100644 --- a/board/ti/am62x/am62x.env +++ b/board/ti/am62x/am62x.env @@ -1,5 +1,4 @@ #include <env/ti/ti_common.env> -#include <env/ti/default_findfdt.env> #include <env/ti/mmc.env>
name_kern=Image diff --git a/board/ti/am62x/evm.c b/board/ti/am62x/evm.c index ad939088402e..b76ef1b94a61 100644 --- a/board/ti/am62x/evm.c +++ b/board/ti/am62x/evm.c @@ -54,6 +54,14 @@ int dram_init(void) return fdtdec_setup_mem_size_base(); }
+#ifdef CONFIG_BOARD_LATE_INIT +int board_late_init(void) +{ + ti_set_fdt_env(NULL, NULL); + return 0; +} +#endif + int dram_init_banksize(void) { return fdtdec_setup_memory_banksize(); diff --git a/configs/am62x_evm_a53_defconfig b/configs/am62x_evm_a53_defconfig index aa96c1b3125c..3cf3b93a93fc 100644 --- a/configs/am62x_evm_a53_defconfig +++ b/configs/am62x_evm_a53_defconfig @@ -32,6 +32,7 @@ CONFIG_BOOTSTD_FULL=y CONFIG_BOOTSTD_DEFAULTS=y CONFIG_SYS_BOOTM_LEN=0x800000 CONFIG_BOOTCOMMAND="run envboot; bootflow scan -lb" +CONFIG_BOARD_LATE_INIT=y CONFIG_SPL_MAX_SIZE=0x58000 CONFIG_SPL_HAS_BSS_LINKER_SECTION=y CONFIG_SPL_BSS_START_ADDR=0x80c80000
We now can provide a map and have the standard fdtfile variable set from code itself. This allows for bootstd to "just work".
While at this, replace findfdt in environment with a warning as it is no longer needed.
Signed-off-by: Nishanth Menon nm@ti.com --- board/ti/am64x/am64x.env | 9 --------- board/ti/am64x/evm.c | 8 ++++++++ 2 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/board/ti/am64x/am64x.env b/board/ti/am64x/am64x.env index efd736b99be4..9a8812d4ee54 100644 --- a/board/ti/am64x/am64x.env +++ b/board/ti/am64x/am64x.env @@ -2,14 +2,6 @@ #include <env/ti/mmc.env> #include <env/ti/k3_dfu.env>
-findfdt= - if test $board_name = am64x_gpevm; then - setenv name_fdt ti/k3-am642-evm.dtb; fi; - if test $board_name = am64x_skevm; then - setenv name_fdt ti/k3-am642-sk.dtb; fi; - if test $name_fdt = undefined; then - echo WARNING: Could not determine device tree to use; fi; - setenv fdtfile ${name_fdt} name_kern=Image console=ttyS2,115200n8 args_all=setenv optargs earlycon=ns16550a,mmio32,0x02800000 ${mtdparts} @@ -43,7 +35,6 @@ get_fit_usb=load usb ${bootpart} ${addr_fit} usbboot=setenv boot usb; setenv bootpart 0:2; usb start; - run findfdt; run init_usb; run get_kern_usb; run get_fdt_usb; diff --git a/board/ti/am64x/evm.c b/board/ti/am64x/evm.c index a6dcff2eb434..e2f506d2c6ea 100644 --- a/board/ti/am64x/evm.c +++ b/board/ti/am64x/evm.c @@ -16,6 +16,7 @@ #include <env.h>
#include "../common/board_detect.h" +#include "../common/fdt_ops.h"
#define board_is_am64x_gpevm() (board_ti_k3_is("AM64-GPEVM") || \ board_ti_k3_is("AM64-HSEVM")) @@ -180,6 +181,12 @@ int checkboard(void) }
#ifdef CONFIG_BOARD_LATE_INIT +static struct ti_fdt_map ti_am64_evm_fdt_map[] = { + {"am64x_gpevm", "k3-am642-evm.dtb"}, + {"am64x_skevm", "k3-am642-sk.dtb"}, + { /* Sentinel. */ } +}; + static void setup_board_eeprom_env(void) { char *name = "am64x_gpevm"; @@ -197,6 +204,7 @@ static void setup_board_eeprom_env(void)
invalid_eeprom: set_board_info_env_am6(name); + ti_set_fdt_env(name, ti_am64_evm_fdt_map); }
static void setup_serial(void)
On 08/01/2024 19:32, Nishanth Menon wrote:
We now can provide a map and have the standard fdtfile variable set from code itself. This allows for bootstd to "just work".
While at this, replace findfdt in environment with a warning as it is no longer needed.
Signed-off-by: Nishanth Menon nm@ti.com
board/ti/am64x/am64x.env | 9 --------- board/ti/am64x/evm.c | 8 ++++++++ 2 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/board/ti/am64x/am64x.env b/board/ti/am64x/am64x.env index efd736b99be4..9a8812d4ee54 100644 --- a/board/ti/am64x/am64x.env +++ b/board/ti/am64x/am64x.env @@ -2,14 +2,6 @@ #include <env/ti/mmc.env> #include <env/ti/k3_dfu.env>
-findfdt=
- if test $board_name = am64x_gpevm; then
setenv name_fdt ti/k3-am642-evm.dtb; fi;- if test $board_name = am64x_skevm; then
setenv name_fdt ti/k3-am642-sk.dtb; fi;- if test $name_fdt = undefined; then
echo WARNING: Could not determine device tree to use; fi;- setenv fdtfile ${name_fdt}
name_kern=Image console=ttyS2,115200n8 args_all=setenv optargs earlycon=ns16550a,mmio32,0x02800000 ${mtdparts} @@ -43,7 +35,6 @@ get_fit_usb=load usb ${bootpart} ${addr_fit} usbboot=setenv boot usb; setenv bootpart 0:2; usb start;
- run findfdt; run init_usb; run get_kern_usb; run get_fdt_usb;
diff --git a/board/ti/am64x/evm.c b/board/ti/am64x/evm.c index a6dcff2eb434..e2f506d2c6ea 100644 --- a/board/ti/am64x/evm.c +++ b/board/ti/am64x/evm.c @@ -16,6 +16,7 @@ #include <env.h>
#include "../common/board_detect.h" +#include "../common/fdt_ops.h"
#define board_is_am64x_gpevm() (board_ti_k3_is("AM64-GPEVM") || \ board_ti_k3_is("AM64-HSEVM")) @@ -180,6 +181,12 @@ int checkboard(void) }
#ifdef CONFIG_BOARD_LATE_INIT +static struct ti_fdt_map ti_am64_evm_fdt_map[] = {
- {"am64x_gpevm", "k3-am642-evm.dtb"},
- {"am64x_skevm", "k3-am642-sk.dtb"},
"am64x_gpevm" and "am64x_skevm" strings are used multiple times in this file. see setup_board_eeprom_env()
Please use a MACRO for them.
What is the logic of choosing this name and can it be updated at this point? e.g."gp" is misleading in the board name as the boards are now shipped with HS-FS chip and are no longer GP.
- { /* Sentinel. */ }
+};
static void setup_board_eeprom_env(void) { char *name = "am64x_gpevm"; @@ -197,6 +204,7 @@ static void setup_board_eeprom_env(void)
invalid_eeprom: set_board_info_env_am6(name);
- ti_set_fdt_env(name, ti_am64_evm_fdt_map);
}
static void setup_serial(void)
On 15:08-20240109, Roger Quadros wrote: [..]
#ifdef CONFIG_BOARD_LATE_INIT +static struct ti_fdt_map ti_am64_evm_fdt_map[] = {
- {"am64x_gpevm", "k3-am642-evm.dtb"},
- {"am64x_skevm", "k3-am642-sk.dtb"},
"am64x_gpevm" and "am64x_skevm" strings are used multiple times in this file. see setup_board_eeprom_env()
Please use a MACRO for them.
What is the logic of choosing this name and can it be updated at this point? e.g."gp" is misleading in the board name as the boards are now shipped with HS-FS chip and are no longer GP.
Intent of the series was to get rid of findfdt - it was not meant to cleanup existing name usage in the files. If there is a desire to do so, please - patches are welcome.
We now can provide a map and have the standard fdtfile variable set from code itself. This allows for bootstd to "just work".
While at this, replace findfdt in environment with a warning as it is no longer needed.
Signed-off-by: Nishanth Menon nm@ti.com --- board/ti/am65x/am65x.env | 3 --- board/ti/am65x/evm.c | 2 ++ 2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/board/ti/am65x/am65x.env b/board/ti/am65x/am65x.env index 286b9c300c05..814374d68cf0 100644 --- a/board/ti/am65x/am65x.env +++ b/board/ti/am65x/am65x.env @@ -5,9 +5,6 @@ #include <env/ti/k3_rproc.env> #endif
-findfdt= - setenv name_fdt ti/k3-am654-base-board.dtb; - setenv fdtfile ${name_fdt} name_kern=Image console=ttyS2,115200n8 args_all=setenv optargs ${optargs} earlycon=ns16550a,mmio32,0x02800000 diff --git a/board/ti/am65x/evm.c b/board/ti/am65x/evm.c index df209021c1b7..3109c9a2acac 100644 --- a/board/ti/am65x/evm.c +++ b/board/ti/am65x/evm.c @@ -22,6 +22,7 @@ #include <linux/printk.h>
#include "../common/board_detect.h" +#include "../common/fdt_ops.h"
#define board_is_am65x_base_board() board_ti_is("AM6-COMPROCEVM")
@@ -141,6 +142,7 @@ static void setup_board_eeprom_env(void)
invalid_eeprom: set_board_info_env_am6(name); + ti_set_fdt_env(NULL, NULL); }
static int init_daughtercard_det_gpio(char *gpio_name, struct gpio_desc *desc)
We now can provide a map and have the standard fdtfile variable set from code itself. This allows for bootstd to "just work".
While at this, replace findfdt in environment with a warning as it is no longer needed.
Signed-off-by: Nishanth Menon nm@ti.com --- board/ti/j721e/evm.c | 8 ++++++++ board/ti/j721e/j721e.env | 10 ---------- 2 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/board/ti/j721e/evm.c b/board/ti/j721e/evm.c index c541880107ec..ad6ef4553e04 100644 --- a/board/ti/j721e/evm.c +++ b/board/ti/j721e/evm.c @@ -16,6 +16,7 @@ #include <dm.h>
#include "../common/board_detect.h" +#include "../common/fdt_ops.h"
#define board_is_j721e_som() (board_ti_k3_is("J721EX-PM1-SOM") || \ board_ti_k3_is("J721EX-PM2-SOM")) @@ -424,6 +425,12 @@ void configure_serdes_sierra(void) }
#ifdef CONFIG_BOARD_LATE_INIT +static struct ti_fdt_map ti_j721e_evm_fdt_map[] = { + {"j721e", "k3-j721e-common-proc-board.dtb"}, + {"j721e-sk", "k3-j721e-sk.dtb"}, + {"j7200", "k3-j7200-common-proc-board.dtb"}, + { /* Sentinel. */ } +}; static void setup_board_eeprom_env(void) { char *name = "j721e"; @@ -443,6 +450,7 @@ static void setup_board_eeprom_env(void)
invalid_eeprom: set_board_info_env_am6(name); + ti_set_fdt_env(name, ti_j721e_evm_fdt_map); }
static void setup_serial(void) diff --git a/board/ti/j721e/j721e.env b/board/ti/j721e/j721e.env index cb27bf5e2b24..38bfd7d49634 100644 --- a/board/ti/j721e/j721e.env +++ b/board/ti/j721e/j721e.env @@ -7,16 +7,6 @@ #include <env/ti/k3_rproc.env> #endif
-default_device_tree=ti/k3-j721e-common-proc-board.dtb -findfdt= - setenv name_fdt ${default_device_tree}; - if test $board_name = j721e; then - setenv name_fdt ti/k3-j721e-common-proc-board.dtb; fi; - if test $board_name = j7200; then - setenv name_fdt ti/k3-j7200-common-proc-board.dtb; fi; - if test $board_name = j721e-eaik || test $board_name = j721e-sk; then - setenv name_fdt ti/k3-j721e-sk.dtb; fi; - setenv fdtfile ${name_fdt} name_kern=Image console=ttyS2,115200n8 args_all=setenv optargs earlycon=ns16550a,mmio32,0x02800000
On 08/01/2024 19:32, Nishanth Menon wrote:
We now can provide a map and have the standard fdtfile variable set from code itself. This allows for bootstd to "just work".
While at this, replace findfdt in environment with a warning as it is no longer needed.
Signed-off-by: Nishanth Menon nm@ti.com
board/ti/j721e/evm.c | 8 ++++++++ board/ti/j721e/j721e.env | 10 ---------- 2 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/board/ti/j721e/evm.c b/board/ti/j721e/evm.c index c541880107ec..ad6ef4553e04 100644 --- a/board/ti/j721e/evm.c +++ b/board/ti/j721e/evm.c @@ -16,6 +16,7 @@ #include <dm.h>
#include "../common/board_detect.h" +#include "../common/fdt_ops.h"
#define board_is_j721e_som() (board_ti_k3_is("J721EX-PM1-SOM") || \ board_ti_k3_is("J721EX-PM2-SOM")) @@ -424,6 +425,12 @@ void configure_serdes_sierra(void) }
#ifdef CONFIG_BOARD_LATE_INIT +static struct ti_fdt_map ti_j721e_evm_fdt_map[] = {
- {"j721e", "k3-j721e-common-proc-board.dtb"},
- {"j721e-sk", "k3-j721e-sk.dtb"},
You missed "j721e-eaik"
- {"j7200", "k3-j7200-common-proc-board.dtb"},
- { /* Sentinel. */ }
+}; static void setup_board_eeprom_env(void) { char *name = "j721e"; @@ -443,6 +450,7 @@ static void setup_board_eeprom_env(void)
invalid_eeprom: set_board_info_env_am6(name);
- ti_set_fdt_env(name, ti_j721e_evm_fdt_map);
}
static void setup_serial(void) diff --git a/board/ti/j721e/j721e.env b/board/ti/j721e/j721e.env index cb27bf5e2b24..38bfd7d49634 100644 --- a/board/ti/j721e/j721e.env +++ b/board/ti/j721e/j721e.env @@ -7,16 +7,6 @@ #include <env/ti/k3_rproc.env> #endif
-default_device_tree=ti/k3-j721e-common-proc-board.dtb -findfdt=
- setenv name_fdt ${default_device_tree};
- if test $board_name = j721e; then
setenv name_fdt ti/k3-j721e-common-proc-board.dtb; fi;- if test $board_name = j7200; then
setenv name_fdt ti/k3-j7200-common-proc-board.dtb; fi;- if test $board_name = j721e-eaik || test $board_name = j721e-sk; then
setenv name_fdt ti/k3-j721e-sk.dtb; fi;- setenv fdtfile ${name_fdt}
name_kern=Image console=ttyS2,115200n8 args_all=setenv optargs earlycon=ns16550a,mmio32,0x02800000
On 18:20-20240109, Roger Quadros wrote:
On 08/01/2024 19:32, Nishanth Menon wrote:
We now can provide a map and have the standard fdtfile variable set from code itself. This allows for bootstd to "just work".
While at this, replace findfdt in environment with a warning as it is no longer needed.
Signed-off-by: Nishanth Menon nm@ti.com
board/ti/j721e/evm.c | 8 ++++++++ board/ti/j721e/j721e.env | 10 ---------- 2 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/board/ti/j721e/evm.c b/board/ti/j721e/evm.c index c541880107ec..ad6ef4553e04 100644 --- a/board/ti/j721e/evm.c +++ b/board/ti/j721e/evm.c @@ -16,6 +16,7 @@ #include <dm.h>
#include "../common/board_detect.h" +#include "../common/fdt_ops.h"
#define board_is_j721e_som() (board_ti_k3_is("J721EX-PM1-SOM") || \ board_ti_k3_is("J721EX-PM2-SOM")) @@ -424,6 +425,12 @@ void configure_serdes_sierra(void) }
#ifdef CONFIG_BOARD_LATE_INIT +static struct ti_fdt_map ti_j721e_evm_fdt_map[] = {
- {"j721e", "k3-j721e-common-proc-board.dtb"},
- {"j721e-sk", "k3-j721e-sk.dtb"},
You missed "j721e-eaik"
Nope, it is intentional - eaik (the older name) is handled as sk in setup_board_eeprom_env board_name[1] ==> board_is_j721e_sk provides that abstraction[2]
[1] https://github.com/u-boot/u-boot/blob/master/board/ti/j721e/evm.c#L427 [2] https://github.com/u-boot/u-boot/blob/master/board/ti/j721e/evm.c#L23
We now can provide a map and have the standard fdtfile variable set from code itself. This allows for bootstd to "just work".
While at this, replace findfdt in environment with a warning as it is no longer needed.
Signed-off-by: Nishanth Menon nm@ti.com --- board/ti/j721s2/evm.c | 8 ++++++++ board/ti/j721s2/j721s2.env | 8 -------- 2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/board/ti/j721s2/evm.c b/board/ti/j721s2/evm.c index 1220cd84519b..5a0281d6b483 100644 --- a/board/ti/j721s2/evm.c +++ b/board/ti/j721s2/evm.c @@ -23,6 +23,7 @@ #include <dm/root.h>
#include "../common/board_detect.h" +#include "../common/fdt_ops.h"
DECLARE_GLOBAL_DATA_PTR;
@@ -114,6 +115,12 @@ int checkboard(void) return 0; }
+static struct ti_fdt_map ti_j721s2_evm_fdt_map[] = { + {"j721s2", "k3-j721s2-common-proc-board.dtb"}, + {"am68-sk", "k3-am68-sk-base-board.dtb"}, + { /* Sentinel. */ } +}; + static void setup_board_eeprom_env(void) { char *name = "j721s2"; @@ -131,6 +138,7 @@ static void setup_board_eeprom_env(void)
invalid_eeprom: set_board_info_env_am6(name); + ti_set_fdt_env(name, ti_j721s2_evm_fdt_map); }
static void setup_serial(void) diff --git a/board/ti/j721s2/j721s2.env b/board/ti/j721s2/j721s2.env index 64e3d9da85f0..9a03b9f30aee 100644 --- a/board/ti/j721s2/j721s2.env +++ b/board/ti/j721s2/j721s2.env @@ -7,14 +7,6 @@ #include <env/ti/k3_rproc.env> #endif
-default_device_tree=ti/k3-j721s2-common-proc-board.dtb -findfdt= - setenv name_fdt ${default_device_tree}; - if test $board_name = j721s2; then \ - setenv name_fdt ti/k3-j721s2-common-proc-board.dtb; fi; - if test $board_name = am68-sk; then - setenv name_fdt ti/k3-am68-sk-base-board.dtb; fi; - setenv fdtfile ${name_fdt} name_kern=Image console=ttyS2,115200n8 args_all=setenv optargs earlycon=ns16550a,mmio32,0x02880000
Stop using the findfdt script and switch to setting the fdtfile from C code.
While at this, replace findfdt in environment with a warning as it is no longer needed
Signed-off-by: Nishanth Menon nm@ti.com --- board/beagle/beagleboneai64/beagleboneai64.c | 14 ++++++++++++++ board/beagle/beagleboneai64/beagleboneai64.env | 1 - configs/j721e_beagleboneai64_a72_defconfig | 3 ++- 3 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/board/beagle/beagleboneai64/beagleboneai64.c b/board/beagle/beagleboneai64/beagleboneai64.c index c8c1c78ae5a2..1982f738b04e 100644 --- a/board/beagle/beagleboneai64/beagleboneai64.c +++ b/board/beagle/beagleboneai64/beagleboneai64.c @@ -28,3 +28,17 @@ int dram_init_banksize(void) { return fdtdec_setup_memory_banksize(); } + +#ifdef CONFIG_BOARD_LATE_INIT +int board_late_init(void) +{ + char fdtfile[50]; + + snprintf(fdtfile, sizeof(fdtfile), "%s/%s.dtb", + CONFIG_TI_EVM_FDT_FOLDER_PATH, CONFIG_DEFAULT_DEVICE_TREE); + + env_set("fdtfile", fdtfile); + + return 0; +} +#endif diff --git a/board/beagle/beagleboneai64/beagleboneai64.env b/board/beagle/beagleboneai64/beagleboneai64.env index 4f0a94a8113e..647b25d14c8e 100644 --- a/board/beagle/beagleboneai64/beagleboneai64.env +++ b/board/beagle/beagleboneai64/beagleboneai64.env @@ -1,5 +1,4 @@ #include <env/ti/ti_common.env> -#include <env/ti/default_findfdt.env> #include <env/ti/mmc.env>
name_kern=Image diff --git a/configs/j721e_beagleboneai64_a72_defconfig b/configs/j721e_beagleboneai64_a72_defconfig index 959f86844d32..9e53658eacb9 100644 --- a/configs/j721e_beagleboneai64_a72_defconfig +++ b/configs/j721e_beagleboneai64_a72_defconfig @@ -34,7 +34,8 @@ CONFIG_AUTOBOOT_PROMPT="Press SPACE to abort autoboot in %d seconds\n" CONFIG_AUTOBOOT_DELAY_STR="d" CONFIG_AUTOBOOT_STOP_STR=" " CONFIG_OF_SYSTEM_SETUP=y -CONFIG_BOOTCOMMAND="run set_led_state_start_load;run findfdt; run envboot; bootflow scan -lb;run set_led_state_fail_load" +CONFIG_BOOTCOMMAND="run set_led_state_start_load; run envboot; bootflow scan -lb;run set_led_state_fail_load" +CONFIG_BOARD_LATE_INIT=y CONFIG_LOGLEVEL=7 CONFIG_SPL_MAX_SIZE=0xc0000 CONFIG_SPL_HAS_BSS_LINKER_SECTION=y
On 1/8/24 11:32 AM, Nishanth Menon wrote:
Stop using the findfdt script and switch to setting the fdtfile from C code.
While at this, replace findfdt in environment with a warning as it is no longer needed
Signed-off-by: Nishanth Menon nm@ti.com
board/beagle/beagleboneai64/beagleboneai64.c | 14 ++++++++++++++ board/beagle/beagleboneai64/beagleboneai64.env | 1 - configs/j721e_beagleboneai64_a72_defconfig | 3 ++- 3 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/board/beagle/beagleboneai64/beagleboneai64.c b/board/beagle/beagleboneai64/beagleboneai64.c index c8c1c78ae5a2..1982f738b04e 100644 --- a/board/beagle/beagleboneai64/beagleboneai64.c +++ b/board/beagle/beagleboneai64/beagleboneai64.c @@ -28,3 +28,17 @@ int dram_init_banksize(void) { return fdtdec_setup_memory_banksize(); }
+#ifdef CONFIG_BOARD_LATE_INIT +int board_late_init(void) +{
- char fdtfile[50];
- snprintf(fdtfile, sizeof(fdtfile), "%s/%s.dtb",
CONFIG_TI_EVM_FDT_FOLDER_PATH, CONFIG_DEFAULT_DEVICE_TREE);
Why not use your new ti_set_fdt_env() helper? It makes this board consistent with TI boards (sets "name_fdt" and adds warning for "findfdt" like your commit message says you do..).
Andrew
- env_set("fdtfile", fdtfile);
- return 0;
+} +#endif diff --git a/board/beagle/beagleboneai64/beagleboneai64.env b/board/beagle/beagleboneai64/beagleboneai64.env index 4f0a94a8113e..647b25d14c8e 100644 --- a/board/beagle/beagleboneai64/beagleboneai64.env +++ b/board/beagle/beagleboneai64/beagleboneai64.env @@ -1,5 +1,4 @@ #include <env/ti/ti_common.env> -#include <env/ti/default_findfdt.env> #include <env/ti/mmc.env>
name_kern=Image diff --git a/configs/j721e_beagleboneai64_a72_defconfig b/configs/j721e_beagleboneai64_a72_defconfig index 959f86844d32..9e53658eacb9 100644 --- a/configs/j721e_beagleboneai64_a72_defconfig +++ b/configs/j721e_beagleboneai64_a72_defconfig @@ -34,7 +34,8 @@ CONFIG_AUTOBOOT_PROMPT="Press SPACE to abort autoboot in %d seconds\n" CONFIG_AUTOBOOT_DELAY_STR="d" CONFIG_AUTOBOOT_STOP_STR=" " CONFIG_OF_SYSTEM_SETUP=y -CONFIG_BOOTCOMMAND="run set_led_state_start_load;run findfdt; run envboot; bootflow scan -lb;run set_led_state_fail_load" +CONFIG_BOOTCOMMAND="run set_led_state_start_load; run envboot; bootflow scan -lb;run set_led_state_fail_load" +CONFIG_BOARD_LATE_INIT=y CONFIG_LOGLEVEL=7 CONFIG_SPL_MAX_SIZE=0xc0000 CONFIG_SPL_HAS_BSS_LINKER_SECTION=y
On 13:00-20240108, Andrew Davis wrote:
+#ifdef CONFIG_BOARD_LATE_INIT +int board_late_init(void) +{
- char fdtfile[50];
- snprintf(fdtfile, sizeof(fdtfile), "%s/%s.dtb",
CONFIG_TI_EVM_FDT_FOLDER_PATH, CONFIG_DEFAULT_DEVICE_TREE);Why not use your new ti_set_fdt_env() helper? It makes this board
I had it originally, but did not want the code to depend on board/ti/common functions. So dropped it in later version.
consistent with TI boards (sets "name_fdt" and adds warning for "findfdt" like your commit message says you do..).
Will fix up the commit message in the next revision
Nishanth Menon nm@ti.com writes:
Stop using the findfdt script and switch to setting the fdtfile from C code.
While at this, replace findfdt in environment with a warning as it is no longer needed
Signed-off-by: Nishanth Menon nm@ti.com
board/beagle/beagleboneai64/beagleboneai64.c | 14 ++++++++++++++ board/beagle/beagleboneai64/beagleboneai64.env | 1 - configs/j721e_beagleboneai64_a72_defconfig | 3 ++- 3 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/board/beagle/beagleboneai64/beagleboneai64.c b/board/beagle/beagleboneai64/beagleboneai64.c index c8c1c78ae5a2..1982f738b04e 100644 --- a/board/beagle/beagleboneai64/beagleboneai64.c +++ b/board/beagle/beagleboneai64/beagleboneai64.c @@ -28,3 +28,17 @@ int dram_init_banksize(void) { return fdtdec_setup_memory_banksize(); }
+#ifdef CONFIG_BOARD_LATE_INIT +int board_late_init(void) +{
- char fdtfile[50];
- snprintf(fdtfile, sizeof(fdtfile), "%s/%s.dtb",
CONFIG_TI_EVM_FDT_FOLDER_PATH, CONFIG_DEFAULT_DEVICE_TREE);
This would set the board to using the control DT, not boot DT. Is that what you meant?
But anyway, why not just hard code the FDT path/name here since there is only one for this board? I don't see the value in the extra logic of using the config values (or having a fdt_map). (Same for beagleplay)
- env_set("fdtfile", fdtfile);
- return 0;
+} +#endif diff --git a/board/beagle/beagleboneai64/beagleboneai64.env b/board/beagle/beagleboneai64/beagleboneai64.env index 4f0a94a8113e..647b25d14c8e 100644 --- a/board/beagle/beagleboneai64/beagleboneai64.env +++ b/board/beagle/beagleboneai64/beagleboneai64.env @@ -1,5 +1,4 @@ #include <env/ti/ti_common.env> -#include <env/ti/default_findfdt.env> #include <env/ti/mmc.env>
name_kern=Image diff --git a/configs/j721e_beagleboneai64_a72_defconfig b/configs/j721e_beagleboneai64_a72_defconfig index 959f86844d32..9e53658eacb9 100644 --- a/configs/j721e_beagleboneai64_a72_defconfig +++ b/configs/j721e_beagleboneai64_a72_defconfig @@ -34,7 +34,8 @@ CONFIG_AUTOBOOT_PROMPT="Press SPACE to abort autoboot in %d seconds\n" CONFIG_AUTOBOOT_DELAY_STR="d" CONFIG_AUTOBOOT_STOP_STR=" " CONFIG_OF_SYSTEM_SETUP=y -CONFIG_BOOTCOMMAND="run set_led_state_start_load;run findfdt; run envboot; bootflow scan -lb;run set_led_state_fail_load" +CONFIG_BOOTCOMMAND="run set_led_state_start_load; run envboot; bootflow scan -lb;run set_led_state_fail_load" +CONFIG_BOARD_LATE_INIT=y CONFIG_LOGLEVEL=7 CONFIG_SPL_MAX_SIZE=0xc0000 CONFIG_SPL_HAS_BSS_LINKER_SECTION=y -- 2.43.0
On 20:24-20240108, Jon Humphreys wrote:
Nishanth Menon nm@ti.com writes:
Stop using the findfdt script and switch to setting the fdtfile from C code.
While at this, replace findfdt in environment with a warning as it is no longer needed
Signed-off-by: Nishanth Menon nm@ti.com
board/beagle/beagleboneai64/beagleboneai64.c | 14 ++++++++++++++ board/beagle/beagleboneai64/beagleboneai64.env | 1 - configs/j721e_beagleboneai64_a72_defconfig | 3 ++- 3 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/board/beagle/beagleboneai64/beagleboneai64.c b/board/beagle/beagleboneai64/beagleboneai64.c index c8c1c78ae5a2..1982f738b04e 100644 --- a/board/beagle/beagleboneai64/beagleboneai64.c +++ b/board/beagle/beagleboneai64/beagleboneai64.c @@ -28,3 +28,17 @@ int dram_init_banksize(void) { return fdtdec_setup_memory_banksize(); }
+#ifdef CONFIG_BOARD_LATE_INIT +int board_late_init(void) +{
- char fdtfile[50];
- snprintf(fdtfile, sizeof(fdtfile), "%s/%s.dtb",
CONFIG_TI_EVM_FDT_FOLDER_PATH, CONFIG_DEFAULT_DEVICE_TREE);This would set the board to using the control DT, not boot DT. Is that what you meant?
control DT file name. not the content.
But anyway, why not just hard code the FDT path/name here since there is only one for this board? I don't see the value in the extra logic of using the config values (or having a fdt_map). (Same for beagleplay)
because there are potentially variants being privately discussed for both the platforms - though the timelines are unknown at this point. This will allow fragments to replace the DEFAULT_DEVICE_TREE to the variant and rest of the logic will *just work*.
Stop using the findfdt script and switch to setting the fdtfile from C code.
While at this, replace findfdt in environment with a warning as it is no longer needed.
Signed-off-by: Nishanth Menon nm@ti.com --- board/beagle/beagleplay/beagleplay.c | 14 ++++++++++++++ board/beagle/beagleplay/beagleplay.env | 1 - configs/am62x_beagleplay_a53_defconfig | 3 ++- 3 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/board/beagle/beagleplay/beagleplay.c b/board/beagle/beagleplay/beagleplay.c index 1c376dea372f..00ff86165790 100644 --- a/board/beagle/beagleplay/beagleplay.c +++ b/board/beagle/beagleplay/beagleplay.c @@ -27,3 +27,17 @@ int dram_init_banksize(void) { return fdtdec_setup_memory_banksize(); } + +#ifdef CONFIG_BOARD_LATE_INIT +int board_late_init(void) +{ + char fdtfile[50]; + + snprintf(fdtfile, sizeof(fdtfile), "%s/%s.dtb", + CONFIG_TI_EVM_FDT_FOLDER_PATH, CONFIG_DEFAULT_DEVICE_TREE); + + env_set("fdtfile", fdtfile); + + return 0; +} +#endif diff --git a/board/beagle/beagleplay/beagleplay.env b/board/beagle/beagleplay/beagleplay.env index 4f0a94a8113e..647b25d14c8e 100644 --- a/board/beagle/beagleplay/beagleplay.env +++ b/board/beagle/beagleplay/beagleplay.env @@ -1,5 +1,4 @@ #include <env/ti/ti_common.env> -#include <env/ti/default_findfdt.env> #include <env/ti/mmc.env>
name_kern=Image diff --git a/configs/am62x_beagleplay_a53_defconfig b/configs/am62x_beagleplay_a53_defconfig index 0be20045a974..1f43891d10bb 100644 --- a/configs/am62x_beagleplay_a53_defconfig +++ b/configs/am62x_beagleplay_a53_defconfig @@ -33,7 +33,8 @@ CONFIG_AUTOBOOT_KEYED=y CONFIG_AUTOBOOT_PROMPT="Press SPACE to abort autoboot in %d seconds\n" CONFIG_AUTOBOOT_DELAY_STR="d" CONFIG_AUTOBOOT_STOP_STR=" " -CONFIG_BOOTCOMMAND="run set_led_state_start_load;run findfdt; run envboot; bootflow scan -lb;run set_led_state_fail_load" +CONFIG_BOOTCOMMAND="run set_led_state_start_load; run envboot; bootflow scan -lb;run set_led_state_fail_load" +CONFIG_BOARD_LATE_INIT=y CONFIG_SPL_MAX_SIZE=0x58000 CONFIG_SPL_HAS_BSS_LINKER_SECTION=y CONFIG_SPL_BSS_START_ADDR=0x80c80000
We shouldn't need finfdt anymore. Drop the env script.
Signed-off-by: Nishanth Menon nm@ti.com --- include/env/ti/default_findfdt.env | 12 ------------ 1 file changed, 12 deletions(-) delete mode 100644 include/env/ti/default_findfdt.env
diff --git a/include/env/ti/default_findfdt.env b/include/env/ti/default_findfdt.env deleted file mode 100644 index a2b51dd923bb..000000000000 --- a/include/env/ti/default_findfdt.env +++ /dev/null @@ -1,12 +0,0 @@ -default_device_tree=CONFIG_DEFAULT_DEVICE_TREE -default_device_tree_arch=ti -#ifdef CONFIG_ARM64 -findfdt= - setenv name_fdt ${default_device_tree_arch}/${default_device_tree}.dtb; - setenv fdtfile ${name_fdt} -#else -default_device_tree_subarch=omap -findfdt= - setenv name_fdt ${default_device_tree_arch}/${default_device_tree_subarch}/${default_device_tree}.dtb; - setenv fdtfile ${name_fdt} -#endif
On Mon, Jan 08, 2024 at 11:32:51AM -0600, Nishanth Menon wrote:
This is a wide cleanup to switch to setting fdtfile using env_set instead of scripted magic. 'fdtfile' is expected to be set by default. This allows the stdboot triggered efi loaders to find the correct OS device tree file.
This is a refresh of https://lore.kernel.org/all/86le9dwz4d.fsf@udb0321960.dhcp.ti.com/ which was the wrong approach.
Bootlogs: https://gist.github.com/nmenon/2f4a142c1bcaa09d544b1f2e206ea134
NOTE: There are a couple of checkpatch WARN (around LATE_INIT) and CHECK (fdt_ops #ifdeffery) that on closer inspection looks fine and consistent with other similar usage.
Based on next branch at: c2c598e87cfe Merge branch 'staging' of https://source.denx.de/u-boot/custodians/u-boot-tegra into next
This overall seems fine, thanks.
participants (5)
-
Andrew Davis -
Jon Humphreys -
Nishanth Menon -
Roger Quadros -
Tom Rini