Re: [PATCH v2] drivers/firmware: fix SYSFB depends to prevent build failures

[Geert Uytterhoeven]

Hi Javier,

On Tue, Jul 27, 2021 at 11:33 AM Javier Martinez Canillas
<javierm@xxxxxxxxxx> wrote:
> The Generic System Framebuffers support is built when the COMPILE_TEST
> option is enabled. But this wrongly assumes that all the architectures
> declare a struct screen_info.
>
> This is true for most architectures, but at least the following do not:
> arc, m68k, microblaze, openrisc, parisc and s390.
>
> By attempting to make this compile testeable on all architectures, it
> leads to linking errors as reported by the kernel test robot for parisc:
>
>   All errors (new ones prefixed by >>):
>
>      hppa-linux-ld: drivers/firmware/sysfb.o: in function `sysfb_init':
>      (.init.text+0x24): undefined reference to `screen_info'
>   >> hppa-linux-ld: (.init.text+0x28): undefined reference to `screen_info'
>
> To prevent these errors only allow sysfb to be built on systems that are
> going to need it, which are x86 BIOS and EFI.
>
> The EFI Kconfig symbol is used instead of (ARM || ARM64 || RISC) because
> some of these architectures only declare a struct screen_info if EFI is
> enabled. And also, because the SYSFB code is only used for EFI on these
> architectures. For !EFI the "simple-framebuffer" device is registered by
> OF when parsing the Device Tree Blob (if a DT node for this was defined).
>
> Fixes: d391c5827107 ("drivers/firmware: move x86 Generic System Framebuffers support")
> Reported-by: kernel test robot <lkp@xxxxxxxxx>
> Signed-off-by: Javier Martinez Canillas <javierm@xxxxxxxxxx>

Thanks for your patch!

> --- a/drivers/firmware/Kconfig
> +++ b/drivers/firmware/Kconfig
> @@ -254,7 +254,7 @@ config QCOM_SCM_DOWNLOAD_MODE_DEFAULT
>  config SYSFB
>         bool
>         default y
> -       depends on X86 || ARM || ARM64 || RISCV || COMPILE_TEST
> +       depends on X86 || EFI

Thanks, much better.
Still, now this worm is crawling out of the X86 can, I'm wondering
why this option is so important that it has to default to y?
It is not just a dependency for SYSFB_SIMPLEFB, but also causes the
inclusion of drivers/firmware/sysfb.c.

>
>  config SYSFB_SIMPLEFB
>         bool "Mark VGA/VBE/EFI FB as generic system framebuffer"

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds