This seems less consistent to me, although tbh I don't understand the
Linux device and driver framework that well. I think the original idea
here was that the coreboot bus represents the coreboot table, and each
entry is represented as one device (regardless of whether any driver
actually wants to do anything with that entry). That's why we're
creating a device for all the tags even though most of them aren't
really interesting for kernel drivers. This also helps with inspecting
things in sysfs.

So the device with TAG_FRAMEBUFFER doesn't mean that we have a
framebuffer, it just means that we have an entry in the table
describing a framebuffer. Whether it should actually be used to set up
a framebuffer should be up to the binding driver. That's why I think
keeping this decision in the driver probe makes more sense, and
excluding it from the devices on the bus is weird (because you're just
randomly excluding one of the entries in the table from being
represented like the others, just because of details about how its
drivers would want to use it).

If you want these devices to be bound by drivers outside this
directory, then I think either that other driver needs to have the
logic to decide when a coreboot framebuffer should actually be used,
or maybe you should have a small stub driver in this directory that
binds to the list entry device, makes the decision whether to actually
use it, and if so creates a sub-device or something (if that's
possible?) which the actual outside driver can find and bind to.

On Thu, Jan 8, 2026 at 3:51 PM Thomas Zimmermann <[email protected]> wrote:
>
> Test sysfb before creating the coreboot framebuffer device. Skip
> device creation if the test fails, as this framebuffer does not exist.
>
> Depending on the system setup, the initial framebuffer can be provided
> by the boot loader via screen_info boot parameters and handled by the
> kernel's sysfb code in drivers/firmware/sysfb.c. With the sysfb test in
> the coreboot-framebuffer probing, the coreboot device is present without
> the framebuffer. Even after the sysfb device has been replaced with a
> native PCI device, the coreboot device persists.
>
> Skipping device creation early avoids all these inconsistencies. It
> further prepares coreboot to support graphics drivers besides the one
> in framebuffer-coreboot.c.
>
> Signed-off-by: Thomas Zimmermann <[email protected]>
> ---
>  drivers/firmware/google/coreboot_table.c       | 17 +++++++++++++++++
>  drivers/firmware/google/coreboot_table.h       |  1 +
>  drivers/firmware/google/framebuffer-coreboot.c | 16 ----------------
>  3 files changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/firmware/google/coreboot_table.c 
> b/drivers/firmware/google/coreboot_table.c
> index 882db32e51be..c34426e5002d 100644
> --- a/drivers/firmware/google/coreboot_table.c
> +++ b/drivers/firmware/google/coreboot_table.c
> @@ -18,6 +18,7 @@
>  #include <linux/of.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
> +#include <linux/sysfb.h>
>
>  #include "coreboot_table.h"
>
> @@ -118,6 +119,22 @@ static int coreboot_table_populate(struct device *dev, 
> void *ptr)
>                         return -EINVAL;
>                 }
>
> +               switch (entry->tag) {
> +               case CB_TAG_FRAMEBUFFER:
> +                       /*
> +                        * On coreboot systems, the advertised 
> CB_TAG_FRAMEBUFFER entry
> +                        * in the coreboot table should only be used if the 
> payload did
> +                        * not pass a framebuffer information to the Linux 
> kernel.
> +                        *
> +                        * If the global screen_info data has been filled, 
> the generic
> +                        * system framebuffers (sysfb) will already register 
> a platform
> +                        * device and pass that screen_info as platform_data 
> to a driver
> +                        * that can scan-out using the system-provided 
> framebuffer.
> +                        */
> +                       if (sysfb_handles_screen_info())
> +                               continue;
> +               }
> +
>                 device = kzalloc(sizeof(device->dev) + entry->size, 
> GFP_KERNEL);
>                 if (!device)
>                         return -ENOMEM;
> diff --git a/drivers/firmware/google/coreboot_table.h 
> b/drivers/firmware/google/coreboot_table.h
> index bb6f0f7299b4..e3c353676940 100644
> --- a/drivers/firmware/google/coreboot_table.h
> +++ b/drivers/firmware/google/coreboot_table.h
> @@ -40,6 +40,7 @@ struct lb_cbmem_ref {
>         u64 cbmem_addr;
>  };
>
> +#define CB_TAG_FRAMEBUFFER 0x12
>  #define LB_TAG_CBMEM_ENTRY 0x31
>
>  /* Corresponds to LB_TAG_CBMEM_ENTRY */
> diff --git a/drivers/firmware/google/framebuffer-coreboot.c 
> b/drivers/firmware/google/framebuffer-coreboot.c
> index c68c9f56370f..bb53d1a47409 100644
> --- a/drivers/firmware/google/framebuffer-coreboot.c
> +++ b/drivers/firmware/google/framebuffer-coreboot.c
> @@ -15,12 +15,9 @@
>  #include <linux/module.h>
>  #include <linux/platform_data/simplefb.h>
>  #include <linux/platform_device.h>
> -#include <linux/sysfb.h>
>
>  #include "coreboot_table.h"
>
> -#define CB_TAG_FRAMEBUFFER 0x12
> -
>  static const struct simplefb_format formats[] = SIMPLEFB_FORMATS;
>
>  static int framebuffer_probe(struct coreboot_device *dev)
> @@ -37,19 +34,6 @@ static int framebuffer_probe(struct coreboot_device *dev)
>                 .format = NULL,
>         };
>
> -       /*
> -        * On coreboot systems, the advertised LB_TAG_FRAMEBUFFER entry
> -        * in the coreboot table should only be used if the payload did
> -        * not pass a framebuffer information to the Linux kernel.
> -        *
> -        * If the global screen_info data has been filled, the Generic
> -        * System Framebuffers (sysfb) will already register a platform
> -        * device and pass that screen_info as platform_data to a driver
> -        * that can scan-out using the system provided framebuffer.
> -        */
> -       if (sysfb_handles_screen_info())
> -               return -ENODEV;
> -
>         if (!fb->physical_address)
>                 return -ENODEV;
>
> --
> 2.52.0
>

Reply via email to