Re: [PATCH V2 01/10] board: ti: common: Introduce a common fdt ops library

Jon Humphreys
2024-01-12T21:26:45Z

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>
> ---
> Changes Since v1:
> * s/TI_EVM_FDT_FOLDER_PATH/TI_FDT_FOLDER_PATH s/name_fdt/board_name
> s/TI_NAME_FDT_MAX/TI_BOARD_NAME_MAX
> * Comment updates in various places for review clarification.
> * Still maintain the fall back using CONFIG_DEFAULT_DEVICE_TREE for
> reasons explained in review comment response.
> * Added a specific u-boot version number for deprecation of legacy
> env variables.
>
> V1: https://lore.kernel.org/r/20240108173301.2692332-2-nm@ti.com
>
> board/ti/common/Kconfig | 12 ++++++++
> board/ti/common/Makefile | 1 +
> board/ti/common/fdt_ops.c | 64 +++++++++++++++++++++++++++++++++++++++
> board/ti/common/fdt_ops.h | 42 +++++++++++++++++++++++++
> 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..de44e4de2115 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_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..eb917be9e0da
> --- /dev/null
> +++ b/board/ti/common/fdt_ops.c
> @@ -0,0 +1,64 @@
> +// 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 *board_name, struct ti_fdt_map *fdt_map)
> +{
> + char *fdt_file_name = NULL;
> + char fdtfile[TI_FDT_FILE_MAX];
> +
> + if (board_name) {
> + while (fdt_map) {
> + /* Check for NULL terminator in the list */
> + if (!fdt_map->board_name)
> + break;
> + if (!strncmp(fdt_map->board_name, board_name, TI_BOARD_NAME_MAX)) {
> + fdt_file_name = fdt_map->fdt_file_name;
> + break;
> + }
> + fdt_map++;
> + }
> + }
> +
> + /* match not found OR null board_name */
> + 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_FDT_FOLDER_PATH, CONFIG_DEFAULT_FDT_FILE);
> + } else
> +#endif
> + {
> + snprintf(fdtfile, sizeof(fdtfile), "%s/%s.dtb",
> + CONFIG_TI_FDT_FOLDER_PATH, CONFIG_DEFAULT_DEVICE_TREE);
> + }
> + } else {
> + snprintf(fdtfile, sizeof(fdtfile), "%s/%s", CONFIG_TI_FDT_FOLDER_PATH,
> + fdt_file_name);
> + }
> +
> + env_set("fdtfile", fdtfile);
> +
> + /*
> + * XXX: DEPRECATION WARNING: 2 u-boot versions (2024.10).
> + *
> + * Maintain compatibility with downstream scripts that may be using
> + * name_fdt
> + */
> + if (board_name)
> + env_set("name_fdt", fdtfile);
> + /* 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..5d304994fb6e
> --- /dev/null
> +++ b/board/ti/common/fdt_ops.h
> @@ -0,0 +1,42 @@
> +/* 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_BOARD_NAME_MAX 20
> +#define TI_FDT_FILE_MAX 200
> +
> +/**
> + * struct ti_fdt_map - mapping of device tree blob name to board name
> + * @board_name: board_name up to TI_BOARD_NAME_MAX long
> + * @fdt_file_name: device tree blob name as described by kernel
> + */
> +struct ti_fdt_map {
> + const char *board_name;
> + char *fdt_file_name;
> +};
> +
> +/**
> + * ti_set_fdt_env - Find the correct device tree file name based on the
> + * board name and set 'fdtfile' env variable with correct folder
> + * structure appropriate to the architecture and Linux kernel's
> + * 'make install_dtbs' conventions. This function is invoked typically
> + * as part of board_late_init.
> + *
> + * fdt name is picked by:
> + * 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)
> + *
> + * @board_name: match to search with (max of TI_BOARD_NAME_MAX chars)
> + * @fdt_map: NULL terminated array of device tree file name matches.
> + */
> +void ti_set_fdt_env(const char *board_name, struct ti_fdt_map *fdt_map);
> +
> +#endif /* __FDT_OPS_H */
> --
> 2.43.0

Reviewed-by: Jonathan Humphreys <j-humphreys@ti.com>