[PATCH v4 2/3] env: Access Environment in SPI flashes before relocation
Tom Rini
trini at konsulko.com
Sat Oct 31 01:07:03 CET 2020
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 <hs at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201030/1e3897bd/attachment.sig>
More information about the U-Boot
mailing list