xPL Proposal

[Tom Rini]

On Mon, Feb 17, 2025 at 12:11:12PM -0700, Simon Glass wrote:
> Hi Tom,
> 
> On Mon, 17 Feb 2025 at 11:50, Tom Rini <trini at konsulko.com> wrote:
> >
> > On Tue, Feb 11, 2025 at 03:22:22PM -0600, Tom Rini wrote:
> > > On Tue, Feb 11, 2025 at 08:03:20AM -0700, Simon Glass wrote:
> > > > Hi,
> > > >
> > > > I just wanted to send a note to (re-)introduce my ideas[1] for the
> > > > next iteration of xPL.
> > > >
> > > > A recent series introduced 'xPL' as the name for the various
> > > > pre-U-Boot phases, so now CONFIG_XPL_BUILD means that this is any xPL
> > > > phase and CONFIG_SPL means this really is the SPL phase, not TPL. We
> > > > still use filenames and function naming which uses 'spl', but could
> > > > potentially adjust that.
> > > >
> > > > The major remaining problem IMO is that it is quite tricky and
> > > > expensive (in terms of time) to add a new phase. We also have some
> > > > medium-sized problems:
> > > >
> > > > a. The $(PHASE_), $(SPL_) rules in the Makefile are visually ugly and
> > > > can be confusing, particularly when combined with ifdef and ifneq
> > > >
> > > > b. We have both CONFIG_IS_ENABLED() and IS_ENABLED() and they mean
> > > > different things. For any given option, some code uses one and some
> > > > the other, depending on what problems people have met along the way.
> > > >
> > > > c. An option like CONFIG_FOO is ambiguous, in that it could mean that
> > > > the option is enabled in one or more xPL phases, or just in U-Boot
> > > > proper. The only way to know is to look for $(PHASE_) etc. in the
> > > > Makefiles and CONFIG_IS_ENABLED() in the code. This is very confusing
> > > > and has not scaled well.
> > > >
> > > > d. We need to retain an important feature: options from different
> > > > phases can depend on each other. As an example, we might want to
> > > > enable MMC in SPL by default, if MMC is enabled in U-Boot proper. We
> > > > may also want to share values between phases, such as TEXT_BASE. We
> > > > can do this easily today, just by adding Kconfig rules.
> > >
> > > I agree with a through c and for d there are likely some cases even if
> > > I'm not sure TEXT_BASE is a good example. But I'm not sure it's as
> > > important as the other ones.
> > >
> > > > Proposal
> > > >
> > > > 1. Adjust kconf to generate separate autoconf.h files for each phase.
> > > > These contain the values for each Kconfig option for that phase. For
> > > > example CONFIG_TEXT_BASE in autoconf_spl.h is SPL's text base.
> > > >
> > > > 2. Add a file to resolve the ambiguity in (c) above, listing the
> > > > Kconfig options which should not be enabled/valid in any xPL build.
> > > > There are around 200 of these.
> > > >
> > > > 3. Introduce CONFIG_PPL as a new prefix, meaning U-Boot proper (only),
> > > > useful in rare cases. This indicates that the option applies only to
> > > > U-Boot proper and is not defined in any xPL build. It is analogous to
> > > > CONFIG_TPL_xxx meaning 'enabled in TPL'. Only a dozen of these are
> > > > needed at present, basically to allow access to the value for another
> > > > phase, e.g. SPL wanting to find CONFIG_PPL_TEXT_BASE so that it knows
> > > > the address to which U-Boot should be loaded.
> > > >
> > > > 4. There is no change to the existing defconfig files, or 'make
> > > > menuconfig', which works just as today, including dependencies between
> > > > options across all phases.
> > > >
> > > > 5. (next) Expand the Kconfig language[2] to support declaring phases
> > > > (SPL, TPL, etc.) and remove the need for duplicating options (DM_MMC,
> > > > SPL_DM_MMC, TPL_DM_MMC, VPL_DM_MMC), so allowing an option to be
> > > > declared once for any/all phases. We can then drop the file in 2
> > > > above.
> > > >
> > > > With this, maintaining Kconfig options, Makefiles and adding a new
> > > > phase should be considerably easier.
> > >
> > > I think this will not make our life easier, it will make things harder.
> > >
> > > I think what we've reached now shows that Yamada-san was correct at the
> > > time in saying that we were going down the wrong path with how we
> > > handled SPL/TPL.
> > >
> > > My request instead is:
> > > - Largely drop SPL/TPL/VPL (so no DM_MMC and SPL_DM_MMC and so on, just
> > >   DM_MMC) as a prefix.
> > > - Likely need to introduce a PPL symbol as you suggest.
> > > - Make PPL/SPL/TPL/VPL be a choice statement when building a defconfig.
> > > - Split something like rockpro64-rk3399_defconfig in to
> > >   rockpro64-rk3399_ppl_defconfig
> > >   rockpro64-rk3399_spl_defconfig rockpro64-rk3399_tpl_defconfig
> > >   and add Makefile logic such that for X_defconfig as a build target but
> > >   not configs/X_defconfig not existing, we see if any of
> > >   configs/X_{ppl,spl,tpl,vpl}_defconfig exist and we run a builds in
> > >   subdirectories of our object directory, and then using binman combine
> > >   as needed.
> > >   - Maybe instead the Makefile logic above we would parse X_defconfig
> > >     and see if it's a different format of say PHASE:file to make it
> > >     easier to say share a single TPL config with all rk3399, have a few
> > >     common SPL configs and then just a board specific PPL.
> > >
> > > This solves (a) by removing them entirely. This solves (b) by removing
> > > the ambiguity entirely (it will be enabled or not). As a bonus for (b)
> > > we can switch everyone to IS_ENABLED(CONFIG_FOO) and match up with the
> > > Linux Kernel again. This solves (c) again by removing it entirely.
> >
> > Lets come back up here, to my proposal, since parts of it seem to have
> > not been clear enough. While what I'm proposing should work for any
> > platform and xPL -> xPL -> ... -> PPL, for this example let us assume
> > rockpro64-rk3399 supports the flow of TPL -> SPL -> PPL. Also, to
> > compare with today, it will be helpful to run "make
> > O=/tmp/rockpro64-rk3399_current rockpro64-rk3399_config" and have the
> > resulting .config file available.
> >
> > There shall be configs/rockpro64-rk3399_tpl_defconfig. This will contain
> > lines such as:
> > CONFIG_ARM=y
> > CONFIG_ARCH_ROCKCHIP=y
> > CONFIG_ROCKCHIP_RK3399=y
> > CONFIG_TPL=y
> >
> > When you run "make O=/tmp/rockpro64-rk3399_tpl rockpro64-rk3399_tpl_defconfig"
> > the resulting .config file will contain lines such as:
> > # CONFIG_ROCKCHIP_EXTERNAL_TPL is not set
> > as this only makes sense in the context of building something that will
> > be TPL.
> >
> > A more complex example is that it will also contain:
> > CONFIG_TPL_ROCKCHIP_COMMON_BOARD=y
> >
> > Because looking at arch/arm/mach-rockchip/Makefile a bunch of that will
> > be able to be simplified (and spl_common.c should be renamed to
> > xpl_common.c) to:
> > obj-$(CONFIG_SPL_ROCKCHIP_COMMON_BOARD) += spl.o spl-boot-order.o xpl_common.o
> > obj-$(CONFIG_TPL_ROCKCHIP_COMMON_BOARD) += tpl.o xpl_common.o
> >
> > The .config file here will also contain:
> > CONFIG_DM_SERIAL=y
> >
> > What it will not contain is:
> > CONFIG_TPL_DM_SERIAL=y
> >
> > This is because there is no 'config TPL_DM_SERIAL' option in
> > drivers/serial/Kconfig anymore.
> >
> > When you next run "make O=/tmp/rockpro64-rk3399_tpl all" the results in
> > /tmp/rockpro64-rk3399_tpl would be similar to the results as under
> > "/tmp/rockpro64-rk3399/tpl/" when building today.
> >
> > The contents of configs/rockpro64-rk3399_spl_defconfig would be similar
> > to the tpl one, except with SPL-only-ever-valid options such as
> > CONFIG_SPL_ROCKCHIP_COMMON_BOARD=y but otherwise have CONFIG_DM_SERIAL=y
> > and no CONFIG_SPL_DM_SERIAL=y, and when building the "all" target, you
> > would only get similar results to what is under the spl/ directory
> > today.
> >
> > Next we have configs/rockpro64-rk3399_ppl_defconfig.  When you run "make
> > O=/tmp/rockpro64-rk3399_ppl rockpro64-rk3399_ppl_defconfig" the
> > important difference is what you do not have. You do not have:
> > CONFIG_SPL=y
> > CONFIG_TPL=y
> >
> > Because we are not building SPL nor TPL. We're just making full U-Boot
> > itself. This is where in more full examples and with additional
> > restructure a "generic-arm64_ppl_defconfig" makes sense and be used
> > instead.
> >
> > This brings up what to do with "ockpro64-rk3399_defconfig". And I'm a
> > little unsure which of the things I mentioned above is best. It's
> > either:
> > a) Does not exist, top-level Makefile says roughly:
> > %_defconfig: %_tpl_defconfig %_spl_defconfig %_ppl_defconfig
> >   make O=$(objdir)/tpl %_tpl_defconfig all
> >   make O=$(objdir)/spl %_spl_defconfig all
> >   make O=$(objdir)/ppl %_ppl_defconfig all
> >
> > But this might be too rigid.
> > b) It contains:
> > PHASE:VPL:rockpro64-rk3399_vpl_defconfig
> > PHASE:TPL:rockpro64-rk3399_tpl_defconfig
> > PHASE:SPL:rockpro64-rk3399_spl_defconfig
> > PHASE:PPL:rockpro64-rk3399_ppl_defconfig
> > And the top-level Makefile looks like:
> > %_defconfig:
> >   grep -q ^PHASE $@ || fatal "Invalid defconfig file, please see..."
> >   foreach line in $@
> >     make O=$(objdir)/$PHASE $CONFIGFILE all
> >
> > It could also be some other suggestion.
> 
> Thanks for writing that up. It is somewhat clearer.
> 
> What happens to the Makefiles? Do they still have $(PHASE_) in them?

No. Because CONFIG_SPL_FIT would never exist, $(CONFIG_$(PHASE_)FIT)
would be meaningless. Only rockpro64-rk3399_spl_defconfig would say
CONFIG_FIT=y (or more likely, only the resulting .config would say
CONFIG_FIT=y just like how configs/rockpro64-rk3399_defconfig doesn't
say CONFIG_FIT=y nor CONFIG_SPL_FIT=y).

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20250217/51d09a98/attachment.sig>