[U-Boot] [PATCH v2 34/34] env: Adjust the load() method to return an error

Simon Glass
2017-07-24T03:20:09Z

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 <sjg@chromium.org>
---

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 | 18 +++++++++++++-----
env/nvram.c | 4 +++-
env/onenand.c | 4 +++-
env/remote.c | 4 +++-
env/sata.c | 15 +++++++++------
env/sf.c | 20 +++++++++++++-------
env/ubi.c | 16 ++++++++++------
include/environment.h | 4 +++-
14 files changed, 85 insertions(+), 43 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 cf85bc232f..c7746492af 100644
--- a/env/mmc.c
+++ b/env/mmc.c
@@ -213,7 +213,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;
@@ -232,13 +232,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;
}

@@ -258,7 +258,7 @@ static void env_mmc_load(void)

if (!crc1_ok && !crc2_ok) {
errmsg = "!bad CRC";
- ret = 1;
+ ret = -EIO;
goto fini;
} else if (crc1_ok && !crc2_ok) {
gd->env_valid = ENV_VALID;
@@ -292,10 +292,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);
@@ -309,18 +311,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;
}

@@ -333,6 +335,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 e0368bf8de..c0d9ae66a1 100644
--- a/env/nand.c
+++ b/env/nand.c
@@ -307,7 +307,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 {
@@ -320,18 +320,22 @@ 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;
int crc1_ok = 0, crc2_ok = 0;
env_t *ep, *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;
}

@@ -351,6 +355,7 @@ static void env_nand_load(void)

if (!crc1_ok && !crc2_ok) {
set_default_env("!bad CRC");
+ ret = -EIO;
goto done;
} else if (crc1_ok && !crc2_ok) {
gd->env_valid = ENV_VALID;
@@ -382,6 +387,7 @@ done:
free(tmp_env1);
free(tmp_env2);

+ return ret;
#endif /* ! ENV_IS_EMBEDDED */
}
#else /* ! CONFIG_ENV_OFFSET_REDUND */
@@ -390,7 +396,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;
@@ -413,11 +419,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 65be2b1bbf..6ac86c8571 100644
--- a/env/ubi.c
+++ b/env/ubi.c
@@ -95,7 +95,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);
@@ -120,7 +120,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,
@@ -140,7 +140,7 @@ static void env_ubi_load(void)

if (!crc1_ok && !crc2_ok) {
set_default_env("!bad CRC");
- return;
+ return -EIO;
} else if (crc1_ok && !crc2_ok) {
gd->env_valid = ENV_VALID;
} else if (!crc1_ok && crc2_ok) {
@@ -166,9 +166,11 @@ static void env_ubi_load(void)

env_flags = ep->flags;
env_import((char *)ep, 0);
+
+ 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);

@@ -186,17 +188,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 bfb6fbc22d..c4453024fe 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.284.gd933b75aa4-goog