[PATCH v2 1/3] Kbuild: Always use $(PHASE_)

[Tom Rini]

On Thu, Mar 13, 2025 at 11:10:28AM +0100, Quentin Schulz wrote:
> Hi Tom,
> 
> On 3/12/25 2:00 AM, Tom Rini wrote:
> > It is confusing to have both "$(PHASE_)" and "$(XPL_)" be used in our
> > Makefiles as part of the macros to determine when to do something in our
> > Makefiles based on what phase of the build we are in. For consistency,
> > bring this down to a single macro and use "$(PHASE_)" only.
> > 
> 
> Haven't gone through the whole thing, but caught a typo at the end, see
> below.
> 
> I assume you did a simple sed here? Or was it more involved than that?

Yes, it was just doing a grep/sed.

> It doesn't seem like any driver has code guards using $(XPL_) so that should
> limit the number of fallouts if any.

There were two problems this exposed, I linked to them in the RFC but
not here as they're general fixes.

> [...]
> > diff --git a/scripts/Makefile.xpl b/scripts/Makefile.xpl
> > index abc49fbe6c93..43f27874f9fe 100644
> > --- a/scripts/Makefile.xpl
> > +++ b/scripts/Makefile.xpl
> > @@ -58,20 +58,18 @@ endif
> >   export SPL_NAME
> > -ifdef CONFIG_XPL_BUILD
> > -XPL_ := SPL_
> > +ifeq ($(CONFIG_SPL_BUILD),y)
> > +PHASE_ := SPL_
> > +else
> >   ifeq ($(CONFIG_VPL_BUILD),y)
> >   PHASE_ := VPL_
> >   else
> >   ifeq ($(CONFIG_TPL_BUILD),y)
> >   PHASE_ := TPL_
> >   else
> > -PHASE_ := SPL_
> > +PHASE_ :=
> >   endif
> >   endif
> > -else
> > -XPL_ :=
> > -PHASE_ :=
> >   endif
> >   ifeq ($(obj)$(CONFIG_SUPPORT_SPL),spl)
> 
> Should the same be done in scripts/Kbuild.include ?

Yes, now that you mention it, I had missed it.

> [...]
> > diff --git a/tools/qconfig.py b/tools/qconfig.py
> > index 259adbe1bc9e..544254f6e78f 100755
> > --- a/tools/qconfig.py
> > +++ b/tools/qconfig.py
> > @@ -1220,8 +1220,6 @@ def scan_makefiles(fnames):
> >       >>> RE_MK_CONFIGS.search('CONFIG_FRED').groups()
> >       (None, 'FRED')
> > -    >>> RE_MK_CONFIGS.search('CONFIG_$(XPL_)MARY').groups()
> > -    ('$(XPL_)', 'MARY')
> 
> Can we update RE_MK_CONFIGS re as well to remove \$\(XPL_\) from it? No clue
> if applicable, just stumbled upon it and wondered.

Likely so, I'll look at that part before posting v3.

> >       >>> RE_MK_CONFIGS.search('CONFIG_$(PHASE_)MARY').groups()
> >       ('$(PHASE_)', 'MARY')
> >       """
> > @@ -1321,7 +1319,7 @@ def do_scan_source(path, do_update):
> >               spl_mode (int): If MODE_SPL, look at source code which implies
> >                   an xPL_ option, but for which there is none;
> >                   for MOD_PROPER, look at source code which implies a Proper
> > -                option (i.e. use of CONFIG_IS_ENABLED() or $(XPL_) or
> > +                option (i.e. use of CONFIG_IS_ENABLED() or $(PHASE_) or
> >                   $(PHASE_) but for which there none;
> 
> You can remove one of the "or $(PHASE_)" here.

Oh hah. I did review this change a bit more manually than the rest
(hence the first part being a remove, the 'sed' just made it check
PHASE_ twice) and missed this, thanks.

-- 
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/20250313/f8357471/attachment.sig>