On 6/2/20 4:43 PM, Tom Rini wrote:
On Tue, Jun 02, 2020 at 02:09:57PM +0200, Marek Vasut wrote:
On 6/2/20 2:05 PM, Rasmus Villemoes wrote:
On 02/06/2020 13.04, Marek Vasut wrote:
On 6/2/20 8:42 AM, Rasmus Villemoes wrote:
On 29/05/2020 19.54, Marek Vasut wrote:
+config ENV_APPEND
- bool "Always append the environment with new data"
- default n
- help
If defined, the environment hash table is only ever appended with newdata, but the existing hash table can never be dropped and reloadedwith newly imported data. This may be used in combination with staticflags to e.g. to protect variables which must not be modified.config ENV_ACCESS_IGNORE_FORCE bool "Block forced environment operations" default n diff --git a/env/env.c b/env/env.c index 024d36fdbe..967a9d36d7 100644 --- a/env/env.c +++ b/env/env.c @@ -204,7 +204,9 @@ int env_load(void) ret = drv->load(); if (!ret) { printf("OK\n"); +#if !CONFIG_IS_ENABLED(ENV_APPEND) return 0; +#endif
Don't use CONFIG_IS_ENABLED() unless you actually introduce both CONFIG_FOO and CONFIG_SPL_FOO. Otherwise the above CONFIG_IS_ENABLED(ENV_APPEND) is guaranteed to evaluate to false in SPL. Of course that only matters if environment support is enabled in SPL, but some actually use that.
We actually want to use CONFIG_IS_ENABLED() as much as possible to make these options future-proof, so that others won't have to chase down all kinds of #ifdef CONFIG stuff and fix it later on for SPL/TPL/etc.
That makes no sense. You're introducing something whose help text doesn't spell out that the option only applies to U-Boot proper, and is completely ignored in SPL (since CONFIG_SPL_ENV_APPEND never exists).
Anything which does not explicitly spell _SPL or _TPL is U-Boot only, except for some remaining options which need fixing.
No, it's not true that every option in Kconfig needs to be listed in triplicate.
The reason it's ignored in SPL is that you use the SPL-or-not-SPL-aware CONFIG_IS_ENABLED() helper, and you say that's so that somebody in the future can implement CONFIG_SPL_ENV_APPEND?
Yes, because you might need to differentiate between the env behavior in TPL/SPL/U-Boot.
I'm not sure it's valid to say that env can behave different (outside specific cases like readonly before full U-Boot).
If you intend for ENV_APPEND to be something that's either set or not set for a given board, then the code needs to use the SPL-agnostic IS_ENABLED(CONFIG_ENV_APPEND). If you intend it to be something that can be set independently for the env support in SPL vs U-Boot proper, you need to add both config options and, as you do, use CONFIG_IS_ENABLED.
I don't have a way to test it in SPL, so I'm not adding untested config options.
Then you should default to making SPL behave the same way as full U-Boot.
That makes no sense e.g. if you only have default env in SPL while multiple envs in U-Boot.