[PATCH] env: env_sf: don't set .init op if not needed

[Rasmus Villemoes]

On 03/11/2020 08.52, Wolfgang Denk wrote:
> Dear Heiko,
> 
> In message <c32e8504-1f86-8579-3240-e4cf41e847c9 at denx.de> you wrote:
>>
>>> Apparently the meaning of CONFIG_ENV_IS_NOWHERE is nowhere
>>> documented :-(
>>
>> env/Kconfig says:
> 
> Ah, I missed that.  I was only checkng the README and the doc/
> files...
> 
>> config ENV_IS_NOWHERE
>>          bool "Environment is not stored"
> 
> OK, but this is a pretty clear statement.
> 
>>          help
>>            Define this if you don't want to or can't have an environment stored
>>            on a storage medium. In this case the environment will still exist
>>            while U-Boot is running, but once U-Boot exits it will not be
>>            stored. U-Boot will therefore always start up with a default
>>            environment.
>>
>>
>>> But common sense says that "IS NOWHERE" means that there is no
>>> storage defined for the environment.  I would expect, that Kconfig
>>
>> Yes and use default one ...
> 
> Correct.  So this matches my understanding of the name of the config
> option: use this if there is no storage defined for the environment.
> 
>>> does not even allow to enable any CONFIG_ENV_IS_IN_* when
>>> CONFIG_ENV_IS_NOWHERE is selected - these are logically exclusive.
>>
>> Hmm...
>>
>>> May I suggest that:
>>>
>>> 1) our Kconfig files are changed such that CONFIG_ENV_IS_NOWHERE and
>>>     CONFIG_ENV_IS_IN_* are indeed exclusive, so that we adhere to the
>>>     POLA [1] ?
>>
>> Yes if we really want such a hard setting without having an environment!
> 
> Well, this is exactly what this config option is supposed to do -
> both according to the name and according to the documentation: "the
> environment ... will not be stored."
> 
>> Currently CONFIG_ENV_IS_NOWHERE is not that hard and means U-Boot has
>> no Environment ... it means use the built in default environment...
> 
> I would rephrase this: Currently the implemantation does not enforce
> the correct behaviour, so it has been misused for other (also
> useful) things.  But this is not clean and should be fixed.
> 
>>> 2) for cases like this one, where there actually _is_ some storage
>>>     defined, but it shall be used in a non-standard way, a new
>>>     CONFIG_ option gets created that expresses in it's name what it
>>>     does?
>>
>> Not in a none standard way! Instead you can define more than one
>> environment storage devices and load them in a board specific order
>> (defined thorugh board specfif function env_get_location())
> 
> Yes, agreed.  But this logically impossible if there is no storage
> at all, which is what CONFIG_ENV_IS_NOWHERE says.

Then should all the current config options CONFIG_ENV_IS_ be renamed to
CONFIG_ENV_MAY_BE_? Because that's really what they mean.

I certainly agree the current naming isn't very accurate, but the
ability to choose CONFIG_ENV_IS_NOWHERE along with some "real" storage
option is quite useful. We use it so that we can use the exact same
U-Boot binary for both development and production, just different .dtbs;
there's a board-specific env_get_location() which reads a DT property to
decide whether to return ENVL_SPI_FLASH or ENVL_NOWHERE. Whether there
actually needs to be a stub nowhere.c driver for that or one could just
return ENVL_UNKNOWN I don't know.

Rasmus