Re: [PATCH 4/7] fbdev/core: Move logo functions into separate source file

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@xxxxxxx> writes:

> Move the fbdev function for displaying boot-up logos into their
> own file fb_logo.c. The file can later be build depending on the
> state of CONFIG_LOGO. No functional changes.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
> ---
>  drivers/video/fbdev/core/Makefile      |   1 +
>  drivers/video/fbdev/core/fb_internal.h |   6 +
>  drivers/video/fbdev/core/fb_logo.c     | 533 +++++++++++++++++++++++++
>  drivers/video/fbdev/core/fbmem.c       | 524 ------------------------
>  include/linux/fb.h                     |   5 -
>  5 files changed, 540 insertions(+), 529 deletions(-)
>  create mode 100644 drivers/video/fbdev/core/fb_logo.c
>
> diff --git a/drivers/video/fbdev/core/Makefile b/drivers/video/fbdev/core/Makefile
> index edfde2948e5c..adce31155e92 100644
> --- a/drivers/video/fbdev/core/Makefile
> +++ b/drivers/video/fbdev/core/Makefile
> @@ -2,6 +2,7 @@
>  obj-$(CONFIG_FB_NOTIFY)           += fb_notify.o
>  obj-$(CONFIG_FB_CORE)             += fb.o
>  fb-y                              := fb_info.o \
> +                                     fb_logo.o \

Maybe instead of adding this unconditionally you could only add it when
CONFIG_LOGO is enabled ?

[...]

> diff --git a/drivers/video/fbdev/core/fb_logo.c b/drivers/video/fbdev/core/fb_logo.c
> new file mode 100644
> index 000000000000..76ba5a2bebae
> --- /dev/null
> +++ b/drivers/video/fbdev/core/fb_logo.c
> @@ -0,0 +1,533 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/export.h>
> +#include <linux/fb.h>
> +#include <linux/linux_logo.h>
> +
> +bool fb_center_logo __read_mostly;
> +int fb_logo_count __read_mostly = -1;
> +
> +#ifdef CONFIG_LOGO

[...]

> +#else
> +int fb_prepare_logo(struct fb_info *info, int rotate)
> +{
> +	return 0;
> +}
> +EXPORT_SYMBOL(fb_prepare_logo);
> +
> +int fb_show_logo(struct fb_info *info, int rotate)
> +{
> +	return 0;
> +}
> +EXPORT_SYMBOL(fb_show_logo);

I would move the CONFIG_LOGO #ifdefery to the header file and make these a
static inline function, instead of having the stubs here and exporting
symbols for functions that are a no-op.

I think that will also fix the issues that the robot complained about.

Other than that, the patch looks good to me.

Acked-by: Javier Martinez Canillas <javierm@xxxxxxxxxx>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat