From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wolfgang Denk Date: Tue, 03 Nov 2020 08:52:42 +0100 Subject: [PATCH] env: env_sf: don't set .init op if not needed In-Reply-To: References: <20201101133812.18701-1-michael@walle.cc> <287855.1604321503@gemini.denx.de> Message-ID: <305086.1604389962@gemini.denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Dear Heiko, In message 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