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

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