From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Walle Date: Sat, 31 Oct 2020 00:51:27 +0100 Subject: [PATCH v4 2/3] env: Access Environment in SPI flashes before relocation In-Reply-To: <20201010082806.2260318-3-hs@denx.de> References: <20201010082806.2260318-1-hs@denx.de> <20201010082806.2260318-3-hs@denx.de> Message-ID: <24cf63b0b269f6beb0c8470b57cab622@walle.cc> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 > --- > > 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. -michael