[U-Boot] [PATCH v3 2/3] env: introduce macro ENV_IS_IN_SOMEWHERE

[Simon Glass]

On Wed, 18 Sep 2019 at 03:30, Patrick Delaunay <patrick.delaunay at st.com> wrote:
>
> This patch introduce a macro ENV_IS_IN_SOMEWHERE to check if the
> the environment can be saved somewhere, in a device or in a file system,
> without assumption on CONFIG$(SPL_TPL_)ENV_IS_NOWHERE.
>
> Since the commit 208bd2b85ecc ("env: allow ENV_IS_NOWHERE with
> other storage target"), is is allowed to activated ENV_IS_NOWHERE with
> other CONFIG_IS_IN...
> It is only the case for U-Boot but the restriction in Kconfig
> could also removed for SPL and TPL
> (depends on !SPL_ENV_IS_NOWHERE / depends on !TPL_ENV_IS_NOWHERE).
>
> This macro ENV_IS_IN_DEVICE can be used in code to check if the
> environment for U-Boot / SPL / TPL is really managed (at least one
> CONFIG$(SPL_TPL_)ENV_IS_IN_.. is activated).
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com>
> ---
>
> Changes in v3: None
> Changes in v2: None
>
>  cmd/nvedit.c  | 29 +++++++----------------------
>  include/env.h | 13 +++++++++++++
>  2 files changed, 20 insertions(+), 22 deletions(-)

I feel this is a bit confusion as we have ENV_IS_NOWHERE and
ENV_IS_IN_SOMEWHERE which are opposites. Can they both be true?

So how about adding a comment as to what your new ENV_IS_NOWHERE means?

Also, how about ENV_IS_SOMEWHERE?


>
> diff --git a/cmd/nvedit.c b/cmd/nvedit.c
> index 1cb0bc1460..7a6ec5ae30 100644
> --- a/cmd/nvedit.c
> +++ b/cmd/nvedit.c
> @@ -40,28 +40,13 @@
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> -#if    defined(CONFIG_ENV_IS_IN_EEPROM)        || \
> -       defined(CONFIG_ENV_IS_IN_FLASH)         || \
> -       defined(CONFIG_ENV_IS_IN_MMC)           || \
> -       defined(CONFIG_ENV_IS_IN_FAT)           || \
> -       defined(CONFIG_ENV_IS_IN_EXT4)          || \
> -       defined(CONFIG_ENV_IS_IN_NAND)          || \
> -       defined(CONFIG_ENV_IS_IN_NVRAM)         || \
> -       defined(CONFIG_ENV_IS_IN_ONENAND)       || \
> -       defined(CONFIG_ENV_IS_IN_SATA)          || \
> -       defined(CONFIG_ENV_IS_IN_SPI_FLASH)     || \
> -       defined(CONFIG_ENV_IS_IN_REMOTE)        || \
> -       defined(CONFIG_ENV_IS_IN_UBI)
> -
> -#define ENV_IS_IN_DEVICE
> -
> -#endif
> -
> -#if    !defined(ENV_IS_IN_DEVICE)              && \
> -       !defined(CONFIG_ENV_IS_NOWHERE)
> +#if !defined(CONFIG_SPL_BUILD) || CONFIG_IS_ENABLED(ENV_SUPPORT)
> +#if    !ENV_IS_IN_SOMEWHERE            && \
> +       !CONFIG_IS_ENABLED(ENV_IS_NOWHERE)
>  # error Define one of CONFIG_ENV_IS_IN_{EEPROM|FLASH|MMC|FAT|EXT4|\
>  NAND|NVRAM|ONENAND|SATA|SPI_FLASH|REMOTE|UBI} or CONFIG_ENV_IS_NOWHERE
>  #endif
> +#endif
>
>  /*
>   * Maximum expected input data size for import command
> @@ -744,7 +729,7 @@ ulong env_get_ulong(const char *name, int base, ulong default_val)
>  }
>
>  #ifndef CONFIG_SPL_BUILD
> -#if defined(CONFIG_CMD_SAVEENV) && defined(ENV_IS_IN_DEVICE)
> +#if defined(CONFIG_CMD_SAVEENV) && ENV_IS_IN_SOMEWHERE
>  static int do_env_save(cmd_tbl_t *cmdtp, int flag, int argc,
>                        char * const argv[])
>  {
> @@ -1309,7 +1294,7 @@ static cmd_tbl_t cmd_env_sub[] = {
>  #if defined(CONFIG_CMD_RUN)
>         U_BOOT_CMD_MKENT(run, CONFIG_SYS_MAXARGS, 1, do_run, "", ""),
>  #endif
> -#if defined(CONFIG_CMD_SAVEENV) && defined(ENV_IS_IN_DEVICE)
> +#if defined(CONFIG_CMD_SAVEENV) && ENV_IS_IN_SOMEWHERE
>         U_BOOT_CMD_MKENT(save, 1, 0, do_env_save, "", ""),
>  #if defined(CONFIG_CMD_ERASEENV)
>         U_BOOT_CMD_MKENT(erase, 1, 0, do_env_erase, "", ""),
> @@ -1392,7 +1377,7 @@ static char env_help_text[] =
>  #if defined(CONFIG_CMD_RUN)
>         "env run var [...] - run commands in an environment variable\n"
>  #endif
> -#if defined(CONFIG_CMD_SAVEENV) && defined(ENV_IS_IN_DEVICE)
> +#if defined(CONFIG_CMD_SAVEENV) && ENV_IS_IN_SOMEWHERE
>         "env save - save environment\n"
>  #if defined(CONFIG_CMD_ERASEENV)
>         "env erase - erase environment\n"
> diff --git a/include/env.h b/include/env.h
> index a74a261337..0088d3b1e8 100644
> --- a/include/env.h
> +++ b/include/env.h
> @@ -35,6 +35,19 @@ struct env_clbk_tbl {
>                         int flags);
>  };
>
> +#define ENV_IS_IN_SOMEWHERE \
> +               (CONFIG_IS_ENABLED(ENV_IS_IN_EEPROM) || \
> +                CONFIG_IS_ENABLED(ENV_IS_IN_EXT4) || \
> +                CONFIG_IS_ENABLED(ENV_IS_IN_FAT) || \
> +                CONFIG_IS_ENABLED(ENV_IS_IN_FLASH) || \
> +                CONFIG_IS_ENABLED(ENV_IS_IN_MMC) || \
> +                CONFIG_IS_ENABLED(ENV_IS_IN_NAND) || \
> +                CONFIG_IS_ENABLED(ENV_IS_IN_NVRAM) || \
> +                CONFIG_IS_ENABLED(ENV_IS_IN_ONENAND) || \
> +                CONFIG_IS_ENABLED(ENV_IS_IN_REMOTE) || \
> +                CONFIG_IS_ENABLED(ENV_IS_IN_SPI_FLASH) || \
> +                CONFIG_IS_ENABLED(ENV_IS_IN_UBI))
> +
>  /*
>   * Define a callback that can be associated with variables.
>   * when associated through the ".callbacks" environment variable, the callback
> --
> 2.17.1
>

Regards,
Simon