On 01.02.21 13:07, Stefan Roese wrote:
On 01.02.21 13:03, Harry Waschkeit wrote:
Instead of implementing redundant environments in two very similar functions env_sf_save(), handle redundancy in one function, placing the few differences in appropriate pre-compiler sections depending on config option CONFIG_ENV_OFFSET_REDUND.
Additionally, several checkpatch complaints were addressed.
This patch is in preparation for adding support for env erase.
Signed-off-by: Harry Waschkeit Harry.Waschkeit@men.de
Reviewed-by: Stefan Roese sr@denx.de
BTW: Please re-add the tags (RB etc) into subsequent patch versions, so that they are available in the git history.
Oh, yeah, of course, sorry for that ... :-(
Thanks, Harry
Thanks, Stefan
Changes in v2: - remove one more #ifdef, instead take advantage of compiler attribute __maybe_unused for one variable used only in case of redundant environments
env/sf.c | 130 ++++++++++++++++++------------------------------------- 1 file changed, 41 insertions(+), 89 deletions(-)
diff --git a/env/sf.c b/env/sf.c index 937778aa37..199114fd3b 100644 --- a/env/sf.c +++ b/env/sf.c @@ -66,13 +66,14 @@ static int setup_flash_device(void) return 0; } -#if defined(CONFIG_ENV_OFFSET_REDUND) static int env_sf_save(void) { env_t env_new; - char *saved_buffer = NULL, flag = ENV_REDUND_OBSOLETE; + char *saved_buffer = NULL; u32 saved_size, saved_offset, sector; + ulong offset; int ret; + __maybe_unused char flag = ENV_REDUND_OBSOLETE; ret = setup_flash_device(); if (ret) @@ -81,27 +82,33 @@ static int env_sf_save(void) ret = env_export(&env_new); if (ret) return -EIO; - env_new.flags = ENV_REDUND_ACTIVE; - if (gd->env_valid == ENV_VALID) { - env_new_offset = CONFIG_ENV_OFFSET_REDUND; - env_offset = CONFIG_ENV_OFFSET; - } else { - env_new_offset = CONFIG_ENV_OFFSET; - env_offset = CONFIG_ENV_OFFSET_REDUND; - } +#if CONFIG_IS_ENABLED(SYS_REDUNDAND_ENVIRONMENT) + env_new.flags = ENV_REDUND_ACTIVE;
+ if (gd->env_valid == ENV_VALID) { + env_new_offset = CONFIG_ENV_OFFSET_REDUND; + env_offset = CONFIG_ENV_OFFSET; + } else { + env_new_offset = CONFIG_ENV_OFFSET; + env_offset = CONFIG_ENV_OFFSET_REDUND; + } + offset = env_new_offset; +#else + offset = CONFIG_ENV_OFFSET; +#endif /* 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 = env_new_offset + CONFIG_ENV_SIZE; + saved_offset = offset + CONFIG_ENV_SIZE; saved_buffer = memalign(ARCH_DMA_MINALIGN, saved_size); if (!saved_buffer) { ret = -ENOMEM; goto done; } - ret = spi_flash_read(env_flash, saved_offset, - saved_size, saved_buffer); + ret = spi_flash_read(env_flash, saved_offset, saved_size, + saved_buffer); if (ret) goto done; } @@ -109,35 +116,39 @@ static int env_sf_save(void) sector = DIV_ROUND_UP(CONFIG_ENV_SIZE, CONFIG_ENV_SECT_SIZE); puts("Erasing SPI flash..."); - ret = spi_flash_erase(env_flash, env_new_offset, - sector * CONFIG_ENV_SECT_SIZE); + ret = spi_flash_erase(env_flash, offset, + sector * CONFIG_ENV_SECT_SIZE); if (ret) goto done; puts("Writing to SPI flash..."); - ret = spi_flash_write(env_flash, env_new_offset, - CONFIG_ENV_SIZE, &env_new); + ret = spi_flash_write(env_flash, offset, + CONFIG_ENV_SIZE, &env_new); if (ret) goto done; if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) { - ret = spi_flash_write(env_flash, saved_offset, - saved_size, saved_buffer); + ret = spi_flash_write(env_flash, saved_offset, saved_size, + saved_buffer); if (ret) goto done; } - ret = spi_flash_write(env_flash, env_offset + offsetof(env_t, flags), - sizeof(env_new.flags), &flag); - if (ret) - goto done; +#if CONFIG_IS_ENABLED(SYS_REDUNDAND_ENVIRONMENT) + ret = spi_flash_write(env_flash, env_offset + offsetof(env_t, flags), + sizeof(env_new.flags), &flag); + if (ret) + goto done; - puts("done\n"); + puts("done\n"); - gd->env_valid = gd->env_valid == ENV_REDUND ? ENV_VALID : ENV_REDUND; + gd->env_valid = gd->env_valid == ENV_REDUND ? ENV_VALID : ENV_REDUND; - printf("Valid environment: %d\n", (int)gd->env_valid); + printf("Valid environment: %d\n", (int)gd->env_valid); +#else + puts("done\n"); +#endif done: if (saved_buffer) @@ -146,6 +157,7 @@ static int env_sf_save(void) return ret; } +#if CONFIG_IS_ENABLED(SYS_REDUNDAND_ENVIRONMENT) static int env_sf_load(void) { int ret; @@ -183,66 +195,6 @@ out: return ret; } #else -static int env_sf_save(void) -{ - u32 saved_size, saved_offset, sector; - char *saved_buffer = NULL; - int ret = 1; - env_t env_new;
- 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; - }
- ret = env_export(&env_new); - 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;
- puts("Writing to SPI flash..."); - ret = spi_flash_write(env_flash, CONFIG_ENV_OFFSET, - CONFIG_ENV_SIZE, &env_new); - if (ret) - goto done;
- if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) { - ret = spi_flash_write(env_flash, saved_offset, - saved_size, saved_buffer); - if (ret) - goto done; - }
- ret = 0; - puts("done\n");
- done:
- if (saved_buffer) - free(saved_buffer);
- return ret; -}
static int env_sf_load(void) { int ret; @@ -258,8 +210,8 @@ static int env_sf_load(void) if (ret) goto out; - ret = spi_flash_read(env_flash, - CONFIG_ENV_OFFSET, CONFIG_ENV_SIZE, buf); + ret = spi_flash_read(env_flash, CONFIG_ENV_OFFSET, CONFIG_ENV_SIZE, + buf); if (ret) { env_set_default("spi_flash_read() failed", 0); goto err_read; @@ -292,7 +244,7 @@ static int env_sf_init(void) env_t *env_ptr = (env_t *)env_sf_get_env_addr(); if (crc32(0, env_ptr->data, ENV_SIZE) == env_ptr->crc) { - gd->env_addr = (ulong)&(env_ptr->data); + gd->env_addr = (ulong)&env_ptr->data; gd->env_valid = 1; } else { gd->env_addr = (ulong)&default_environment[0];
Viele Grüße, Stefan