The load() methods have inconsistent behaviour on error. Some of them load
an empty default environment. Some load an environment containing an error
message. Others do nothing.

As a step in the right direction, have the method return an error code.
Then the caller could handle this itself in a consistent way.

Signed-off-by: Simon Glass <s...@chromium.org>
---

Changes in v4: None
Changes in v3:
- Rebase to master
- Rebase to master, dropping patches already applied

Changes in v2:
- Rebase to master

 env/dataflash.c       |  4 +++-
 env/eeprom.c          |  4 +++-
 env/ext4.c            |  6 ++++--
 env/fat.c             |  6 ++++--
 env/flash.c           |  4 +++-
 env/mmc.c             | 19 +++++++++++--------
 env/nand.c            | 17 ++++++++++++-----
 env/nvram.c           |  4 +++-
 env/onenand.c         |  4 +++-
 env/remote.c          |  4 +++-
 env/sata.c            | 15 +++++++++------
 env/sf.c              | 20 +++++++++++++-------
 env/ubi.c             | 14 +++++++++-----
 include/environment.h |  4 +++-
 14 files changed, 83 insertions(+), 42 deletions(-)

diff --git a/env/dataflash.c b/env/dataflash.c
index afa08f8fd5..77bc595e0d 100644
--- a/env/dataflash.c
+++ b/env/dataflash.c
@@ -23,7 +23,7 @@ static int env_dataflash_get_char(int index)
        return c;
 }
 
-static void env_dataflash_load(void)
+static int env_dataflash_load(void)
 {
        ulong crc, new = 0;
        unsigned off;
@@ -44,6 +44,8 @@ static void env_dataflash_load(void)
                env_import(buf, 1);
        else
                set_default_env("!bad CRC");
+
+       return 0;
 }
 
 #ifdef CONFIG_ENV_OFFSET_REDUND
diff --git a/env/eeprom.c b/env/eeprom.c
index fbe4fd4efc..08ef6307fc 100644
--- a/env/eeprom.c
+++ b/env/eeprom.c
@@ -76,7 +76,7 @@ static int env_eeprom_get_char(int index)
        return c;
 }
 
-static void env_eeprom_load(void)
+static int env_eeprom_load(void)
 {
        char buf_env[CONFIG_ENV_SIZE];
        unsigned int off = CONFIG_ENV_OFFSET;
@@ -182,6 +182,8 @@ static void env_eeprom_load(void)
                off, (uchar *)buf_env, CONFIG_ENV_SIZE);
 
        env_import(buf_env, 1);
+
+       return 0;
 }
 
 static int env_eeprom_save(void)
diff --git a/env/ext4.c b/env/ext4.c
index ee073a8b7c..65202213d3 100644
--- a/env/ext4.c
+++ b/env/ext4.c
@@ -75,7 +75,7 @@ static int env_ext4_save(void)
 }
 #endif /* CONFIG_CMD_SAVEENV */
 
-static void env_ext4_load(void)
+static int env_ext4_load(void)
 {
        ALLOC_CACHE_ALIGN_BUFFER(char, buf, CONFIG_ENV_SIZE);
        struct blk_desc *dev_desc = NULL;
@@ -109,10 +109,12 @@ static void env_ext4_load(void)
        }
 
        env_import(buf, 1);
-       return;
+       return 0;
 
 err_env_relocate:
        set_default_env(NULL);
+
+       return -EIO;
 }
 
 U_BOOT_ENV_LOCATION(ext4) = {
diff --git a/env/fat.c b/env/fat.c
index 69319f8834..83d4ba1340 100644
--- a/env/fat.c
+++ b/env/fat.c
@@ -74,7 +74,7 @@ static int env_fat_save(void)
 #endif /* CMD_SAVEENV */
 
 #ifdef LOADENV
-static void env_fat_load(void)
+static int env_fat_load(void)
 {
        ALLOC_CACHE_ALIGN_BUFFER(char, buf, CONFIG_ENV_SIZE);
        struct blk_desc *dev_desc = NULL;
@@ -103,10 +103,12 @@ static void env_fat_load(void)
        }
 
        env_import(buf, 1);
-       return;
+       return 0;
 
 err_env_relocate:
        set_default_env(NULL);
+
+       return -EIO;
 }
 #endif /* LOADENV */
 
diff --git a/env/flash.c b/env/flash.c
index 2d72c51622..b60be57a8d 100644
--- a/env/flash.c
+++ b/env/flash.c
@@ -308,7 +308,7 @@ done:
 #endif /* CONFIG_ENV_ADDR_REDUND */
 
 #ifdef LOADENV
-static void env_flash_load(void)
+static int env_flash_load(void)
 {
 #ifdef CONFIG_ENV_ADDR_REDUND
        if (gd->env_addr != (ulong)&(flash_addr->data)) {
@@ -352,6 +352,8 @@ static void env_flash_load(void)
 #endif /* CONFIG_ENV_ADDR_REDUND */
 
        env_import((char *)flash_addr, 1);
+
+       return 0;
 }
 #endif /* LOADENV */
 
diff --git a/env/mmc.c b/env/mmc.c
index 88ffc91f0b..3f3092d975 100644
--- a/env/mmc.c
+++ b/env/mmc.c
@@ -207,7 +207,7 @@ static inline int read_env(struct mmc *mmc, unsigned long 
size,
 }
 
 #ifdef CONFIG_ENV_OFFSET_REDUND
-static void env_mmc_load(void)
+static int env_mmc_load(void)
 {
 #if !defined(ENV_IS_EMBEDDED)
        struct mmc *mmc;
@@ -224,13 +224,13 @@ static void env_mmc_load(void)
 
        errmsg = init_mmc_for_env(mmc);
        if (errmsg) {
-               ret = 1;
+               ret = -EIO;
                goto err;
        }
 
        if (mmc_get_env_addr(mmc, 0, &offset1) ||
            mmc_get_env_addr(mmc, 1, &offset2)) {
-               ret = 1;
+               ret = -EIO;
                goto fini;
        }
 
@@ -245,7 +245,7 @@ static void env_mmc_load(void)
 
        if (read1_fail && read2_fail) {
                errmsg = "!bad CRC";
-               ret = 1;
+               ret = -EIO;
                goto fini;
        } else if (!read1_fail && read2_fail) {
                gd->env_valid = ENV_VALID;
@@ -264,10 +264,12 @@ fini:
 err:
        if (ret)
                set_default_env(errmsg);
+
 #endif
+       return ret;
 }
 #else /* ! CONFIG_ENV_OFFSET_REDUND */
-static void env_mmc_load(void)
+static int env_mmc_load(void)
 {
 #if !defined(ENV_IS_EMBEDDED)
        ALLOC_CACHE_ALIGN_BUFFER(char, buf, CONFIG_ENV_SIZE);
@@ -281,18 +283,18 @@ static void env_mmc_load(void)
 
        errmsg = init_mmc_for_env(mmc);
        if (errmsg) {
-               ret = 1;
+               ret = -EIO;
                goto err;
        }
 
        if (mmc_get_env_addr(mmc, 0, &offset)) {
-               ret = 1;
+               ret = -EIO;
                goto fini;
        }
 
        if (read_env(mmc, CONFIG_ENV_SIZE, offset, buf)) {
                errmsg = "!read failed";
-               ret = 1;
+               ret = -EIO;
                goto fini;
        }
 
@@ -305,6 +307,7 @@ err:
        if (ret)
                set_default_env(errmsg);
 #endif
+       return ret;
 }
 #endif /* CONFIG_ENV_OFFSET_REDUND */
 
diff --git a/env/nand.c b/env/nand.c
index e74a8c674e..dea7b00720 100644
--- a/env/nand.c
+++ b/env/nand.c
@@ -302,7 +302,7 @@ int get_nand_env_oob(struct mtd_info *mtd, unsigned long 
*result)
        }
 
        if (oob_buf[0] == ENV_OOB_MARKER) {
-               *result = oob_buf[1] * mtd->erasesize;
+               *result = ovoid ob_buf[1] * mtd->erasesize;
        } else if (oob_buf[0] == ENV_OOB_MARKER_OLD) {
                *result = oob_buf[1];
        } else {
@@ -315,17 +315,21 @@ int get_nand_env_oob(struct mtd_info *mtd, unsigned long 
*result)
 #endif
 
 #ifdef CONFIG_ENV_OFFSET_REDUND
-static void env_nand_load(void)
+static int env_nand_load(void)
 {
-#if !defined(ENV_IS_EMBEDDED)
+#if defined(ENV_IS_EMBEDDED)
+       return 0;
+#else
        int read1_fail = 0, read2_fail = 0;
        env_t *tmp_env1, *tmp_env2;
+       int ret = 0;
 
        tmp_env1 = (env_t *)malloc(CONFIG_ENV_SIZE);
        tmp_env2 = (env_t *)malloc(CONFIG_ENV_SIZE);
        if (tmp_env1 == NULL || tmp_env2 == NULL) {
                puts("Can't allocate buffers for environment\n");
                set_default_env("!malloc() failed");
+               ret = -EIO;
                goto done;
        }
 
@@ -355,6 +359,7 @@ done:
        free(tmp_env1);
        free(tmp_env2);
 
+       return ret;
 #endif /* ! ENV_IS_EMBEDDED */
 }
 #else /* ! CONFIG_ENV_OFFSET_REDUND */
@@ -363,7 +368,7 @@ done:
  * device i.e., nand_dev_desc + 0. This is also the behaviour using
  * the new NAND code.
  */
-static void env_nand_load(void)
+static int env_nand_load(void)
 {
 #if !defined(ENV_IS_EMBEDDED)
        int ret;
@@ -386,11 +391,13 @@ static void env_nand_load(void)
        ret = readenv(CONFIG_ENV_OFFSET, (u_char *)buf);
        if (ret) {
                set_default_env("!readenv() failed");
-               return;
+               return -EIO;
        }
 
        env_import(buf, 1);
 #endif /* ! ENV_IS_EMBEDDED */
+
+       return 0;
 }
 #endif /* CONFIG_ENV_OFFSET_REDUND */
 
diff --git a/env/nvram.c b/env/nvram.c
index 85af37d4a0..5fb3115ce6 100644
--- a/env/nvram.c
+++ b/env/nvram.c
@@ -51,7 +51,7 @@ static int env_nvram_get_char(int index)
 }
 #endif
 
-static void env_nvram_load(void)
+static int env_nvram_load(void)
 {
        char buf[CONFIG_ENV_SIZE];
 
@@ -61,6 +61,8 @@ static void env_nvram_load(void)
        memcpy(buf, (void *)CONFIG_ENV_ADDR, CONFIG_ENV_SIZE);
 #endif
        env_import(buf, 1);
+
+       return 0;
 }
 
 static int env_nvram_save(void)
diff --git a/env/onenand.c b/env/onenand.c
index 319f553262..2e3045c5f5 100644
--- a/env/onenand.c
+++ b/env/onenand.c
@@ -26,7 +26,7 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-static void env_onenand_load(void)
+static int env_onenand_load(void)
 {
        struct mtd_info *mtd = &onenand_mtd;
 #ifdef CONFIG_ENV_ADDR_FLEX
@@ -59,6 +59,8 @@ static void env_onenand_load(void)
        rc = env_import(buf, 1);
        if (rc)
                gd->env_valid = ENV_VALID;
+
+       return rc ? 0 : -EIO;
 }
 
 static int env_onenand_save(void)
diff --git a/env/remote.c b/env/remote.c
index 0d8865bd67..c013fdd4b0 100644
--- a/env/remote.c
+++ b/env/remote.c
@@ -46,11 +46,13 @@ static int env_remote_save(void)
 }
 #endif /* CONFIG_CMD_SAVEENV */
 
-static void env_remote_load(void)
+static int env_remote_load(void)
 {
 #ifndef ENV_IS_EMBEDDED
        env_import((char *)env_ptr, 1);
 #endif
+
+       return 0;
 }
 
 U_BOOT_ENV_LOCATION(remote) = {
diff --git a/env/sata.c b/env/sata.c
index 16d8f939db..a77029774e 100644
--- a/env/sata.c
+++ b/env/sata.c
@@ -98,21 +98,24 @@ static void env_sata_load(void)
        int env_sata;
 
        if (sata_initialize())
-               return;
+               return -EIO;
 
        env_sata = sata_get_env_dev();
 
        sata = sata_get_dev(env_sata);
        if (sata == NULL) {
-               printf("Unknown SATA(%d) device for environment!\n",
-                      env_sata);
-               return;
+               printf("Unknown SATA(%d) device for environment!\n", env_sata);
+               return -EIO;
        }
 
-       if (read_env(sata, CONFIG_ENV_SIZE, CONFIG_ENV_OFFSET, buf))
-               return set_default_env(NULL);
+       if (read_env(sata, CONFIG_ENV_SIZE, CONFIG_ENV_OFFSET, buf)) {
+               set_default_env(NULL);
+               return -EIO;
+       }
 
        env_import(buf, 1);
+
+       return 0;
 }
 
 U_BOOT_ENV_LOCATION(sata) = {
diff --git a/env/sf.c b/env/sf.c
index 07386c629a..6f74371c09 100644
--- a/env/sf.c
+++ b/env/sf.c
@@ -61,7 +61,7 @@ static int setup_flash_device(void)
                                     0, 0, &new);
        if (ret) {
                set_default_env("!spi_flash_probe_bus_cs() failed");
-               return 1;
+               return ret;
        }
 
        env_flash = dev_get_uclass_priv(new);
@@ -73,7 +73,7 @@ static int setup_flash_device(void)
                        CONFIG_ENV_SPI_MAX_HZ, CONFIG_ENV_SPI_MODE);
                if (!env_flash) {
                        set_default_env("!spi_flash_probe() failed");
-                       return 1;
+                       return -EIO;
                }
        }
 #endif
@@ -95,7 +95,7 @@ static int env_sf_save(void)
 
        ret = env_export(&env_new);
        if (ret)
-               return ret;
+               return -EIO;
        env_new.flags   = ACTIVE_FLAG;
 
        if (gd->env_valid == ENV_VALID) {
@@ -112,7 +112,7 @@ static int env_sf_save(void)
                saved_offset = env_new_offset + CONFIG_ENV_SIZE;
                saved_buffer = memalign(ARCH_DMA_MINALIGN, saved_size);
                if (!saved_buffer) {
-                       ret = 1;
+                       ret = -ENOMEM;
                        goto done;
                }
                ret = spi_flash_read(env_flash, saved_offset,
@@ -162,7 +162,7 @@ static int env_sf_save(void)
 }
 #endif /* CMD_SAVEENV */
 
-static void env_sf_load(void)
+static int env_sf_load(void)
 {
        int ret;
        int crc1_ok = 0, crc2_ok = 0;
@@ -176,6 +176,7 @@ static void env_sf_load(void)
                        CONFIG_ENV_SIZE);
        if (!tmp_env1 || !tmp_env2) {
                set_default_env("!malloc() failed");
+               ret = -EIO;
                goto out;
        }
 
@@ -202,6 +203,7 @@ static void env_sf_load(void)
 
        if (!crc1_ok && !crc2_ok) {
                set_default_env("!bad CRC");
+               ret = -EIO;
                goto err_read;
        } else if (crc1_ok && !crc2_ok) {
                gd->env_valid = ENV_VALID;
@@ -244,6 +246,8 @@ err_read:
 out:
        free(tmp_env1);
        free(tmp_env2);
+
+       return ret;
 }
 #else
 #ifdef CMD_SAVEENV
@@ -308,7 +312,7 @@ static int env_sf_save(void)
 }
 #endif /* CMD_SAVEENV */
 
-static void env_sf_load(void)
+static int env_sf_load(void)
 {
        int ret;
        char *buf = NULL;
@@ -316,7 +320,7 @@ static void env_sf_load(void)
        buf = (char *)memalign(ARCH_DMA_MINALIGN, CONFIG_ENV_SIZE);
        if (!buf) {
                set_default_env("!malloc() failed");
-               return;
+               return -EIO;
        }
 
        ret = setup_flash_device();
@@ -339,6 +343,8 @@ err_read:
        env_flash = NULL;
 out:
        free(buf);
+
+       return ret;
 }
 #endif
 
diff --git a/env/ubi.c b/env/ubi.c
index 9399f943dc..1c4653d4f6 100644
--- a/env/ubi.c
+++ b/env/ubi.c
@@ -91,7 +91,7 @@ static int env_ubi_save(void)
 #endif /* CONFIG_CMD_SAVEENV */
 
 #ifdef CONFIG_SYS_REDUNDAND_ENVIRONMENT
-static void env_ubi_load(void)
+static int env_ubi_load(void)
 {
        ALLOC_CACHE_ALIGN_BUFFER(char, env1_buf, CONFIG_ENV_SIZE);
        ALLOC_CACHE_ALIGN_BUFFER(char, env2_buf, CONFIG_ENV_SIZE);
@@ -115,7 +115,7 @@ static void env_ubi_load(void)
                printf("\n** Cannot find mtd partition \"%s\"\n",
                       CONFIG_ENV_UBI_PART);
                set_default_env(NULL);
-               return;
+               return -EIO;
        }
 
        if (ubi_volume_read(CONFIG_ENV_UBI_VOLUME, (void *)tmp_env1,
@@ -131,9 +131,11 @@ static void env_ubi_load(void)
        }
 
        env_import_redund((char *)tmp_env1, (char *)tmp_env2);
+
+       return 0;
 }
 #else /* ! CONFIG_SYS_REDUNDAND_ENVIRONMENT */
-static void env_ubi_load(void)
+static int env_ubi_load(void)
 {
        ALLOC_CACHE_ALIGN_BUFFER(char, buf, CONFIG_ENV_SIZE);
 
@@ -151,17 +153,19 @@ static void env_ubi_load(void)
                printf("\n** Cannot find mtd partition \"%s\"\n",
                       CONFIG_ENV_UBI_PART);
                set_default_env(NULL);
-               return;
+               return -EIO;
        }
 
        if (ubi_volume_read(CONFIG_ENV_UBI_VOLUME, buf, CONFIG_ENV_SIZE)) {
                printf("\n** Unable to read env from %s:%s **\n",
                       CONFIG_ENV_UBI_PART, CONFIG_ENV_UBI_VOLUME);
                set_default_env(NULL);
-               return;
+               return -EIO;
        }
 
        env_import(buf, 1);
+
+       return 0;
 }
 #endif /* CONFIG_SYS_REDUNDAND_ENVIRONMENT */
 
diff --git a/include/environment.h b/include/environment.h
index ba8af28414..03b41e0c51 100644
--- a/include/environment.h
+++ b/include/environment.h
@@ -236,8 +236,10 @@ struct env_driver {
         *
         * This method is optional. If not provided, no environment will be
         * loaded.
+        *
+        * @return 0 if OK, -ve on error
         */
-       void (*load)(void);
+       int (*load)(void);
 
        /**
         * save() - Save the environment to storage
-- 
2.14.0.rc0.400.g1c36432dff-goog

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to