[U-Boot] [PATCH v3 1/2] env: register erase command

[Simon Goldschmidt]

On Fri, Apr 12, 2019 at 11:38 AM Frank Wunderlich
<frank-w at public-files.de> wrote:
>
> Signed-off-by: Frank Wunderlich <frank-w at public-files.de>
> ---
>  cmd/nvedit.c          | 13 +++++++++++++
>  env/env.c             | 30 ++++++++++++++++++++++++++++++
>  include/environment.h | 17 +++++++++++++++++
>  3 files changed, 60 insertions(+)
>
> diff --git a/cmd/nvedit.c b/cmd/nvedit.c
> index 24a6cf7824..3b5c62d629 100644
> --- a/cmd/nvedit.c
> +++ b/cmd/nvedit.c
> @@ -756,11 +756,22 @@ static int do_env_save(cmd_tbl_t *cmdtp, int flag, int argc,
>         return env_save() ? 1 : 0;
>  }
>
> +static int do_env_erase(cmd_tbl_t *cmdtp, int flag, int argc,
> +                       char * const argv[])
> +{
> +       return env_erase() ? 1 : 0;
> +}
> +
>  U_BOOT_CMD(
>         saveenv, 1, 0,  do_env_save,
>         "save environment variables to persistent storage",
>         ""
>  );
> +U_BOOT_CMD(
> +       eraseenv, 1, 0, do_env_erase,
> +       "erase environment variables from persistent storage",
> +       ""
> +);
>  #endif
>  #endif /* CONFIG_SPL_BUILD */
>
> @@ -1207,6 +1218,7 @@ static cmd_tbl_t cmd_env_sub[] = {
>  #endif
>  #if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_ENV_IS_NOWHERE)
>         U_BOOT_CMD_MKENT(save, 1, 0, do_env_save, "", ""),
> +       U_BOOT_CMD_MKENT(erase, 1, 0, do_env_erase, "", ""),

Guarding this with the same Kconfig option as 'saveenv' might be a bit
confusing?

>  #endif
>         U_BOOT_CMD_MKENT(set, CONFIG_SYS_MAXARGS, 0, do_env_set, "", ""),
>  #if defined(CONFIG_CMD_ENV_EXISTS)
> @@ -1282,6 +1294,7 @@ static char env_help_text[] =
>  #endif
>  #if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_ENV_IS_NOWHERE)
>         "env save - save environment\n"
> +       "env erase - erase environment\n"
>  #endif
>  #if defined(CONFIG_CMD_NVEDIT_EFI)
>         "env set -e name [arg ...] - set UEFI variable; unset if 'arg' not specified\n"
> diff --git a/env/env.c b/env/env.c
> index 4b417b90a2..25441370e6 100644
> --- a/env/env.c
> +++ b/env/env.c
> @@ -24,6 +24,8 @@ void env_fix_drivers(void)
>                         entry->load += gd->reloc_off;
>                 if (entry->save)
>                         entry->save += gd->reloc_off;
> +               if (entry->erase)
> +                       entry->erase += gd->reloc_off;
>                 if (entry->init)
>                         entry->init += gd->reloc_off;
>         }
> @@ -254,6 +256,34 @@ int env_save(void)
>         return -ENODEV;
>  }
>
> +int env_erase(void)
> +{
> +       struct env_driver *drv;
> +
> +       drv = env_driver_lookup(ENVOP_ERASE, gd->env_load_prio);
> +       if (drv) {
> +               int ret;
> +
> +               if (!drv->erase)
> +                       return -ENODEV;
> +
> +               if (!env_has_inited(drv->location))
> +                       return -ENODEV;
> +
> +               printf("Erase Environment on %s... ", drv->name);

Following the other prints, this should probably start with "Erasing".

> +               ret = drv->erase();
> +               if (ret)
> +                       printf("Failed (%d)\n", ret);
> +               else
> +                       printf("OK\n");
> +
> +               if (!ret)
> +                       return 0;
> +       }
> +
> +       return -ENODEV;
> +}
> +
>  int env_init(void)
>  {
>         struct env_driver *drv;
> diff --git a/include/environment.h b/include/environment.h
> index cd96676141..5024cbea7e 100644
> --- a/include/environment.h
> +++ b/include/environment.h
> @@ -200,6 +200,7 @@ enum env_operation {
>         ENVOP_INIT,     /* we want to call the init function */
>         ENVOP_LOAD,     /* we want to call the load function */
>         ENVOP_SAVE,     /* we want to call the save function */
> +       ENVOP_ERASE,    /* we want to call the erase function */
>  };
>
>  struct env_driver {
> @@ -225,6 +226,15 @@ struct env_driver {
>          */
>         int (*save)(void);
>
> +       /**
> +        * erase() - Erase the environment on storage
> +        *
> +        * This method is required for 'eraseenv' to work.

I know you copied this from 'save', but I think you should mention that
it's optional.

Regards,
Simon

> +        *
> +        * @return 0 if OK, -ve on error
> +        */
> +       int (*erase)(void);
> +
>         /**
>          * init() - Set up the initial pre-relocation environment
>          *
> @@ -303,6 +313,13 @@ int env_load(void);
>   */
>  int env_save(void);
>
> +/**
> + * env_erase() - Erase the environment on storage
> + *
> + * @return 0 if OK, -ve on error
> + */
> +int env_erase(void);
> +
>  /**
>   * env_fix_drivers() - Updates envdriver as per relocation
>   */
> --
> 2.17.1