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.waschk...@men.de>

Reviewed-by: Stefan Roese <s...@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;
                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

--
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de

Reply via email to