Re: RFC: Support for U-Boot phases in Kconfig

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 4845 bytes --]

On Wed, Aug 11, 2021 at 08:47:24AM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On Wed, 11 Aug 2021 at 08:31, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Wed, Aug 11, 2021 at 08:11:41AM -0600, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Wed, 11 Aug 2021 at 08:02, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Wed, Aug 11, 2021 at 06:56:31AM -0600, Simon Glass wrote:
> > > > [snip]
> > > > > Having thought a bit more, perhaps we have the wrong attitude to
> > > > > Kconfig. The CONFIG() macro I am talking about works by building an
> > > > > xxx or SPL_xxx config. If we have separate autoconf.h files for each
> > > > > phase (autoconf_spl.h etc.) then we don't actually need this. We just
> > > > > need to include the correct file. Any SPL_xxx config can be written as
> > > > > xxx. Similarly the Makefile rules can drop the $(P) I was proposing.
> > > > >
> > > > > We can, in fact, generate separate autoconf.h files for each phase
> > > > > today, with no other changes. Unless I am missing something...?
> > > >
> > > > If we can spit out {spl_,tpl_,}autoconf.h files that might help a bit.
> > > > But would it help with the recent case of SPL has SATA+AHCI+!PCI while
> > > > full U-Boot has SATA+AHCI+!PCI AND SATA+AHCI+PCI ?  Today we can't
> > > > support the SPL case without adding the handful of SPL_xxx symbols so
> > > > that we can say we have SATA+AHCI without PCI.
> > >
> > > My thought is that:
> > >
> > > - where there is no SPL_xxx symbol, it we would have CONFIG_xxx=y in
> > > all autoconf.h files
> > > - where there is an SPL_xxx symbol, it we would only have it in
> > > spl_autoconf.h if the SPL_xxx symbol is enabled
> > >
> > > So it does not reduce the power/flexibility of what we have to cover
> > > all cases. It is just a phase-specific way of presenting the configs
> > > to the build, so we can do:
> > >
> > > obj-$(CONFIG_FOO) += foo.o
> > >
> > > as well as
> > >
> > > if (CONFIG(FOO))
> > >
> > > I'm still thinking about Kconfig. To me it seems that separating the
> > > phases so completely is giving up quite a bit. There is no-longer a
> > > unified build, so dependencies between phases may become a problem. I
> > > think in fact our problem is the use of SPL_ and TPL_ prefixes on
> > > Kconfigs, which you have highlighted. Perhaps we just shouldn't do
> > > that. It would be nice if kconfig could support multiple interrelated
> > > build phases and output a separate autoconf.h for each one.
> >
> > What are the dependencies we have between phases?  You've mentioned
> > bloblist, but to me that's like BOARD_INIT and MISC_INIT_R and all of
> > the other things you need to have select'd on a platform because they're
> > non-optional.
> 
> Well if you enable BLOBLIST in U-Boot proper then it had better be
> enabled in SPL or it won't work. Same with HANDOFF. Other things on my
> list in this vein are console recording through the phases. Logging is
> best enabled globally, with different default levels for SPL. We also
> have a lot of things like LOCALVERSION. The main Makefile looks at
> CONFIG_SPL_FRAMEWORK to decide whether to expect u-boot.img or not, so
> we'd have to have another symbol like CONFIG_FRAMEWORK that people
> keep in sync (or just complete the *&^#$&^# migration :-)
> 
> We have quite a bit of:
> 
> config SPL_SYS_ICACHE_OFF
>    depends on SPL
>    default SYS_ICACHE_OFF
> 
> I think we are throwing away a lot by separating them at the
> configuration stage. I'm not saying it's a disaster but I am worried
> that it might not lead to a good place.

My first reaction is that things will be fine with either select's (a
platform using HANDOFF/BLOBLIST should be select'ing that since it's
require) or matching defaults.  The *CACHE_OFF example isn't actually a
needs to be in sync thing, there's platforms today that disable in SPL
but not full.

I'm fully willing to admit there's pitfalls I'm not seeing.  And I'll
further say I don't think this should be our top goal right now.

> > And I'm really not seeing now how we would support the example I gave as
> > for them SPL with SATA+AHCI+PCI is not desired nor possible.  I asked.
> > The answer was no, don't want it.  Or do you really just mean that if we
> > had spl_autoconf.h the only thing that would change is that we would
> > never test on CONFIG_SPL_xxx only CONFIG_xxx, but we would still need to
> > Kconfig SPL_xxx?
> 
> Yes, that's what I am saying. We can make that change now (to clean up
> Makefile and add CONFIG()) with no change to Kconfig.
> 
> We can support the case you mention and yes we want it and need it, of
> course. Otherwise we are back to the SPL #undef horror.

OK.  We can certainly see how it works out, if you make a patch for it.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]