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
