[PATCH v3 61/95] kconfig: Support writing separate SPL files

[Simon Glass]

Hi Tom,

On Tue, 14 Feb 2023 at 10:38, Tom Rini <trini at konsulko.com> wrote:
>
> On Tue, Feb 14, 2023 at 09:58:59AM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On Tue, 14 Feb 2023 at 09:31, Tom Rini <trini at konsulko.com> wrote:
> > >
> > > On Sun, Feb 12, 2023 at 04:16:04PM -0700, Simon Glass wrote:
> > >
> > > > At present kconfig writes out several files, including:
> > > >
> > > >    auto.conf  - CONFIG settings used by make
> > > >    autoconf.h - header file used by C code
> > > >
> > > > This works well but is a bit ugly in places, for example requiring the use
> > > > of a SPL_TPL_ macro in Makefiles to distinguish between options intended
> > > > for SPL and U-Boot proper.
> > > >
> > > > Update the kconfig tool to also output separate files for each phase: e.g.
> > > > auto_spl.conf and autoconf_spl.h
> > > >
> > > > These are similar to the existing files, but drop the SPL_ prefix so that
> > > > SPL_TPL_ is not needed. It also allows the CONFIG_IS_ENABLED() macro to be
> > > > simplified, in a later patch, eventually replacing it with IS_ENABLED().
> > > >
> > > > When CONFIG_FOO is used within SPL, it means that FOO is enabled in that
> > > > SPL phase. For example if CONFIG_SPL_FOO is enabled in the Kconfig, that
> > > > means that CONFIG_FOO will be enabled in the SPL phase. So the SPL builds
> > > > can just use CONFIG_FOO to check it. There is no need to use
> > > > CONFIG_SPL_FOO or CONFIG_IS_ENABLED() anymore.
> > > >
> > > > This of course means that if there is a need to access a PPL symbol from
> > > > an SPL build, there is no way to do it. To copy with that, we need a
> > > > CONFIG_PPL_FOO to be visibilty to all SPL builds.
> > > >
> > > > So this change also adds new PPL_ output for U-Boot proper (Primary
> > > > Program Loader). So every CONFIG_FOO that is enabled in PPL also has a
> > > > CONFIG_PPL_FOO
> > > >
> > > > This allows SPL to access the TEXT_BASE for U-Boot proper, for example, so
> > > > it knows where to load it. There are about 30 places where this is needed,
> > > > in addition to TEXT_BASE. The environment has the same problem, adding
> > > > another dozen or so caes in include/config_distro_bootcmd.h but it has
> > > > been decided to ignore that for now.
> > > >
> > > > The feature is controlled by an environment variable, since it seems to be
> > > > bad form to add flags to the conf tool.
> > > >
> > > > Rebuild the autoconf files if the split config is not present. This allows
> > > > building this commit as part of a chain, without generating build errors.
> > > >
> > > > These changes may benefit from some reworking to send upstream, e.g. to
> > > > use a struct for the 'arg' parameter.
> > > >
> > > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > >
> > > This patch, I think, is where my largest problem is. We go from being
> > > able to say "if CONFIG_SPL_FOO is undefined, it is false" to "we must
> > > define CONFIG_SPL_FOO to false". There's around 150 cases of this, with
> > > the series. Why can we not extend the PPL logic (which I'm not super
> > > happy with, but, I understand and I think an audit of everything
> > > not-TEXT_BASE should be fairly straight forward), to say that if
> > > CONFIG_FOO exists and CONFIG_SPL_FOO does not exist, say CONFIG_SPL_FOO
> > > is now false.
> >
> > Well it is all about choices.
> >
> > We don't have to add a CONFIG_SPL_xxx to tell Kconfig that the PPL
> > symbol controls all phases. We can use the conf_nospl file instead.
> > But then the entire description is not in Kconfig. Of course, we might
> > expect that some of those things in conf_nospl might end up needing to
> > be controlled in SPL, so perhaps that file would shrink? Not sure
> > about that, though.
> >
> > It isn't just SPL , BTW. We might have any xPL symbol defined.
> >
> > I suppose you are thinking of something like:
> >
> > #define CONFIG_IS_ENABLED(x)  IS_ENABLED(CONFIG_ ## xpl_prefix ## x) ||
> >    ( IS_ENABLED(CONFIG_ ## x) && !IS_ENABLED(CONFIG_ SPL_ ## x) &&
> > !IS_ENABLED(CONFIG_TPL_ ##x) ..)
> >
> > But how do we deal with Makefiles? We still end up with the SPL_TPL_ stuff.
>
> I see it as a very useful feature that today if we don't set
> CONFIG_xPL_FOO, it evaluates to false, anywhere we need it to. In all of
> the prep work for split config, I think we've seen one case where we got
> things wrong (in that it lead to failure). Split config build is doing a
> whole bunch of things to then remove this feature. And the series
> intentionally ignores the Makefile design issue / feature of wrapping
> large chunks with a check for being/not-being in an xPL_BUILD phase.

In another thread you suggested moving to separate defconfig files for
each phase (I think). Presumably that would operate the same way as
this series, in that you would not be able to handle the special case
you mention. So I see this series as the best of both worlds - a
unified Kconfig (and .config) but with separate auto.conf files for
each phase. Plus we can drop the SPL_TPL_ stuff.

We can add things to conf_nospl and then the behaviour is as you want
it, isn't it?

For the Makefile xPL ifdefs, is it essential to do anything there? We
could remove them, using the same techniques in this series. I would
quite like to, but we don't have to, if you like them. My reason for
wanting to remove them is that we end up refactoring things to enable
features in xPL, instead of just adding an xPL option.

Finally, with the series I have, it is quite easy to add a new phase
if we want to.

>
> > Yes, adding PPL moves a step forward and reduces the audit to only
> > uses of PPL, instead of the whole Kconfig.
> >
> > This series does get us closer to separate configs (it is really easy
> > to see what is enabled in each build just by looking at the
> > auto_xpl.conf files) and I think those 150 exceptions are not a big
> > price to pay.
> >
> > Ultimately I am coming to the view that we should extend the Kconfig
> > language to support multiple build phases, using a 'phase' property.
> > Zephyr is going to need it too, fairly soon[1].
> >
> > Regards,
> > Simon
> >
> > [1] https://github.com/zephyrproject-rtos/zephyr/issues/54534
>
> Yes, if the Kconfig language moves in a new direction, it would be good
> to follow and use that.

Well I am thinking of moving it there...but would need to start by
upstreaming the two patches in this series.

Regards,
Simon