On Sat, Oct 31, 2020 at 12:51:27AM +0100, Michael Walle wrote: > Hi Heiko, Hi Tom, > > Am 2020-10-10 10:28, schrieb Heiko Schocher: > > 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 <h...@denx.de> > > --- > > > > Changes in v4: > > - rebased to current master 5dcf7cc590 > > > > Changes in v3: > > - env_sf_init_early() always return 0 now. If we do not return > > 0 in this function, env_set_inited() never get called, > > which has the consequence that env_load/save/erase never > > work, because they check if the init bit is set. > > - add comment from Simon Glass > > - add missing function comments > > - use if(IS_ENABLED...) > > - drop extra brackets > > - let env_sf_init() decide, which function to call > > add comment that it is necessary to return env_sf_init() > > with 0. > > > > env/Kconfig | 8 +++++ > > env/sf.c | 100 ++++++++++++++++++++++++++++++++++++++++++++++++++-- > > 2 files changed, 105 insertions(+), 3 deletions(-) > > > > diff --git a/env/Kconfig b/env/Kconfig > > index c6ba08878d..393b191a30 100644 > > --- a/env/Kconfig > > +++ b/env/Kconfig > > @@ -376,6 +376,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 937778aa37..2eb2de1a4e 100644 > > --- a/env/sf.c > > +++ b/env/sf.c > > @@ -287,7 +287,10 @@ __weak void *env_sf_get_env_addr(void) > > #endif > > > > #if defined(INITENV) && (CONFIG_ENV_ADDR != 0x0) > > -static int env_sf_init(void) > > +/* > > + * check if Environment on CONFIG_ENV_ADDR is valid. > > + */ > > +static int env_sf_init_addr(void) > > { > > env_t *env_ptr = (env_t *)env_sf_get_env_addr(); > > > > @@ -303,12 +306,103 @@ static int env_sf_init(void) > > } > > #endif > > > > +#if defined(CONFIG_ENV_SPI_EARLY) > > +/* > > + * early load environment from SPI flash (before relocation) > > + * and check if it is valid. > > + */ > > +static int env_sf_init_early(void) > > +{ > > + int ret; > > + int read1_fail; > > + int read2_fail; > > + int crc1_ok; > > + env_t *tmp_env2 = NULL; > > + env_t *tmp_env1; > > + > > + /* > > + * if malloc is not ready yet, we cannot use > > + * this part yet. > > + */ > > + if (!gd->malloc_limit) > > + return -ENOENT; > > + > > + tmp_env1 = (env_t *)memalign(ARCH_DMA_MINALIGN, > > + CONFIG_ENV_SIZE); > > + if (IS_ENABLED(CONFIG_SYS_REDUNDAND_ENVIRONMENT)) > > + tmp_env2 = (env_t *)memalign(ARCH_DMA_MINALIGN, > > + CONFIG_ENV_SIZE); > > + > > + 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); > > + > > + if (IS_ENABLED(CONFIG_SYS_REDUNDAND_ENVIRONMENT)) { > > + read2_fail = spi_flash_read(env_flash, > > + CONFIG_ENV_OFFSET_REDUND, > > + CONFIG_ENV_SIZE, tmp_env2); > > + ret = env_check_redund((char *)tmp_env1, read1_fail, > > + (char *)tmp_env2, read2_fail); > > + > > + if (ret == -EIO || ret == -ENOMSG) > > + goto err_read; > > + > > + 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; > > + > > + crc1_ok = crc32(0, tmp_env1->data, ENV_SIZE) == > > + tmp_env1->crc; > > + if (!crc1_ok) > > + goto err_read; > > + > > + /* if valid -> this is our env */ > > + gd->env_valid = ENV_VALID; > > + gd->env_addr = (unsigned long)&tmp_env1->data; > > + } > > + > > + return 0; > > +err_read: > > + spi_flash_free(env_flash); > > + env_flash = NULL; > > + free(tmp_env1); > > + if (IS_ENABLED(CONFIG_SYS_REDUNDAND_ENVIRONMENT)) > > + free(tmp_env2); > > +out: > > + /* env is not valid. always return 0 */ > > + gd->env_valid = ENV_INVALID; > > + return 0; > > +} > > +#endif > > + > > +static int env_sf_init(void) > > +{ > > +#if defined(INITENV) && (CONFIG_ENV_ADDR != 0x0) > > + return env_sf_init_addr(); > > +#elif defined(CONFIG_ENV_SPI_EARLY) > > + return env_sf_init_early(); > > +#endif > > + /* > > + * we must return with 0 if there is nothing done, > > + * else env_set_inited() get not called in env_init() > > + */ > > + return 0; > > +} > > + > > U_BOOT_ENV_LOCATION(sf) = { > > .location = ENVL_SPI_FLASH, > > ENV_NAME("SPIFlash") > > .load = env_sf_load, > > .save = CONFIG_IS_ENABLED(SAVEENV) ? > > ENV_SAVE_PTR(env_sf_save) : > > NULL, > > -#if defined(INITENV) && (CONFIG_ENV_ADDR != 0x0) > > .init = env_sf_init, > > -#endif > > This will break my board (the new kontron sl28). Before the > change, the environment is loaded from SPI, after this patch > the environment won't be loaded anymore. If I delete the > .init callback, the behavior is the same as before. > > The problem here is that returning 0 in env_sf_init() is not > enough because if the .init callback is not set, env_init() will > have ret defaulting to -ENOENT and in this case will load the > default environment and setting env_valid to ENV_VALID. I didn't > follow the whole call chain then. But this will then eventually > lead to loading the actual environment in a later stage.
Ugh. The games we play in the env area really just need to be rewritten. So at this point I've merged the series, should I revert or do you see an easy fix for your platform? Thanks! -- Tom
signature.asc
Description: PGP signature