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

[Matthias Schiffer]

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

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
> 

-- 
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/