| Message ID | 20210413224345.2692591-2-rasmus.villemoes@prevas.dk |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | allow environment to be updated from dtb | expand |
Hi Rasmus, On Wed, 14 Apr 2021 at 10:43, Rasmus Villemoes <rasmus.villemoes@prevas.dk> wrote: > > It can be useful to use the same U-Boot binary for multiple purposes, > say the normal one, one for developers that allow breaking into the > U-Boot shell, and one for use during bootstrapping which runs a > special-purpose bootcmd. Or one can have several board variants that > can share almost all boot logic, but just needs a few tweaks in the > variables used by the boot script. > > To that end, allow the control dtb to contain a /config/enviroment > node (or whatever one puts in fdt_env_path variable), whose > property/value pairs are used to update the run-time environment after > it has been loaded from its persistent location. > > The indirection via fdt_env_path is for maximum flexibility - for > example, should the user wish (or board logic dictate) that the values > in the DTB should no longer be applied, one simply needs to delete the > fdt_env_path variable; that can even be done automatically by > including a > > fdt_env_path = ""; > > property in the DTB node. > > Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk> > --- > common/board_r.c | 2 ++ > env/Kconfig | 18 ++++++++++++++++++ > env/common.c | 23 +++++++++++++++++++++++ > include/env.h | 15 +++++++++++++++ > include/env_default.h | 3 +++ > 5 files changed, 61 insertions(+) > Reviewed-by: Simon Glass <sjg@chromium.org> Some suggestions below > diff --git a/common/board_r.c b/common/board_r.c > index c835ff8e26..3f82404772 100644 > --- a/common/board_r.c > +++ b/common/board_r.c > @@ -459,6 +459,8 @@ static int initr_env(void) > else > env_set_default(NULL, 0); > > + env_import_fdt(); > + > if (IS_ENABLED(CONFIG_OF_CONTROL)) > env_set_hex("fdtcontroladdr", > (unsigned long)map_to_sysmem(gd->fdt_blob)); > diff --git a/env/Kconfig b/env/Kconfig > index b473d7cfe1..aa800e37ce 100644 > --- a/env/Kconfig > +++ b/env/Kconfig > @@ -647,6 +647,24 @@ config DELAY_ENVIRONMENT > later by U-Boot code. With CONFIG_OF_CONTROL this is instead > controlled by the value of /config/load-environment. > > +config ENV_IMPORT_FDT > + bool "Amend environment by FDT properties" > + depends on OF_CONTROL > + help > + If selected, after the environment has been loaded from its > + persistent location, the "env_fdt_path" variable is looked > + up and used as a path to a node in the control DTB. The > + property/value pairs in that node is then used to update the > + run-time environment. This can be useful to use the same > + U-Boot binary with different board variants. > + > +config ENV_FDT_PATH > + string "Default value for env_fdt_path variable" > + depends on ENV_IMPORT_FDT > + default "/config/environment" > + help > + The initial value of the env_fdt_path variable. > + > config ENV_APPEND > bool "Always append the environment with new data" > default n > diff --git a/env/common.c b/env/common.c > index 2ee423beb5..af45e561ce 100644 > --- a/env/common.c > +++ b/env/common.c > @@ -345,3 +345,26 @@ int env_complete(char *var, int maxv, char *cmdv[], int bufsz, char *buf, > return found; > } > #endif > + > +#ifdef CONFIG_ENV_IMPORT_FDT > +void env_import_fdt(void) > +{ > + const void *blob = gd->fdt_blob; > + const char *path; > + int offset, prop; > + > + path = env_get("env_fdt_path"); > + if (!path || !path[0]) > + return; How about returning an error code indicating what happened? > + offset = fdt_path_offset(blob, path); Could we use the livetree API here (ofnode)? > + if (offset < 0) > + return; > + > + fdt_for_each_property_offset(prop, blob, offset) { > + const char *name, *val; > + > + val = fdt_getprop_by_offset(blob, prop, &name, NULL); > + env_set(name, val); > + } > +} > +#endif > diff --git a/include/env.h b/include/env.h > index c15339a93f..6296502595 100644 > --- a/include/env.h > +++ b/include/env.h > @@ -377,4 +377,19 @@ int env_get_char(int index); > * This is used for those unfortunate archs with crappy toolchains > */ > void env_reloc(void); > + > + > +/** > + * env_import_fdt() - Import environment values from device tree blob > + * > + * This uses the value of the environment variable "env_fdt_path" as a > + * path to an fdt node, whose property/value pairs are added to the > + * environment. > + */ > +#ifdef CONFIG_ENV_IMPORT_FDT > +void env_import_fdt(void); > +#else > +static inline void env_import_fdt(void) {} > +#endif > + > #endif > diff --git a/include/env_default.h b/include/env_default.h > index ea31a8eddf..1ddd64ba8f 100644 > --- a/include/env_default.h > +++ b/include/env_default.h > @@ -103,6 +103,9 @@ const uchar default_environment[] = { > #ifdef CONFIG_SYS_SOC > "soc=" CONFIG_SYS_SOC "\0" > #endif > +#ifdef CONFIG_ENV_IMPORT_FDT > + "env_fdt_path=" CONFIG_ENV_FDT_PATH "\0" > +#endif > #endif > #if defined(CONFIG_BOOTCOUNT_BOOTLIMIT) && (CONFIG_BOOTCOUNT_BOOTLIMIT > 0) > "bootlimit=" __stringify(CONFIG_BOOTCOUNT_BOOTLIMIT)"\0" > -- > 2.29.2 > Regards, Simon
On 21/04/2021 09.14, Simon Glass wrote: > Hi Rasmus, > > On Wed, 14 Apr 2021 at 10:43, Rasmus Villemoes > <rasmus.villemoes@prevas.dk> wrote: >> >> It can be useful to use the same U-Boot binary for multiple purposes, >> say the normal one, one for developers that allow breaking into the >> U-Boot shell, and one for use during bootstrapping which runs a >> special-purpose bootcmd. Or one can have several board variants that >> can share almost all boot logic, but just needs a few tweaks in the >> variables used by the boot script. >> >> To that end, allow the control dtb to contain a /config/enviroment >> node (or whatever one puts in fdt_env_path variable), whose >> property/value pairs are used to update the run-time environment after >> it has been loaded from its persistent location. >> >> The indirection via fdt_env_path is for maximum flexibility - for >> example, should the user wish (or board logic dictate) that the values >> in the DTB should no longer be applied, one simply needs to delete the >> fdt_env_path variable; that can even be done automatically by >> including a >> >> fdt_env_path = ""; >> >> property in the DTB node. >> >> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk> >> --- >> common/board_r.c | 2 ++ >> env/Kconfig | 18 ++++++++++++++++++ >> env/common.c | 23 +++++++++++++++++++++++ >> include/env.h | 15 +++++++++++++++ >> include/env_default.h | 3 +++ >> 5 files changed, 61 insertions(+) >> > > Reviewed-by: Simon Glass <sjg@chromium.org> > > Some suggestions below > >> diff --git a/common/board_r.c b/common/board_r.c >> index c835ff8e26..3f82404772 100644 >> --- a/common/board_r.c >> +++ b/common/board_r.c >> @@ -459,6 +459,8 @@ static int initr_env(void) >> else >> env_set_default(NULL, 0); >> >> + env_import_fdt(); >> + >> if (IS_ENABLED(CONFIG_OF_CONTROL)) >> env_set_hex("fdtcontroladdr", >> (unsigned long)map_to_sysmem(gd->fdt_blob)); >> diff --git a/env/Kconfig b/env/Kconfig >> index b473d7cfe1..aa800e37ce 100644 >> --- a/env/Kconfig >> +++ b/env/Kconfig >> @@ -647,6 +647,24 @@ config DELAY_ENVIRONMENT >> later by U-Boot code. With CONFIG_OF_CONTROL this is instead >> controlled by the value of /config/load-environment. >> >> +config ENV_IMPORT_FDT >> + bool "Amend environment by FDT properties" >> + depends on OF_CONTROL >> + help >> + If selected, after the environment has been loaded from its >> + persistent location, the "env_fdt_path" variable is looked >> + up and used as a path to a node in the control DTB. The >> + property/value pairs in that node is then used to update the >> + run-time environment. This can be useful to use the same >> + U-Boot binary with different board variants. >> + >> +config ENV_FDT_PATH >> + string "Default value for env_fdt_path variable" >> + depends on ENV_IMPORT_FDT >> + default "/config/environment" >> + help >> + The initial value of the env_fdt_path variable. >> + >> config ENV_APPEND >> bool "Always append the environment with new data" >> default n >> diff --git a/env/common.c b/env/common.c >> index 2ee423beb5..af45e561ce 100644 >> --- a/env/common.c >> +++ b/env/common.c >> @@ -345,3 +345,26 @@ int env_complete(char *var, int maxv, char *cmdv[], int bufsz, char *buf, >> return found; >> } >> #endif >> + >> +#ifdef CONFIG_ENV_IMPORT_FDT >> +void env_import_fdt(void) >> +{ >> + const void *blob = gd->fdt_blob; >> + const char *path; >> + int offset, prop; >> + >> + path = env_get("env_fdt_path"); >> + if (!path || !path[0]) >> + return; > > How about returning an error code indicating what happened? I considered that, but I'm not sure what the (single) caller would do with that. Not having env_fdt_path set is a supported use case as I explain. So here it's not really an error. However, if env_fdt_path is set, but there's no such node in the DT blob, it might be worth printing a warning (i.e. in the "offset < 0" case below). >> + offset = fdt_path_offset(blob, path); > > Could we use the livetree API here (ofnode)? Dunno, what's that? Have U-Boot started deserializing the FDT blob like the linux kernel does and build an in-memory representation that's easier to traverse? Thanks, Rasmus
On 21/04/2021 10.02, Rasmus Villemoes wrote: > On 21/04/2021 09.14, Simon Glass wrote: >> Hi Rasmus, >> >> On Wed, 14 Apr 2021 at 10:43, Rasmus Villemoes >> <rasmus.villemoes@prevas.dk> wrote: >>> >>> It can be useful to use the same U-Boot binary for multiple purposes, >>> say the normal one, one for developers that allow breaking into the >>> U-Boot shell, and one for use during bootstrapping which runs a >>> special-purpose bootcmd. Or one can have several board variants that >>> can share almost all boot logic, but just needs a few tweaks in the >>> variables used by the boot script. >>> >>> To that end, allow the control dtb to contain a /config/enviroment >>> node (or whatever one puts in fdt_env_path variable), whose >>> property/value pairs are used to update the run-time environment after >>> it has been loaded from its persistent location. >>> >>> The indirection via fdt_env_path is for maximum flexibility - for >>> example, should the user wish (or board logic dictate) that the values >>> in the DTB should no longer be applied, one simply needs to delete the >>> fdt_env_path variable; that can even be done automatically by >>> including a >>> >>> fdt_env_path = ""; >>> >>> property in the DTB node. >>> >>> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk> >>> --- >>> common/board_r.c | 2 ++ >>> env/Kconfig | 18 ++++++++++++++++++ >>> env/common.c | 23 +++++++++++++++++++++++ >>> include/env.h | 15 +++++++++++++++ >>> include/env_default.h | 3 +++ >>> 5 files changed, 61 insertions(+) >>> >> >> Reviewed-by: Simon Glass <sjg@chromium.org> >> >> Some suggestions below >> >>> diff --git a/common/board_r.c b/common/board_r.c >>> index c835ff8e26..3f82404772 100644 >>> --- a/common/board_r.c >>> +++ b/common/board_r.c >>> @@ -459,6 +459,8 @@ static int initr_env(void) >>> else >>> env_set_default(NULL, 0); >>> >>> + env_import_fdt(); >>> + >>> if (IS_ENABLED(CONFIG_OF_CONTROL)) >>> env_set_hex("fdtcontroladdr", >>> (unsigned long)map_to_sysmem(gd->fdt_blob)); >>> diff --git a/env/Kconfig b/env/Kconfig >>> index b473d7cfe1..aa800e37ce 100644 >>> --- a/env/Kconfig >>> +++ b/env/Kconfig >>> @@ -647,6 +647,24 @@ config DELAY_ENVIRONMENT >>> later by U-Boot code. With CONFIG_OF_CONTROL this is instead >>> controlled by the value of /config/load-environment. >>> >>> +config ENV_IMPORT_FDT >>> + bool "Amend environment by FDT properties" >>> + depends on OF_CONTROL >>> + help >>> + If selected, after the environment has been loaded from its >>> + persistent location, the "env_fdt_path" variable is looked >>> + up and used as a path to a node in the control DTB. The >>> + property/value pairs in that node is then used to update the >>> + run-time environment. This can be useful to use the same >>> + U-Boot binary with different board variants. >>> + >>> +config ENV_FDT_PATH >>> + string "Default value for env_fdt_path variable" >>> + depends on ENV_IMPORT_FDT >>> + default "/config/environment" >>> + help >>> + The initial value of the env_fdt_path variable. >>> + >>> config ENV_APPEND >>> bool "Always append the environment with new data" >>> default n >>> diff --git a/env/common.c b/env/common.c >>> index 2ee423beb5..af45e561ce 100644 >>> --- a/env/common.c >>> +++ b/env/common.c >>> @@ -345,3 +345,26 @@ int env_complete(char *var, int maxv, char *cmdv[], int bufsz, char *buf, >>> return found; >>> } >>> #endif >>> + >>> +#ifdef CONFIG_ENV_IMPORT_FDT >>> +void env_import_fdt(void) >>> +{ >>> + const void *blob = gd->fdt_blob; >>> + const char *path; >>> + int offset, prop; >>> + >>> + path = env_get("env_fdt_path"); >>> + if (!path || !path[0]) >>> + return; >> >> How about returning an error code indicating what happened? > > I considered that, but I'm not sure what the (single) caller would do > with that. Not having env_fdt_path set is a supported use case as I > explain. So here it's not really an error. However, if env_fdt_path is > set, but there's no such node in the DT blob, it might be worth printing > a warning (i.e. in the "offset < 0" case below). > >>> + offset = fdt_path_offset(blob, path); >> >> Could we use the livetree API here (ofnode)? > > Dunno, what's that? Have U-Boot started deserializing the FDT blob like > the linux kernel does and build an in-memory representation that's > easier to traverse? Ah, ok, I see, something like node = ofnode_path(path); if (!ofnode_valid(node)) /* no such node */ But I can't find any ofnode_for_each_property, though I guess it's just as easy to open-code it like test/dm/ofread.c does. Thanks, I'll give it a try. Rasmus
diff --git a/common/board_r.c b/common/board_r.c index c835ff8e26..3f82404772 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -459,6 +459,8 @@ static int initr_env(void) else env_set_default(NULL, 0); + env_import_fdt(); + if (IS_ENABLED(CONFIG_OF_CONTROL)) env_set_hex("fdtcontroladdr", (unsigned long)map_to_sysmem(gd->fdt_blob)); diff --git a/env/Kconfig b/env/Kconfig index b473d7cfe1..aa800e37ce 100644 --- a/env/Kconfig +++ b/env/Kconfig @@ -647,6 +647,24 @@ config DELAY_ENVIRONMENT later by U-Boot code. With CONFIG_OF_CONTROL this is instead controlled by the value of /config/load-environment. +config ENV_IMPORT_FDT + bool "Amend environment by FDT properties" + depends on OF_CONTROL + help + If selected, after the environment has been loaded from its + persistent location, the "env_fdt_path" variable is looked + up and used as a path to a node in the control DTB. The + property/value pairs in that node is then used to update the + run-time environment. This can be useful to use the same + U-Boot binary with different board variants. + +config ENV_FDT_PATH + string "Default value for env_fdt_path variable" + depends on ENV_IMPORT_FDT + default "/config/environment" + help + The initial value of the env_fdt_path variable. + config ENV_APPEND bool "Always append the environment with new data" default n diff --git a/env/common.c b/env/common.c index 2ee423beb5..af45e561ce 100644 --- a/env/common.c +++ b/env/common.c @@ -345,3 +345,26 @@ int env_complete(char *var, int maxv, char *cmdv[], int bufsz, char *buf, return found; } #endif + +#ifdef CONFIG_ENV_IMPORT_FDT +void env_import_fdt(void) +{ + const void *blob = gd->fdt_blob; + const char *path; + int offset, prop; + + path = env_get("env_fdt_path"); + if (!path || !path[0]) + return; + offset = fdt_path_offset(blob, path); + if (offset < 0) + return; + + fdt_for_each_property_offset(prop, blob, offset) { + const char *name, *val; + + val = fdt_getprop_by_offset(blob, prop, &name, NULL); + env_set(name, val); + } +} +#endif diff --git a/include/env.h b/include/env.h index c15339a93f..6296502595 100644 --- a/include/env.h +++ b/include/env.h @@ -377,4 +377,19 @@ int env_get_char(int index); * This is used for those unfortunate archs with crappy toolchains */ void env_reloc(void); + + +/** + * env_import_fdt() - Import environment values from device tree blob + * + * This uses the value of the environment variable "env_fdt_path" as a + * path to an fdt node, whose property/value pairs are added to the + * environment. + */ +#ifdef CONFIG_ENV_IMPORT_FDT +void env_import_fdt(void); +#else +static inline void env_import_fdt(void) {} +#endif + #endif diff --git a/include/env_default.h b/include/env_default.h index ea31a8eddf..1ddd64ba8f 100644 --- a/include/env_default.h +++ b/include/env_default.h @@ -103,6 +103,9 @@ const uchar default_environment[] = { #ifdef CONFIG_SYS_SOC "soc=" CONFIG_SYS_SOC "\0" #endif +#ifdef CONFIG_ENV_IMPORT_FDT + "env_fdt_path=" CONFIG_ENV_FDT_PATH "\0" +#endif #endif #if defined(CONFIG_BOOTCOUNT_BOOTLIMIT) && (CONFIG_BOOTCOUNT_BOOTLIMIT > 0) "bootlimit=" __stringify(CONFIG_BOOTCOUNT_BOOTLIMIT)"\0"
It can be useful to use the same U-Boot binary for multiple purposes, say the normal one, one for developers that allow breaking into the U-Boot shell, and one for use during bootstrapping which runs a special-purpose bootcmd. Or one can have several board variants that can share almost all boot logic, but just needs a few tweaks in the variables used by the boot script. To that end, allow the control dtb to contain a /config/enviroment node (or whatever one puts in fdt_env_path variable), whose property/value pairs are used to update the run-time environment after it has been loaded from its persistent location. The indirection via fdt_env_path is for maximum flexibility - for example, should the user wish (or board logic dictate) that the values in the DTB should no longer be applied, one simply needs to delete the fdt_env_path variable; that can even be done automatically by including a fdt_env_path = ""; property in the DTB node. Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk> --- common/board_r.c | 2 ++ env/Kconfig | 18 ++++++++++++++++++ env/common.c | 23 +++++++++++++++++++++++ include/env.h | 15 +++++++++++++++ include/env_default.h | 3 +++ 5 files changed, 61 insertions(+)