[PATCH] boot: add support for fdt_fixup command in environment

[Simon Glass]

On Mon, 11 Dec 2023 at 04:04, Matthias Schiffer
<matthias.schiffer at ew.tq-group.com> wrote:
>
> The "fdt" command is convenient for making small changes to the OS FDT,
> especially during development. This is easy when the kernel and FDT are
> loaded separately, but can be cumbersome for FIT images, requiring to
> unpack the image, manually apply overlays, etc.
>
> Add an option to execute a command "fdt_fixup" from the environment at
> the beginning of image_setup_libfdt() (after overlays are applied, and
> before the other fixups).
>
> Signed-off-by: Matthias Schiffer <matthias.schiffer at ew.tq-group.com>
> ---
>  boot/Kconfig     |  9 +++++++++
>  boot/image-fdt.c | 19 +++++++++++++++++--
>  2 files changed, 26 insertions(+), 2 deletions(-)

Reviewed-by: Simon Glass <sjg at chromium.org>

How about a test?


>
> diff --git a/boot/Kconfig b/boot/Kconfig
> index ef71883a502..7eea935f490 100644
> --- a/boot/Kconfig
> +++ b/boot/Kconfig
> @@ -1502,6 +1502,15 @@ if OF_LIBFDT
>
>  menu "Devicetree fixup"
>
> +config OF_ENV_SETUP
> +       bool "Run a command from environment to set up device tree before boot"
> +       depends on CMD_FDT
> +       help
> +         This causes U-Boot to run a command from the environment variable
> +         fdt_fixup before booting into the operating system, which can use the
> +         fdt command to modify the device tree. The device tree is then passed
> +         to the OS.
> +
>  config OF_BOARD_SETUP
>         bool "Set up board-specific details in device tree before boot"
>         help
> diff --git a/boot/image-fdt.c b/boot/image-fdt.c
> index f10200f6474..78b5c639381 100644
> --- a/boot/image-fdt.c
> +++ b/boot/image-fdt.c
> @@ -9,6 +9,7 @@
>   */
>
>  #include <common.h>
> +#include <command.h>
>  #include <fdt_support.h>
>  #include <fdtdec.h>
>  #include <env.h>
> @@ -608,8 +609,22 @@ int image_setup_libfdt(struct bootm_headers *images, void *blob,
>  {
>         ulong *initrd_start = &images->initrd_start;
>         ulong *initrd_end = &images->initrd_end;
> -       int ret = -EPERM;
> -       int fdt_ret;
> +       int ret, fdt_ret;
> +
> +       if (IS_ENABLED(CONFIG_OF_ENV_SETUP)) {
> +               const char *fdt_fixup;
> +
> +               fdt_fixup = env_get("fdt_fixup");
> +               if (fdt_fixup) {
> +                       set_working_fdt_addr(map_to_sysmem(blob));

Is that not already done?

> +                       ret = run_command_list(fdt_fixup, -1, 0);
> +                       if (ret)
> +                               printf("WARNING: fdt_fixup command returned %d\n",
> +                                      ret);
> +               }

Would it make sense to put this code near the end of the function,
after other fixups have been done?

> +       }
> +
> +       ret = -EPERM;
>
>         if (fdt_root(blob) < 0) {
>                 printf("ERROR: root node setup failed\n");
> --
> TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
> Amtsgericht München, HRB 105018
> Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
> https://www.tq-group.com/
>

Regards,
Simon