Hi,

Am 2020-10-31 01:07, schrieb Tom Rini:
[..]
> +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!

Mh, my board cannot be the only one with a late environment
from SPI flash, can it?

Returning 0 will call env_set_inited() and returning -ENOENT
will call the second part. I can't tell why it was done that
way. So I don't have a simple fix.

But I guess the following ugly ifdeffery could do it:

#if (defined(INITENV) && (CONFIG_ENV_ADDR != 0x0)) || defined(CONFIG_ENV_SPI_EARLY)
        .init           = env_sf_init,
#endif

I can try that tomorrow. Also the comment about the return
value should be removed (?).

-michael

Reply via email to