[PATCH] env: sf: add support for env erase

Sean Anderson seanga2 at gmail.com
Fri Oct 9 15:55:04 CEST 2020


On 10/8/20 1:27 PM, Harry Waschkeit wrote:
> Command "env erase" didn't work even though CONFIG_CMD_ERASEENV was
> defined, because serial flash environment routines didn't implement
> erase method.
> 
> Signed-off-by: Waschkeit, Harry <Harry.Waschkeit at men.de>
> ---

Hi Harry,

I wanted to test out your patch, but I couldn't get it to apply. It
appears that your mail program has replaced the tabs with spaces, so git
can't figure out how to apply it. I tried to fix it by performing the
substitutions

	s/^\(.\)       /\1\t/g
	s/        /\t/g

but it still wouldn't apply. In addition, checkpatch has a few warnings:

> $ scripts/checkpatch.pl env-sf-add-support-for-env-erase.patch
> WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible
> #152: FILE: env/sf.c:149:
> +#ifdef CONFIG_CMD_ERASEENV
> 
> WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible
> #240: FILE: env/sf.c:318:
> +#ifdef CONFIG_CMD_ERASEENV
> 
> CHECK: Alignment should match open parenthesis
> #260: FILE: env/sf.c:338:
> +		ret = spi_flash_read(env_flash, saved_offset,
> +			saved_size, saved_buffer);
> 
> CHECK: Alignment should match open parenthesis
> #269: FILE: env/sf.c:347:
> +	ret = spi_flash_erase(env_flash, CONFIG_ENV_OFFSET,
> +		sector * CONFIG_ENV_SECT_SIZE);
> 
> CHECK: Alignment should match open parenthesis
> #276: FILE: env/sf.c:354:
> +		ret = spi_flash_write(env_flash, saved_offset,
> +			saved_size, saved_buffer);
> 
> WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible
> #307: FILE: env/sf.c:437:
> +#if defined(CONFIG_CMD_ERASEENV) && defined(CONFIG_ENV_ADDR)
> 
> WARNING: Missing Signed-off-by: line by nominal patch author 'Harry Waschkeit <harry.waschkeit at men.de>'
> 
> total: 0 errors, 4 warnings, 3 checks, 158 lines checked
> 
> NOTE: For some of the reported defects, checkpatch may be able to
>       mechanically convert to the typical style using --fix or --fix-inplace.
> 
> env-sf-add-support-for-env-erase.patch has style problems, please review.
> 
> NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX MULTISTATEMENT_MACRO_USE_DO_WHILE NETWORKING_BLOCK_COMMENT_STYLE PREFER_ETHER_ADDR_COPY USLEEP_RANGE
> 
> NOTE: If any of the errors are false positives, please report
>       them to the maintainer, see CHECKPATCH in MAINTAINERS.

Please fix these issues and resend, thanks!

--Sean

>  env/sf.c | 130 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 128 insertions(+), 2 deletions(-)
> 
> diff --git a/env/sf.c b/env/sf.c
> index 937778aa37..9cda192a73 100644
> --- a/env/sf.c
> +++ b/env/sf.c
> @@ -146,6 +146,78 @@ static int env_sf_save(void)
>         return ret;
>  }
>  
> +#ifdef CONFIG_CMD_ERASEENV
> +static int env_sf_erase(void)
> +{
> +       char    *saved_buffer = NULL;
> +       u32     saved_size, saved_offset, sector;
> +       ulong   offset;
> +       ulong   offsets[2] = { CONFIG_ENV_OFFSET, CONFIG_ENV_OFFSET_REDUND };
> +       int     i;
> +       int     ret;
> +
> +       ret = setup_flash_device();
> +       if (ret)
> +               return ret;
> +
> +       /* get temporary storage if sector is larger than env (i.e. embedded) */
> +       if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) {
> +               saved_size = CONFIG_ENV_SECT_SIZE - CONFIG_ENV_SIZE;
> +               saved_buffer = memalign(ARCH_DMA_MINALIGN, saved_size);
> +               if (!saved_buffer) {
> +                       ret = -ENOMEM;
> +                       goto done;
> +               }
> +       }
> +
> +       sector = DIV_ROUND_UP(CONFIG_ENV_SIZE, CONFIG_ENV_SECT_SIZE);
> +
> +       /* simply erase both environments, retaining non-env data (if any) */
> +       for (i = 0; i < ARRAY_SIZE(offsets); i++) {
> +               offset = offsets[i];
> +
> +               if (saved_buffer) {
> +                       saved_offset = offset + CONFIG_ENV_SIZE;
> +                       ret = spi_flash_read(env_flash, saved_offset,
> +                                            saved_size, saved_buffer);
> +                       if (ret)
> +                               goto done;
> +               }
> +
> +               if (i)
> +                       puts("Redund:");
> +
> +               puts("Erasing SPI flash...");
> +               ret = spi_flash_erase(env_flash, offset,
> +                                     sector * CONFIG_ENV_SECT_SIZE);
> +               if (ret)
> +                       goto done;
> +
> +               if (saved_buffer) {
> +                       puts("Writing non-environment data to SPI flash...");
> +                       ret = spi_flash_write(env_flash, saved_offset,
> +                                             saved_size, saved_buffer);
> +                       if (ret)
> +                               goto done;
> +               }
> +
> +               puts("done\n");
> +       }
> +
> +       /* here we know that both env sections are cleared */
> +       env_new_offset = CONFIG_ENV_OFFSET;
> +       env_offset = CONFIG_ENV_OFFSET_REDUND;
> +
> +       gd->env_valid = ENV_INVALID;
> +
> + done:
> +       if (saved_buffer)
> +               free(saved_buffer);
> +
> +       return ret;
> +}
> +#endif /* CONFIG_CMD_ERASEENV */
> +
>  static int env_sf_load(void)
>  {
>         int ret;
> @@ -182,7 +254,7 @@ out:
>  
>         return ret;
>  }
> -#else
> +#else  /* #if defined(CONFIG_ENV_OFFSET_REDUND) */
>  static int env_sf_save(void)
>  {
>         u32     saved_size, saved_offset, sector;
> @@ -243,6 +315,57 @@ static int env_sf_save(void)
>         return ret;
>  }
>  
> +#ifdef CONFIG_CMD_ERASEENV
> +static int env_sf_erase(void)
> +{
> +       u32     saved_size, saved_offset, sector;
> +       char    *saved_buffer = NULL;
> +       int     ret = 1;
> +
> +       ret = setup_flash_device();
> +       if (ret)
> +               return ret;
> +
> +       /* Is the sector larger than the env (i.e. embedded) */
> +       if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) {
> +               saved_size = CONFIG_ENV_SECT_SIZE - CONFIG_ENV_SIZE;
> +               saved_offset = CONFIG_ENV_OFFSET + CONFIG_ENV_SIZE;
> +               saved_buffer = malloc(saved_size);
> +               if (!saved_buffer)
> +                       goto done;
> +
> +               ret = spi_flash_read(env_flash, saved_offset,
> +                       saved_size, saved_buffer);
> +               if (ret)
> +                       goto done;
> +       }
> +
> +       sector = DIV_ROUND_UP(CONFIG_ENV_SIZE, CONFIG_ENV_SECT_SIZE);
> +
> +       puts("Erasing SPI flash...");
> +       ret = spi_flash_erase(env_flash, CONFIG_ENV_OFFSET,
> +               sector * CONFIG_ENV_SECT_SIZE);
> +       if (ret)
> +               goto done;
> +
> +       if (saved_buffer) {
> +               puts("Writing non-environment data to SPI flash...");
> +               ret = spi_flash_write(env_flash, saved_offset,
> +                       saved_size, saved_buffer);
> +               if (ret)
> +                       goto done;
> +       }
> +
> +       puts("done\n");
> +
> + done:
> +       if (saved_buffer)
> +               free(saved_buffer);
> +
> +       return ret;
> +}
> +#endif /* CONFIG_CMD_ERASEENV */
> +
>  static int env_sf_load(void)
>  {
>         int ret;
> @@ -277,7 +400,7 @@ out:
>  
>         return ret;
>  }
> -#endif
> +#endif  /* #if defined(CONFIG_ENV_OFFSET_REDUND) #else */
>  
>  #if CONFIG_ENV_ADDR != 0x0
>  __weak void *env_sf_get_env_addr(void)
> @@ -311,4 +434,7 @@ U_BOOT_ENV_LOCATION(sf) = {
>  #if defined(INITENV) && (CONFIG_ENV_ADDR != 0x0)
>         .init           = env_sf_init,
>  #endif
> +#if defined(CONFIG_CMD_ERASEENV) && defined(CONFIG_ENV_ADDR)
> +       .erase          = env_sf_erase,
> +#endif
>  };



More information about the U-Boot mailing list