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?
Cheers
Julien
> >
> > /* register cyclic as soon as the first video device is probed */
> > if (CONFIG_IS_ENABLED(CYCLIC) && (gd->flags && GD_FLG_RELOC) &&
> > diff --git a/include/video.h b/include/video.h
> > index 9ea6b676463..8e4c1544e56 100644
> > --- a/include/video.h
> > +++ b/include/video.h
> > @@ -418,9 +418,16 @@ bool video_is_active(void);
> > /**
> > * video_get_u_boot_logo() - Get a pointer to the U-Boot logo
> > *
> > - * Returns: Pointer to logo
> > + * Returns: Pointer to logo, or NULL if CONFIG_VIDEO_LOGO is disabled
> > */
> > +#if CONFIG_IS_ENABLED(VIDEO_LOGO)
> > void *video_get_u_boot_logo(void);
> > +#else
> > +static inline void *video_get_u_boot_logo(void)
> > +{
> > + return NULL;
> > +}
> > +#endif
>
> This part does make sense.
>
> --
> Tom