[PATCH 1/2] env: allow environment to be amended from control dtb

[Simon Glass]

Hi Rasmus,

On Wed, 14 Apr 2021 at 10:43, Rasmus Villemoes
<rasmus.villemoes at 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 at 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 at 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