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 v5: - Rebase to master
Changes in v4: - Rebase to master, dropping patches already applied
Changes in v3: - Rebase to master
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 a5a156c88b..ec49c39053 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