[U-Boot] [PATCH v3 2/2] env: mmc: add erase-function

[Simon Goldschmidt]

On Fri, Apr 12, 2019 at 11:38 AM Frank Wunderlich
<frank-w at public-files.de> wrote:
>

Missing description here. I just saw I forgot this comment for the 1/2
patch, too.
There's no description there as well.

> Signed-off-by: Frank Wunderlich <frank-w at public-files.de>
> ---
>  env/mmc.c | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
>
> diff --git a/env/mmc.c b/env/mmc.c
> index c3cf35d01b..f387a53970 100644
> --- a/env/mmc.c
> +++ b/env/mmc.c
> @@ -242,6 +242,33 @@ fini:
>         fini_mmc_for_env(mmc);
>         return ret;
>  }
> +
> +static int env_mmc_erase(void)
> +{
> +       int dev = mmc_get_env_dev();
> +       struct mmc *mmc = find_mmc_device(dev);
> +       int     n, blk, cnt;

Bogus indent?

> +
> +       if (!mmc)
> +               return CMD_RET_FAILURE;
> +
> +       blk = CONFIG_ENV_OFFSET / mmc->read_bl_len;
> +       cnt = CONFIG_ENV_SIZE / mmc->read_bl_len;

Hmm, this doesn't work with redundant env. To fix this, you probably
need to touch patch 1/2, add a parameter to the command specifying
which env to erase (or both) and pass it to this callback.

> +
> +       printf("\nMMC erase env: dev # %d, block # %d (0x%x), count %d (0x%x)\n",
> +              dev, blk, blk * mmc->read_bl_len,
> +              cnt, cnt * mmc->read_bl_len);
> +
> +       if (mmc_getwp(mmc) == 1) {
> +               printf("Error: card is write protected!\n");
> +               return CMD_RET_FAILURE;
> +       }
> +       n = blk_derase(mmc_get_blk_desc(mmc), blk, cnt);
> +       printf("%d blocks erased: %s\n", n, (n == cnt) ? "OK" : "ERROR");
> +
> +       return (n == cnt) ? CMD_RET_SUCCESS : CMD_RET_FAILURE;
> +}

Overall, I think this function should more resemble the save function.
That should make it easier to follow for someone trying to understand
this file.

Regards,
Simon

> +
>  #endif /* CONFIG_CMD_SAVEENV && !CONFIG_SPL_BUILD */
>
>  static inline int read_env(struct mmc *mmc, unsigned long size,
> @@ -351,5 +378,6 @@ U_BOOT_ENV_LOCATION(mmc) = {
>         .load           = env_mmc_load,
>  #ifndef CONFIG_SPL_BUILD
>         .save           = env_save_ptr(env_mmc_save),
> +       .erase          = env_mmc_erase,
>  #endif
>  };
> --
> 2.17.1