* [PATCH v8 1/7] misc: fs_loader: fix ubifs not unmounted on dev_get_priv error
2026-10-22 19:40 [PATCH v8 0/7] misc: fs_loader: reorg and split to FS and FW loader + FIP loader Christian Marangi
@ 2026-10-22 19:40 ` Christian Marangi
2026-10-22 19:40 ` [PATCH v8 2/7] misc: fs_loader: reorganize and split to FS loader and FW UCLASS Christian Marangi
` (6 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Christian Marangi @ 2026-10-22 19:40 UTC (permalink / raw)
To: Simon Glass, Tom Rini, Mattijs Korpershoek, Marek Vasut,
Heiko Schocher, Neil Armstrong, Alexey Charkov, Daniel Golle,
Aristo Chen, Christian Marangi, Lucien.Jheng, Mehmet Fide,
Casey Connolly, Michal Simek, Quentin Schulz, Johan Jonker,
Anshul Dalal, Romain Gantois, Mateus Lima Alves,
Marcello Sylvester Bauer, Tan Siewert, Peng Fan, Patrice Chotard,
Yao Zi, Heinrich Schuchardt, Martin Schwan, u-boot
Cc: Neha Malcom Francis
When dev_get_priv errors out, the ubifs is not umounted (if used).
Correctly handle this handle condition and while at it also return -EINVAL
instead of -ENOMEM as a better error since no memory is allocated but is
actually an invalid scenario for fw_get_filesystem_firmware().
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Neha Malcom Francis <n-francis@ti.com>
---
drivers/misc/fs_loader.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/misc/fs_loader.c b/drivers/misc/fs_loader.c
index 6af4c7f15e7b..cd4695b08eea 100644
--- a/drivers/misc/fs_loader.c
+++ b/drivers/misc/fs_loader.c
@@ -175,8 +175,10 @@ static int fw_get_filesystem_firmware(struct udevice *dev)
struct firmware *firmwarep = dev_get_priv(dev);
- if (!firmwarep)
- return -ENOMEM;
+ if (!firmwarep) {
+ ret = -EINVAL;
+ goto out;
+ }
ret = fs_read(firmwarep->name, (ulong)map_to_sysmem(firmwarep->data),
firmwarep->offset, firmwarep->size, &actread);
--
2.55.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v8 2/7] misc: fs_loader: reorganize and split to FS loader and FW UCLASS
2026-10-22 19:40 [PATCH v8 0/7] misc: fs_loader: reorg and split to FS and FW loader + FIP loader Christian Marangi
2026-10-22 19:40 ` [PATCH v8 1/7] misc: fs_loader: fix ubifs not unmounted on dev_get_priv error Christian Marangi
@ 2026-10-22 19:40 ` Christian Marangi
2026-10-23 13:12 ` Simon Glass
2026-10-22 19:40 ` [PATCH v8 3/7] misc: fw_loader: implement generic get_fw_loader_from_node() Christian Marangi
` (5 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Christian Marangi @ 2026-10-22 19:40 UTC (permalink / raw)
To: Simon Glass, Tom Rini, Mattijs Korpershoek, Marek Vasut,
Heiko Schocher, Neil Armstrong, Alexey Charkov, Daniel Golle,
Aristo Chen, Christian Marangi, Lucien.Jheng, Mehmet Fide,
Casey Connolly, Michal Simek, Quentin Schulz, Johan Jonker,
Anshul Dalal, Romain Gantois, Mateus Lima Alves,
Marcello Sylvester Bauer, Tan Siewert, Peng Fan, Patrice Chotard,
Yao Zi, Heinrich Schuchardt, Martin Schwan, u-boot
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.
Doc are renamed and updated to a more generic name and for the new UCLASS.
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@gmail.com>
---
...irmware_loader.rst => firmware_loader.rst} | 6 +-
doc/develop/driver-model/index.rst | 2 +-
drivers/misc/Kconfig | 8 +
drivers/misc/Makefile | 2 +-
drivers/misc/fs_loader.c | 328 ------------------
drivers/misc/fw_loader/Makefile | 4 +
drivers/misc/fw_loader/fs_loader.c | 176 ++++++++++
drivers/misc/fw_loader/fw_loader-uclass.c | 188 ++++++++++
drivers/misc/fw_loader/internal.h | 91 +++++
include/dm/uclass-id.h | 2 +-
include/fs_loader.h | 47 +--
include/fw_loader.h | 19 +
12 files changed, 494 insertions(+), 379 deletions(-)
rename doc/develop/driver-model/{fs_firmware_loader.rst => firmware_loader.rst} (95%)
delete mode 100644 drivers/misc/fs_loader.c
create mode 100644 drivers/misc/fw_loader/Makefile
create mode 100644 drivers/misc/fw_loader/fs_loader.c
create mode 100644 drivers/misc/fw_loader/fw_loader-uclass.c
create mode 100644 drivers/misc/fw_loader/internal.h
diff --git a/doc/develop/driver-model/fs_firmware_loader.rst b/doc/develop/driver-model/firmware_loader.rst
similarity index 95%
rename from doc/develop/driver-model/fs_firmware_loader.rst
rename to doc/develop/driver-model/firmware_loader.rst
index 630daf636f18..2d11a74d5e2f 100644
--- a/doc/develop/driver-model/fs_firmware_loader.rst
+++ b/doc/develop/driver-model/firmware_loader.rst
@@ -4,7 +4,7 @@
File System Firmware Loader
===========================
-This is file system firmware loader for U-Boot framework, which has very close
+This is firmware loader for U-Boot framework, which has very close
to some Linux Firmware API. For the details of Linux Firmware API, you can refer
to https://docs.kernel.org/driver-api/firmware/index.html.
@@ -87,7 +87,7 @@ For example of getting DT phandle from /chosen and creating instance:
}
phandle = fdt32_to_cpu(*phandle_p);
- ret = uclass_get_device_by_phandle_id(UCLASS_FS_FIRMWARE_LOADER,
+ ret = uclass_get_device_by_phandle_id(UCLASS_FIRMWARE_LOADER,
phandle, &dev);
if (ret)
return ret;
@@ -145,7 +145,7 @@ instance:
.. code-block:: c
- ret = uclass_get_device_by_phandle_id(UCLASS_FS_FIRMWARE_LOADER,
+ ret = uclass_get_device_by_phandle_id(UCLASS_FIRMWARE_LOADER,
phandle, &dev);
if (ret)
return ret;
diff --git a/doc/develop/driver-model/index.rst b/doc/develop/driver-model/index.rst
index 8e12bbd9366a..b2d46da4e239 100644
--- a/doc/develop/driver-model/index.rst
+++ b/doc/develop/driver-model/index.rst
@@ -15,7 +15,7 @@ subsystems
design
ethernet
fdt-fixup
- fs_firmware_loader
+ firmware_loader
i2c-howto
livetree
migration
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 5f22bcb330ee..1cab469a3434 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -621,8 +621,12 @@ config MPC83XX_SERDES
help
Support for serdes found on MPC83xx SoCs.
+config FW_LOADER_UCLASS
+ bool
+
config FS_LOADER
bool "Enable loader driver for file system"
+ select FW_LOADER_UCLASS
help
This is file system generic loader which can be used to load
the file image from the storage into target such as memory.
@@ -630,8 +634,12 @@ config FS_LOADER
The consumer driver would then use this loader to program whatever,
ie. the FPGA device.
+config SPL_FW_LOADER_UCLASS
+ bool
+
config SPL_FS_LOADER
bool "Enable loader driver for file system in SPL"
+ select SPL_FW_LOADER_UCLASS
depends on SPL
help
This is file system generic loader which can be used to load
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 9ddbe9cd1142..f138ff9126f1 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -36,7 +36,7 @@ obj-$(CONFIG_FSL_DEVICE_DISABLE) += fsl_devdis.o
obj-$(CONFIG_FSL_IFC) += fsl_ifc.o
obj-$(CONFIG_FSL_IIM) += fsl_iim.o
obj-$(CONFIG_FSL_SEC_MON) += fsl_sec_mon.o
-obj-$(CONFIG_$(PHASE_)FS_LOADER) += fs_loader.o
+obj-$(CONFIG_$(PHASE_)FW_LOADER_UCLASS) += fw_loader/
obj-$(CONFIG_GATEWORKS_SC) += gsc.o
obj-$(CONFIG_GXP_EEPROM) += gxp_eeprom.o
obj-$(CONFIG_GDSYS_IOEP) += gdsys_ioep.o
diff --git a/drivers/misc/fs_loader.c b/drivers/misc/fs_loader.c
deleted file mode 100644
index cd4695b08eea..000000000000
--- a/drivers/misc/fs_loader.c
+++ /dev/null
@@ -1,328 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
- /*
- * Copyright (C) 2018-2019 Intel Corporation <www.intel.com>
- *
- */
-
-#define LOG_CATEGORY UCLASS_FS_FIRMWARE_LOADER
-
-#include <dm.h>
-#include <env.h>
-#include <errno.h>
-#include <blk.h>
-#include <fs.h>
-#include <fs_loader.h>
-#include <log.h>
-#include <dm/device-internal.h>
-#include <dm/root.h>
-#include <linux/string.h>
-#include <mapmem.h>
-#include <malloc.h>
-#include <spl.h>
-
-#ifdef CONFIG_CMD_UBIFS
-#include <ubi_uboot.h>
-#endif
-
-/**
- * struct firmware - A place for storing firmware and its attribute data.
- *
- * This holds information about a firmware and its content.
- *
- * @size: Size of a file
- * @data: Buffer for file
- * @priv: Firmware loader private fields
- * @name: Filename
- * @offset: Offset of reading a file
- */
-struct firmware {
- size_t size;
- const u8 *data;
- const char *name;
- u32 offset;
-};
-
-#ifdef CONFIG_CMD_UBIFS
-static int mount_ubifs(char *mtdpart, char *ubivol)
-{
- int ret = ubi_part(mtdpart, NULL);
-
- if (ret) {
- debug("Cannot find mtd partition %s\n", mtdpart);
- return ret;
- }
-
- return cmd_ubifs_mount(ubivol);
-}
-
-static int umount_ubifs(void)
-{
- return cmd_ubifs_umount();
-}
-#else
-static int mount_ubifs(char *mtdpart, char *ubivol)
-{
- debug("Error: Cannot load image: no UBIFS support\n");
- return -ENOSYS;
-}
-#endif
-
-static int select_fs_dev(struct device_plat *plat)
-{
- int ret;
-
- if (plat->phandlepart.phandle) {
- ofnode node;
-
- node = ofnode_get_by_phandle(plat->phandlepart.phandle);
-
- struct udevice *dev;
-
- ret = device_get_global_by_ofnode(node, &dev);
- if (!ret) {
- struct blk_desc *desc = blk_get_by_device(dev);
- if (desc) {
- ret = fs_set_blk_dev_with_part(desc,
- plat->phandlepart.partition);
- } else {
- debug("%s: No device found\n", __func__);
- return -ENODEV;
- }
- }
- } else if (plat->mtdpart && plat->ubivol) {
- ret = mount_ubifs(plat->mtdpart, plat->ubivol);
- if (ret)
- return ret;
-
- ret = fs_set_blk_dev("ubi", NULL, FS_TYPE_UBIFS);
- } else {
- debug("Error: unsupported storage device.\n");
- return -ENODEV;
- }
-
- if (ret)
- debug("Error: could not access storage.\n");
-
- return ret;
-}
-
-/**
- * _request_firmware_prepare - Prepare firmware struct.
- *
- * @dev: An instance of a driver.
- * @name: Name of firmware file.
- * @dbuf: Address of buffer to load firmware into.
- * @size: Size of buffer.
- * @offset: Offset of a file for start reading into buffer.
- *
- * Return: Negative value if fail, 0 for successful.
- */
-static int _request_firmware_prepare(struct udevice *dev,
- const char *name, void *dbuf,
- size_t size, u32 offset)
-{
- if (!name || name[0] == '\0')
- return -EINVAL;
-
- struct firmware *firmwarep = dev_get_priv(dev);
-
- if (!firmwarep)
- return -ENOMEM;
-
- firmwarep->name = name;
- firmwarep->offset = offset;
- firmwarep->data = dbuf;
- firmwarep->size = size;
-
- return 0;
-}
-
-/**
- * fw_get_filesystem_firmware - load firmware into an allocated buffer.
- * @dev: An instance of a driver.
- *
- * Return: Size of total read, negative value when error.
- */
-static int fw_get_filesystem_firmware(struct udevice *dev)
-{
- loff_t actread = 0;
- char *storage_interface, *dev_part, *ubi_mtdpart, *ubi_volume;
- int ret;
-
- storage_interface = env_get("storage_interface");
- dev_part = env_get("fw_dev_part");
- ubi_mtdpart = env_get("fw_ubi_mtdpart");
- ubi_volume = env_get("fw_ubi_volume");
-
- if (storage_interface && dev_part) {
- ret = fs_set_blk_dev(storage_interface, dev_part, FS_TYPE_ANY);
- } else if (storage_interface && ubi_mtdpart && ubi_volume) {
- ret = mount_ubifs(ubi_mtdpart, ubi_volume);
- if (ret)
- return ret;
-
- if (!strcmp("ubi", storage_interface))
- ret = fs_set_blk_dev(storage_interface, NULL,
- FS_TYPE_UBIFS);
- else
- ret = -ENODEV;
- } else {
- ret = select_fs_dev(dev_get_plat(dev));
- }
-
- if (ret)
- goto out;
-
- struct firmware *firmwarep = dev_get_priv(dev);
-
- if (!firmwarep) {
- ret = -EINVAL;
- goto out;
- }
-
- ret = fs_read(firmwarep->name, (ulong)map_to_sysmem(firmwarep->data),
- firmwarep->offset, firmwarep->size, &actread);
-
- if (ret) {
- debug("Error: %d Failed to read %s from flash %lld != %zu.\n",
- ret, firmwarep->name, actread, firmwarep->size);
- } else {
- ret = actread;
- }
-
-out:
-#ifdef CONFIG_CMD_UBIFS
- umount_ubifs();
-#endif
- return ret;
-}
-
-/**
- * request_firmware_into_buf - Load firmware into a previously allocated buffer.
- * @dev: An instance of a driver.
- * @name: Name of firmware file.
- * @buf: Address of buffer to load firmware into.
- * @size: Size of buffer.
- * @offset: Offset of a file for start reading into buffer.
- *
- * The firmware is loaded directly into the buffer pointed to by @buf.
- *
- * Return: Size of total read, negative value when error.
- */
-int request_firmware_into_buf(struct udevice *dev,
- const char *name,
- void *buf, size_t size, u32 offset)
-{
- int ret;
-
- if (!dev)
- return -EINVAL;
-
- ret = _request_firmware_prepare(dev, name, buf, size, offset);
- if (ret < 0) /* error */
- return ret;
-
- ret = fw_get_filesystem_firmware(dev);
-
- return ret;
-}
-
-static int fs_loader_of_to_plat(struct udevice *dev)
-{
- u32 phandlepart[2];
-
- ofnode fs_loader_node = dev_ofnode(dev);
-
- if (ofnode_valid(fs_loader_node)) {
- struct device_plat *plat;
-
- plat = dev_get_plat(dev);
- if (!ofnode_read_u32_array(fs_loader_node,
- "phandlepart",
- phandlepart, 2)) {
- plat->phandlepart.phandle = phandlepart[0];
- plat->phandlepart.partition = phandlepart[1];
- }
-
- plat->mtdpart = (char *)ofnode_read_string(
- fs_loader_node, "mtdpart");
-
- plat->ubivol = (char *)ofnode_read_string(
- fs_loader_node, "ubivol");
- }
-
- return 0;
-}
-
-static int fs_loader_probe(struct udevice *dev)
-{
-#if CONFIG_IS_ENABLED(DM) && CONFIG_IS_ENABLED(BLK)
- int ret;
- struct device_plat *plat = dev_get_plat(dev);
-
- if (plat->phandlepart.phandle) {
- ofnode node = ofnode_get_by_phandle(plat->phandlepart.phandle);
- struct udevice *parent_dev = NULL;
-
- ret = device_get_global_by_ofnode(node, &parent_dev);
- if (!ret) {
- struct udevice *dev;
-
- ret = blk_get_from_parent(parent_dev, &dev);
- if (ret) {
- debug("fs_loader: No block device: %d\n",
- ret);
-
- return ret;
- }
- }
- }
-#endif
-
- return 0;
-};
-
-static const struct udevice_id fs_loader_ids[] = {
- { .compatible = "u-boot,fs-loader"},
- { }
-};
-
-U_BOOT_DRIVER(fs_loader) = {
- .name = "fs-loader",
- .id = UCLASS_FS_FIRMWARE_LOADER,
- .of_match = fs_loader_ids,
- .probe = fs_loader_probe,
- .of_to_plat = fs_loader_of_to_plat,
- .plat_auto = sizeof(struct device_plat),
- .priv_auto = sizeof(struct firmware),
-};
-
-static struct device_plat default_plat = { 0 };
-
-int get_fs_loader(struct udevice **dev)
-{
- int ret;
- ofnode node = ofnode_get_chosen_node("firmware-loader");
-
- if (ofnode_valid(node))
- return uclass_get_device_by_ofnode(UCLASS_FS_FIRMWARE_LOADER,
- node, dev);
-
- /* Try the first device if none was chosen */
- ret = uclass_first_device_err(UCLASS_FS_FIRMWARE_LOADER, dev);
- if (ret != -ENODEV)
- return ret;
-
- /* Just create a new device */
- ret = device_bind(dm_root(), DM_DRIVER_REF(fs_loader), "default-loader",
- &default_plat, ofnode_null(), dev);
- if (ret)
- return ret;
-
- return device_probe(*dev);
-}
-
-UCLASS_DRIVER(fs_loader) = {
- .id = UCLASS_FS_FIRMWARE_LOADER,
- .name = "fs-loader",
-};
diff --git a/drivers/misc/fw_loader/Makefile b/drivers/misc/fw_loader/Makefile
new file mode 100644
index 000000000000..f5df0db61c22
--- /dev/null
+++ b/drivers/misc/fw_loader/Makefile
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0+
+
+obj-$(CONFIG_$(PHASE_)FW_LOADER_UCLASS) += fw_loader-uclass.o
+obj-$(CONFIG_$(PHASE_)FS_LOADER) += fs_loader.o
diff --git a/drivers/misc/fw_loader/fs_loader.c b/drivers/misc/fw_loader/fs_loader.c
new file mode 100644
index 000000000000..9923fb5e123d
--- /dev/null
+++ b/drivers/misc/fw_loader/fs_loader.c
@@ -0,0 +1,176 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018-2019 Intel Corporation <www.intel.com>
+ *
+ */
+
+#define LOG_CATEGORY UCLASS_FIRMWARE_LOADER
+
+#include <dm.h>
+#include <errno.h>
+#include <blk.h>
+#include <fs.h>
+#include <fs_loader.h>
+#include <log.h>
+#include <dm/device-internal.h>
+#include <dm/root.h>
+#include <linux/string.h>
+#include <mapmem.h>
+#include <malloc.h>
+#include <spl.h>
+
+#ifdef CONFIG_CMD_UBIFS
+#include <ubi_uboot.h>
+#endif
+
+#include "internal.h"
+
+#ifdef CONFIG_CMD_UBIFS
+static int mount_ubifs(char *mtdpart, char *ubivol)
+{
+ int ret;
+
+ ret = generic_fw_loader_ubi_select(mtdpart);
+ if (ret)
+ return ret;
+
+ return cmd_ubifs_mount(ubivol);
+}
+
+static int umount_ubifs(void)
+{
+ return cmd_ubifs_umount();
+}
+#else
+static int mount_ubifs(char *mtdpart, char *ubivol)
+{
+ debug("Error: Cannot load image: no UBIFS support\n");
+ return -ENOSYS;
+}
+#endif
+
+static int select_fs_dev(struct fw_loader_plat *plat)
+{
+ int ret;
+
+ if (plat->phandlepart.phandle) {
+ struct blk_desc *desc;
+
+ ret = generic_fw_loader_get_phandle_desc(plat, &desc);
+ if (!desc)
+ debug("%s: No device found\n", __func__);
+
+ if (!ret)
+ ret = fs_set_blk_dev_with_part(desc,
+ plat->phandlepart.partition);
+ } else if (plat->mtdpart && plat->ubivol) {
+ ret = mount_ubifs(plat->mtdpart, plat->ubivol);
+ if (ret)
+ return ret;
+
+ ret = fs_set_blk_dev("ubi", NULL, FS_TYPE_UBIFS);
+ } else {
+ debug("Error: unsupported storage device.\n");
+ return -ENODEV;
+ }
+
+ if (ret)
+ debug("Error: could not access storage.\n");
+
+ return ret;
+}
+
+/**
+ * fw_get_filesystem_firmware - load firmware into an allocated buffer.
+ * @dev: An instance of a driver.
+ *
+ * Return: Size of total read, negative value when error.
+ */
+static int fw_get_filesystem_firmware(struct udevice *dev)
+{
+ struct firmware *upriv = dev_get_uclass_priv(dev);
+ struct fw_loader_plat *plat = dev_get_uclass_plat(dev);
+ struct fw_loader_env_info info = { };
+ loff_t actread = 0;
+ int ret;
+
+ generic_fw_loader_env_info(&info);
+
+ if (info.storage_interface && info.dev_part) {
+ ret = fs_set_blk_dev(info.storage_interface,
+ info.dev_part, FS_TYPE_ANY);
+ } else if (info.storage_interface && info.ubi_mtdpart &&
+ info.ubi_volume) {
+ ret = mount_ubifs(info.ubi_mtdpart, info.ubi_volume);
+ if (ret)
+ return ret;
+
+ if (!strcmp("ubi", info.storage_interface))
+ ret = fs_set_blk_dev(info.storage_interface, NULL,
+ FS_TYPE_UBIFS);
+ else
+ ret = -ENODEV;
+ } else {
+ ret = select_fs_dev(plat);
+ }
+
+ if (ret)
+ goto out;
+
+ ret = fs_read(upriv->name, (ulong)map_to_sysmem(upriv->data),
+ upriv->offset, upriv->size, &actread);
+
+ if (ret) {
+ debug("Error: %d Failed to read %s from flash %lld != %zu.\n",
+ ret, upriv->name, actread, upriv->size);
+ } else {
+ ret = actread;
+ }
+
+out:
+#ifdef CONFIG_CMD_UBIFS
+ umount_ubifs();
+#endif
+ return ret;
+}
+
+static const struct fw_loader_ops fs_loader_ops = {
+ .get_firmware = fw_get_filesystem_firmware,
+};
+
+static const struct udevice_id fs_loader_ids[] = {
+ { .compatible = "u-boot,fs-loader"},
+ { }
+};
+
+U_BOOT_DRIVER(fs_loader) = {
+ .name = "fs-loader",
+ .id = UCLASS_FIRMWARE_LOADER,
+ .of_match = fs_loader_ids,
+ .ops = &fs_loader_ops,
+};
+
+static struct fw_loader_plat default_plat = { 0 };
+
+int get_fs_loader(struct udevice **dev)
+{
+ int ret;
+ ofnode node = ofnode_get_chosen_node("firmware-loader");
+
+ if (ofnode_valid(node))
+ return uclass_get_device_by_ofnode(UCLASS_FIRMWARE_LOADER,
+ node, dev);
+
+ /* Try the first device if none was chosen */
+ ret = uclass_first_device_err(UCLASS_FIRMWARE_LOADER, dev);
+ if (ret != -ENODEV)
+ return ret;
+
+ /* Just create a new device */
+ ret = device_bind(dm_root(), DM_DRIVER_REF(fs_loader), "default-loader",
+ &default_plat, ofnode_null(), dev);
+ if (ret)
+ return ret;
+
+ return device_probe(*dev);
+}
diff --git a/drivers/misc/fw_loader/fw_loader-uclass.c b/drivers/misc/fw_loader/fw_loader-uclass.c
new file mode 100644
index 000000000000..0d95fac8f42b
--- /dev/null
+++ b/drivers/misc/fw_loader/fw_loader-uclass.c
@@ -0,0 +1,188 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018-2019 Intel Corporation <www.intel.com>
+ *
+ */
+
+#include <blk.h>
+#include <dm.h>
+#include <dm/device.h>
+#include <env.h>
+#include <errno.h>
+#include <fw_loader.h>
+#include <linux/types.h>
+
+#if CONFIG_IS_ENABLED(CMD_UBIFS)
+#include <ubi_uboot.h>
+#endif
+
+#include "internal.h"
+
+#if CONFIG_IS_ENABLED(CMD_UBIFS)
+int generic_fw_loader_ubi_select(char *mtdpart)
+{
+ int ret;
+
+ ret = ubi_part(mtdpart, NULL);
+ if (ret)
+ debug("Cannot find mtd partition %s\n", mtdpart);
+
+ return ret;
+}
+#else
+int generic_fw_loader_ubi_select(char *mtdpart)
+{
+ debug("Error: Cannot select ubi partition: no UBIFS support\n");
+ return -ENOSYS;
+}
+#endif
+
+#if CONFIG_IS_ENABLED(BLK)
+static int fw_loader_get_blk_dev_from_node(ofnode node)
+{
+ struct udevice *parent_dev = NULL;
+ struct udevice *blk_dev;
+ int ret;
+
+ ret = device_get_global_by_ofnode(node, &parent_dev);
+ if (ret)
+ return ret;
+
+ ret = blk_get_from_parent(parent_dev, &blk_dev);
+ if (ret)
+ debug("fw_loader: No block device: %d\n", ret);
+
+ return ret;
+}
+#else
+static int fw_loader_get_blk_dev_from_node(ofnode node)
+{
+ debug("fw_loader: No block device support\n");
+ return 0;
+}
+#endif
+
+void generic_fw_loader_env_info(struct fw_loader_env_info *info)
+{
+ info->storage_interface = env_get("storage_interface");
+ info->dev_part = env_get("fw_dev_part");
+ info->ubi_mtdpart = env_get("fw_ubi_mtdpart");
+ info->ubi_volume = env_get("fw_ubi_volume");
+}
+
+int generic_fw_loader_get_phandle_desc(struct fw_loader_plat *plat,
+ struct blk_desc **desc)
+{
+ struct blk_desc *disk_desc;
+ struct udevice *disk_dev;
+ ofnode node;
+ int ret;
+
+ node = ofnode_get_by_phandle(plat->phandlepart.phandle);
+
+ ret = device_get_global_by_ofnode(node, &disk_dev);
+ if (ret)
+ return ret;
+
+ disk_desc = blk_get_by_device(disk_dev);
+ if (!disk_desc)
+ return -ENODEV;
+
+ *desc = disk_desc;
+
+ return 0;
+}
+
+static int fw_loader_pre_probe(struct udevice *dev)
+{
+#if CONFIG_IS_ENABLED(DM)
+ struct fw_loader_plat *plat = dev_get_uclass_plat(dev);
+ ofnode fw_loader_node;
+
+ fw_loader_node = dev_ofnode(dev);
+ if (ofnode_valid(fw_loader_node)) {
+ u32 phandlepart[2];
+
+ if (!ofnode_read_u32_array(fw_loader_node, "phandlepart",
+ phandlepart, 2)) {
+ plat->phandlepart.phandle = phandlepart[0];
+ plat->phandlepart.partition = phandlepart[1];
+ }
+
+ plat->mtdpart = (char *)ofnode_read_string(fw_loader_node,
+ "mtdpart");
+
+ plat->ubivol = (char *)ofnode_read_string(fw_loader_node,
+ "ubivol");
+ }
+
+ if (plat->phandlepart.phandle) {
+ ofnode node = ofnode_get_by_phandle(plat->phandlepart.phandle);
+ int ret;
+
+ ret = fw_loader_get_blk_dev_from_node(node);
+ if (ret)
+ return ret;
+ }
+#endif
+
+ return 0;
+}
+
+UCLASS_DRIVER(fw_loader) = {
+ .id = UCLASS_FIRMWARE_LOADER,
+ .name = "fw_loader",
+ .pre_probe = fw_loader_pre_probe,
+ .per_device_plat_auto = sizeof(struct fw_loader_plat),
+ .per_device_auto = sizeof(struct firmware),
+};
+
+/**
+ * _request_firmware_prepare - Prepare firmware struct.
+ *
+ * @dev: An instance of a driver.
+ * @name: Name of firmware file.
+ * @dbuf: Address of buffer to load firmware into.
+ * @size: Size of buffer.
+ * @offset: Offset of a file for start reading into buffer.
+ *
+ * Return: Negative value if fail, 0 for successful.
+ */
+static int _request_firmware_prepare(struct udevice *dev,
+ const char *name, void *dbuf,
+ size_t size, u32 offset)
+{
+ struct firmware *upriv = dev_get_uclass_priv(dev);
+
+ if (!name || name[0] == '\0')
+ return -EINVAL;
+
+ upriv->name = name;
+ upriv->offset = offset;
+ upriv->data = dbuf;
+ upriv->size = size;
+
+ return 0;
+}
+
+int request_firmware_into_buf(struct udevice *dev,
+ const char *name,
+ void *buf, size_t size, u32 offset)
+{
+ const struct fw_loader_ops *ops;
+ int ret;
+
+ if (!dev)
+ return -EINVAL;
+
+ ret = _request_firmware_prepare(dev, name, buf, size, offset);
+ if (ret < 0) /* error */
+ return ret;
+
+ ops = fw_loader_get_ops(dev);
+
+ if (!ops->get_firmware)
+ return -EOPNOTSUPP;
+
+ return ops->get_firmware(dev);
+}
diff --git a/drivers/misc/fw_loader/internal.h b/drivers/misc/fw_loader/internal.h
new file mode 100644
index 000000000000..9ff43e872554
--- /dev/null
+++ b/drivers/misc/fw_loader/internal.h
@@ -0,0 +1,91 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2018-2019 Intel Corporation <www.intel.com>
+ */
+#ifndef _FW_LOADER_INTERNAL_H_
+#define _FW_LOADER_INTERNAL_H_
+
+/**
+ * struct phandle_part - A place for storing phandle of node and its partition
+ *
+ * This holds information about a phandle of the block device, and its
+ * partition where the firmware would be loaded from.
+ *
+ * @phandle: Phandle of storage device node
+ * @partition: Partition of block device
+ */
+struct phandle_part {
+ u32 phandle;
+ u32 partition;
+};
+
+/**
+ * struct fw_loader_ops - driver operations for Firmware Loader uclass
+ *
+ * Drivers MUST support these operation. These operations are intended
+ * to be used by uclass code, not directly from other code.
+ */
+struct fw_loader_ops {
+ /**
+ * get_firmware() - get firmware from Firmware Loader driver
+ *
+ * @dev: Firmware Loader device to read firmware from
+ */
+ int (*get_firmware)(struct udevice *dev);
+};
+
+#define fw_loader_get_ops(dev) ((const struct fw_loader_ops *)(dev)->driver->ops)
+
+/**
+ * struct fw_loader_plat - A place for storing all supported storage devices
+ *
+ * This holds information about all supported storage devices for driver use.
+ *
+ * @phandlepart: Attribute data for block device.
+ * @mtdpart: MTD partition for ubi partition.
+ * @ubivol: UBI volume-name for ubifsmount.
+ */
+struct fw_loader_plat {
+ struct phandle_part phandlepart;
+ char *mtdpart;
+ char *ubivol;
+};
+
+/**
+ * struct firmware - A place for storing firmware and its attribute data.
+ *
+ * This holds information about a firmware and its content.
+ *
+ * @size: Size of a file
+ * @data: Buffer for file
+ * @name: Filename
+ * @offset: Offset of reading a file
+ */
+struct firmware {
+ size_t size;
+ const u8 *data;
+ const char *name;
+ u32 offset;
+};
+
+/**
+ * struct fw_loader_env_info - struct for FW loader configuration from ENV
+ *
+ * @storage_interface: "storage_interface" ENV value
+ * @dev_part: "fw_dev_part" ENV value
+ * @ubi_mtdpart: "fw_ubi_mtdpart" ENV value
+ * @ubi_volume: "fw_ubi_volume" ENV value
+ */
+struct fw_loader_env_info {
+ char *storage_interface;
+ char *dev_part;
+ char *ubi_mtdpart;
+ char *ubi_volume;
+};
+
+int generic_fw_loader_ubi_select(char *mtdpart);
+void generic_fw_loader_env_info(struct fw_loader_env_info *info);
+int generic_fw_loader_get_phandle_desc(struct fw_loader_plat *plat,
+ struct blk_desc **desc);
+
+#endif
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
index 7de9312cde7a..48f36e88fee9 100644
--- a/include/dm/uclass-id.h
+++ b/include/dm/uclass-id.h
@@ -69,7 +69,7 @@ enum uclass_id {
UCLASS_FIRMWARE, /* Firmware */
UCLASS_FPGA, /* FPGA device */
UCLASS_FUZZING_ENGINE, /* Fuzzing engine */
- UCLASS_FS_FIRMWARE_LOADER, /* Generic loader */
+ UCLASS_FIRMWARE_LOADER, /* Firmware loader */
UCLASS_FWU_MDATA, /* FWU Metadata Access */
UCLASS_GPIO, /* Bank of general-purpose I/O pins */
UCLASS_HASH, /* Hash device */
diff --git a/include/fs_loader.h b/include/fs_loader.h
index 7e16e0f70309..3c64efe1b439 100644
--- a/include/fs_loader.h
+++ b/include/fs_loader.h
@@ -6,52 +6,9 @@
#ifndef _FS_LOADER_H_
#define _FS_LOADER_H_
-struct udevice;
-
-/**
- * struct phandle_part - A place for storing phandle of node and its partition
- *
- * This holds information about a phandle of the block device, and its
- * partition where the firmware would be loaded from.
- *
- * @phandle: Phandle of storage device node
- * @partition: Partition of block device
- */
-struct phandle_part {
- u32 phandle;
- u32 partition;
-};
-
-/**
- * struct phandle_part - A place for storing all supported storage devices
- *
- * This holds information about all supported storage devices for driver use.
- *
- * @phandlepart: Attribute data for block device.
- * @mtdpart: MTD partition for ubi partition.
- * @ubivol: UBI volume-name for ubifsmount.
- */
-struct device_plat {
- struct phandle_part phandlepart;
- char *mtdpart;
- char *ubivol;
-};
+#include <fw_loader.h>
-/**
- * request_firmware_into_buf - Load firmware into a previously allocated buffer.
- * @dev: An instance of a driver.
- * @name: Name of firmware file.
- * @buf: Address of buffer to load firmware into.
- * @size: Size of buffer.
- * @offset: Offset of a file for start reading into buffer.
- *
- * The firmware is loaded directly into the buffer pointed to by @buf.
- *
- * Return: Size of total read, negative value when error.
- */
-int request_firmware_into_buf(struct udevice *dev,
- const char *name,
- void *buf, size_t size, u32 offset);
+struct udevice;
/**
* get_fs_loader() - Get the chosen filesystem loader
diff --git a/include/fw_loader.h b/include/fw_loader.h
index 35574482b2b9..56f5e3be6195 100644
--- a/include/fw_loader.h
+++ b/include/fw_loader.h
@@ -1,10 +1,29 @@
/* SPDX-License-Identifier: GPL-2.0 */
/*
+ * Copyright (C) 2018-2019 Intel Corporation <www.intel.com>
* Copyright (C) 2025 Lucien Jheng <lucienzx159@gmail.com>
*/
#ifndef _FW_LOADER_H_
#define _FW_LOADER_H_
+struct udevice;
+
+/**
+ * request_firmware_into_buf - Load firmware into a previously allocated buffer.
+ * @dev: An instance of a driver.
+ * @name: Name of firmware file.
+ * @buf: Address of buffer to load firmware into.
+ * @size: Size of buffer.
+ * @offset: Offset of a file for start reading into buffer.
+ *
+ * The firmware is loaded directly into the buffer pointed to by @buf.
+ *
+ * Return: Size of total read, negative value when error.
+ */
+int request_firmware_into_buf(struct udevice *dev,
+ const char *name,
+ void *buf, size_t size, u32 offset);
+
/**
* request_firmware_into_buf_via_script() -
* Load firmware using a U-Boot script and copy to buffer
--
2.55.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v8 2/7] misc: fs_loader: reorganize and split to FS loader and FW UCLASS
2026-10-22 19:40 ` [PATCH v8 2/7] misc: fs_loader: reorganize and split to FS loader and FW UCLASS Christian Marangi
@ 2026-10-23 13:12 ` Simon Glass
2026-10-23 16:22 ` Christian Marangi
0 siblings, 1 reply; 16+ messages in thread
From: Simon Glass @ 2026-10-23 13:12 UTC (permalink / raw)
To: ansuelsmth
Cc: Simon Glass, Tom Rini, Mattijs Korpershoek, Marek Vasut,
Heiko Schocher, Neil Armstrong, Alexey Charkov, Daniel Golle,
Aristo Chen, Lucien.Jheng, Mehmet Fide, Casey Connolly,
Michal Simek, Quentin Schulz, Johan Jonker, Anshul Dalal,
Romain Gantois, Mateus Lima Alves, Marcello Sylvester Bauer,
Tan Siewert, Peng Fan, Patrice Chotard, Yao Zi,
Heinrich Schuchardt, Martin Schwan, u-boot
Hi Christian,
On 2026-10-22T19:40:35, Christian Marangi (Ansuel) <ansuelsmth@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.
> Doc are renamed and updated to a more generic name and for the new UCLASS.
>
> 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@gmail.com>
>
> ...{fs_firmware_loader.rst => firmware_loader.rst} | 6 +-
> doc/develop/driver-model/index.rst | 2 +-
> drivers/misc/Kconfig | 8 +
> drivers/misc/Makefile | 2 +-
> drivers/misc/fs_loader.c | 328 ---------------------
> drivers/misc/fw_loader/Makefile | 4 +
> drivers/misc/fw_loader/fs_loader.c | 176 +++++++++++
> drivers/misc/fw_loader/fw_loader-uclass.c | 188 ++++++++++++
> drivers/misc/fw_loader/internal.h | 91 ++++++
> include/dm/uclass-id.h | 2 +-
> include/fs_loader.h | 47 +--
> include/fw_loader.h | 19 ++
> 12 files changed, 494 insertions(+), 379 deletions(-)
> diff --git a/drivers/misc/fw_loader/fs_loader.c b/drivers/misc/fw_loader/fs_loader.c
> @@ -0,0 +1,176 @@
> + if (plat->phandlepart.phandle) {
> + struct blk_desc *desc;
> +
> + ret = generic_fw_loader_get_phandle_desc(plat, &desc);
> + if (!desc)
> + debug("%s: No device found\n", __func__);
generic_fw_loader_get_phandle_desc() only sets *desc on success, so if
device_get_global_by_ofnode() or blk_get_by_device() fails, 'desc' is
read uninitialised. Please can you check 'ret' instead, e.g.
ret = generic_fw_loader_get_phandle_desc(plat, &desc);
if (ret)
debug("%s: No device found\n", __func__);
else
ret = fs_set_blk_dev_with_part(desc,
plat->phandlepart.partition);
Regards,
Simon
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v8 2/7] misc: fs_loader: reorganize and split to FS loader and FW UCLASS
2026-10-23 13:12 ` Simon Glass
@ 2026-10-23 16:22 ` Christian Marangi
0 siblings, 0 replies; 16+ messages in thread
From: Christian Marangi @ 2026-10-23 16:22 UTC (permalink / raw)
To: Simon Glass
Cc: Tom Rini, Mattijs Korpershoek, Marek Vasut, Heiko Schocher,
Neil Armstrong, Alexey Charkov, Daniel Golle, Aristo Chen,
Lucien.Jheng, Mehmet Fide, Casey Connolly, Michal Simek,
Quentin Schulz, Johan Jonker, Anshul Dalal, Romain Gantois,
Mateus Lima Alves, Marcello Sylvester Bauer, Tan Siewert,
Peng Fan, Patrice Chotard, Yao Zi, Heinrich Schuchardt,
Martin Schwan, u-boot
On Fri, Oct 23, 2026 at 09:12:26AM -0400, Simon Glass wrote:
> Hi Christian,
>
> On 2026-10-22T19:40:35, Christian Marangi (Ansuel) <ansuelsmth@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.
> > Doc are renamed and updated to a more generic name and for the new UCLASS.
> >
> > 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@gmail.com>
> >
> > ...{fs_firmware_loader.rst => firmware_loader.rst} | 6 +-
> > doc/develop/driver-model/index.rst | 2 +-
> > drivers/misc/Kconfig | 8 +
> > drivers/misc/Makefile | 2 +-
> > drivers/misc/fs_loader.c | 328 ---------------------
> > drivers/misc/fw_loader/Makefile | 4 +
> > drivers/misc/fw_loader/fs_loader.c | 176 +++++++++++
> > drivers/misc/fw_loader/fw_loader-uclass.c | 188 ++++++++++++
> > drivers/misc/fw_loader/internal.h | 91 ++++++
> > include/dm/uclass-id.h | 2 +-
> > include/fs_loader.h | 47 +--
> > include/fw_loader.h | 19 ++
> > 12 files changed, 494 insertions(+), 379 deletions(-)
>
> > diff --git a/drivers/misc/fw_loader/fs_loader.c b/drivers/misc/fw_loader/fs_loader.c
> > @@ -0,0 +1,176 @@
> > + if (plat->phandlepart.phandle) {
> > + struct blk_desc *desc;
> > +
> > + ret = generic_fw_loader_get_phandle_desc(plat, &desc);
> > + if (!desc)
> > + debug("%s: No device found\n", __func__);
>
> generic_fw_loader_get_phandle_desc() only sets *desc on success, so if
> device_get_global_by_ofnode() or blk_get_by_device() fails, 'desc' is
> read uninitialised. Please can you check 'ret' instead, e.g.
>
> ret = generic_fw_loader_get_phandle_desc(plat, &desc);
> if (ret)
> debug("%s: No device found\n", __func__);
> else
> ret = fs_set_blk_dev_with_part(desc,
> plat->phandlepart.partition);
>
Hi Simon,
isn't the current logic the exact implementation you are suggesting?
But yes I will update with the compacted if-else since it's more readable.
--
Ansuel
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v8 3/7] misc: fw_loader: implement generic get_fw_loader_from_node()
2026-10-22 19:40 [PATCH v8 0/7] misc: fs_loader: reorg and split to FS and FW loader + FIP loader Christian Marangi
2026-10-22 19:40 ` [PATCH v8 1/7] misc: fs_loader: fix ubifs not unmounted on dev_get_priv error Christian Marangi
2026-10-22 19:40 ` [PATCH v8 2/7] misc: fs_loader: reorganize and split to FS loader and FW UCLASS Christian Marangi
@ 2026-10-22 19:40 ` Christian Marangi
2026-10-22 19:40 ` [PATCH v8 4/7] misc: fw_loader: implement request_firmware_size() OP Christian Marangi
` (4 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Christian Marangi @ 2026-10-22 19:40 UTC (permalink / raw)
To: Simon Glass, Tom Rini, Mattijs Korpershoek, Marek Vasut,
Heiko Schocher, Neil Armstrong, Alexey Charkov, Daniel Golle,
Aristo Chen, Christian Marangi, Lucien.Jheng, Mehmet Fide,
Casey Connolly, Michal Simek, Quentin Schulz, Johan Jonker,
Anshul Dalal, Romain Gantois, Mateus Lima Alves,
Marcello Sylvester Bauer, Tan Siewert, Peng Fan, Patrice Chotard,
Yao Zi, Heinrich Schuchardt, Martin Schwan, u-boot
Implement a generic function to get a FW loader dev from a node.
There is currently only get_fs_loader() but that is limited to chosen
node and is limited only to FS loader.
Introduce get_fw_loader_from_node() that will parse the
"firmware-loader" from a specified node and will search and probe a
matching FW loader device, if found.
Reviewed-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
drivers/misc/fw_loader/fw_loader-uclass.c | 24 +++++++++++++++++++++++
include/fw_loader.h | 14 +++++++++++++
2 files changed, 38 insertions(+)
diff --git a/drivers/misc/fw_loader/fw_loader-uclass.c b/drivers/misc/fw_loader/fw_loader-uclass.c
index 0d95fac8f42b..7f19d6c4af4c 100644
--- a/drivers/misc/fw_loader/fw_loader-uclass.c
+++ b/drivers/misc/fw_loader/fw_loader-uclass.c
@@ -7,6 +7,8 @@
#include <blk.h>
#include <dm.h>
#include <dm/device.h>
+#include <dm/device-internal.h>
+#include <dm/uclass.h>
#include <env.h>
#include <errno.h>
#include <fw_loader.h>
@@ -137,6 +139,28 @@ UCLASS_DRIVER(fw_loader) = {
.per_device_auto = sizeof(struct firmware),
};
+int get_fw_loader_from_node(ofnode node, struct udevice **dev)
+{
+ struct udevice *fw_dev;
+ int ret;
+
+ node = ofnode_parse_phandle(node, "firmware-loader", 0);
+ if (!ofnode_valid(node))
+ return -ENODEV;
+
+ ret = uclass_get_device_by_ofnode(UCLASS_FIRMWARE_LOADER,
+ node, &fw_dev);
+ if (ret)
+ return ret;
+
+ ret = device_probe(fw_dev);
+ if (ret)
+ return ret;
+
+ *dev = fw_dev;
+ return 0;
+}
+
/**
* _request_firmware_prepare - Prepare firmware struct.
*
diff --git a/include/fw_loader.h b/include/fw_loader.h
index 56f5e3be6195..99c47380a172 100644
--- a/include/fw_loader.h
+++ b/include/fw_loader.h
@@ -6,8 +6,22 @@
#ifndef _FW_LOADER_H_
#define _FW_LOADER_H_
+#include <dm/ofnode_decl.h>
+
struct udevice;
+/**
+ * get_fw_loader_from_node - Get FW loader dev from @node.
+ *
+ * @node: ofnode where "firmware-loader" phandle is stored.
+ * @dev: pointer where to store the FW loader dev.
+ *
+ * Search and probe a matching FW loader device, if found.
+ *
+ * Return: Negative value if fail, 0 for successful.
+ */
+int get_fw_loader_from_node(ofnode node, struct udevice **dev);
+
/**
* request_firmware_into_buf - Load firmware into a previously allocated buffer.
* @dev: An instance of a driver.
--
2.55.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v8 4/7] misc: fw_loader: implement request_firmware_size() OP
2026-10-22 19:40 [PATCH v8 0/7] misc: fs_loader: reorg and split to FS and FW loader + FIP loader Christian Marangi
` (2 preceding siblings ...)
2026-10-22 19:40 ` [PATCH v8 3/7] misc: fw_loader: implement generic get_fw_loader_from_node() Christian Marangi
@ 2026-10-22 19:40 ` Christian Marangi
2026-10-22 19:40 ` [PATCH v8 5/7] misc: fw_loader: introduce FIP loader driver Christian Marangi
` (3 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Christian Marangi @ 2026-10-22 19:40 UTC (permalink / raw)
To: Simon Glass, Tom Rini, Mattijs Korpershoek, Marek Vasut,
Heiko Schocher, Neil Armstrong, Alexey Charkov, Daniel Golle,
Aristo Chen, Christian Marangi, Lucien.Jheng, Mehmet Fide,
Casey Connolly, Michal Simek, Quentin Schulz, Johan Jonker,
Anshul Dalal, Romain Gantois, Mateus Lima Alves,
Marcello Sylvester Bauer, Tan Siewert, Peng Fan, Patrice Chotard,
Yao Zi, Heinrich Schuchardt, Martin Schwan, u-boot
It might be useful to get the firmware size before reading it to allocate
the correct size. This is needed for case where the firmware size change
across different firmware version and is not always the same. In such case
the only way to handle this scenario is to allocate a big-enough buffer to
read the firmware.
To better handle this, introduce the request_firmware_size() OP to get the
firmware size before reading it. This way proper buffer can be allocated
and dynamic firmware size can be better supported.
FS loader is updated to handle the new .get_size() handler.
Reviewed-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
drivers/misc/fw_loader/fs_loader.c | 68 +++++++++++++++++++----
drivers/misc/fw_loader/fw_loader-uclass.c | 19 +++++++
drivers/misc/fw_loader/internal.h | 7 +++
include/fw_loader.h | 9 +++
4 files changed, 91 insertions(+), 12 deletions(-)
diff --git a/drivers/misc/fw_loader/fs_loader.c b/drivers/misc/fw_loader/fs_loader.c
index 9923fb5e123d..c7aef99be8bb 100644
--- a/drivers/misc/fw_loader/fs_loader.c
+++ b/drivers/misc/fw_loader/fs_loader.c
@@ -80,18 +80,10 @@ static int select_fs_dev(struct fw_loader_plat *plat)
return ret;
}
-/**
- * fw_get_filesystem_firmware - load firmware into an allocated buffer.
- * @dev: An instance of a driver.
- *
- * Return: Size of total read, negative value when error.
- */
-static int fw_get_filesystem_firmware(struct udevice *dev)
+static int fw_get_filesystem_prepare(struct udevice *dev)
{
- struct firmware *upriv = dev_get_uclass_priv(dev);
struct fw_loader_plat *plat = dev_get_uclass_plat(dev);
struct fw_loader_env_info info = { };
- loff_t actread = 0;
int ret;
generic_fw_loader_env_info(&info);
@@ -114,6 +106,29 @@ static int fw_get_filesystem_firmware(struct udevice *dev)
ret = select_fs_dev(plat);
}
+ return ret;
+}
+
+static void fw_get_filesystem_release(struct udevice *dev)
+{
+#ifdef CONFIG_CMD_UBIFS
+ umount_ubifs();
+#endif
+}
+
+/**
+ * fw_get_filesystem_firmware - load firmware into an allocated buffer.
+ * @dev: An instance of a driver.
+ *
+ * Return: Size of total read, negative value when error.
+ */
+static int fw_get_filesystem_firmware(struct udevice *dev)
+{
+ struct firmware *upriv = dev_get_uclass_priv(dev);
+ loff_t actread = 0;
+ int ret;
+
+ ret = fw_get_filesystem_prepare(dev);
if (ret)
goto out;
@@ -128,14 +143,43 @@ static int fw_get_filesystem_firmware(struct udevice *dev)
}
out:
-#ifdef CONFIG_CMD_UBIFS
- umount_ubifs();
-#endif
+ fw_get_filesystem_release(dev);
+ return ret;
+}
+
+/**
+ * fw_get_filesystem_firmware_size - get firmware size.
+ * @dev: An instance of a driver.
+ *
+ * Return: Size of firmware, negative value when error.
+ */
+static int fw_get_filesystem_firmware_size(struct udevice *dev)
+{
+ struct firmware *upriv = dev_get_uclass_priv(dev);
+ loff_t size = 0;
+ int ret;
+
+ ret = fw_get_filesystem_prepare(dev);
+ if (ret)
+ goto out;
+
+ ret = fs_size(upriv->name, &size);
+ if (ret) {
+ debug("Error: %d Failed to get size for %s.\n",
+ ret, upriv->name);
+ goto out;
+ }
+
+ ret = size;
+
+out:
+ fw_get_filesystem_release(dev);
return ret;
}
static const struct fw_loader_ops fs_loader_ops = {
.get_firmware = fw_get_filesystem_firmware,
+ .get_size = fw_get_filesystem_firmware_size,
};
static const struct udevice_id fs_loader_ids[] = {
diff --git a/drivers/misc/fw_loader/fw_loader-uclass.c b/drivers/misc/fw_loader/fw_loader-uclass.c
index 7f19d6c4af4c..4e67f2559fa2 100644
--- a/drivers/misc/fw_loader/fw_loader-uclass.c
+++ b/drivers/misc/fw_loader/fw_loader-uclass.c
@@ -210,3 +210,22 @@ int request_firmware_into_buf(struct udevice *dev,
return ops->get_firmware(dev);
}
+
+int request_firmware_size(struct udevice *dev, const char *name)
+{
+ const struct fw_loader_ops *ops;
+ int ret;
+
+ if (!dev)
+ return -EINVAL;
+
+ ret = _request_firmware_prepare(dev, name, NULL, 0, 0);
+ if (ret < 0) /* error */
+ return ret;
+
+ ops = fw_loader_get_ops(dev);
+ if (!ops->get_size)
+ return -EOPNOTSUPP;
+
+ return ops->get_size(dev);
+}
diff --git a/drivers/misc/fw_loader/internal.h b/drivers/misc/fw_loader/internal.h
index 9ff43e872554..1abe7504a81e 100644
--- a/drivers/misc/fw_loader/internal.h
+++ b/drivers/misc/fw_loader/internal.h
@@ -32,6 +32,13 @@ struct fw_loader_ops {
* @dev: Firmware Loader device to read firmware from
*/
int (*get_firmware)(struct udevice *dev);
+
+ /**
+ * get_size() - get firmware size from Firmware Loader driver
+ *
+ * @dev: Firmware Loader device to get firmware size from
+ */
+ int (*get_size)(struct udevice *dev);
};
#define fw_loader_get_ops(dev) ((const struct fw_loader_ops *)(dev)->driver->ops)
diff --git a/include/fw_loader.h b/include/fw_loader.h
index 99c47380a172..01f242dcbefd 100644
--- a/include/fw_loader.h
+++ b/include/fw_loader.h
@@ -38,6 +38,15 @@ int request_firmware_into_buf(struct udevice *dev,
const char *name,
void *buf, size_t size, u32 offset);
+/**
+ * request_firmware_size - Get firmware size.
+ * @dev: An instance of a driver.
+ * @name: Name of firmware file.
+ *
+ * Return: Size of firmware, negative value when error.
+ */
+int request_firmware_size(struct udevice *dev, const char *name);
+
/**
* request_firmware_into_buf_via_script() -
* Load firmware using a U-Boot script and copy to buffer
--
2.55.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v8 5/7] misc: fw_loader: introduce FIP loader driver
2026-10-22 19:40 [PATCH v8 0/7] misc: fs_loader: reorg and split to FS and FW loader + FIP loader Christian Marangi
` (3 preceding siblings ...)
2026-10-22 19:40 ` [PATCH v8 4/7] misc: fw_loader: implement request_firmware_size() OP Christian Marangi
@ 2026-10-22 19:40 ` Christian Marangi
2026-10-23 13:12 ` Simon Glass
2026-10-22 19:40 ` [PATCH v8 6/7] doc: dtbinding: Update documentation for Generic Firmware loader Christian Marangi
` (2 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Christian Marangi @ 2026-10-22 19:40 UTC (permalink / raw)
To: Simon Glass, Tom Rini, Mattijs Korpershoek, Marek Vasut,
Heiko Schocher, Neil Armstrong, Alexey Charkov, Daniel Golle,
Aristo Chen, Christian Marangi, Lucien.Jheng, Mehmet Fide,
Casey Connolly, Michal Simek, Quentin Schulz, Johan Jonker,
Anshul Dalal, Romain Gantois, Mateus Lima Alves,
Marcello Sylvester Bauer, Tan Siewert, Peng Fan, Patrice Chotard,
Yao Zi, Heinrich Schuchardt, Martin Schwan, u-boot
Introduce a variant of the FS loader driver to extract images from FIP
image. These image can contain additional binary used to init Network
accelerator or PHY firmware blob.
The way FIP handle image type is with the usage of UUID.
This FIP loader driver implement a simple FIP image parser that check
every entry for a matching UUID.
Similar to FS loader, this driver also support both UBI and Block
devices.
Also an additional property is added to handle special case with eMMC
that doesn't have a GPT partition and require a global offset to
reference the FIP partition.
An example usage of this driver is the following:
Entry in DTS:
fs_loader0: fip-loader {
bootph-all;
compatible = "u-boot,fip-loader";
phandlepart = <&mmc0 0>;
partoffset = <0x100>;
};
ethernet@1fb50000 {
firmware-loader = <&fs_loader0>;
}
FIP loader user:
/* get the FW loader from the ethernet node */
get_fw_loader_from_node(dev_ofnode(dev), &fw_loader);
/* read the blob identified by "58704aef-389f-3e52-b475-e0bf2234a6a2" UUID */
request_firmware_into_buf(fw_loader, "58704aef-389f-3e52-b475-e0bf2234a6a2",
buf, 261400, 0);
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
drivers/misc/Kconfig | 23 ++
drivers/misc/fw_loader/Makefile | 1 +
drivers/misc/fw_loader/fip_loader.c | 556 ++++++++++++++++++++++++++++
drivers/misc/fw_loader/internal.h | 2 +
4 files changed, 582 insertions(+)
create mode 100644 drivers/misc/fw_loader/fip_loader.c
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 1cab469a3434..f1091e96775e 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -624,6 +624,17 @@ config MPC83XX_SERDES
config FW_LOADER_UCLASS
bool
+config FIP_LOADER
+ bool "Enable loader driver from FIP partition"
+ select LIB_UUID
+ select FW_LOADER_UCLASS
+ help
+ This is FIP partition generic loader which can be used to load
+ the file image from the FIP image into target such as memory.
+
+ The consumer driver would then use this loader to program whatever,
+ ie. the FPGA device/PHY firmware.
+
config FS_LOADER
bool "Enable loader driver for file system"
select FW_LOADER_UCLASS
@@ -637,6 +648,18 @@ config FS_LOADER
config SPL_FW_LOADER_UCLASS
bool
+config SPL_FIP_LOADER
+ bool "Enable loader driver from FIP partition in SPL"
+ select LIB_UUID
+ select SPL_FW_LOADER_UCLASS
+ depends on SPL
+ help
+ This is FIP partition generic loader which can be used to load
+ the file image from the FIP image into target such as memory.
+
+ The consumer driver would then use this loader to program whatever,
+ ie. the FPGA device/PHY firmware.
+
config SPL_FS_LOADER
bool "Enable loader driver for file system in SPL"
select SPL_FW_LOADER_UCLASS
diff --git a/drivers/misc/fw_loader/Makefile b/drivers/misc/fw_loader/Makefile
index f5df0db61c22..120722c4ed17 100644
--- a/drivers/misc/fw_loader/Makefile
+++ b/drivers/misc/fw_loader/Makefile
@@ -1,4 +1,5 @@
# SPDX-License-Identifier: GPL-2.0+
obj-$(CONFIG_$(PHASE_)FW_LOADER_UCLASS) += fw_loader-uclass.o
+obj-$(CONFIG_$(PHASE_)FIP_LOADER) += fip_loader.o
obj-$(CONFIG_$(PHASE_)FS_LOADER) += fs_loader.o
diff --git a/drivers/misc/fw_loader/fip_loader.c b/drivers/misc/fw_loader/fip_loader.c
new file mode 100644
index 000000000000..1df5b46af1d0
--- /dev/null
+++ b/drivers/misc/fw_loader/fip_loader.c
@@ -0,0 +1,556 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2025 Christian Marangi <ansuelsmth@gmail.com>
+ *
+ */
+
+#define LOG_CATEGORY UCLASS_FIRMWARE_LOADER
+
+#include <blk.h>
+#include <dm.h>
+#include <div64.h>
+#include <env.h>
+#include <errno.h>
+#include <fs.h>
+#include <linux/kernel.h>
+#include <log.h>
+#include <malloc.h>
+#include <memalign.h>
+#include <part.h>
+#include <u-boot/uuid.h>
+
+#if CONFIG_IS_ENABLED(CMD_UBIFS)
+#include <ubi_uboot.h>
+#endif
+
+#include "internal.h"
+
+#define TOC_HEADER_NAME 0xaa640001
+
+/*
+ * FIP toc and entry is always in the
+ * native byte order of the system.
+ */
+struct fip_toc_header {
+ u32 name;
+ u32 serial_number;
+ u64 flags;
+} __packed;
+
+struct fip_toc_entry {
+ struct uuid uuid;
+ u64 offset_address;
+ u64 size;
+ u64 flags;
+} __packed;
+
+enum fip_storage_interface {
+ FIP_STORAGE_INTERFACE_BLK,
+ FIP_STORAGE_INTERFACE_UBI,
+};
+
+struct fip_storage_info {
+ enum fip_storage_interface storage_interface;
+
+ /* BLK info */
+ struct disk_partition part_info;
+ struct blk_desc *desc;
+ unsigned int part_offset;
+
+ /* UBI info */
+ char *ubi_volume;
+};
+
+static bool validate_fip_toc_header(struct fip_toc_header *hdr)
+{
+ if (hdr->name != TOC_HEADER_NAME) {
+ log_err("Invalid FIP header\n");
+ return false;
+ }
+
+ return true;
+}
+
+static int firmware_name_to_uuid(struct firmware *firmwarep,
+ struct uuid *uuid)
+{
+ const char *uuid_str = firmwarep->name;
+ int ret;
+
+ ret = uuid_str_to_bin(uuid_str, (unsigned char *)uuid,
+ UUID_STR_FORMAT_STD);
+ if (ret)
+ log_err("Invalid UUID str: %s\n", uuid_str);
+
+ return ret;
+}
+
+static int check_fip_toc_entry(struct fip_toc_entry *ent,
+ struct uuid *uuid,
+ struct fip_toc_entry *dent)
+{
+ struct uuid uuid_null = { };
+
+ /* NULL uuid. We parsed every entry */
+ if (!memcmp(&ent->uuid, &uuid_null, sizeof(uuid_null)))
+ return -ENOENT;
+
+ /* We found the related uuid */
+ if (!memcmp(&ent->uuid, uuid, sizeof(*uuid))) {
+ log_debug("Found matching FIP entry. offset: 0x%llx size: %lld\n",
+ ent->offset_address, ent->size);
+ memcpy(dent, ent, sizeof(*ent));
+ return 0;
+ }
+
+ return -EAGAIN;
+}
+
+static int blk_read_fip_toc_header(struct blk_desc *desc, lbaint_t offset,
+ char *buf, struct fip_toc_header *hdr)
+{
+ lbaint_t blkcnt = BLOCK_CNT(sizeof(*hdr), desc);
+ lbaint_t to_read, read = 0;
+ int i, ret;
+
+ for (i = 0; i < blkcnt && read < sizeof(*hdr); i++) {
+ to_read = min(desc->blksz,
+ (lbaint_t)(sizeof(*hdr) - read));
+
+ ret = blk_dread(desc, offset + i, 1, buf);
+ if (ret != 1)
+ return -EINVAL;
+
+ memcpy((u8 *)hdr + read, buf, to_read);
+ read += to_read;
+ }
+
+ return read;
+}
+
+static int blk_read_fip_toc_entry(struct blk_desc *desc, lbaint_t offset,
+ int pos, char *buf,
+ struct fip_toc_entry *ent)
+{
+ lbaint_t left, consumed, to_read, read = 0;
+ lbaint_t blkstart, blkcnt;
+ int i, ret;
+
+ consumed = pos % desc->blksz;
+ left = desc->blksz - consumed;
+ to_read = min(left, (lbaint_t)sizeof(*ent));
+
+ blkstart = BLOCK_CNT(pos, desc);
+ blkcnt = BLOCK_CNT(sizeof(*ent) - to_read, desc);
+
+ /* Read data from previous cached block if present */
+ if (consumed) {
+ memcpy(ent, buf + consumed, to_read);
+ read += to_read;
+ }
+
+ for (i = 0; i < blkcnt && read < sizeof(*ent); i++) {
+ to_read = min(desc->blksz,
+ (lbaint_t)(sizeof(*ent) - read));
+
+ ret = blk_dread(desc, offset + blkstart + i, 1, buf);
+ if (ret != 1)
+ return -EINVAL;
+
+ memcpy((u8 *)ent + read, buf, to_read);
+ read += to_read;
+ }
+
+ return read;
+}
+
+static int blk_parse_fip_firmware(struct firmware *firmwarep,
+ struct blk_desc *desc,
+ struct disk_partition *part_info,
+ unsigned int part_offset,
+ struct fip_toc_entry *dent)
+{
+ lbaint_t offset = part_info->start + part_offset;
+ struct fip_toc_header hdr;
+ struct fip_toc_entry ent;
+ struct uuid uuid;
+ unsigned int pos;
+ char *read_buf;
+ int ret;
+
+ /* Allocate a Scratch Buffer for FIP parsing */
+ read_buf = malloc(desc->blksz);
+ if (!read_buf)
+ return -ENOMEM;
+
+ ret = blk_read_fip_toc_header(desc, offset, read_buf, &hdr);
+ if (ret < 0) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ pos = ret;
+
+ if (!validate_fip_toc_header(&hdr)) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ ret = firmware_name_to_uuid(firmwarep, &uuid);
+ if (ret)
+ goto out;
+
+ /* Loop for every FIP entry searching for uuid */
+ while (true) {
+ ret = blk_read_fip_toc_entry(desc, offset, pos,
+ read_buf, &ent);
+ if (ret < 0)
+ goto out;
+
+ pos += ret;
+
+ ret = check_fip_toc_entry(&ent, &uuid, dent);
+ if (ret != -EAGAIN)
+ break;
+ }
+
+out:
+ free(read_buf);
+ return ret;
+}
+
+#if CONFIG_IS_ENABLED(CMD_UBIFS)
+static int ubi_parse_fip_firmware(struct firmware *firmwarep,
+ char *ubi_vol,
+ struct fip_toc_entry *dent)
+{
+ struct fip_toc_header hdr;
+ struct fip_toc_entry ent;
+ struct uuid uuid;
+ unsigned int pos;
+ int ret;
+
+ ret = ubi_volume_read(ubi_vol, (char *)&hdr, 0, sizeof(hdr));
+ if (ret)
+ return ret;
+
+ pos = sizeof(hdr);
+
+ if (!validate_fip_toc_header(&hdr))
+ return -EINVAL;
+
+ ret = firmware_name_to_uuid(firmwarep, &uuid);
+ if (ret)
+ return ret;
+
+ /* Loop for every FIP entry searching for uuid */
+ while (true) {
+ ret = ubi_volume_read(ubi_vol, (char *)&ent, pos,
+ sizeof(ent));
+ if (ret)
+ return ret;
+
+ ret = check_fip_toc_entry(&ent, &uuid, dent);
+ if (ret != -EAGAIN)
+ break;
+
+ pos += sizeof(ent);
+ }
+
+ return ret;
+}
+#endif
+
+static int parse_fip_firmware(struct firmware *firmwarep,
+ struct fip_storage_info *info,
+ struct fip_toc_entry *dent)
+{
+ switch (info->storage_interface) {
+ case FIP_STORAGE_INTERFACE_BLK:
+ return blk_parse_fip_firmware(firmwarep, info->desc,
+ &info->part_info,
+ info->part_offset,
+ dent);
+#if CONFIG_IS_ENABLED(CMD_UBIFS)
+ case FIP_STORAGE_INTERFACE_UBI:
+ return ubi_parse_fip_firmware(firmwarep,
+ info->ubi_volume,
+ dent);
+#endif
+ default:
+ return -EINVAL;
+ }
+}
+
+static int blk_read_fip_firmware(struct firmware *firmwarep,
+ struct blk_desc *desc,
+ struct disk_partition *part_info,
+ unsigned int part_offset,
+ const struct fip_toc_entry *ent)
+{
+ unsigned int offset = part_info->start + part_offset;
+ unsigned long pos, to_read, read = 0;
+ unsigned long long blkstart;
+ unsigned int blkcnt;
+ char *read_buf;
+ size_t size;
+ int i, ret;
+
+ read_buf = malloc(desc->blksz);
+ if (!read_buf)
+ return -ENOMEM;
+
+ size = min(firmwarep->size, (size_t)ent->size - firmwarep->offset);
+ blkcnt = BLOCK_CNT(size, desc);
+ blkstart = ent->offset_address + firmwarep->offset;
+ pos = do_div(blkstart, desc->blksz);
+
+ /* Read data in the middle of a block */
+ if (pos) {
+ to_read = min(desc->blksz - pos, (unsigned long)size);
+ ret = blk_dread(desc, offset + blkstart, 1, read_buf);
+ if (ret != 1) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ memcpy((u8 *)firmwarep->data, read_buf + pos, to_read);
+ read += to_read;
+ blkstart++;
+ }
+
+ /* Consume all the remaining block */
+ for (i = 0; i < blkcnt && read < size; i++) {
+ to_read = min(desc->blksz, (unsigned long)(size - read));
+ ret = blk_dread(desc, offset + blkstart + i, 1, read_buf);
+ if (ret != 1) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ memcpy((u8 *)firmwarep->data + read, read_buf, to_read);
+ read += to_read;
+ }
+
+ ret = read;
+
+out:
+ free(read_buf);
+ return ret;
+}
+
+#if CONFIG_IS_ENABLED(CMD_UBIFS)
+static int ubi_read_fip_firmware(struct firmware *firmwarep,
+ char *ubi_vol,
+ const struct fip_toc_entry *ent)
+{
+ unsigned int offset = firmwarep->offset;
+ size_t size;
+ int ret;
+
+ size = min(firmwarep->size, (size_t)ent->size - offset);
+ ret = ubi_volume_read(ubi_vol,
+ (u8 *)firmwarep->data,
+ ent->offset_address + offset,
+ size);
+ if (ret)
+ return ret;
+
+ return size - firmwarep->offset;
+}
+#endif
+
+static int read_fip_firmware(struct firmware *firmwarep,
+ struct fip_storage_info *info,
+ const struct fip_toc_entry *dent)
+{
+ switch (info->storage_interface) {
+ case FIP_STORAGE_INTERFACE_BLK:
+ return blk_read_fip_firmware(firmwarep, info->desc,
+ &info->part_info,
+ info->part_offset,
+ dent);
+#if CONFIG_IS_ENABLED(CMD_UBIFS)
+ case FIP_STORAGE_INTERFACE_UBI:
+ return ubi_read_fip_firmware(firmwarep,
+ info->ubi_volume,
+ dent);
+#endif
+ default:
+ return -EINVAL;
+ }
+}
+
+static int fw_parse_storage_info(struct udevice *dev,
+ struct fip_storage_info *info)
+{
+ struct fw_loader_plat *plat = dev_get_uclass_plat(dev);
+ struct fw_loader_env_info env_info = { };
+ int ret;
+
+ generic_fw_loader_env_info(&env_info);
+ info->part_offset = env_get_hex("fw_partoffset", 0);
+
+ if (env_info.storage_interface && env_info.dev_part) {
+ int part;
+
+ part = part_get_info_by_dev_and_name_or_num(env_info.storage_interface,
+ env_info.dev_part,
+ &info->desc,
+ &info->part_info, 1);
+ if (part < 0)
+ return part;
+
+ info->storage_interface = FIP_STORAGE_INTERFACE_BLK;
+
+ return 0;
+ }
+
+ if (env_info.storage_interface && env_info.ubi_mtdpart &&
+ env_info.ubi_volume) {
+ if (strcmp("ubi", env_info.storage_interface))
+ return -ENODEV;
+
+ ret = generic_fw_loader_ubi_select(env_info.ubi_mtdpart);
+ if (ret)
+ return ret;
+
+ info->ubi_volume = env_info.ubi_volume;
+ info->storage_interface = FIP_STORAGE_INTERFACE_UBI;
+
+ return 0;
+ }
+
+ info->part_offset = plat->partoffset;
+
+ if (plat->phandlepart.phandle) {
+ int part;
+
+ ret = generic_fw_loader_get_phandle_desc(plat, &info->desc);
+ if (ret)
+ return ret;
+
+ part = plat->phandlepart.partition;
+ if (part >= 1)
+ ret = part_get_info(info->desc, part,
+ &info->part_info);
+ else
+ ret = part_get_info_whole_disk(info->desc,
+ &info->part_info);
+
+ info->storage_interface = FIP_STORAGE_INTERFACE_BLK;
+
+ return ret;
+ }
+
+ if (plat->mtdpart && plat->ubivol) {
+ ret = generic_fw_loader_ubi_select(plat->mtdpart);
+ if (ret)
+ return ret;
+
+ info->ubi_volume = plat->ubivol;
+ info->storage_interface = FIP_STORAGE_INTERFACE_UBI;
+
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
+/**
+ * fw_get_fip_firmware - load firmware into an allocated buffer.
+ * @dev: An instance of a driver.
+ *
+ * Return: Size of total read, negative value when error.
+ */
+static int fw_get_fip_firmware(struct udevice *dev)
+{
+ struct fip_storage_info info = { };
+ struct firmware *firmwarep;
+ struct fip_toc_entry ent;
+ int ret;
+
+ ret = fw_parse_storage_info(dev, &info);
+ if (ret)
+ return ret;
+
+ firmwarep = dev_get_uclass_priv(dev);
+ if (!firmwarep)
+ return -EINVAL;
+
+ ret = parse_fip_firmware(firmwarep, &info, &ent);
+ if (ret)
+ return ret;
+
+ if (firmwarep->offset > ent.size) {
+ log_err("Invalid offset. Entry too small\n");
+ return -EINVAL;
+ }
+
+ ret = read_fip_firmware(firmwarep, &info, &ent);
+ if (ret < 0)
+ log_err("Failed to read %s from FIP: %d.\n",
+ firmwarep->name, ret);
+
+ return ret;
+}
+
+/**
+ * fw_get_fip_firmware_size - get firmware size.
+ * @dev: An instance of a driver.
+ *
+ * Return: Size of firmware, negative value when error.
+ */
+static int fw_get_fip_firmware_size(struct udevice *dev)
+{
+ struct fip_storage_info info = { };
+ struct firmware *firmwarep;
+ struct fip_toc_entry ent;
+ int ret;
+
+ ret = fw_parse_storage_info(dev, &info);
+ if (ret)
+ return ret;
+
+ firmwarep = dev_get_uclass_priv(dev);
+ if (!firmwarep)
+ return -EINVAL;
+
+ ret = parse_fip_firmware(firmwarep, &info, &ent);
+ if (ret)
+ return ret;
+
+ return ent.size - firmwarep->offset;
+}
+
+static const struct fw_loader_ops fip_loader_ops = {
+ .get_firmware = fw_get_fip_firmware,
+ .get_size = fw_get_fip_firmware_size,
+};
+
+static int fip_loader_of_to_plat(struct udevice *dev)
+{
+ struct fw_loader_plat *plat = dev_get_uclass_plat(dev);
+ ofnode fw_loader_node;
+
+ fw_loader_node = dev_ofnode(dev);
+ if (ofnode_valid(fw_loader_node)) {
+ ofnode_read_u32(fw_loader_node, "partoffset",
+ &plat->partoffset);
+ }
+
+ return 0;
+}
+
+static const struct udevice_id fip_loader_ids[] = {
+ { .compatible = "u-boot,fip-loader"},
+ { }
+};
+
+U_BOOT_DRIVER(fip_loader) = {
+ .name = "fip-loader",
+ .id = UCLASS_FIRMWARE_LOADER,
+ .of_match = fip_loader_ids,
+ .of_to_plat = fip_loader_of_to_plat,
+ .ops = &fip_loader_ops,
+};
diff --git a/drivers/misc/fw_loader/internal.h b/drivers/misc/fw_loader/internal.h
index 1abe7504a81e..5af061321512 100644
--- a/drivers/misc/fw_loader/internal.h
+++ b/drivers/misc/fw_loader/internal.h
@@ -49,11 +49,13 @@ struct fw_loader_ops {
* This holds information about all supported storage devices for driver use.
*
* @phandlepart: Attribute data for block device.
+ * @partoffset: Global offset for BLK partition.
* @mtdpart: MTD partition for ubi partition.
* @ubivol: UBI volume-name for ubifsmount.
*/
struct fw_loader_plat {
struct phandle_part phandlepart;
+ u32 partoffset;
char *mtdpart;
char *ubivol;
};
--
2.55.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v8 5/7] misc: fw_loader: introduce FIP loader driver
2026-10-22 19:40 ` [PATCH v8 5/7] misc: fw_loader: introduce FIP loader driver Christian Marangi
@ 2026-10-23 13:12 ` Simon Glass
2026-10-23 16:22 ` Christian Marangi
0 siblings, 1 reply; 16+ messages in thread
From: Simon Glass @ 2026-10-23 13:12 UTC (permalink / raw)
To: ansuelsmth
Cc: Simon Glass, Tom Rini, Mattijs Korpershoek, Marek Vasut,
Heiko Schocher, Neil Armstrong, Alexey Charkov, Daniel Golle,
Aristo Chen, Lucien.Jheng, Mehmet Fide, Casey Connolly,
Michal Simek, Quentin Schulz, Johan Jonker, Anshul Dalal,
Romain Gantois, Mateus Lima Alves, Marcello Sylvester Bauer,
Tan Siewert, Peng Fan, Patrice Chotard, Yao Zi,
Heinrich Schuchardt, Martin Schwan, u-boot
Hi Christian,
On 2026-10-22T19:40:35, Christian Marangi (Ansuel) <ansuelsmth@gmail.com> wrote:
> misc: fw_loader: introduce FIP loader driver
>
> Introduce a variant of the FS loader driver to extract images from FIP
> image. These image can contain additional binary used to init Network
> accelerator or PHY firmware blob.
>
> The way FIP handle image type is with the usage of UUID.
>
> This FIP loader driver implement a simple FIP image parser that check
> every entry for a matching UUID.
>
> Similar to FS loader, this driver also support both UBI and Block
> devices.
>
> Also an additional property is added to handle special case with eMMC
> that doesn't have a GPT partition and require a global offset to
> reference the FIP partition.
>
> An example usage of this driver is the following:
>
> [...]
>
> drivers/misc/Kconfig | 23 ++
> drivers/misc/fw_loader/Makefile | 1 +
> drivers/misc/fw_loader/fip_loader.c | 556 ++++++++++++++++++++++++++++++++++++
> drivers/misc/fw_loader/internal.h | 2 +
> 4 files changed, 582 insertions(+)
> diff --git a/drivers/misc/fw_loader/fip_loader.c b/drivers/misc/fw_loader/fip_loader.c
> @@ -0,0 +1,556 @@
> +static int blk_read_fip_firmware(struct firmware *firmwarep,
> + struct blk_desc *desc,
> + struct disk_partition *part_info,
> + unsigned int part_offset,
> + const struct fip_toc_entry *ent)
> +{
> + unsigned int offset = part_info->start + part_offset;
The parse path now uses lbaint_t, but this read path still truncates
part_info->start to unsigned int, so on a large device the header is
found but the data is read from the wrong block. Please can you use
lbaint_t here too (and for blkcnt/blkstart, to match
blk_parse_fip_firmware())?
> diff --git a/drivers/misc/fw_loader/fip_loader.c b/drivers/misc/fw_loader/fip_loader.c
> @@ -0,0 +1,556 @@
> + size = min(firmwarep->size, (size_t)ent->size - offset);
> + ret = ubi_volume_read(ubi_vol,
> + (u8 *)firmwarep->data,
> + ent->offset_address + offset,
> + size);
> + if (ret)
> + return ret;
> +
> + return size - firmwarep->offset;
'size' already has the offset taken off, so this subtracts it twice.
With a non-zero offset the UBI path returns less than it actually
read, whereas the block path returns the full 'read' count. I think
this should just be 'return size;'
Also, ubi_volume_read() treats size == 0 as 'read the whole volume',
which overruns the buffer. That happens if offset == ent->size (which
the check in fw_get_fip_firmware() allows) or if the caller passes a
zero-sized buffer. Please can you return early when size is 0?
Regards,
Simon
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v8 5/7] misc: fw_loader: introduce FIP loader driver
2026-10-23 13:12 ` Simon Glass
@ 2026-10-23 16:22 ` Christian Marangi
0 siblings, 0 replies; 16+ messages in thread
From: Christian Marangi @ 2026-10-23 16:22 UTC (permalink / raw)
To: Simon Glass
Cc: Tom Rini, Mattijs Korpershoek, Marek Vasut, Heiko Schocher,
Neil Armstrong, Alexey Charkov, Daniel Golle, Aristo Chen,
Lucien.Jheng, Mehmet Fide, Casey Connolly, Michal Simek,
Quentin Schulz, Johan Jonker, Anshul Dalal, Romain Gantois,
Mateus Lima Alves, Marcello Sylvester Bauer, Tan Siewert,
Peng Fan, Patrice Chotard, Yao Zi, Heinrich Schuchardt,
Martin Schwan, u-boot
On Fri, Oct 23, 2026 at 09:12:32AM -0400, Simon Glass wrote:
> Hi Christian,
>
> On 2026-10-22T19:40:35, Christian Marangi (Ansuel) <ansuelsmth@gmail.com> wrote:
> > misc: fw_loader: introduce FIP loader driver
> >
> > Introduce a variant of the FS loader driver to extract images from FIP
> > image. These image can contain additional binary used to init Network
> > accelerator or PHY firmware blob.
> >
> > The way FIP handle image type is with the usage of UUID.
> >
> > This FIP loader driver implement a simple FIP image parser that check
> > every entry for a matching UUID.
> >
> > Similar to FS loader, this driver also support both UBI and Block
> > devices.
> >
> > Also an additional property is added to handle special case with eMMC
> > that doesn't have a GPT partition and require a global offset to
> > reference the FIP partition.
> >
> > An example usage of this driver is the following:
> >
> > [...]
> >
> > drivers/misc/Kconfig | 23 ++
> > drivers/misc/fw_loader/Makefile | 1 +
> > drivers/misc/fw_loader/fip_loader.c | 556 ++++++++++++++++++++++++++++++++++++
> > drivers/misc/fw_loader/internal.h | 2 +
> > 4 files changed, 582 insertions(+)
>
> > diff --git a/drivers/misc/fw_loader/fip_loader.c b/drivers/misc/fw_loader/fip_loader.c
> > @@ -0,0 +1,556 @@
> > +static int blk_read_fip_firmware(struct firmware *firmwarep,
> > + struct blk_desc *desc,
> > + struct disk_partition *part_info,
> > + unsigned int part_offset,
> > + const struct fip_toc_entry *ent)
> > +{
> > + unsigned int offset = part_info->start + part_offset;
>
> The parse path now uses lbaint_t, but this read path still truncates
> part_info->start to unsigned int, so on a large device the header is
> found but the data is read from the wrong block. Please can you use
> lbaint_t here too (and for blkcnt/blkstart, to match
> blk_parse_fip_firmware())?
>
> > diff --git a/drivers/misc/fw_loader/fip_loader.c b/drivers/misc/fw_loader/fip_loader.c
> > @@ -0,0 +1,556 @@
> > + size = min(firmwarep->size, (size_t)ent->size - offset);
> > + ret = ubi_volume_read(ubi_vol,
> > + (u8 *)firmwarep->data,
> > + ent->offset_address + offset,
> > + size);
> > + if (ret)
> > + return ret;
> > +
> > + return size - firmwarep->offset;
>
> 'size' already has the offset taken off, so this subtracts it twice.
> With a non-zero offset the UBI path returns less than it actually
> read, whereas the block path returns the full 'read' count. I think
> this should just be 'return size;'
>
> Also, ubi_volume_read() treats size == 0 as 'read the whole volume',
> which overruns the buffer. That happens if offset == ent->size (which
> the check in fw_get_fip_firmware() allows) or if the caller passes a
> zero-sized buffer. Please can you return early when size is 0?
>
!!! No idea ubi_volume_read() 0 is read all. Thanks a lot for noticing
this.
--
Ansuel
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v8 6/7] doc: dtbinding: Update documentation for Generic Firmware loader
2026-10-22 19:40 [PATCH v8 0/7] misc: fs_loader: reorg and split to FS and FW loader + FIP loader Christian Marangi
` (4 preceding siblings ...)
2026-10-22 19:40 ` [PATCH v8 5/7] misc: fw_loader: introduce FIP loader driver Christian Marangi
@ 2026-10-22 19:40 ` Christian Marangi
2026-10-23 13:12 ` Simon Glass
2026-10-22 19:40 ` [PATCH v8 7/7] test: dm: Add test for FW Loader feature Christian Marangi
2026-10-23 13:12 ` [PATCH v8 0/7] misc: fs_loader: reorg and split to FS and FW loader + FIP loader Simon Glass
7 siblings, 1 reply; 16+ messages in thread
From: Christian Marangi @ 2026-10-22 19:40 UTC (permalink / raw)
To: Simon Glass, Tom Rini, Mattijs Korpershoek, Marek Vasut,
Heiko Schocher, Neil Armstrong, Alexey Charkov, Daniel Golle,
Aristo Chen, Christian Marangi, Lucien.Jheng, Mehmet Fide,
Casey Connolly, Michal Simek, Quentin Schulz, Johan Jonker,
Anshul Dalal, Romain Gantois, Mateus Lima Alves,
Marcello Sylvester Bauer, Tan Siewert, Peng Fan, Patrice Chotard,
Yao Zi, Heinrich Schuchardt, Martin Schwan, u-boot
Update documentation for Generic Firmware loader to YAML schema,
generalize it from FS specific and add new property and example
for FIP loader.
Also add details on the usage of 'phandlepart', 'mtdpart' and 'ubivol'
property.
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
doc/device-tree-bindings/misc/fs_loader.txt | 48 -------
doc/device-tree-bindings/misc/fw_loader.yaml | 126 +++++++++++++++++++
2 files changed, 126 insertions(+), 48 deletions(-)
delete mode 100644 doc/device-tree-bindings/misc/fs_loader.txt
create mode 100644 doc/device-tree-bindings/misc/fw_loader.yaml
diff --git a/doc/device-tree-bindings/misc/fs_loader.txt b/doc/device-tree-bindings/misc/fs_loader.txt
deleted file mode 100644
index 542be4b25a0a..000000000000
--- a/doc/device-tree-bindings/misc/fs_loader.txt
+++ /dev/null
@@ -1,48 +0,0 @@
-* File system firmware loader
-
-Required properties:
---------------------
-
-- compatible: should contain "u-boot,fs-loader"
-- phandlepart: which block storage device and partition the image loading from,
- this property is required for mmc, usb and sata. This is unsigned
- 32-bit array. For example phandlepart=<&mmc_0 1>, meaning use
- that MMC0 node pointer, partition 1.
-- mdtpart: which partition of ubi the image loading from, this property is
- required for ubi and mounting.
-- ubivol: which volume of ubi the image loading from, this property is required
- for ubi and mounting.
-
-Example of storage device and partition search set for mmc, usb, sata and
-ubi in device tree source as shown in below:
-
- Example of storage type and device partition search set for mmc, usb,
- sata and ubi as shown in below:
- Example for mmc:
- fs_loader0: fs-loader@0 {
- bootph-all;
- compatible = "u-boot,fs-loader";
- phandlepart = <&mmc_0 1>;
- };
-
- Example for usb:
- fs_loader1: fs-loader@1 {
- bootph-all;
- compatible = "u-boot,fs-loader";
- phandlepart = <&usb0 1>;
- };
-
- Example for sata:
- fs_loader2: fs-loader@2 {
- bootph-all;
- compatible = "u-boot,fs-loader";
- phandlepart = <&sata0 1>;
- };
-
- Example for ubi:
- fs_loader3: fs-loader@3 {
- bootph-all;
- compatible = "u-boot,fs-loader";
- mtdpart = "UBI",
- ubivol = "ubi0";
- };
diff --git a/doc/device-tree-bindings/misc/fw_loader.yaml b/doc/device-tree-bindings/misc/fw_loader.yaml
new file mode 100644
index 000000000000..0a99509e0881
--- /dev/null
+++ b/doc/device-tree-bindings/misc/fw_loader.yaml
@@ -0,0 +1,126 @@
+# SPDX-License-Identifier: GPL-2.0+
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/misc/fw_loader.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: U-Boot Generic Firmware Loader
+
+maintainers:
+ - Christian Marangi <ansuelsmth@gmail.com>
+
+description: |
+ Generic firmware loader for loading images from block storage devices
+ (MMC, USB, SATA) and MTD/UBI storage.
+
+ The fs-loader loads firmware images, while the fip-loader locates a
+ Firmware Image Package (FIP), optionally at a specified byte offset
+ within the selected partition.
+
+properties:
+ compatible:
+ enum:
+ - u-boot,fs-loader
+ - u-boot,fip-loader
+
+ phandlepart:
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ description: |
+ Specifies the block storage device and partition from which the
+ image is loaded. Required for MMC, USB, and SATA devices.
+
+ The first cell is a phandle to the storage device, and the second
+ cell is the partition number.
+
+ mtdpart:
+ $ref: /schemas/types.yaml#/definitions/string
+ description: |
+ Specifies the MTD partition from which the image is loaded.
+ Required for MTD and UBI devices.
+
+ ubivol:
+ $ref: /schemas/types.yaml#/definitions/string
+ description: |
+ Specifies the UBI volume from which the image is loaded.
+ Required when loading from a UBI device.
+
+ partoffset:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: |
+ Byte offset within the selected partition where the FIP header
+ is located.
+
+required:
+ - compatible
+
+oneOf:
+ - required:
+ - phandlepart
+
+ properties:
+ mtdpart: false
+
+ - required:
+ - mtdpart
+
+ properties:
+ phandlepart: false
+
+allOf:
+ - if:
+ properties:
+ compatible:
+ const: u-boot,fs-loader
+ then:
+ properties:
+ partoffset: false
+
+ - if:
+ required:
+ - phandlepart
+ then:
+ properties:
+ mtdpart: false
+
+ ubivol: false
+
+ - if:
+ required:
+ - mtdpart
+ then:
+ properties:
+ phandlepart: false
+
+additionalProperties: false
+
+examples:
+ - |
+ fs_loader0: fs-loader@0 {
+ bootph-all;
+ compatible = "u-boot,fs-loader";
+ phandlepart = <&mmc_0 1>;
+ };
+
+ - |
+ fs_loader3: fs-loader@3 {
+ bootph-all;
+ compatible = "u-boot,fs-loader";
+ mtdpart = "UBI";
+ ubivol = "ubi0";
+ };
+
+ - |
+ fs_loader4: fip-loader@4 {
+ bootph-all;
+ compatible = "u-boot,fip-loader";
+ phandlepart = <&mmc0 0>;
+ partoffset = <0x100>;
+ };
+
+ - |
+ fs_loader5: fip-loader@5 {
+ bootph-all;
+ compatible = "u-boot,fip-loader";
+ mtdpart = "ubi";
+ ubivol = "bl31";
+ };
--
2.55.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v8 6/7] doc: dtbinding: Update documentation for Generic Firmware loader
2026-10-22 19:40 ` [PATCH v8 6/7] doc: dtbinding: Update documentation for Generic Firmware loader Christian Marangi
@ 2026-10-23 13:12 ` Simon Glass
0 siblings, 0 replies; 16+ messages in thread
From: Simon Glass @ 2026-10-23 13:12 UTC (permalink / raw)
To: ansuelsmth
Cc: Simon Glass, Tom Rini, Mattijs Korpershoek, Marek Vasut,
Heiko Schocher, Neil Armstrong, Alexey Charkov, Daniel Golle,
Aristo Chen, Lucien.Jheng, Mehmet Fide, Casey Connolly,
Michal Simek, Quentin Schulz, Johan Jonker, Anshul Dalal,
Romain Gantois, Mateus Lima Alves, Marcello Sylvester Bauer,
Tan Siewert, Peng Fan, Patrice Chotard, Yao Zi,
Heinrich Schuchardt, Martin Schwan, u-boot
Hi Christian,
On 2026-10-22T19:40:35, Christian Marangi (Ansuel) <ansuelsmth@gmail.com> wrote:
> doc: dtbinding: Update documentation for Generic Firmware loader
>
> Update documentation for Generic Firmware loader to YAML schema,
> generalize it from FS specific and add new property and example
> for FIP loader.
>
> Also add details on the usage of 'phandlepart', 'mtdpart' and 'ubivol'
> property.
>
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
>
> doc/device-tree-bindings/misc/fs_loader.txt | 48 ----------
> doc/device-tree-bindings/misc/fw_loader.yaml | 126 +++++++++++++++++++++++++++
> 2 files changed, 126 insertions(+), 48 deletions(-)
> diff --git a/doc/device-tree-bindings/misc/fw_loader.yaml b/doc/device-tree-bindings/misc/fw_loader.yaml
> @@ -0,0 +1,126 @@
> + mtdpart:
> + $ref: /schemas/types.yaml#/definitions/string
> + description: |
> + Specifies the MTD partition from which the image is loaded.
> + Required for MTD and UBI devices.
Both loaders only look at mtdpart when ubivol is also set
(plat->mtdpart && plat->ubivol in fs_loader.c and fip_loader.c), so
there is no MTD-only path. Please can you say 'Required for UBI
devices' here, and make ubivol required alongside mtdpart in the
schema? The 'MTD/UBI storage' wording in the top-level description has
the same problem.
> diff --git a/doc/device-tree-bindings/misc/fw_loader.yaml b/doc/device-tree-bindings/misc/fw_loader.yaml
> @@ -0,0 +1,126 @@
> +oneOf:
> + - required:
> + - phandlepart
> +
> + properties:
> + mtdpart: false
> +
> + - required:
> + - mtdpart
> +
> + properties:
> + phandlepart: false
> +
> +allOf:
> + - if:
> + properties:
> + compatible:
> + const: u-boot,fs-loader
> + then:
> + properties:
> + partoffset: false
> +
> + - if:
> + required:
> + - phandlepart
> + then:
> + properties:
> + mtdpart: false
> +
> + ubivol: false
> +
> + - if:
> + required:
> + - mtdpart
> + then:
> + properties:
> + phandlepart: false
The second and third allOf entries repeat the oneOf. The third adds
nothing and the second only adds 'ubivol: false'. I suggest putting it
all in the oneOf, along these lines:
oneOf:
- required:
- phandlepart
properties:
mtdpart: false
ubivol: false
- required:
- mtdpart
- ubivol
properties:
phandlepart: false
and keeping only the partoffset rule in the allOf list. What do you think?
Regards,
Simon
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v8 7/7] test: dm: Add test for FW Loader feature
2026-10-22 19:40 [PATCH v8 0/7] misc: fs_loader: reorg and split to FS and FW loader + FIP loader Christian Marangi
` (5 preceding siblings ...)
2026-10-22 19:40 ` [PATCH v8 6/7] doc: dtbinding: Update documentation for Generic Firmware loader Christian Marangi
@ 2026-10-22 19:40 ` Christian Marangi
2026-10-23 13:12 ` Simon Glass
2026-10-23 13:12 ` [PATCH v8 0/7] misc: fs_loader: reorg and split to FS and FW loader + FIP loader Simon Glass
7 siblings, 1 reply; 16+ messages in thread
From: Christian Marangi @ 2026-10-22 19:40 UTC (permalink / raw)
To: Simon Glass, Tom Rini, Mattijs Korpershoek, Marek Vasut,
Heiko Schocher, Neil Armstrong, Alexey Charkov, Daniel Golle,
Aristo Chen, Christian Marangi, Lucien.Jheng, Mehmet Fide,
Casey Connolly, Michal Simek, Quentin Schulz, Johan Jonker,
Anshul Dalal, Romain Gantois, Mateus Lima Alves,
Marcello Sylvester Bauer, Tan Siewert, Peng Fan, Patrice Chotard,
Yao Zi, Heinrich Schuchardt, Martin Schwan, u-boot
Add basic test for FW Loader feature for the Filesystem way and FIP.
Additional MMC testing image are added that loads a dummy firmware.bin in
a fat partition or a simple FIP blob.
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
arch/sandbox/dts/test.dts | 38 +++++
configs/sandbox64_defconfig | 2 +
configs/sandbox_defconfig | 2 +
configs/sandbox_flattree_defconfig | 2 +
configs/sandbox_noinst_defconfig | 1 +
configs/sandbox_spl_defconfig | 2 +
configs/sandbox_vpl_defconfig | 2 +
test/dm/Makefile | 1 +
test/dm/fw_loader.c | 218 +++++++++++++++++++++++++++++
test/py/tests/fip/test.fip | Bin 0 -> 101 bytes
test/py/tests/test_ut.py | 26 ++++
11 files changed, 294 insertions(+)
create mode 100644 test/dm/fw_loader.c
create mode 100644 test/py/tests/fip/test.fip
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index 25c99ef81472..772d8dd548bc 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -189,6 +189,18 @@
};
};
+ fs_loader: fs-loader {
+ bootph-all;
+ compatible = "u-boot,fs-loader";
+ phandlepart = <&mmc12 1>;
+ };
+
+ fip_loader: fip-loader {
+ bootph-all;
+ compatible = "u-boot,fip-loader";
+ phandlepart = <&mmc13 1>;
+ };
+
cedit: cedit {
};
@@ -1398,6 +1410,20 @@
filename = "mmc11.img";
};
+ /* This is used for FS Loader tests */
+ mmc12: mmc12 {
+ status = "disabled";
+ compatible = "sandbox,mmc";
+ filename = "mmc12.img";
+ };
+
+ /* This is used for FIP Loader tests */
+ mmc13: mmc13 {
+ status = "disabled";
+ compatible = "sandbox,mmc";
+ filename = "mmc13.img";
+ };
+
pch {
compatible = "sandbox,pch";
};
@@ -2143,6 +2169,18 @@
ethphy1: ethernet-phy@1 {
reg = <1>;
};
+
+ ethernet-phy@2 {
+ reg = <2>;
+
+ firmware-loader = <&fs_loader>;
+ };
+
+ ethernet-phy@3 {
+ reg = <3>;
+
+ firmware-loader = <&fip_loader>;
+ };
};
pm-bus-test {
diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
index 59e08d03f776..7d56ab984c1d 100644
--- a/configs/sandbox64_defconfig
+++ b/configs/sandbox64_defconfig
@@ -266,9 +266,11 @@ CONFIG_WDT=y
CONFIG_WDT_GPIO=y
CONFIG_WDT_SANDBOX=y
CONFIG_WDT_ALARM_SANDBOX=y
+CONFIG_FIP_LOADER=y
CONFIG_FS_CBFS=y
CONFIG_FS_EXFAT=y
CONFIG_FS_CRAMFS=y
+CONFIG_FS_LOADER=y
CONFIG_CMD_DHRYSTONE=y
CONFIG_TPM=y
CONFIG_ERRNO_STR=y
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index f307972a762d..cf3c858b4b72 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -383,9 +383,11 @@ CONFIG_WDT_GPIO=y
CONFIG_WDT_SANDBOX=y
CONFIG_WDT_ALARM_SANDBOX=y
CONFIG_WDT_FTWDT010=y
+CONFIG_FIP_LOADER=y
CONFIG_FS_CBFS=y
CONFIG_FS_EXFAT=y
CONFIG_FS_CRAMFS=y
+CONFIG_FS_LOADER=y
CONFIG_ADDR_MAP=y
CONFIG_PANIC_HANG=y
CONFIG_PMBUS=y
diff --git a/configs/sandbox_flattree_defconfig b/configs/sandbox_flattree_defconfig
index 57f915d4ba9a..3488036ddf8f 100644
--- a/configs/sandbox_flattree_defconfig
+++ b/configs/sandbox_flattree_defconfig
@@ -12,10 +12,12 @@ CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y
CONFIG_EFI_CAPSULE_AUTHENTICATE=y
CONFIG_EFI_CAPSULE_CRT_FILE="board/sandbox/capsule_pub_key_good.crt"
CONFIG_BUTTON_CMD=y
+CONFIG_FIP_LOADER=y
CONFIG_FIT=y
CONFIG_FIT_SIGNATURE=y
CONFIG_FIT_VERITY=y
CONFIG_FIT_VERBOSE=y
+CONFIG_FS_LOADER=y
CONFIG_LEGACY_IMAGE_FORMAT=y
CONFIG_BOOTSTAGE=y
CONFIG_BOOTSTAGE_REPORT=y
diff --git a/configs/sandbox_noinst_defconfig b/configs/sandbox_noinst_defconfig
index 7a7550127956..941bf2d69431 100644
--- a/configs/sandbox_noinst_defconfig
+++ b/configs/sandbox_noinst_defconfig
@@ -21,6 +21,7 @@ CONFIG_SYS_MEMTEST_END=0x00101000
CONFIG_FIT=y
CONFIG_FIT_SIGNATURE=y
CONFIG_FIT_VERBOSE=y
+CONFIG_FS_LOADER=y
CONFIG_SPL_LOAD_FIT=y
CONFIG_SPL_HAS_LOAD_FIT_ADDRESS=y
CONFIG_SPL_LOAD_FIT_ADDRESS=0x0
diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig
index 60b497f48886..183611632ef8 100644
--- a/configs/sandbox_spl_defconfig
+++ b/configs/sandbox_spl_defconfig
@@ -223,8 +223,10 @@ CONFIG_I2C_EDID=y
CONFIG_VIDEO_SANDBOX_SDL=y
CONFIG_OSD=y
CONFIG_SANDBOX_OSD=y
+CONFIG_FIP_LOADER=y
CONFIG_FS_CBFS=y
CONFIG_FS_CRAMFS=y
+CONFIG_FS_LOADER=y
# CONFIG_SPL_USE_TINY_PRINTF is not set
CONFIG_CMD_DHRYSTONE=y
CONFIG_RSA_VERIFY_WITH_PKEY=y
diff --git a/configs/sandbox_vpl_defconfig b/configs/sandbox_vpl_defconfig
index c85bfbfb3b15..445e979915e8 100644
--- a/configs/sandbox_vpl_defconfig
+++ b/configs/sandbox_vpl_defconfig
@@ -231,8 +231,10 @@ CONFIG_CONSOLE_TRUETYPE_CANTORAONE=y
CONFIG_VIDEO_SANDBOX_SDL=y
CONFIG_OSD=y
CONFIG_SANDBOX_OSD=y
+CONFIG_FIP_LOADER=y
CONFIG_FS_CBFS=y
CONFIG_FS_CRAMFS=y
+CONFIG_FS_LOADER=y
# CONFIG_SPL_USE_TINY_PRINTF is not set
CONFIG_CMD_DHRYSTONE=y
CONFIG_RSA_VERIFY_WITH_PKEY=y
diff --git a/test/dm/Makefile b/test/dm/Makefile
index 5dd6b57bbe84..2f095ceedd8c 100644
--- a/test/dm/Makefile
+++ b/test/dm/Makefile
@@ -59,6 +59,7 @@ obj-$(CONFIG_EXTCON) += extcon.o
ifneq ($(CONFIG_EFI_PARTITION),)
obj-$(CONFIG_FASTBOOT_FLASH_MMC) += fastboot.o
endif
+obj-$(CONFIG_FW_LOADER_UCLASS) += fw_loader.o
obj-$(CONFIG_FIRMWARE) += firmware.o
obj-$(CONFIG_DM_FPGA) += fpga.o
obj-$(CONFIG_FWU_MDATA_GPT_BLK) += fwu_mdata.o
diff --git a/test/dm/fw_loader.c b/test/dm/fw_loader.c
new file mode 100644
index 000000000000..71065602cb58
--- /dev/null
+++ b/test/dm/fw_loader.c
@@ -0,0 +1,218 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include <dm.h>
+#include <dm/device.h>
+#include <dm/lists.h>
+#include <dm/test.h>
+#include <fw_loader.h>
+#include <test/test.h>
+#include <test/ut.h>
+
+static int bind_mmc(struct unit_test_state *uts, const char *mmc)
+{
+ struct udevice *dev;
+ ofnode root, node;
+
+ /* Enable the mmc12 node for this test */
+ root = oftree_root(oftree_default());
+ node = ofnode_find_subnode(root, mmc);
+ ut_assert(ofnode_valid(node));
+ ut_assertok(lists_bind_fdt(gd->dm_root, node, &dev, NULL, false));
+
+ return 0;
+}
+
+static int dm_test_fw_loader_get_dev(struct unit_test_state *uts,
+ const char *name, struct udevice **dev)
+{
+ struct udevice *mdio_dev;
+ ofnode mdio_node, phy_node;
+
+ ut_assertok(uclass_get_device_by_name(UCLASS_MDIO, "mdio-test",
+ &mdio_dev));
+
+ mdio_node = dev_ofnode(mdio_dev);
+ ut_assert(ofnode_valid(mdio_node));
+
+ phy_node = ofnode_find_subnode(mdio_node, name);
+ ut_assert(ofnode_valid(phy_node));
+
+ return get_fw_loader_from_node(phy_node, dev);
+}
+
+static int dm_test_fs_loader_get(struct unit_test_state *uts)
+{
+ struct udevice *fw_loader_dev = NULL;
+ int ret;
+
+ ut_assertok(bind_mmc(uts, "mmc12"));
+
+ ret = dm_test_fw_loader_get_dev(uts, "ethernet-phy@1", &fw_loader_dev);
+ ut_asserteq(-ENODEV, ret);
+ ut_assertnull(fw_loader_dev);
+
+ fw_loader_dev = NULL;
+ ret = dm_test_fw_loader_get_dev(uts, "ethernet-phy@2", &fw_loader_dev);
+ ut_assertok(ret);
+ ut_assertnonnull(fw_loader_dev);
+
+ return 0;
+}
+DM_TEST(dm_test_fs_loader_get, UTF_SCAN_FDT);
+
+static int dm_test_fip_loader_get(struct unit_test_state *uts)
+{
+ struct udevice *fw_loader_dev = NULL;
+ int ret;
+
+ ut_assertok(bind_mmc(uts, "mmc13"));
+
+ ret = dm_test_fw_loader_get_dev(uts, "ethernet-phy@3", &fw_loader_dev);
+ ut_assertok(ret);
+ ut_assertnonnull(fw_loader_dev);
+
+ return 0;
+}
+DM_TEST(dm_test_fip_loader_get, UTF_SCAN_FDT);
+
+static int dm_test_fs_loader_get_fw_size(struct unit_test_state *uts)
+{
+ struct udevice *fw_loader_dev = NULL;
+ int ret;
+
+ ut_assertok(bind_mmc(uts, "mmc12"));
+
+ ret = dm_test_fw_loader_get_dev(uts, "ethernet-phy@2", &fw_loader_dev);
+ ut_assertok(ret);
+ ut_assertnonnull(fw_loader_dev);
+
+ ret = request_firmware_size(fw_loader_dev, "firmware.bin");
+ ut_asserteq(256, ret);
+
+ return 0;
+}
+DM_TEST(dm_test_fs_loader_get_fw_size, UTF_SCAN_FDT);
+
+static int dm_test_fip_loader_get_fw_size(struct unit_test_state *uts)
+{
+ struct udevice *fw_loader_dev = NULL;
+ int ret;
+
+ ut_assertok(bind_mmc(uts, "mmc13"));
+
+ ret = dm_test_fw_loader_get_dev(uts, "ethernet-phy@3", &fw_loader_dev);
+ ut_assertok(ret);
+ ut_assertnonnull(fw_loader_dev);
+
+ ret = request_firmware_size(fw_loader_dev,
+ "45a113ac-c7f2-30b0-90a5-a399ab912716");
+ ut_asserteq(5, ret);
+
+ return 0;
+}
+DM_TEST(dm_test_fip_loader_get_fw_size, UTF_SCAN_FDT);
+
+static int dm_test_fs_loader_get_fw_in_buf(struct unit_test_state *uts)
+{
+ struct udevice *fw_loader_dev = NULL;
+ int firmware_size = 256;
+ char *buf;
+ int ret;
+
+ ut_assertok(bind_mmc(uts, "mmc12"));
+
+ ret = dm_test_fw_loader_get_dev(uts, "ethernet-phy@2", &fw_loader_dev);
+ ut_assertok(ret);
+ ut_assertnonnull(fw_loader_dev);
+
+ ret = request_firmware_size(fw_loader_dev, "firmware.bin");
+ ut_asserteq(firmware_size, ret);
+
+ buf = calloc(firmware_size, sizeof(*buf));
+ ut_assertnonnull(buf);
+
+ ret = request_firmware_into_buf(fw_loader_dev, "firmware.bin",
+ buf, firmware_size, 0);
+ ut_asserteq(firmware_size, ret);
+
+ ret = request_firmware_into_buf(fw_loader_dev, "firmware.bin",
+ buf, firmware_size - 10, 10);
+ ut_asserteq(firmware_size - 10, ret);
+
+ ret = request_firmware_into_buf(fw_loader_dev, "firmware.bin",
+ buf, firmware_size, firmware_size);
+ ut_asserteq(0, ret);
+
+ ret = request_firmware_into_buf(fw_loader_dev, "firmware.bin",
+ buf, firmware_size, firmware_size + 1);
+ ut_asserteq(0, ret);
+
+ ret = request_firmware_into_buf(fw_loader_dev, "firmware2.bin",
+ buf, firmware_size, 0);
+ ut_asserteq(-ENOENT, ret);
+
+ free(buf);
+ return 0;
+}
+DM_TEST(dm_test_fs_loader_get_fw_in_buf, UTF_SCAN_FDT);
+
+static int dm_test_fip_loader_get_fw_in_buf(struct unit_test_state *uts)
+{
+ struct udevice *fw_loader_dev = NULL;
+ int firmware_size = 5;
+ char *buf;
+ int ret;
+
+ ut_assertok(bind_mmc(uts, "mmc13"));
+
+ ret = dm_test_fw_loader_get_dev(uts, "ethernet-phy@3", &fw_loader_dev);
+ ut_assertok(ret);
+ ut_assertnonnull(fw_loader_dev);
+
+ /* uuidgen -n @dns --md5 --name test = 45a113ac-c7f2-30b0-90a5-a399ab912716*/
+ ret = request_firmware_size(fw_loader_dev,
+ "45a113ac-c7f2-30b0-90a5-a399ab912716");
+ ut_asserteq(firmware_size, ret);
+
+ buf = calloc(firmware_size, sizeof(*buf));
+ ut_assertnonnull(buf);
+
+ ret = request_firmware_into_buf(fw_loader_dev,
+ "45a113ac-c7f2-30b0-90a5-a399ab912716",
+ buf, firmware_size, 0);
+ ut_asserteq(firmware_size, ret);
+ ut_asserteq_mem("test\n", buf, firmware_size);
+
+ memset(buf, 0, firmware_size);
+ ret = request_firmware_into_buf(fw_loader_dev,
+ "45a113ac-c7f2-30b0-90a5-a399ab912716",
+ buf, firmware_size - 1, 0);
+ ut_asserteq(firmware_size - 1, ret);
+ ut_asserteq_mem("test", buf, firmware_size - 1);
+
+ memset(buf, 0, firmware_size);
+ ret = request_firmware_into_buf(fw_loader_dev,
+ "45a113ac-c7f2-30b0-90a5-a399ab912716",
+ buf, firmware_size - 2, 2);
+ ut_asserteq(firmware_size - 2, ret);
+ ut_asserteq_mem("st\n", buf, firmware_size - 2);
+
+ ret = request_firmware_into_buf(fw_loader_dev,
+ "45a113ac-c7f2-30b0-90a5-a399ab912716",
+ buf, firmware_size, firmware_size);
+ ut_asserteq(0, ret);
+
+ ret = request_firmware_into_buf(fw_loader_dev,
+ "45a113ac-c7f2-30b0-90a5-a399ab912716",
+ buf, firmware_size, firmware_size + 1);
+ ut_asserteq(-EINVAL, ret);
+
+ ret = request_firmware_into_buf(fw_loader_dev,
+ "300b74d6-cc34-4e68-ba5d-733157f633f7",
+ buf, firmware_size, 0);
+ ut_asserteq(-ENOENT, ret);
+
+ free(buf);
+ return 0;
+}
+DM_TEST(dm_test_fip_loader_get_fw_in_buf, UTF_SCAN_FDT);
diff --git a/test/py/tests/fip/test.fip b/test/py/tests/fip/test.fip
new file mode 100644
index 0000000000000000000000000000000000000000..a793dfd05c016b2bfe693a0b908877689abe8e5b
GIT binary patch
literal 101
zcmZQ%NLf`8W+KD@1+EK)*Bt+3uwlZ|#WPn=R2NHtinGEL;Gk1+h?k@mmv8|9cc={D
literal 0
HcmV?d00001
diff --git a/test/py/tests/test_ut.py b/test/py/tests/test_ut.py
index 2a5acc18c612..f3da9f3ef005 100644
--- a/test/py/tests/test_ut.py
+++ b/test/py/tests/test_ut.py
@@ -685,6 +685,31 @@ def test_ut_dm_init(ubman):
with open(fn, 'wb') as fh:
fh.write(data)
+def setup_fw_loader_image(ubman):
+ """Create a 2MB disk image with a dummy firmware on it"""
+ mmc_dev = 12
+ fname, mnt = fs_helper.setup_image(ubman, mmc_dev, 0xc, img_size=2)
+
+ fw = os.path.join(mnt, 'firmware.bin')
+ data = b'\xaa' * 256
+ with open(fw, 'wb') as fh:
+ fh.write(data)
+
+ fsfile = 'vfat1M.img'
+ utils.run_and_log(ubman, f'fallocate -l 1M {fsfile}')
+ utils.run_and_log(ubman, f'mkfs.vfat {fsfile}')
+ utils.run_and_log(ubman, ['sh', '-c', f'mcopy -i {fsfile} {mnt}/* ::/'])
+ copy_partition(ubman, fsfile, fname)
+ utils.run_and_log(ubman, f'rm -rf {mnt}')
+ utils.run_and_log(ubman, f'rm -f {fsfile}')
+
+ mmc_dev = 13
+ fname, mnt = fs_helper.setup_image(ubman, mmc_dev, 0x83, img_size=2)
+
+ fipfile = os.path.join(ubman.config.source_dir,
+ 'test/py/tests/fip/test.fip')
+ copy_partition(ubman, fipfile, fname)
+ utils.run_and_log(ubman, f'rm -rf {mnt}')
def setup_efi_image(ubman):
"""Create a 20MB disk image with an EFI app on it"""
@@ -752,6 +777,7 @@ def test_ut_dm_init_bootstd(ubman):
setup_cedit_file(ubman)
setup_cros_image(ubman)
setup_android_image(ubman)
+ setup_fw_loader_image(ubman)
setup_efi_image(ubman)
setup_rauc_image(ubman)
setup_bls_prefix_image(ubman)
--
2.55.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v8 7/7] test: dm: Add test for FW Loader feature
2026-10-22 19:40 ` [PATCH v8 7/7] test: dm: Add test for FW Loader feature Christian Marangi
@ 2026-10-23 13:12 ` Simon Glass
2026-10-23 16:27 ` Christian Marangi
0 siblings, 1 reply; 16+ messages in thread
From: Simon Glass @ 2026-10-23 13:12 UTC (permalink / raw)
To: ansuelsmth
Cc: Simon Glass, Tom Rini, Mattijs Korpershoek, Marek Vasut,
Heiko Schocher, Neil Armstrong, Alexey Charkov, Daniel Golle,
Aristo Chen, Lucien.Jheng, Mehmet Fide, Casey Connolly,
Michal Simek, Quentin Schulz, Johan Jonker, Anshul Dalal,
Romain Gantois, Mateus Lima Alves, Marcello Sylvester Bauer,
Tan Siewert, Peng Fan, Patrice Chotard, Yao Zi,
Heinrich Schuchardt, Martin Schwan, u-boot
Hi Christian,
On 2026-10-22T19:40:35, Christian Marangi (Ansuel) <ansuelsmth@gmail.com> wrote:
> test: dm: Add test for FW Loader feature
>
> Add basic test for FW Loader feature for the Filesystem way and FIP.
>
> Additional MMC testing image are added that loads a dummy firmware.bin in
> a fat partition or a simple FIP blob.
>
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
>
> arch/sandbox/dts/test.dts | 38 +++++++
> configs/sandbox64_defconfig | 2 +
> configs/sandbox_defconfig | 2 +
> configs/sandbox_flattree_defconfig | 2 +
> configs/sandbox_noinst_defconfig | 1 +
> configs/sandbox_spl_defconfig | 2 +
> configs/sandbox_vpl_defconfig | 2 +
> test/dm/Makefile | 1 +
> test/dm/fw_loader.c | 218 +++++++++++++++++++++++++++++++++++++
> test/py/tests/fip/test.fip | Bin 0 -> 101 bytes
> test/py/tests/test_ut.py | 26 +++++
> 11 files changed, 294 insertions(+)
> diff --git a/test/dm/fw_loader.c b/test/dm/fw_loader.c
> @@ -0,0 +1,218 @@
> + ret = request_firmware_into_buf(fw_loader_dev, "firmware.bin",
> + buf, firmware_size, firmware_size + 1);
> + ut_asserteq(0, ret);
These tests show that the two loaders behave differently when the
offset is past the end: FS returns 0 here, but FIP returns -EINVAL for
the same request (see below). Callers go through the uclass and should
not need to know which loader is behind the phandle, so please can you
pick one behaviour for both drivers? I think -EINVAL is better, and
the cover letter says FIP is meant to match FS anyway.
> diff --git a/test/dm/fw_loader.c b/test/dm/fw_loader.c
> @@ -0,0 +1,218 @@
> + ret = request_firmware_into_buf(fw_loader_dev,
> + "45a113ac-c7f2-30b0-90a5-a399ab912716",
> + buf, firmware_size, firmware_size + 1);
> + ut_asserteq(-EINVAL, ret);
This is the FIP case mentioned above.
> diff --git a/test/dm/fw_loader.c b/test/dm/fw_loader.c
> @@ -0,0 +1,218 @@
> + /* Enable the mmc12 node for this test */
> + root = oftree_root(oftree_default());
This helper is used for mmc13 too, so the comment should not name a
specific node.
> diff --git a/test/py/tests/test_ut.py b/test/py/tests/test_ut.py
> @@ -685,6 +685,31 @@ def test_ut_dm_init(ubman):
> + fipfile = os.path.join(ubman.config.source_dir,
> + 'test/py/tests/fip/test.fip')
Please can you explain in the commit message how test.fip was created
(e.g. the fiptool command line), so the next person can update it?
Better still, since the format is just a header plus one ToC entry,
you could build the 101 bytes in Python here and avoid a binary blob
in the tree. What do you think?
Binman has a atf_fip.py but you can't use that here, unfortunately.
Regards,
Simon
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v8 7/7] test: dm: Add test for FW Loader feature
2026-10-23 13:12 ` Simon Glass
@ 2026-10-23 16:27 ` Christian Marangi
0 siblings, 0 replies; 16+ messages in thread
From: Christian Marangi @ 2026-10-23 16:27 UTC (permalink / raw)
To: Simon Glass
Cc: Tom Rini, Mattijs Korpershoek, Marek Vasut, Heiko Schocher,
Neil Armstrong, Alexey Charkov, Daniel Golle, Aristo Chen,
Lucien.Jheng, Mehmet Fide, Casey Connolly, Michal Simek,
Quentin Schulz, Johan Jonker, Anshul Dalal, Romain Gantois,
Mateus Lima Alves, Marcello Sylvester Bauer, Tan Siewert,
Peng Fan, Patrice Chotard, Yao Zi, Heinrich Schuchardt,
Martin Schwan, u-boot
On Fri, Oct 23, 2026 at 06:12:38AM -0700, Simon Glass wrote:
> Hi Christian,
>
> On 2026-10-22T19:40:35, Christian Marangi (Ansuel) <ansuelsmth@gmail.com> wrote:
> > test: dm: Add test for FW Loader feature
> >
> > Add basic test for FW Loader feature for the Filesystem way and FIP.
> >
> > Additional MMC testing image are added that loads a dummy firmware.bin in
> > a fat partition or a simple FIP blob.
> >
> > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> >
> > arch/sandbox/dts/test.dts | 38 +++++++
> > configs/sandbox64_defconfig | 2 +
> > configs/sandbox_defconfig | 2 +
> > configs/sandbox_flattree_defconfig | 2 +
> > configs/sandbox_noinst_defconfig | 1 +
> > configs/sandbox_spl_defconfig | 2 +
> > configs/sandbox_vpl_defconfig | 2 +
> > test/dm/Makefile | 1 +
> > test/dm/fw_loader.c | 218 +++++++++++++++++++++++++++++++++++++
> > test/py/tests/fip/test.fip | Bin 0 -> 101 bytes
> > test/py/tests/test_ut.py | 26 +++++
> > 11 files changed, 294 insertions(+)
>
> > diff --git a/test/dm/fw_loader.c b/test/dm/fw_loader.c
> > @@ -0,0 +1,218 @@
> > + ret = request_firmware_into_buf(fw_loader_dev, "firmware.bin",
> > + buf, firmware_size, firmware_size + 1);
> > + ut_asserteq(0, ret);
>
> These tests show that the two loaders behave differently when the
> offset is past the end: FS returns 0 here, but FIP returns -EINVAL for
> the same request (see below). Callers go through the uclass and should
> not need to know which loader is behind the phandle, so please can you
> pick one behaviour for both drivers? I think -EINVAL is better, and
> the cover letter says FIP is meant to match FS anyway.
>
Yes exactly... the problem is that I don't want to cause regression in the
use of FS loader with this change of error handling.
Ok I will change FS loader to return -EINVAL as a separate commit.
> > diff --git a/test/dm/fw_loader.c b/test/dm/fw_loader.c
> > @@ -0,0 +1,218 @@
> > + ret = request_firmware_into_buf(fw_loader_dev,
> > + "45a113ac-c7f2-30b0-90a5-a399ab912716",
> > + buf, firmware_size, firmware_size + 1);
> > + ut_asserteq(-EINVAL, ret);
>
> This is the FIP case mentioned above.
>
> > diff --git a/test/dm/fw_loader.c b/test/dm/fw_loader.c
> > @@ -0,0 +1,218 @@
> > + /* Enable the mmc12 node for this test */
> > + root = oftree_root(oftree_default());
>
> This helper is used for mmc13 too, so the comment should not name a
> specific node.
>
> > diff --git a/test/py/tests/test_ut.py b/test/py/tests/test_ut.py
> > @@ -685,6 +685,31 @@ def test_ut_dm_init(ubman):
> > + fipfile = os.path.join(ubman.config.source_dir,
> > + 'test/py/tests/fip/test.fip')
>
> Please can you explain in the commit message how test.fip was created
> (e.g. the fiptool command line), so the next person can update it?
> Better still, since the format is just a header plus one ToC entry,
> you could build the 101 bytes in Python here and avoid a binary blob
> in the tree. What do you think?
Ehhhhh I checked and it's a lot of effort on creating a python shim to
simulate the format of a FIP blob. I will check it again but providing the
3kb blob was more practical than polluting the python script with something
used for just one task.
>
> Binman has a atf_fip.py but you can't use that here, unfortunately.
>
> Regards,
> Simon
--
Ansuel
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v8 0/7] misc: fs_loader: reorg and split to FS and FW loader + FIP loader
2026-10-22 19:40 [PATCH v8 0/7] misc: fs_loader: reorg and split to FS and FW loader + FIP loader Christian Marangi
` (6 preceding siblings ...)
2026-10-22 19:40 ` [PATCH v8 7/7] test: dm: Add test for FW Loader feature Christian Marangi
@ 2026-10-23 13:12 ` Simon Glass
7 siblings, 0 replies; 16+ messages in thread
From: Simon Glass @ 2026-10-23 13:12 UTC (permalink / raw)
To: ansuelsmth
Cc: Simon Glass, Tom Rini, Mattijs Korpershoek, Marek Vasut,
Heiko Schocher, Neil Armstrong, Alexey Charkov, Daniel Golle,
Aristo Chen, Lucien.Jheng, Mehmet Fide, Casey Connolly,
Michal Simek, Quentin Schulz, Johan Jonker, Anshul Dalal,
Romain Gantois, Mateus Lima Alves, Marcello Sylvester Bauer,
Tan Siewert, Peng Fan, Patrice Chotard, Yao Zi,
Heinrich Schuchardt, Martin Schwan, u-boot
Hi Christian,
On 2026-10-22T19:40:35, Christian Marangi (Ansuel) <ansuelsmth@gmail.com> wrote:
> Changes v8:
> - Address all the comments from Simon
Just to check, two series-level points from v7 still seem to be open.
The binding update (patch 6) still comes after the FIP loader (patch
5), so u-boot,fip-loader and partoffset are used before they are
documented. Please can you move the binding before the driver, or fold
it into the driver patch?
> fs_loader0: fip-loader {
The 'fs_loader0' label is still used for the fip-loader node, here and
in the commit message of patch 5, and the binding example in patch 6
has 'fs_loader4' and 'fs_loader5' for its fip-loader nodes. Now that
FS and FW are separate, please can you rename these to something like
'fw_loader0'?
Regards,
Simon
^ permalink raw reply [flat|nested] 16+ messages in thread