[PATCH v2 1/1] env: sf: single function env_sf_save()
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 ---
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];
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.
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;
ret = spi_flash_read(env_flash, saved_offset,saved_size, saved_buffer);
ret = spi_flash_read(env_flash, saved_offset, saved_size,saved_buffer);@@ -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);
- 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);@@ -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;
Viele Grüße, Stefan
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
participants (3)
-
Harry Waschkeit -
Harry Waschkeit
-
Stefan Roese