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

[Wolfgang Denk]

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.

For such cases we must not define CONFIG_ENV_IS_NOWHERE, but
something else / something new instead.

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

In my understanding, ENV_IS_IN_* defines the storage device used to
save the persistent copy (the external representation(s)) of the
environment, which get written when you use "env save".  So both
your suggestions are not acceptable, as the the envrionment never
gets saved to the default environment - this is just the data that
is used to initialize it in the same was as in an emergency when the
persistent the environment (more precisely: when all copies of it)
cannot be read (I/O or checksum error).

> If we really want a hard "there is no storage" switch (which really

You miss the fact that we already have this: CONFIG_ENV_IS_NOWHERE

It is just not implemented correctly, and I ask for a cleanup.

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

You misunderstand.  "No storage" means no storage for writing the
_persistent_ environment, see above.  Int his case there is still
the same emergency handling as in cases of corrupted peristent
environment - the fallback to the builtin (socalled "default")
environment.

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-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Uncertain fortune is thoroughly mastered by the equity of the  calcu-
lation.                                               - Blaise Pascal