Re: [PATCH v2] coreboot_table: skip failing entries instead of aborting populate

From: Brian Norris

Date: Thu Apr 30 2026 - 17:08:13 EST


On Thu, Apr 30, 2026 at 10:23:05PM +0200, Titouan Ameline de Cadeville wrote:
> coreboot_table_populate() registers devices one by one. If
> device_register() fails for one entry, the current code returns
> immediately, leaving previously registered devices orphaned on the
> coreboot bus with no cleanup path.
>
> Since coreboot table entries are independent of each other, a failure
> on one entry should not prevent the others from being registered.
> This mirrors the strategy used by of_platform_populate(), which skips
> individual failures rather than aborting.
>
> Log a warning and continue the loop on device_register() failure.
>
> Signed-off-by: Titouan Ameline de Cadeville <titouan.ameline@xxxxxxxxx>
> ---
> v2: continue the loop on failure instead of unregistering all devices
> (suggested by Julius Werner, with reference to of_platform_populate()
> from Brian Norris)
>
> drivers/firmware/google/coreboot_table.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/google/coreboot_table.c b/drivers/firmware/google/coreboot_table.c
> index c769631ea15d..82be21434be4 100644
> --- a/drivers/firmware/google/coreboot_table.c
> +++ b/drivers/firmware/google/coreboot_table.c
> @@ -150,8 +150,9 @@ static int coreboot_table_populate(struct device *dev, void *ptr)
>
> ret = device_register(&device->dev);
> if (ret) {
> + dev_warn(dev, "failed to register coreboot device: %d\n", ret);
> put_device(&device->dev);
> - return ret;
> + continue;
> }
>
> ptr_entry += entry->size;

Isn't it important to still increment ptr_entry on failure/continue?
Otherwise, you may re-populate a new device with the same contents, and
you may not actually register all devices. (You'll be off-by-1, at
least.)

To fix this, you could either drop the 'continue' (let it fall through)
or else move this line up.

Brian

> --
> 2.44.2
>