[U-Boot] [PATCH v3 5/7] kconfig: switch to single .config configuration

[Scott Wood]

On Wed, 2015-02-25 at 15:14 +0900, Masahiro Yamada wrote:
> Hi Scott,
> 
> 
> On Tue, 24 Feb 2015 18:17:59 -0600
> Scott Wood <scottwood at freescale.com> wrote:
> 
> > On Tue, 2015-02-24 at 16:20 +0900, Masahiro Yamada wrote:
> > > Hi Scott,
> > > 
> > > 
> > > On Mon, 23 Feb 2015 19:22:51 -0600
> > > Scott Wood <scottwood at freescale.com> wrote:
> > > 
> > > > On Fri, 2015-02-20 at 14:24 +0900, Masahiro Yamada wrote:
> > > > > When Kconfig for U-boot was examined, one of the biggest issues was
> > > > > how to support multiple images (Normal, SPL, TPL).  There were
> > > > > actually two options, "single .config" and "multiple .config".
> > > > > After some discussions and thought experiments, I chose the latter,
> > > > > i.e. to create ".config", "spl/.config", "tpl/.config" for Normal,
> > > > > SPL, TPL, respectively.
> > > > > 
> > > > > It is true that the "multiple .config" strategy provided us the
> > > > > maximum flexibility and helped to avoid duplicating CONFIGs among
> > > > > Normal, SPL, TPL, but I have noticed some fatal problems:
> > > > > 
> > > > > [1] It is impossible to share CONFIG options across the images.
> > > > >   If you change the configuration of Main image, you often have to
> > > > >   adjust some SPL configurations correspondingly.  Currently, we
> > > > >   cannot handle the dependencies between them.  It means one of the
> > > > >   biggest advantages of Kconfig is lost.
> > > > 
> > > > Sharing can happen in the defconfig with "+S:"...
> > > 
> > > Yes, it can as for "make *_defconfig".
> > > 
> > > If we modify some options in .config for example by "make menuconfig",
> > > we also modify some in spl/.config correspondingly.
> > > 
> > > Users are responsible for configure .config and spl/.config in sync
> > > in the sane combination.
> > > 
> > > 
> > > 
> > > > What sort of dependencies are people wanting?  Would it be possible to
> > > > modify kconfig to import SPL .config into the main config (or vice
> > > > versa?) with a name prefix so that dependencies could happen, without
> > > > sacrificing the ability to set symbols independently?
> > > 
> > > To have independent symboles coexist in a single .config,
> > > I can only suggest to duplicate options like
> > > CONFIG_FOO=0x100
> > > CONFIG_SPL_FOO=0x200
> > > CONFIG_TPL_FOO=0x300
> > 
> > What I meant was a way to keep the configs separate, but automatically
> > import the CONFIG_FOO from the SPL .config as CONFIG_SPL_FOO (or some
> > other prefix that doesn't conflict with SPL-specific options).
> 
> What is the benefit of doing this?

So that you can express dependencies in the main U-Boot on SPL symbols,
which you said was one of the problems that motivated this change.

> > > > Or as Ian suggested, have only the main config be user-editable, but
> > > > still let select/depends turn certain things on/off for the
> > > > auto-generated SPL config.
> > > 
> > > I guess it is possible for boolean options,
> > > but impossible to set hex/int options independently.
> > 
> > How many hex/int options are there, that need to be different in SPL
> > versus the main U-Boot?  Having a few CONFIG_SPL_xxx for those is better
> > than having a bunch.
> 
> OK.
> But, I do not think we need to tweak the Kconfig just for saving boolean options.

Most options are boolean (especially if you ignore hardware description
that isn't going to differ between SPL and non-SPL).  I think it makes a
lot of sense to not want to duplicate them, and especially to not
complicate each place that ifdefs on the symbol by having to check for
SPL.

> > > BTW, Ian's idea had been already achieved by include/config_uncmd_spl.h
> > 
> > So, the answer is to avoid kconfig and go back to using the preprocessor
> > for configuration? :-(
> 
> I am not saying I prefer the preprocessor.
> 
> Indeed, include/config_uncmd_spl.h is ugly,
> so I'd like to propose a better solution.
> 
> If we introduce CONFIG_SPL_DM, for example, the ifdef conditional in source files
> will be like this:
> 
>    #if  (!defined(CONFIG_SPL_BUILD) && defined(CONFIG_DM)) || \
>               (defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_DM))
> 
>               [Driver Model Code]
> 
>    #else
>               [Non Driver Model Code]
>    #endif
> 
> This is too ugly to be written in each conditional.

That's not what I was suggesting.

I was suggesting something like:

config DM
	bool "Enable Driver Model"
	depends on !SPL_BUILD || SPL_DM
	...

Then the ifdef would just be

#ifdef CONFIG_DM
	[Driver Model Code]
#else
	[Non Driver Model Code]
#endif


> > > > > [2] It is too painful to change both ".config" and "spl/.config".
> > > > >   Sunxi guys started to work around this problem by creating a new
> > > > >   configuration target.  Commit cbdd9a9737cc (sunxi: kconfig: Add
> > > > >   %_felconfig rule to enable FEL build of sunxi platforms.) added
> > > > >   "make *_felconfig" to enable CONFIG_SPL_FEL on both images.
> > > > >   Changing the configuration of multiple images in one command is a
> > > > >   generic demand.  The current implementation cannot propose any
> > > > >   good solution about this.
> > > > 
> > > > How about defconfig fragments?  Instead of having script infrastructure
> > > > specifically for CONFIG_SPL_FEL, merge a fragment containing
> > > > "+S:CONFIG_SPL_FEL".
> > > 
> > > Do you mean something like this?
> > > U-boot proper :   common/.config +     .config
> > > SPL           :   common/.config +  spl/.config
> > > TPL           :   common/.config +  tpl/.config
> > 
> > No, I meant having a fragment containing only "+S:CONFIG_SPL_FEL" that
> > could be merged into any other config.
> 
> So, the fragment is something like the _common_ .config, right?

The fragment would be part of a library of common config tweaks that
users can apply -- providing similar convenience as _felconfig but with
generic infrastructure.

> > > > > [4] The build system got more complicated than it should be.
> > > > >   To adjust Linux-originated Kconfig to U-Boot, the helper script
> > > > >   "scripts/multiconfig.sh" was introduced.  Writing a complicated
> > > > >   text processor is a shell script sometimes caused problems.
> > > > > 
> > > > > Now I believe the "single .config" will serve us better.  With it,
> > > > > all the problems above would go away.  Instead, we will have to add
> > > > > some CONFIG_SPL_* (and CONFIG_TPL_*) options such as CONFIG_SPL_DM,
> > > > > but we will not have much.  Anyway, this is what we do now in
> > > > > scripts/Makefile.spl.
> > > > 
> > > > I had been hoping that the split configs would let us get rid of many of
> > > > the CONFIG_SPL_* options that we already have.
> > > > 
> > > > How will TPL be handled?  Are you going to duplicate all the SPL
> > > > symbols?  Or just avoid ever kconfigizing them?
> > > 
> > > Not all, but I expect some duplicated CONFIG_TPL_* such as CONFIG_TPL_TEXT_BASE.
> > 
> > I'm not talking about TEXT_BASE.  I'm talking about stuff like this:
> 
> We have to add some CONFIG_TPL_*, but we will just have 20.

Just?  And how much makefile crud to check for those options when
building?  To achieve what?

> > If you redefine TPL to mean SPL that doesn't use certain code, you'll
> > end up with targets that have TPL but no SPL.  Are you sure this is
> > simplifying anything?
> 
> Sorry, I can't get it.
> What I expect is like follows:
> 
> CONFIG_TPL still depends on CONFIG_SPL.
> 
> We have three options for the boot procedure:
> 
>  [1] U-Boot-proper   (CONFIG_SPL is not defined)
> 
>  [2] SPL + U-Boot-proper  (CONFIG_SPL is defined)
> 
>  [3] TPL + SPL + U-Boot-proper   (CONFIG_SPL and CONFIG_TPL are defined)
> 
> 
>  The image size:   TPL < SPL < U-Boot-proper
> 
>  Driver Model and some other features are available on SPL if CONFIG_SPL_* is defined.
> 
>  Almost no systematic infrastructure is available on TPL, so we will have
>   very small number of CONFIG_TPL_*.

So instead of CONFIG_TPL_* we'd have more hardcoded makefile hackery to
disable things in TPL -- and we'd have to duplicate that hackery for
SPLs that are just as small as what you're now calling TPL, if you're
not going to allow TPL without SPL.

-Scott