[U-Boot] [PATCH] env: Access Environment in SPI flashes before relocation

Enable the new Kconfig option ENV_SPI_EARLY if you want to use Environment in SPI flash before relocation. Call env_init() and than you can use env_get_f() for accessing Environment variables.
Signed-off-by: Heiko Schocher hs@denx.de
--- travis build: https://travis-ci.org/hsdenx/u-boot-test/builds/609101712
env/Kconfig | 8 ++++ env/sf.c | 109 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 117 insertions(+)
diff --git a/env/Kconfig b/env/Kconfig index bc03816bc8..f2e1e1ba87 100644 --- a/env/Kconfig +++ b/env/Kconfig @@ -370,6 +370,14 @@ config ENV_SPI_MODE Value of the SPI work mode for environment. See include/spi.h for value.
+config ENV_SPI_EARLY + bool "Access Environment in SPI flashes before relocation" + depends on ENV_IS_IN_SPI_FLASH + help + Enable this if you want to use Environment in SPI flash + before relocation. Call env_init() and than you can use + env_get_f() for accessing Environment variables. + config ENV_IS_IN_UBI bool "Environment in a UBI volume" depends on !CHAIN_OF_TRUST diff --git a/env/sf.c b/env/sf.c index 590d0cedd8..c4dd7dc611 100644 --- a/env/sf.c +++ b/env/sf.c @@ -308,6 +308,113 @@ static int env_sf_init(void) } #endif
+#if defined(CONFIG_ENV_SPI_EARLY) +static int env_sf_init_early(void) +{ + int ret; + int read1_fail; + int crc1_ok; +#ifdef CONFIG_SYS_REDUNDAND_ENVIRONMENT + int crc2_ok; + int read2_fail; +#endif + + /* + * if malloc is not ready yet, we cannot use + * this part yet. + */ + if (!gd->malloc_limit) + return -ENOENT; + + env_t *tmp_env2 = NULL; + env_t *tmp_env1; + + tmp_env1 = (env_t *)memalign(ARCH_DMA_MINALIGN, + CONFIG_ENV_SIZE); +#ifdef CONFIG_SYS_REDUNDAND_ENVIRONMENT + tmp_env2 = (env_t *)memalign(ARCH_DMA_MINALIGN, + CONFIG_ENV_SIZE); +#endif + if (!tmp_env1 || !tmp_env2) + goto out; + + ret = setup_flash_device(); + if (ret) + goto out; + + read1_fail = spi_flash_read(env_flash, CONFIG_ENV_OFFSET, + CONFIG_ENV_SIZE, tmp_env1); + crc1_ok = crc32(0, tmp_env1->data, ENV_SIZE) == + tmp_env1->crc; +#ifdef CONFIG_SYS_REDUNDAND_ENVIRONMENT + read2_fail = spi_flash_read(env_flash, CONFIG_ENV_OFFSET_REDUND, + CONFIG_ENV_SIZE, tmp_env2); + crc2_ok = crc32(0, tmp_env2->data, ENV_SIZE) == + tmp_env2->crc; + if (read1_fail && read2_fail) { + goto err_read; + } else if (!read1_fail && read2_fail) { + if (!crc1_ok) + goto err_read; + gd->env_valid = ENV_VALID; + } else if (read1_fail && !read2_fail) { + if (!crc2_ok) + goto err_read; + gd->env_valid = ENV_REDUND; + } else { + /* both environments read */ + if (!crc1_ok && !crc2_ok) { + goto err_read; + } else if (crc1_ok && !crc2_ok) { + gd->env_valid = ENV_VALID; + } else if (!crc1_ok && crc2_ok) { + gd->env_valid = ENV_REDUND; + } else { + /* both ok - check serial */ + if (tmp_env1->flags == 255 && tmp_env2->flags == 0) + gd->env_valid = ENV_REDUND; + else if (tmp_env2->flags == 255 && tmp_env1->flags == 0) + gd->env_valid = ENV_VALID; + else if (tmp_env1->flags > tmp_env2->flags) + gd->env_valid = ENV_VALID; + else if (tmp_env2->flags > tmp_env1->flags) + gd->env_valid = ENV_REDUND; + else /* flags are equal - almost impossible */ + gd->env_valid = ENV_VALID; + } + } + if (gd->env_valid == ENV_VALID) + gd->env_addr = (unsigned long)&tmp_env1->data; + else + gd->env_addr = (unsigned long)&tmp_env2->data; + +#else + if (read1_fail) + goto err_read; + + /* check crc */ + if (!crc1_ok) + goto err_read; + + /* if valid -> this is our env */ + gd->env_valid = ENV_VALID; + gd->env_addr = tmp_env1; +#endif + + return 0; +err_read: + spi_flash_free(env_flash); + env_flash = NULL; + free(tmp_env1); +#ifdef CONFIG_SYS_REDUNDAND_ENVIRONMENT + free(tmp_env2); +#endif +out: + /* return ENOENT else U-Boot will hang */ + return -ENOENT; +} +#endif + U_BOOT_ENV_LOCATION(sf) = { .location = ENVL_SPI_FLASH, ENV_NAME("SPI Flash") @@ -317,5 +424,7 @@ U_BOOT_ENV_LOCATION(sf) = { #endif #if defined(INITENV) && defined(CONFIG_ENV_ADDR) .init = env_sf_init, +#elif defined(CONFIG_ENV_SPI_EARLY) + .init = env_sf_init_early, #endif };

Heiko Schocher hs@denx.de schrieb am Sa., 9. Nov. 2019, 05:02:
Enable the new Kconfig option ENV_SPI_EARLY if you want to use Environment in SPI flash before relocation. Call env_init() and than you can use env_get_f() for accessing Environment variables.
Signed-off-by: Heiko Schocher hs@denx.de
travis build: https://travis-ci.org/hsdenx/u-boot-test/builds/609101712
env/Kconfig | 8 ++++ env/sf.c | 109 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 117 insertions(+)
diff --git a/env/Kconfig b/env/Kconfig index bc03816bc8..f2e1e1ba87 100644 --- a/env/Kconfig +++ b/env/Kconfig @@ -370,6 +370,14 @@ config ENV_SPI_MODE Value of the SPI work mode for environment. See include/spi.h for value.
+config ENV_SPI_EARLY
bool "Access Environment in SPI flashes before relocation"
depends on ENV_IS_IN_SPI_FLASH
help
Enable this if you want to use Environment in SPI flash
before relocation. Call env_init() and than you can use
env_get_f() for accessing Environment variables.
config ENV_IS_IN_UBI bool "Environment in a UBI volume" depends on !CHAIN_OF_TRUST diff --git a/env/sf.c b/env/sf.c index 590d0cedd8..c4dd7dc611 100644 --- a/env/sf.c +++ b/env/sf.c @@ -308,6 +308,113 @@ static int env_sf_init(void) } #endif
+#if defined(CONFIG_ENV_SPI_EARLY) +static int env_sf_init_early(void) +{
int ret;
int read1_fail;
int crc1_ok;
+#ifdef CONFIG_SYS_REDUNDAND_ENVIRONMENT
int crc2_ok;
int read2_fail;
+#endif
/*
* if malloc is not ready yet, we cannot use
* this part yet.
*/
if (!gd->malloc_limit)
return -ENOENT;
env_t *tmp_env2 = NULL;
env_t *tmp_env1;
tmp_env1 = (env_t *)memalign(ARCH_DMA_MINALIGN,
CONFIG_ENV_SIZE);
+#ifdef CONFIG_SYS_REDUNDAND_ENVIRONMENT
tmp_env2 = (env_t *)memalign(ARCH_DMA_MINALIGN,
CONFIG_ENV_SIZE);
+#endif
if (!tmp_env1 || !tmp_env2)
goto out;
ret = setup_flash_device();
if (ret)
goto out;
read1_fail = spi_flash_read(env_flash, CONFIG_ENV_OFFSET,
CONFIG_ENV_SIZE, tmp_env1);
crc1_ok = crc32(0, tmp_env1->data, ENV_SIZE) ==
tmp_env1->crc;
+#ifdef CONFIG_SYS_REDUNDAND_ENVIRONMENT
read2_fail = spi_flash_read(env_flash, CONFIG_ENV_OFFSET_REDUND,
CONFIG_ENV_SIZE, tmp_env2);
crc2_ok = crc32(0, tmp_env2->data, ENV_SIZE) ==
tmp_env2->crc;
if (read1_fail && read2_fail) {
goto err_read;
} else if (!read1_fail && read2_fail) {
if (!crc1_ok)
goto err_read;
gd->env_valid = ENV_VALID;
} else if (read1_fail && !read2_fail) {
if (!crc2_ok)
goto err_read;
gd->env_valid = ENV_REDUND;
} else {
/* both environments read */
if (!crc1_ok && !crc2_ok) {
goto err_read;
} else if (crc1_ok && !crc2_ok) {
gd->env_valid = ENV_VALID;
} else if (!crc1_ok && crc2_ok) {
gd->env_valid = ENV_REDUND;
} else {
/* both ok - check serial */
if (tmp_env1->flags == 255 && tmp_env2->flags == 0)
gd->env_valid = ENV_REDUND;
else if (tmp_env2->flags == 255 && tmp_env1->flags
== 0)
gd->env_valid = ENV_VALID;
else if (tmp_env1->flags > tmp_env2->flags)
gd->env_valid = ENV_VALID;
else if (tmp_env2->flags > tmp_env1->flags)
gd->env_valid = ENV_REDUND;
else /* flags are equal - almost impossible */
gd->env_valid = ENV_VALID;
}
}
if (gd->env_valid == ENV_VALID)
gd->env_addr = (unsigned long)&tmp_env1->data;
else
gd->env_addr = (unsigned long)&tmp_env2->data;
+#else
if (read1_fail)
goto err_read;
/* check crc */
if (!crc1_ok)
goto err_read;
/* if valid -> this is our env */
gd->env_valid = ENV_VALID;
gd->env_addr = tmp_env1;
+#endif
return 0;
+err_read:
spi_flash_free(env_flash);
env_flash = NULL;
free(tmp_env1);
+#ifdef CONFIG_SYS_REDUNDAND_ENVIRONMENT
free(tmp_env2);
+#endif +out:
/* return ENOENT else U-Boot will hang */
return -ENOENT;
+} +#endif
Sorry I haven't looked at the file in detail yet, but doesn't this duplicate quite a lot of env decision code?
Regards, Simon
+
U_BOOT_ENV_LOCATION(sf) = { .location = ENVL_SPI_FLASH, ENV_NAME("SPI Flash") @@ -317,5 +424,7 @@ U_BOOT_ENV_LOCATION(sf) = { #endif #if defined(INITENV) && defined(CONFIG_ENV_ADDR) .init = env_sf_init, +#elif defined(CONFIG_ENV_SPI_EARLY)
.init = env_sf_init_early,
#endif }; -- 2.21.0

Hello Simon,
Am 10.11.2019 um 15:51 schrieb Simon Goldschmidt:
Heiko Schocher <hs@denx.de mailto:hs@denx.de> schrieb am Sa., 9. Nov. 2019, 05:02:
Enable the new Kconfig option ENV_SPI_EARLY if you want to use Environment in SPI flash before relocation. Call env_init() and than you can use env_get_f() for accessing Environment variables. Signed-off-by: Heiko Schocher <hs@denx.de <mailto:hs@denx.de>> --- travis build: https://travis-ci.org/hsdenx/u-boot-test/builds/609101712 env/Kconfig | 8 ++++ env/sf.c | 109 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 117 insertions(+) diff --git a/env/Kconfig b/env/Kconfig index bc03816bc8..f2e1e1ba87 100644 --- a/env/Kconfig +++ b/env/Kconfig @@ -370,6 +370,14 @@ config ENV_SPI_MODE Value of the SPI work mode for environment. See include/spi.h for value. +config ENV_SPI_EARLY + bool "Access Environment in SPI flashes before relocation" + depends on ENV_IS_IN_SPI_FLASH + help + Enable this if you want to use Environment in SPI flash + before relocation. Call env_init() and than you can use + env_get_f() for accessing Environment variables. + config ENV_IS_IN_UBI bool "Environment in a UBI volume" depends on !CHAIN_OF_TRUST diff --git a/env/sf.c b/env/sf.c index 590d0cedd8..c4dd7dc611 100644 --- a/env/sf.c +++ b/env/sf.c @@ -308,6 +308,113 @@ static int env_sf_init(void)
[...]
Sorry I haven't looked at the file in detail yet, but doesn't this duplicate quite a lot of env decision code?
Yes, correct, I take a look to remove this duplication ...
Hmm... just detected when booting with invalid environment in spi nor flash, that the board selects correct the default environment (before relocation) but I see in U-Boot commandshell:
=> printenv
Environment size: 1/12283 bytes =>
and even worser, saveenv never calls the save function for the spi nor... it seems, the reason is:
diff --git a/env/env.c b/env/env.c index 9237bb9c74..6faef1136d 100644 --- a/env/env.c +++ b/env/env.c @@ -189,7 +189,7 @@ int env_load(void) if (!drv->load) continue;
- if (!env_has_inited(drv->location)) + if (env_has_inited(drv->location)) continue;
printf("Loading Environment from %s... ", drv->name);
With this change, all is fine again. This seems a bug to me ... but wonder why this does not pop up on other boards ... what do you think?
bye, Heiko
Regards, Simon
+ U_BOOT_ENV_LOCATION(sf) = { .location = ENVL_SPI_FLASH, ENV_NAME("SPI Flash") @@ -317,5 +424,7 @@ U_BOOT_ENV_LOCATION(sf) = { #endif #if defined(INITENV) && defined(CONFIG_ENV_ADDR) .init = env_sf_init, +#elif defined(CONFIG_ENV_SPI_EARLY) + .init = env_sf_init_early, #endif }; -- 2.21.0

On Mon, Nov 11, 2019 at 7:15 AM Heiko Schocher hs@denx.de wrote:
Hello Simon,
Am 10.11.2019 um 15:51 schrieb Simon Goldschmidt:
Heiko Schocher <hs@denx.de mailto:hs@denx.de> schrieb am Sa., 9. Nov. 2019, 05:02:
Enable the new Kconfig option ENV_SPI_EARLY if you want to use Environment in SPI flash before relocation. Call env_init() and than you can use env_get_f() for accessing Environment variables. Signed-off-by: Heiko Schocher <hs@denx.de <mailto:hs@denx.de>> --- travis build: https://travis-ci.org/hsdenx/u-boot-test/builds/609101712 env/Kconfig | 8 ++++ env/sf.c | 109 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 117 insertions(+) diff --git a/env/Kconfig b/env/Kconfig index bc03816bc8..f2e1e1ba87 100644 --- a/env/Kconfig +++ b/env/Kconfig @@ -370,6 +370,14 @@ config ENV_SPI_MODE Value of the SPI work mode for environment. See include/spi.h for value. +config ENV_SPI_EARLY + bool "Access Environment in SPI flashes before relocation" + depends on ENV_IS_IN_SPI_FLASH + help + Enable this if you want to use Environment in SPI flash + before relocation. Call env_init() and than you can use + env_get_f() for accessing Environment variables. + config ENV_IS_IN_UBI bool "Environment in a UBI volume" depends on !CHAIN_OF_TRUST diff --git a/env/sf.c b/env/sf.c index 590d0cedd8..c4dd7dc611 100644 --- a/env/sf.c +++ b/env/sf.c @@ -308,6 +308,113 @@ static int env_sf_init(void)
[...]
Sorry I haven't looked at the file in detail yet, but doesn't this duplicate quite a lot of env decision code?
Yes, correct, I take a look to remove this duplication ...
Hmm... just detected when booting with invalid environment in spi nor flash, that the board selects correct the default environment (before relocation) but I see in U-Boot commandshell:
=> printenv
Environment size: 1/12283 bytes =>
and even worser, saveenv never calls the save function for the spi nor... it seems, the reason is:
diff --git a/env/env.c b/env/env.c index 9237bb9c74..6faef1136d 100644 --- a/env/env.c +++ b/env/env.c @@ -189,7 +189,7 @@ int env_load(void) if (!drv->load) continue;
if (!env_has_inited(drv->location))
if (env_has_inited(drv->location)) continue; printf("Loading Environment from %s... ", drv->name);
With this change, all is fine again. This seems a bug to me ... but wonder why this does not pop up on other boards ... what do you think?
I don't think I really get what your problem is, yet. But the code above looks correct: only call load/save/erase on env drivers that have successfully been initialized via env_init.
Regards, Simon
bye, Heiko
Regards, Simon
+ U_BOOT_ENV_LOCATION(sf) = { .location = ENVL_SPI_FLASH, ENV_NAME("SPI Flash") @@ -317,5 +424,7 @@ U_BOOT_ENV_LOCATION(sf) = { #endif #if defined(INITENV) && defined(CONFIG_ENV_ADDR) .init = env_sf_init, +#elif defined(CONFIG_ENV_SPI_EARLY) + .init = env_sf_init_early, #endif }; -- 2.21.0
-- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: hs@denx.de

Hello Simon,
Am 11.11.2019 um 21:07 schrieb Simon Goldschmidt:
On Mon, Nov 11, 2019 at 7:15 AM Heiko Schocher hs@denx.de wrote:
Hello Simon,
Am 10.11.2019 um 15:51 schrieb Simon Goldschmidt:
Heiko Schocher <hs@denx.de mailto:hs@denx.de> schrieb am Sa., 9. Nov. 2019, 05:02:
Enable the new Kconfig option ENV_SPI_EARLY if you want to use Environment in SPI flash before relocation. Call env_init() and than you can use env_get_f() for accessing Environment variables. Signed-off-by: Heiko Schocher <hs@denx.de <mailto:hs@denx.de>> --- travis build: https://travis-ci.org/hsdenx/u-boot-test/builds/609101712 env/Kconfig | 8 ++++ env/sf.c | 109 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 117 insertions(+) diff --git a/env/Kconfig b/env/Kconfig index bc03816bc8..f2e1e1ba87 100644 --- a/env/Kconfig +++ b/env/Kconfig @@ -370,6 +370,14 @@ config ENV_SPI_MODE Value of the SPI work mode for environment. See include/spi.h for value. +config ENV_SPI_EARLY + bool "Access Environment in SPI flashes before relocation" + depends on ENV_IS_IN_SPI_FLASH + help + Enable this if you want to use Environment in SPI flash + before relocation. Call env_init() and than you can use + env_get_f() for accessing Environment variables. + config ENV_IS_IN_UBI bool "Environment in a UBI volume" depends on !CHAIN_OF_TRUST diff --git a/env/sf.c b/env/sf.c index 590d0cedd8..c4dd7dc611 100644 --- a/env/sf.c +++ b/env/sf.c @@ -308,6 +308,113 @@ static int env_sf_init(void)
[...]
Sorry I haven't looked at the file in detail yet, but doesn't this duplicate quite a lot of env decision code?
Yes, correct, I take a look to remove this duplication ...
Hmm... just detected when booting with invalid environment in spi nor flash, that the board selects correct the default environment (before relocation) but I see in U-Boot commandshell:
=> printenv
Environment size: 1/12283 bytes =>
and even worser, saveenv never calls the save function for the spi nor... it seems, the reason is:
diff --git a/env/env.c b/env/env.c index 9237bb9c74..6faef1136d 100644 --- a/env/env.c +++ b/env/env.c @@ -189,7 +189,7 @@ int env_load(void) if (!drv->load) continue;
if (!env_has_inited(drv->location))
if (env_has_inited(drv->location)) continue; printf("Loading Environment from %s... ", drv->name);
With this change, all is fine again. This seems a bug to me ... but wonder why this does not pop up on other boards ... what do you think?
I don't think I really get what your problem is, yet. But the code above looks correct: only call load/save/erase on env drivers that have successfully been initialized via env_init.
Yes, you are right, sorry for your time. I try to find out, whats going wrong...
bye, Heiko
participants (2)
-
Heiko Schocher
-
Simon Goldschmidt