[PATCH 2/2] Nokia RX-51: Remove CONFIG_PREBOOT from defconfig

[Pali Rohár]

On Tuesday 12 July 2022 17:39:06 Tom Rini wrote:
> On Tue, Jul 12, 2022 at 10:11:00AM +0200, Pali Rohár wrote:
> > On Monday 11 July 2022 19:23:26 Tom Rini wrote:
> > > On Sun, Jul 10, 2022 at 01:42:56PM +0200, Pali Rohár wrote:
> > > 
> > > > CONFIG_PREBOOT just cause putting "preboot=CONFIG_PREBOOT" into env list.
> > > > Value CONFIG_PREBOOT="run preboot" in defconfig is just nonsense and does
> > > > not do anything useful (it is infinite recursion). Config file for this
> > > > board already contains default preboot= env variable with correct value,
> > > > which has higher priority than CONFIG_PREBOOT and this is reason why
> > > > nonsense CONFIG_PREBOOT is ignored.
> > > > 
> > > > Remove nonsense and unused CONFIG_PREBOOT from nokia_rx51_defconfig file.
> > > > 
> > > > Signed-off-by: Pali Rohár <pali at kernel.org>
> > > > ---
> > > >  configs/nokia_rx51_defconfig | 1 -
> > > >  1 file changed, 1 deletion(-)
> > > > 
> > > > diff --git a/configs/nokia_rx51_defconfig b/configs/nokia_rx51_defconfig
> > > > index 309cf28269c1..46b794f168d9 100644
> > > > --- a/configs/nokia_rx51_defconfig
> > > > +++ b/configs/nokia_rx51_defconfig
> > > > @@ -24,7 +24,6 @@ CONFIG_AUTOBOOT_MENU_SHOW=y
> > > >  CONFIG_USE_BOOTCOMMAND=y
> > > >  CONFIG_BOOTCOMMAND="run sdboot;run emmcboot;run attachboot;echo"
> > > >  CONFIG_USE_PREBOOT=y
> > > > -CONFIG_PREBOOT="run preboot"
> > > >  # CONFIG_SYS_DEVICE_NULLDEV is not set
> > > >  CONFIG_HUSH_PARSER=y
> > > >  CONFIG_SYS_PROMPT="Nokia RX-51 # "
> > > 
> > > These changes are actually a bit puzzling.  There are other platforms
> > > that set preboot in their default environment, rather than via
> > > CONFIG_PREBOOT, and their final value ends up being the one set in
> > > CONFIG_EXTRA_ENV_SETTINGS rather than the empty string that
> > > CONFIG_PREBOOT is.  I assume you've confirmed that at run-time you end
> > > up with preboot="run preboot" being set, and not preboot="long command"
> > > ?
> > 
> > At nokia n900 runtime is always "preboot=long command" and not
> > "preboot=run preboot". It also was before these changes.
> 
> OK, so then we really just need this patch, and not the other.

Note that this is observation and all automated tests depend on this
behavior. So if this behavior somehow change (e.g. by changing
implementation or hacking LTO to change order, or etc...) then tests
start failing and you would see it in failed CI. I hope that n900 test
is still running in CI and in case of issues somebody inform me about
it...

PATCH 1/2 avoids inclusion of "preboot=CONFIG_PREBOOT" into default
environment when CONFIG_PREBOOT is not defined in defconfig file. And
PATCH 2/2 then unsets CONFIG_PREBOOT from nokia defconfig file.

> And long
> term figure out how to better handle this along with the other
> half-dozen places the user may want to configure the default environment
> a bit more, on top of what's already provided.  There are a handful of
> other environment variables that are handled the same (confusing) way
> right now.
> 
> -- 
> Tom

Apparently this problem appeared by converting config.h options into
Kconfig. And continue to grow by conversion of new and new options.

Really Kconfig is not ideal tool for generating environment variables.

For all this stuff is needed some stronger tool/language than Kconfig,
e.g. C preprocessor (like it was before in config.h) or any similar
stronger macro language (e.g. m4) or script languages (shell / python).