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

On Wed, 2023-12-13 at 19:52 +0000, Simon Glass wrote:

---

Achtung externe E-Mail: Öffnen Sie Anhänge und Links nur, wenn Sie wissen, dass diese aus einer sicheren Quelle stammen und sicher sind. Leiten Sie die E-Mail im Zweifelsfall zur Prüfung an den IT-Helpdesk weiter. Attention external email: Open attachments and links only if you know that they are from a secure source and are safe. In doubt forward the email to the IT-Helpdesk to check it.

---

On Mon, 11 Dec 2023 at 04:04, Matthias Schiffer matthias.schiffer@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@ew.tq-group.com

boot/Kconfig | 9 +++++++++ boot/image-fdt.c | 19 +++++++++++++++++-- 2 files changed, 26 insertions(+), 2 deletions(-)

Reviewed-by: Simon Glass sjg@chromium.org

How about a test?

Makes sense, I'll add a unit 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?

Not necessarily in all code paths that lead to image_setup_libfdt().

...

•                   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?

With separate image and FDT, one would usually run the fdt command to modify the FDT before bootm, so the modifications also happen before other fixups. My intention was to get the same behavior for FIT images.

I don't care much either way though, for most fixups the order shouldn't matter too much, and we can always add an fdt_fixup_early or fdt_fixup_late if an actual usecase for such a sequence of fixups comes up.

Regards, Matthias

...

•   }
  

•   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