Nishanth Menon <n...@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 <n...@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.
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