| Message ID | 20231211110317.697011-1-matthias.schiffer@ew.tq-group.com |
|---|---|
| State | Accepted |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | boot: add support for fdt_fixup command in environment | expand |
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? > > 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
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 >
On Mon, Dec 11, 2023 at 12:03:17PM +0100, Matthias Schiffer 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> > Reviewed-by: Simon Glass <sjg@chromium.org> Applied to u-boot/next, thanks!
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)); + ret = run_command_list(fdt_fixup, -1, 0); + if (ret) + printf("WARNING: fdt_fixup command returned %d\n", + ret); + } + } + + ret = -EPERM; if (fdt_root(blob) < 0) { printf("ERROR: root node setup failed\n");
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(-)