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

[Heiko Schocher]

Hello Wolfgang,

Am 02.11.2020 um 13:51 schrieb Wolfgang Denk:
> Dear Heiko,
> 
> In message <a80c8548-a8a5-fb98-f9e3-fc659c2bdfec at denx.de> you wrote:
>>
>> I enabled now ENV_APPEND on this board and
>>
>> CONFIG_ENV_IS_NOWHERE
>> CONFIG_ENV_IS_IN_SPI_FLASH
> 
> This gives me the creeps.  I know this is not cause by anything in
> your patch, but anyway...
> 
> Apparently the meaning of CONFIG_ENV_IS_NOWHERE is nowhere
> documented :-(

env/Kconfig says:

config ENV_IS_NOWHERE
         bool "Environment is not stored"
         default y if !ENV_IS_IN_EEPROM && !ENV_IS_IN_EXT4 && \
                      !ENV_IS_IN_FAT && !ENV_IS_IN_FLASH && \
                      !ENV_IS_IN_MMC && !ENV_IS_IN_NAND && \
                      !ENV_IS_IN_NVRAM && !ENV_IS_IN_ONENAND && \
                      !ENV_IS_IN_REMOTE && !ENV_IS_IN_SPI_FLASH && \
                      !ENV_IS_IN_UBI
         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 ...

> 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!

Currently CONFIG_ENV_IS_NOWHERE is not that hard and means U-Boot has
no Environment ... it means use the built in default environment...

> 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())

For example:
- load first default environment (and you are correct ENV_IS_NOWHERE
     is here really a misleading name).

- load environment from SPI NOR ...

May we only should do a simple rename ?

ENV_IS_NOWHERE -> ENV_IS_IN_UBOOT (or ENV_IS_IN_DEFAULT) ?

If we really want a hard "there is no storage" switch (which really
means there is *no* environment ... I do not even know, if U-boot
works without!) we should introduce a new ENV_IS_IN_DEFAULT which
loads the default environment...

(I do not like this, as all? boards have a default environment, so
  it is enabled for all? boards ... which makes it obsolete...)

better suggestions?

bye,
Heiko
> 
> [1] https://en.wikipedia.org/wiki/Principle_of_least_astonishment
> 
> Thanks!
> 
> Best regards,
> 
> Wolfgang Denk
> 

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de