[PATCH v7 2/7] misc: fs_loader: reorganize and split to FS loader and FW UCLASS

[Simon Glass]

Hi Christian,

On 2026-06-16T19:18:35, Christian Marangi <ansuelsmth at gmail.com> wrote:
> misc: fs_loader: reorganize and split to FS loader and FW UCLASS
>
> In preparation to the introduction of variant of the FS loader,
> reorganize and split the driver to generic fw_loader UCLASS and
> specific fs_loader function. Create a dedicated directory for the loader
> and move the internal structs and functions to a dedicated header file.
> While at it also drop some redundant check and rename some variables as we
> are updating the code for the UCLASS rework.
>
> This will permit to reuse all the property and logic of FS loader
> with container that are not exactly a readable filesystem.
>
> Signed-off-by: Christian Marangi <ansuelsmth at gmail.com>
>
> drivers/misc/Kconfig                      |   5 +
>  drivers/misc/Makefile                     |   2 +-
>  drivers/misc/fs_loader.c                  | 328 ------------------------------
>  drivers/misc/fw_loader/Makefile           |   4 +
>  drivers/misc/fw_loader/fs_loader.c        | 186 +++++++++++++++++
>  drivers/misc/fw_loader/fw_loader-uclass.c | 159 +++++++++++++++
>  drivers/misc/fw_loader/internal.h         |  73 +++++++
>  include/dm/uclass-id.h                    |   2 +-
>  include/fs_loader.h                       |  47 +----
>  include/fw_loader.h                       |  19 ++
>  10 files changed, 450 insertions(+), 375 deletions(-)

> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
> @@ -69,7 +69,7 @@ enum uclass_id {
> -     UCLASS_FS_FIRMWARE_LOADER,              /* Generic loader */
> +     UCLASS_FIRMWARE_LOADER, /* Firmware loader */

This rename leaves doc/develop/driver-model/fs_firmware_loader.rst
stale - it still has two references to UCLASS_FS_FIRMWARE_LOADER
(around lines 90 and 148). Please update the rst file (or rename it)
in this patch.

> diff --git a/drivers/misc/fw_loader/fw_loader-uclass.c b/drivers/misc/fw_loader/fw_loader-uclass.c
> @@ -0,0 +1,159 @@
> +static int fw_loader_pre_probe(struct udevice *dev)
> +{
> +#if CONFIG_IS_ENABLED(DM)
> +     struct device_plat *plat = dev_get_uclass_plat(dev);
> +     ofnode fw_loader_node;
> +
> +     if (!IS_ENABLED(CONFIG_DM))
> +             return 0;

The outer #if CONFIG_IS_ENABLED(DM) already gates this body, so the
runtime if (!IS_ENABLED(CONFIG_DM)) is dead code - and it confuses the
meaning, since CONFIG_IS_ENABLED() is phase-aware while
IS_ENABLED(CONFIG_DM) is not. Please drop the inner check.

> diff --git a/drivers/misc/fw_loader/fw_loader-uclass.c b/drivers/misc/fw_loader/fw_loader-uclass.c
> @@ -0,0 +1,159 @@
> +#else
> +static int fw_loader_get_blk_dev_from_node(ofnode node)
> +{
> +     debug("fw_loader: No block device support\n");
> +     return -EINVAL;
> +}
> +#endif

This changes behaviour from the original driver. Previously the entire
phandlepart block was wrapped in #if CONFIG_IS_ENABLED(DM) &&
CONFIG_IS_ENABLED(BLK), so a board with BLK disabled would probe with
the phandle silently ignored. With this patch, the stub returns
-EINVAL and pre_probe propagates it, so probe fails. If intentional,
please call it out in the commit message; otherwise the stub should
return 0.

> diff --git a/drivers/misc/fw_loader/internal.h b/drivers/misc/fw_loader/internal.h
> @@ -0,0 +1,73 @@
> +struct device_plat {
> +     struct phandle_part phandlepart;
> +     char *mtdpart;
> +     char *ubivol;
> +};

Please rename struct device_plat to something like struct
fw_loader_plat - device_plat reads as a generic DM type and is
misleading even when internal to the uclass.

> diff --git a/drivers/misc/fw_loader/internal.h b/drivers/misc/fw_loader/internal.h
> @@ -0,0 +1,73 @@
> +#define fw_loader_get_ops(dev)       ((struct fw_loader_ops *)(dev)->driver->ops)

The fs_loader_ops table is static const, so this macro silently casts
the const away. Please return const struct fw_loader_ops * - the
callers in request_firmware_into_buf() only need read access.

> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> @@ -36,7 +36,7 @@ obj-$(CONFIG_FSL_DEVICE_DISABLE) += fsl_devdis.o
> -obj-$(CONFIG_$(PHASE_)FS_LOADER) += fs_loader.o
> +obj-$(CONFIG_FW_LOADER_UCLASS) += fw_loader/

FW_LOADER_UCLASS is not phase-aware, so once any phase selects
FS_LOADER the uclass object is linked into every phase that descends
here. Worth making FW_LOADER_UCLASS phase-scoped (SPL_/TPL_/VPL_) and
gating both the subdir and fw_loader-uclass.o on
CONFIG_$(PHASE_)FW_LOADER_UCLASS.

Regards,
Simon