[PATCH v2 03/18] boot: Add UCLASS support for extension boards
Simon Glass
sjg at chromium.org
Fri Oct 10 13:13:11 CEST 2025
Hi Kory,
On Thu, 9 Oct 2025 at 15:51, Kory Maincent (TI.com)
<kory.maincent at bootlin.com> wrote:
>
> Introduce UCLASS-based extension board support to enable more
> standardized and automatic loading of extension board device tree
> overlays in preparation for integration with bootstd and pxe_utils.
>
> Signed-off-by: Kory Maincent (TI.com) <kory.maincent at bootlin.com>
> ---
> MAINTAINERS | 1 +
> boot/Kconfig | 3 +
> boot/Makefile | 1 +
> boot/extension-uclass.c | 196 ++++++++++++++++++++++++++++++++++++++++++++
> cmd/Kconfig | 2 +-
> cmd/extension_board.c | 50 ++++++++++-
> doc/usage/cmd/extension.rst | 24 +++---
> include/dm/uclass-id.h | 1 +
> include/extension_board.h | 64 +++++++++++++++
> 9 files changed, 326 insertions(+), 16 deletions(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c67d5ae9d6b..a63ffa14ef5 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1187,6 +1187,7 @@ M: Kory Maincent <kory.maincent at bootlin.com>
> S: Maintained
> F: board/sunxi/chip.c
> F: board/ti/common/cape_detect.c
> +F: boot/extension-uclass.c
> F: boot/extension.c
> F: cmd/extension_board.c
> F: include/extension_board.h
> diff --git a/boot/Kconfig b/boot/Kconfig
> index 62aa301f18f..159da81bec5 100644
> --- a/boot/Kconfig
> +++ b/boot/Kconfig
> @@ -1909,6 +1909,9 @@ endif # OF_LIBFDT
> config SUPPORT_EXTENSION_SCAN
> bool
>
> +config SUPPORT_DM_EXTENSION_SCAN
> + bool
> +
> config USE_BOOTARGS
> bool "Enable boot arguments"
> help
> diff --git a/boot/Makefile b/boot/Makefile
> index f60d13130b1..aa26070fbb8 100644
> --- a/boot/Makefile
> +++ b/boot/Makefile
> @@ -10,6 +10,7 @@ obj-$(CONFIG_CMD_BOOTM) += bootm.o bootm_os.o
> obj-$(CONFIG_CMD_BOOTZ) += bootm.o bootm_os.o
> obj-$(CONFIG_CMD_BOOTI) += bootm.o bootm_os.o
> obj-$(CONFIG_SUPPORT_EXTENSION_SCAN) += extension.o
> +obj-$(CONFIG_SUPPORT_DM_EXTENSION_SCAN) += extension-uclass.o
>
> obj-$(CONFIG_PXE_UTILS) += pxe_utils.o
>
> diff --git a/boot/extension-uclass.c b/boot/extension-uclass.c
> new file mode 100644
> index 00000000000..9dfbeb60d20
> --- /dev/null
> +++ b/boot/extension-uclass.c
> @@ -0,0 +1,196 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * (C) Copyright 2025 Köry Maincent <kory.maincent at bootlin.com>
> + */
> +
> +#include <alist.h>
> +#include <command.h>
> +#include <dm/device-internal.h>
> +#include <dm/lists.h>
> +#include <dm/uclass.h>
> +#include <env.h>
> +#include <extension_board.h>
> +#include <fdt_support.h>
> +#include <malloc.h>
> +#include <mapmem.h>
nit: Please correct the ordering
https://docs.u-boot.org/en/latest/develop/codingstyle.html#include-files
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +/* For now, bind the extension device manually if none are found */
> +static struct udevice *extension_get_dev(void)
This should really return the error rather than gobbling it. Normally
we use 'int' as the return value, so:
static int extension_get_dev(struct udevice *devp)
> +{
> + struct driver *drv = ll_entry_start(struct driver, driver);
> + const int n_ents = ll_entry_count(struct driver, driver);
> + struct udevice *dev;
> + int i, ret;
> +
> + /* These are not needed before relocation */
> + if (!(gd->flags & GD_FLG_RELOC))
> + return NULL;
> +
> + uclass_first_device(UCLASS_EXTENSION, &dev);
> + if (dev)
> + return dev;
> +
> + /* Create the extension device if not already bound */
> + for (i = 0; i < n_ents; i++, drv++) {
> + if (drv->id == UCLASS_EXTENSION) {
If this is really what you want, you might as well just put a
U_BOOT_DRVINFO() declaration in the source. Then driver model does it
for you. We are supposed to use the devicetree, but it seems that that
idea is WIP at best.
> + char name[32];
> +
> + snprintf(name, sizeof(name), "%s_dev", drv->name);
> + ret = device_bind_driver(gd->dm_root, drv->name,
> + name, &dev);
> + if (ret) {
> + printf("Bind extension driver %s error=%d\n",
> + drv->name, ret);
> + return NULL;
> + }
> +
> + ret = device_probe(dev);
> + if (ret) {
> + printf("Probe extension driver %s error=%d\n",
> + drv->name, ret);
> + return NULL;
> + }
> +
> + /* We manage only one extension driver for now */
> + return dev;
> + }
> + }
> +
> + return NULL;
> +}
> +
> +struct alist *dm_extension_get_list(void)
> +{
> + struct udevice *dev = extension_get_dev();
> +
> + if (!dev)
> + return NULL;
> +
> + return dev_get_priv(dev);
> +}
> +
> +int dm_extension_probe(struct udevice *dev)
> +{
> + struct alist *extension_list = dev_get_priv(dev);
> +
> + alist_init_struct(extension_list, struct extension);
> + return 0;
> +}
> +
> +int dm_extension_remove(struct udevice *dev)
> +{
> + struct alist *extension_list = dev_get_priv(dev);
> +
> + alist_uninit(extension_list);
> + return 0;
> +}
> +
> +int dm_extension_scan(void)
> +{
> + struct alist *extension_list = dm_extension_get_list();
> + struct udevice *dev = extension_get_dev();
> + const struct extension_ops *ops;
> +
> + if (!dev || !extension_list)
> + return -ENODEV;
> +
> + ops = device_get_ops(dev);
extension_get_ops()
> + if (!ops->scan)
> + return -ENODEV;
That is normally -ENOSYS - at this point we have a device, it's just
that it doesn't have the require method.
But it seem strange to me to allow a device that has no means to scan.
> +
> + alist_empty(extension_list);
> + return ops->scan(extension_list);
> +}
> +
> +static int _extension_apply(const struct extension *extension)
> +{
> + ulong extrasize, overlay_addr;
> + struct fdt_header *blob;
> + char *overlay_cmd;
> + int ret;
> +
> + if (!working_fdt) {
> + printf("No FDT memory address configured. Please configure\n"
> + "the FDT address via \"fdt addr <address>\" command.\n");
> + return -EINVAL;
> + }
> +
> + overlay_cmd = env_get("extension_overlay_cmd");
> + if (!overlay_cmd) {
> + printf("Environment extension_overlay_cmd is missing\n");
> + return -EINVAL;
> + }
> +
> + overlay_addr = env_get_hex("extension_overlay_addr", 0);
> + if (!overlay_addr) {
> + printf("Environment extension_overlay_addr is missing\n");
> + return -EINVAL;
> + }
> +
> + env_set("extension_overlay_name", extension->overlay);
> + ret = run_command(overlay_cmd, 0);
> + if (ret)
> + return ret;
> +
> + extrasize = env_get_hex("filesize", 0);
> + if (!extrasize)
> + return -EINVAL;
> +
> + fdt_shrink_to_minimum(working_fdt, extrasize);
> +
> + blob = map_sysmem(overlay_addr, 0);
> + if (!fdt_valid(&blob)) {
> + printf("Invalid overlay devicetree %s\n", extension->overlay);
> + return -EINVAL;
> + }
> +
> + /* Apply method prints messages on error */
> + ret = fdt_overlay_apply_verbose(working_fdt, blob);
> + if (ret)
> + printf("Failed to apply overlay %s\n", extension->overlay);
> +
> + return ret;
> +}
> +
> +int dm_extension_apply(int extension_num)
> +{
> + struct alist *extension_list = dm_extension_get_list();
> + const struct extension *extension;
> +
> + if (!extension_list)
> + return -ENODEV;
-ENODEV has a special meaning with driver model, which is that there
is no device. I think -ENOENT would be better.
> +
> + extension = alist_get(extension_list, extension_num,
> + struct extension);
> + if (!extension) {
> + printf("Wrong extension number\n");
> + return -ENODEV;
-EINVAL ?
> + }
> +
> + return _extension_apply(extension);
> +}
> +
> +int dm_extension_apply_all(void)
> +{
> + struct alist *extension_list = dm_extension_get_list();
> + const struct extension *extension;
> + int ret;
> +
> + if (!extension_list)
> + return -ENODEV;
-ENOENT
> +
> + alist_for_each(extension, extension_list) {
> + ret = _extension_apply(extension);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +UCLASS_DRIVER(extension) = {
> + .name = "extension",
> + .id = UCLASS_EXTENSION,
> +};
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 986eeeba807..721bdb87c8a 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -548,7 +548,7 @@ config CMD_FDT
> config CMD_EXTENSION
> bool "Extension board management command"
> select CMD_FDT
> - depends on SUPPORT_EXTENSION_SCAN
> + depends on SUPPORT_EXTENSION_SCAN || SUPPORT_DM_EXTENSION_SCAN
> help
> Enables the "extension" command, which allows to detect
> extension boards connected to the system, and apply
> diff --git a/cmd/extension_board.c b/cmd/extension_board.c
> index 78d937ee6b6..d70394f36c7 100644
> --- a/cmd/extension_board.c
> +++ b/cmd/extension_board.c
> @@ -4,6 +4,7 @@
> * Köry Maincent, Bootlin, <kory.maincent at bootlin.com>
> */
>
> +#include <alist.h>
> #include <exports.h>
> #include <command.h>
> #include <extension_board.h>
> @@ -13,9 +14,28 @@ LIST_HEAD(extension_list);
> static int do_extension_list(struct cmd_tbl *cmdtp, int flag,
> int argc, char *const argv[])
> {
> +#if CONFIG_IS_ENABLED(SUPPORT_DM_EXTENSION_SCAN)
> + struct alist *dm_extension_list;
> +#endif
> struct extension *extension;
> int i = 0;
>
> +#if CONFIG_IS_ENABLED(SUPPORT_DM_EXTENSION_SCAN)
Is it possible to use if() ? Perhaps at the end of your series, when
all the conversions are done and you can delete the old code?
> + dm_extension_list = dm_extension_get_list();
> +
> + if (!alist_get_ptr(dm_extension_list, 0)) {
> + printf("No extension registered - Please run \"extension scan\"\n");
> + return CMD_RET_SUCCESS;
> + }
> +
> + alist_for_each(extension, dm_extension_list) {
> + printf("Extension %d: %s\n", i++, extension->name);
> + printf("\tManufacturer: \t\t%s\n", extension->owner);
> + printf("\tVersion: \t\t%s\n", extension->version);
> + printf("\tDevicetree overlay: \t%s\n", extension->overlay);
> + printf("\tOther information: \t%s\n", extension->other);
> + }
> +#else
> if (list_empty(&extension_list)) {
> printf("No extension registered - Please run \"extension scan\"\n");
> return CMD_RET_SUCCESS;
> @@ -28,6 +48,7 @@ static int do_extension_list(struct cmd_tbl *cmdtp, int flag,
> printf("\tDevicetree overlay: \t%s\n", extension->overlay);
> printf("\tOther information: \t%s\n", extension->other);
> }
> +#endif
> return CMD_RET_SUCCESS;
> }
>
> @@ -36,9 +57,19 @@ static int do_extension_scan(struct cmd_tbl *cmdtp, int flag,
> {
> int extension_num;
>
> +#if CONFIG_IS_ENABLED(SUPPORT_DM_EXTENSION_SCAN)
I'm hoping you can use if() instead of #if for all of these?
> + extension_num = dm_extension_scan();
> + if (extension_num == -ENODEV)
> + extension_num = 0;
> + else if (extension_num < 0)
> + return CMD_RET_FAILURE;
> +
> + printf("Found %d extension board(s).\n", extension_num);
> +#else
> extension_num = extension_scan(true);
> - if (extension_num < 0)
> + if (extension_num < 0 && extension_num != -ENODEV)
> return CMD_RET_FAILURE;
> +#endif
>
> return CMD_RET_SUCCESS;
> }
> @@ -46,22 +77,34 @@ static int do_extension_scan(struct cmd_tbl *cmdtp, int flag,
> static int do_extension_apply(struct cmd_tbl *cmdtp, int flag,
> int argc, char *const argv[])
> {
> +#if !CONFIG_IS_ENABLED(SUPPORT_DM_EXTENSION_SCAN)
> struct extension *extension = NULL;
> - int i = 0, extension_id, ret;
> struct list_head *entry;
> + int i = 0;
> +#endif
> + int extension_id, ret;
>
> if (argc < 2)
> return CMD_RET_USAGE;
>
> if (strcmp(argv[1], "all") == 0) {
> ret = CMD_RET_FAILURE;
> +#if CONFIG_IS_ENABLED(SUPPORT_DM_EXTENSION_SCAN)
> + if (dm_extension_apply_all())
> + return CMD_RET_FAILURE;
> +#else
> list_for_each_entry(extension, &extension_list, list) {
> ret = extension_apply(extension);
> if (ret != CMD_RET_SUCCESS)
> break;
> }
> +#endif
> } else {
> extension_id = simple_strtol(argv[1], NULL, 10);
> +#if CONFIG_IS_ENABLED(SUPPORT_DM_EXTENSION_SCAN)
> + if (dm_extension_apply(extension_id))
> + return CMD_RET_FAILURE;
> +#else
> list_for_each(entry, &extension_list) {
> if (i == extension_id) {
> extension = list_entry(entry, struct extension, list);
> @@ -76,9 +119,10 @@ static int do_extension_apply(struct cmd_tbl *cmdtp, int flag,
> }
>
> ret = extension_apply(extension);
> +#endif
> }
>
> - return ret;
> + return CMD_RET_SUCCESS;
or just 0
> }
>
> static struct cmd_tbl cmd_extension[] = {
> diff --git a/doc/usage/cmd/extension.rst b/doc/usage/cmd/extension.rst
> index 4c261e74951..d82c3fbef0a 100644
> --- a/doc/usage/cmd/extension.rst
> +++ b/doc/usage/cmd/extension.rst
> @@ -25,9 +25,8 @@ Device Tree overlays depending on the detected extension boards.
>
> The "extension" command comes with three sub-commands:
>
> - - "extension scan" makes the generic code call the board-specific
> - extension_board_scan() function to retrieve the list of detected
> - extension boards.
> + - "extension scan" makes the generic code call a board-specific extension
> + function to retrieve the list of detected extension boards.
>
> - "extension list" allows to list the detected extension boards.
>
> @@ -98,17 +97,18 @@ Simple extension_board_scan function example
>
> .. code-block:: c
>
> - int extension_board_scan(struct list_head *extension_list)
> + static int foo_extension_board_scan(struct alist *extension_list)
> {
> - struct extension *extension;
> + struct extension extension = {0};
>
> - extension = calloc(1, sizeof(struct extension));
> - snprintf(extension->overlay, sizeof(extension->overlay), "overlay.dtbo");
> - snprintf(extension->name, sizeof(extension->name), "extension board");
> - snprintf(extension->owner, sizeof(extension->owner), "sandbox");
> - snprintf(extension->version, sizeof(extension->version), "1.1");
> - snprintf(extension->other, sizeof(extension->other), "Extension board information");
> - list_add_tail(&extension->list, extension_list);
> + snprintf(extension.overlay, sizeof(extension.overlay), "overlay.dtbo");
> + snprintf(extension.name, sizeof(extension.name), "extension board");
> + snprintf(extension.owner, sizeof(extension.owner), "sandbox");
> + snprintf(extension.version, sizeof(extension.version), "1.1");
> + snprintf(extension.other, sizeof(extension.other), "Extension board information");
> + alist_add(extension_list, extension);
This can fail, so:
if (!alist_add(...))
return -ENOMEM
>
> return 1;
> }
> +
> + U_BOOT_EXTENSION(foo_extension_name, foo_extension_board_scan);
> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
> index 6be59093160..eb6416b5917 100644
> --- a/include/dm/uclass-id.h
> +++ b/include/dm/uclass-id.h
> @@ -63,6 +63,7 @@ enum uclass_id {
> UCLASS_ETH, /* Ethernet device */
> UCLASS_ETH_PHY, /* Ethernet PHY device */
> UCLASS_EXTCON, /* External Connector Class */
> + UCLASS_EXTENSION, /* Extension board */
> UCLASS_FFA, /* Arm Firmware Framework for Armv8-A */
> UCLASS_FFA_EMUL, /* sandbox FF-A device emulator */
> UCLASS_FIRMWARE, /* Firmware */
> diff --git a/include/extension_board.h b/include/extension_board.h
> index 3f70416f005..78139cd7489 100644
> --- a/include/extension_board.h
> +++ b/include/extension_board.h
> @@ -8,9 +8,51 @@
> #define __EXTENSION_SUPPORT_H
>
> #include <linux/list.h>
> +#include <alist.h>
> +#include <dm/device.h>
>
> extern struct list_head extension_list;
>
> +/**
> + * dm_extension_get_list - Get the extension list
> + * Return: The extension alist pointer, or NULL if no such list exists.
caller must free the list?
> + */
> +struct alist *dm_extension_get_list(void);
Is this the list of all extensions from all extension devices?
> +
> +/**
> + * dm_extension_probe - Probe extension device
> + * @dev: Extension device that needs to be probed
> + * Return: Zero on success, negative on failure.
> + */
> +int dm_extension_probe(struct udevice *dev);
> +
> +/**
> + * dm_extension_remove - Remove extension device
> + * @dev: Extension device that needs to be removed
> + * Return: Zero on success, negative on failure.
> + */
> +int dm_extension_remove(struct udevice *dev);
> +
> +/**
> + * dm_extension_scan - Scan extension boards available.
> + * Return: Zero on success, negative on failure.
> + */
> +int dm_extension_scan(void);
> +
> +/**
> + * dm_extension_apply - Apply extension board overlay to the devicetree
> + * @extension_num: Extension number to be applied
> + * Return: Zero on success, negative on failure.
> + */
> +int dm_extension_apply(int extension_num);
The uclass should have a method like apply(struct uclass *dev, int
extension_num). Is the numbering global across all devices?
> +
> +/**
> + * dm_extension_apply_all - Apply all extension board overlays to the
> + * devicetree
> + * Return: Zero on success, negative on failure.
> + */
> +int dm_extension_apply_all(void);
> +
> struct extension {
> struct list_head list;
> char name[32];
> @@ -20,6 +62,28 @@ struct extension {
> char other[32];
> };
At some point in this series, please comment this struct
>
> +struct extension_ops {
> + /**
> + * scan - Add system-specific function to scan extension boards.
> + * @dev: extension device to use
need to document extension_list here - I believe it is a list of
struct extension? Or is it struct extension * ?
> + * Return: The number of extension or a negative value in case of
> + * error.
> + */
> + int (*scan)(struct alist *extension_list);
This is a bit of a strange uclass function! Since you are probing
drivers in this uclass, you can iterate through them using
uclass_foreach...() etc.
I am guessing that you just need to add a struct udevice * as the first arg.
> +};
We should have an extension_get_ops() and extension_scan() stub as
well, for calling this. See how other uclasses do this.
> +
> +#define U_BOOT_EXTENSION(_name, _scan_func) \
> + U_BOOT_DRIVER(_name) = { \
> + .name = #_name, \
> + .id = UCLASS_EXTENSION, \
> + .probe = dm_extension_probe, \
> + .remove = dm_extension_remove, \
> + .ops = &(struct extension_ops) { \
> + .scan = _scan_func, \
> + }, \
> + .priv_auto = sizeof(struct alist), \
> + }
> +
> /**
> * extension_board_scan - Add system-specific function to scan extension board.
> * @param extension_list List of extension board information to update.
>
> --
> 2.43.0
>
Regards,
Simon
More information about the U-Boot
mailing list