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
signature.asc
Description: PGP signature
