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

[Simon Glass <sjg@chromium.org>]

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