Re: [PATCH 11/11] spl: Use CONFIG_IS_ENABLED() for LIBCOMMON_SUPPORT checks

Hi Quentin,

On Tue, 21 Jul 2026 at 03:58, Quentin Schulz via U-Boot
<[email protected]> wrote:
>
> Hi Jonas,
>
> On 7/9/26 12:05 AM, Jonas Karlman wrote:
> > Use CONFIG_IS_ENABLED(LIBCOMMON_SUPPORT) instead of directly checking
> > CONFIG_SPL_LIBCOMMON_SUPPORT when guarding printf/log calls to depend on
> > correct xPL_LIBCOMMON_SUPPORT symbol. Also change to use IS_ENABLED()
> > for the XPL_BUILD symbol checks for consistency.
> >
> > Signed-off-by: Jonas Karlman <[email protected]>
> > ---
> >   boot/common_fit.c        |  2 +-
> >   common/spl/spl_usb.c     |  2 +-
> >   drivers/mmc/mmc-uclass.c |  4 ++--
> >   drivers/mmc/mmc.c        | 16 ++++++++--------
> >   drivers/mmc/mmc_legacy.c |  4 ++--
> >   include/spl.h            |  6 +++---
> >   lib/hang.c               |  2 +-
> >   7 files changed, 18 insertions(+), 18 deletions(-)
> >
> > diff --git a/boot/common_fit.c b/boot/common_fit.c
> > index fd434fe28e19..9dcec1cd7d7d 100644
> > --- a/boot/common_fit.c
> > +++ b/boot/common_fit.c
> > @@ -53,7 +53,7 @@ int fit_find_config_node(const void *fdt)
> >            node = fdt_next_subnode(fdt, node)) {
> >               name = fdt_getprop(fdt, node, FIT_DESC_PROP, &len);
> >               if (!name) {
> > -#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
> > +#if !IS_ENABLED(CONFIG_XPL_BUILD) || CONFIG_IS_ENABLED(LIBCOMMON_SUPPORT)
>
> I'm stuck on this and cannot make sense of it. I understand we don't
> have a CONFIG_LIBCOMMON_SUPPORT and we may want to printf in proper
> anyway...
>
> I guess the issue is that if we use
> CONFIG_IS_ENABLED(LIBCOMMON_SUPPORT), we will not check for
> CONFIG_SPL_LIBCOMMON_SUPPORT in proper (which is incorrect but what
> we're doing today) anymore and thus the printf won't be compiled in.
>
> I'm thinking the right approach would be to define a
>
> config LIBCOMMON_SUPPORT
>      def_bool y
>      help
>        Enable support for common U-Boot libraries. [...]
>
> and replace all
>
> #if !defined(CONFIG_XPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>
> with
>
> #if CONFIG_IS_ENABLED(LIBCOMMON_SUPPORT)

Seems good, although we should not need the _SUPPORT suffix

>
> I'm assuming we don't have an issue with TPL using SPL_LIBCOMMON_SUPPORT
> without TPL_LIBCOMMON_SUPPORT as it won't be able to link due to missing
> library support in TPL?
>
> Then we can simply replace the above in boot/common_fit.c with
> CONFIG_IS_ENABLED(LIBCOMMON_SUPPORT).

[..]

>
> A bit of a tangent, one of my biggest gripes about U-Boot is that we do
> not differentiate symbols that are meant to be phase-agnostic and those
> which are specific to a phase. I'm thinking we should have a PROPER_
> prefix for symbols that are expected to apply to U-Boot proper only. If
> you have neither xPL or PROPER prefix, then it's meant to apply to all
> phases. But we often have mixes and it's difficult to know what's
> supposed to be used or not. I think it would make things much clearer as
> well.

With the xPL work I did some years back, I had a PPL_ prefix to mean
it is proper (when we want to distinguish it from a symbol which
applies to all phases). So for example, CONFIG_PPL_THIS would only
apply in proper, whereas CONFIG_THIS would apply in all phases. There
are not that many cases where it is important, but there are some.

>
> For example, we have some ifdeffery in lib/Makefile around
> USE_TINY_PRINTF which we wouldn't need, had we a symbol for the proper
> phase (which can always be n/y and non-selectable by the user), then we
> simply can remove a bunch of ifeq ($(CONFIG_XPL_BUILD),y) checks.
>
> Anyway, nothing required for this series and likely a big rework that
> may not be worth it in the end.

Regards,
Simon