Re: [PATCH 2/2] video: fix build with CONFIG_VIDEO && !CONFIG_VIDEO_LOGO

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 3675 bytes --]

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 <jstephan@baylibre.com>
> ---
>  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.

>  
>  	/* 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]