On Wed, Sep 02, 2026 at 10:48:38AM +0200, Julien Stephan wrote: > Le mar. 1 sept. 2026 à 18:42, Tom Rini <[email protected]> a écrit : > > > > On Tue, Sep 01, 2026 at 10:52:43AM +0200, Julien Stephan wrote: > > > Building with CONFIG_VIDEO enabled but CONFIG_VIDEO_LOGO disabled fails > > > at link time: > > > > > > video-uclass.o: in function `video_get_u_boot_logo': > > > video-uclass.c:593: undefined reference to `__splash_u_boot_logo_begin' > > > > > > The __splash_u_boot_logo_begin/_end symbols are provided by > > > u_boot_logo.bmp.o, which is only built when CONFIG_VIDEO_LOGO is set: > > > > > > obj-$(CONFIG_VIDEO_LOGO) += u_boot_logo.bmp.o > > > > > > video_get_u_boot_logo() and show_splash() reference those symbols > > > unconditionally, so with the logo disabled the reference is left > > > dangling. show_splash() alone would be dead-code eliminated (it is > > > static and only reached under a CONFIG_IS_ENABLED(VIDEO_LOGO) guard), > > > but video_get_u_boot_logo() is an exported function and is always > > > emitted. > > > > > > Guard the splash helpers and their symbol references with > > > CONFIG_IS_ENABLED(VIDEO_LOGO), and provide a static inline > > > video_get_u_boot_logo() stub returning NULL for the disabled case in > > > video.h. Callers already handle a NULL logo pointer (e.g. > > > bootflow_menu.c), so no caller changes are needed. > > > > > > Reproduce with any board that enables VIDEO without VIDEO_LOGO or > > > enabling SPLASH_SCREEN (it disables automatically VIDEO_LOGO). > > > > > > Fixes: 0d3890188d6b ("video: Add function to obtain the U-Boot logo") > > > Signed-off-by: Julien Stephan <[email protected]> > > > --- > > > drivers/video/video-uclass.c | 7 +++++-- > > > include/video.h | 9 ++++++++- > > > 2 files changed, 13 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c > > > index de161054d52..4c959a57619 100644 > > > --- a/drivers/video/video-uclass.c > > > +++ b/drivers/video/video-uclass.c > > > @@ -579,6 +579,7 @@ int video_get_ysize(struct udevice *dev) > > > return priv->ysize; > > > } > > > > > > +#if CONFIG_IS_ENABLED(VIDEO_LOGO) > > > #define SPLASH_DECL(_name) \ > > > extern u8 __splash_ ## _name ## _begin[]; \ > > > extern u8 __splash_ ## _name ## _end[] > > > @@ -598,6 +599,7 @@ static int show_splash(struct udevice *dev) > > > > > > return video_bmp_display(dev, map_to_sysmem(data), -4, 4, true); > > > } > > > +#endif > > > > > > int video_default_font_height(struct udevice *dev) > > > { > > > @@ -716,14 +718,15 @@ static int video_post_probe(struct udevice *dev) > > > return ret; > > > } > > > > > > - if (CONFIG_IS_ENABLED(VIDEO_LOGO) && > > > - !CONFIG_IS_ENABLED(SPLASH_SCREEN) && !plat->hide_logo) { > > > +#if CONFIG_IS_ENABLED(VIDEO_LOGO) > > > + if (!CONFIG_IS_ENABLED(SPLASH_SCREEN) && !plat->hide_logo) { > > > ret = show_splash(dev); > > > if (ret) { > > > log_debug("Cannot show splash screen\n"); > > > return ret; > > > } > > > } > > > +#endif > > > > Is this hunk really needed? I can see getting here as part of debugging > > the problem, but before the change it should evaluate to 'if (0 && ...)' > > and be link-time eliminated. > > > > Hi Tom, > > Yes it is needed since I moved show_splash() inside the #if > CONFIG_IS_ENABLED(VIDEO_LOGO) guard above. > I can go back to the runtime check, and keep show_splash() outside of > the guard, but I'll have to use video_get_u_boot_logo() instead of > relying on SPLASH_START(u_boot_logo); > > What do you prefer?
Ah, I see now, that wasn't clear to me from the context. Looking at 1/2 and then 2/2 now, can we just remove show_splash() and call video_bmp_display directly? That means not guarding SPLASH_START but again it should optimize away. I do complain about how if (CONFIG_IS_ENABLED(...)) isn't always great, but I think mixing that and #if together just reads awkwardly too. So if we can't restructure the code to keep if (CONFIG_IS_ENABLED(...)) then we should #if the whole block there. Thanks. -- Tom
signature.asc
Description: PGP signature
