* [PATCH v5 1/6] misc: fs_loader: fix ubifs not unmounted on dev_get_priv error
2026-04-03 13:51 [PATCH v5 0/6] misc: fs_loader: reorg and split to FS and FW loader + FIP loader Christian Marangi
@ 2026-04-03 13:51 ` Christian Marangi
2026-04-03 13:51 ` [PATCH v5 2/6] misc: fs_loader: reorganize and split to FS and FW loader Christian Marangi
` (4 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Christian Marangi @ 2026-04-03 13:51 UTC (permalink / raw)
To: Tom Rini, Simon Glass, Christian Marangi, Casey Connolly,
Quentin Schulz, Peng Fan, Justin Klaassen, Neha Malcom Francis,
Heinrich Schuchardt, Jamie Gibbons, Leo Yu-Chi Liang,
Harsha Vardhan V M, Weijie Gao, Marek Vasut, Patrice Chotard,
Yao Zi, Alif Zakuan Yuslaimi, Lucien.Jheng, u-boot
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.53.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v5 2/6] misc: fs_loader: reorganize and split to FS and FW loader
2026-04-03 13:51 [PATCH v5 0/6] misc: fs_loader: reorg and split to FS and FW loader + FIP loader Christian Marangi
2026-04-03 13:51 ` [PATCH v5 1/6] misc: fs_loader: fix ubifs not unmounted on dev_get_priv error Christian Marangi
@ 2026-04-03 13:51 ` Christian Marangi
2026-04-06 17:10 ` Simon Glass
2026-04-03 13:52 ` [PATCH v5 3/6] misc: fw_loader: implement generic get_fw_loader_from_node() Christian Marangi
` (3 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Christian Marangi @ 2026-04-03 13:51 UTC (permalink / raw)
To: Tom Rini, Simon Glass, Christian Marangi, Casey Connolly,
Quentin Schulz, Peng Fan, Justin Klaassen, Neha Malcom Francis,
Heinrich Schuchardt, Jamie Gibbons, Leo Yu-Chi Liang,
Harsha Vardhan V M, Weijie Gao, Marek Vasut, Patrice Chotard,
Yao Zi, Alif Zakuan Yuslaimi, Lucien.Jheng, u-boot
In preparation to the introduction of variant of the FS loader,
reorganize and split the driver to generic fw_loader function and
specific fs_loader function. Create a dedicated directory for the loader
and move the internal structs and functions to a dedicated header file.
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>
---
drivers/misc/Kconfig | 5 +
drivers/misc/Makefile | 2 +-
drivers/misc/fw_loader/Makefile | 4 +
drivers/misc/{ => fw_loader}/fs_loader.c | 141 ++--------------------
drivers/misc/fw_loader/fw_loader.c | 144 +++++++++++++++++++++++
drivers/misc/fw_loader/internal.h | 61 ++++++++++
include/fs_loader.h | 47 +-------
include/fw_loader.h | 19 +++
8 files changed, 247 insertions(+), 176 deletions(-)
create mode 100644 drivers/misc/fw_loader/Makefile
rename drivers/misc/{ => fw_loader}/fs_loader.c (58%)
create mode 100644 drivers/misc/fw_loader/fw_loader.c
create mode 100644 drivers/misc/fw_loader/internal.h
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index b2dfc7f5b663..58f1a4c07ff5 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -612,8 +612,12 @@ config MPC83XX_SERDES
help
Support for serdes found on MPC83xx SoCs.
+config FW_LOADER_LIB
+ bool
+
config FS_LOADER
bool "Enable loader driver for file system"
+ select FW_LOADER_DRV
help
This is file system generic loader which can be used to load
the file image from the storage into target such as memory.
@@ -623,6 +627,7 @@ config FS_LOADER
config SPL_FS_LOADER
bool "Enable loader driver for file system in SPL"
+ select FW_LOADER_DRV
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 e2170212e5ad..b514d1351ca5 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_FW_LOADER_DRV) += fw_loader/
obj-$(CONFIG_GATEWORKS_SC) += gsc.o
obj-$(CONFIG_GDSYS_IOEP) += gdsys_ioep.o
obj-$(CONFIG_GDSYS_RXAUI_CTRL) += gdsys_rxaui_ctrl.o
diff --git a/drivers/misc/fw_loader/Makefile b/drivers/misc/fw_loader/Makefile
new file mode 100644
index 000000000000..96baebede788
--- /dev/null
+++ b/drivers/misc/fw_loader/Makefile
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0+
+
+obj-y += fw_loader.o
+obj-$(CONFIG_$(PHASE_)FS_LOADER) += fs_loader.o
diff --git a/drivers/misc/fs_loader.c b/drivers/misc/fw_loader/fs_loader.c
similarity index 58%
rename from drivers/misc/fs_loader.c
rename to drivers/misc/fw_loader/fs_loader.c
index cd4695b08eea..2748d9f041e8 100644
--- a/drivers/misc/fs_loader.c
+++ b/drivers/misc/fw_loader/fs_loader.c
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-2.0
- /*
+/*
* Copyright (C) 2018-2019 Intel Corporation <www.intel.com>
*
*/
@@ -24,33 +24,16 @@
#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;
-};
+#include "internal.h"
#ifdef CONFIG_CMD_UBIFS
static int mount_ubifs(char *mtdpart, char *ubivol)
{
- int ret = ubi_part(mtdpart, NULL);
+ int ret;
- if (ret) {
- debug("Cannot find mtd partition %s\n", mtdpart);
+ ret = generic_fw_loader_ubi_select(mtdpart);
+ if (ret)
return ret;
- }
return cmd_ubifs_mount(ubivol);
}
@@ -106,37 +89,6 @@ static int select_fs_dev(struct device_plat *plat)
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.
@@ -197,87 +149,16 @@ out:
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);
+ int ret;
- 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);
+ ret = generic_fw_loader_probe(dev);
+ if (ret)
+ return ret;
- return ret;
- }
- }
- }
-#endif
+ plat->get_firmware = fw_get_filesystem_firmware;
return 0;
};
@@ -292,7 +173,7 @@ U_BOOT_DRIVER(fs_loader) = {
.id = UCLASS_FS_FIRMWARE_LOADER,
.of_match = fs_loader_ids,
.probe = fs_loader_probe,
- .of_to_plat = fs_loader_of_to_plat,
+ .of_to_plat = generic_fw_loader_of_to_plat,
.plat_auto = sizeof(struct device_plat),
.priv_auto = sizeof(struct firmware),
};
diff --git a/drivers/misc/fw_loader/fw_loader.c b/drivers/misc/fw_loader/fw_loader.c
new file mode 100644
index 000000000000..644b98de9f6c
--- /dev/null
+++ b/drivers/misc/fw_loader/fw_loader.c
@@ -0,0 +1,144 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018-2019 Intel Corporation <www.intel.com>
+ *
+ */
+
+#include <errno.h>
+#include <blk.h>
+#include <linux/types.h>
+#include <dm/device.h>
+#include <fw_loader.h>
+
+#ifdef CONFIG_CMD_UBIFS
+#include <ubi_uboot.h>
+#endif
+
+#include "internal.h"
+
+#ifdef CONFIG_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
+
+int generic_fw_loader_of_to_plat(struct udevice *dev)
+{
+ u32 phandlepart[2];
+
+ ofnode fw_loader_node = dev_ofnode(dev);
+
+ if (ofnode_valid(fw_loader_node)) {
+ struct device_plat *plat;
+
+ plat = dev_get_plat(dev);
+ 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");
+ }
+
+ return 0;
+}
+
+int generic_fw_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 *blk_dev;
+
+ ret = blk_get_from_parent(parent_dev, &blk_dev);
+ if (ret) {
+ debug("fw_loader: No block device: %d\n",
+ ret);
+
+ return ret;
+ }
+ }
+ }
+#endif
+
+ return 0;
+}
+
+/**
+ * _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;
+}
+
+int request_firmware_into_buf(struct udevice *dev,
+ const char *name,
+ void *buf, size_t size, u32 offset)
+{
+ struct device_plat *plat;
+ int ret;
+
+ if (!dev)
+ return -EINVAL;
+
+ ret = _request_firmware_prepare(dev, name, buf, size, offset);
+ if (ret < 0) /* error */
+ return ret;
+
+ plat = dev_get_plat(dev);
+
+ if (!plat->get_firmware)
+ return -EOPNOTSUPP;
+
+ return plat->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..fa006b7e6077
--- /dev/null
+++ b/drivers/misc/fw_loader/internal.h
@@ -0,0 +1,61 @@
+/* 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 device_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 device_plat {
+ struct phandle_part phandlepart;
+ char *mtdpart;
+ char *ubivol;
+
+ int (*get_firmware)(struct udevice *dev);
+};
+
+/**
+ * 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;
+};
+
+int generic_fw_loader_ubi_select(char *mtdpart);
+int generic_fw_loader_probe(struct udevice *dev);
+int generic_fw_loader_of_to_plat(struct udevice *dev);
+
+#endif
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.53.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v5 2/6] misc: fs_loader: reorganize and split to FS and FW loader
2026-04-03 13:51 ` [PATCH v5 2/6] misc: fs_loader: reorganize and split to FS and FW loader Christian Marangi
@ 2026-04-06 17:10 ` Simon Glass
0 siblings, 0 replies; 13+ messages in thread
From: Simon Glass @ 2026-04-06 17:10 UTC (permalink / raw)
To: ansuelsmth
Cc: Tom Rini, Simon Glass, Casey Connolly, Quentin Schulz, Peng Fan,
Justin Klaassen, Neha Malcom Francis, Heinrich Schuchardt,
Jamie Gibbons, Leo Yu-Chi Liang, Harsha Vardhan V M, Weijie Gao,
Marek Vasut, Patrice Chotard, Yao Zi, Alif Zakuan Yuslaimi,
Lucien.Jheng, u-boot
Hi Christian,
On 2026-04-03T13:51:57, Christian Marangi <ansuelsmth@gmail.com> wrote:
> misc: fs_loader: reorganize and split to FS and FW loader
>
> In preparation to the introduction of variant of the FS loader,
> reorganize and split the driver to generic fw_loader function and
> specific fs_loader function. Create a dedicated directory for the loader
> and move the internal structs and functions to a dedicated header file.
>
> 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>
> +struct device_plat {
> + struct phandle_part phandlepart;
> + char *mtdpart;
> + char *ubivol;
> +
> + int (*get_firmware)(struct udevice *dev);
> +};
This series introduces a generic firmware-loader framework with two
implementations (FS and FIP), which is a good idea. But the dispatch
mechanism uses function pointers in platform data rather than U-Boot's
driver model ops.
In U-Boot's driver model, polymorphic behaviour is handled by uclass
operations. Instead of storing function pointers in device_plat and
setting them during probe, please can you define a proper ops
structure:
struct fw_loader_ops {
int (*get_firmware)(struct udevice *dev);
int (*get_size)(struct udevice *dev);
};
Each driver would then declare its ops:
static const struct fw_loader_ops fs_loader_ops = {
.get_firmware = fw_get_filesystem_firmware,
.get_size = fw_get_filesystem_firmware_size,
};
And fw_loader.c would use dev_get_uclass_ops() to dispatch. This also
means you only need a single UCLASS_FIRMWARE_LOADER rather than
separate UCLASS_FS_FIRMWARE_LOADER and UCLASS_FIP_FIRMWARE_LOADER -
the ops handle the polymorphism, which is the whole point of having a
uclass.
See drivers/spi/spi-uclass.c or drivers/i2c/i2c-uclass.c for examples
of this pattern.
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> @@ -612,8 +612,12 @@ config MPC83XX_SERDES
> +config FW_LOADER_LIB
> + bool
> +
> config FS_LOADER
> bool "Enable loader driver for file system"
> + select FW_LOADER_DRV
FW_LOADER_LIB is defined but never used. FW_LOADER_DRV is selected but
never defined. I suspect you meant to define FW_LOADER_DRV here
instead of FW_LOADER_LIB.
> diff --git a/drivers/misc/fw_loader/fw_loader.c b/drivers/misc/fw_loader/fw_loader.c
> @@ -0,0 +1,144 @@
> +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);
Please can you move the variable declaration to the top of the
function, before the statements.
> diff --git a/drivers/misc/fw_loader/internal.h b/drivers/misc/fw_loader/internal.h
> @@ -0,0 +1,61 @@
> +/**
> + * 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;
> +};
The kernel-doc mentions @priv but there is no priv member in the struct.
Regards,
Simon
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v5 3/6] misc: fw_loader: implement generic get_fw_loader_from_node()
2026-04-03 13:51 [PATCH v5 0/6] misc: fs_loader: reorg and split to FS and FW loader + FIP loader Christian Marangi
2026-04-03 13:51 ` [PATCH v5 1/6] misc: fs_loader: fix ubifs not unmounted on dev_get_priv error Christian Marangi
2026-04-03 13:51 ` [PATCH v5 2/6] misc: fs_loader: reorganize and split to FS and FW loader Christian Marangi
@ 2026-04-03 13:52 ` Christian Marangi
2026-04-06 17:10 ` Simon Glass
2026-04-03 13:52 ` [PATCH v5 4/6] misc: fw_loader: implement request_firmware_size() OP Christian Marangi
` (2 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Christian Marangi @ 2026-04-03 13:52 UTC (permalink / raw)
To: Tom Rini, Simon Glass, Christian Marangi, Casey Connolly,
Quentin Schulz, Peng Fan, Justin Klaassen, Neha Malcom Francis,
Heinrich Schuchardt, Jamie Gibbons, Leo Yu-Chi Liang,
Harsha Vardhan V M, Weijie Gao, Marek Vasut, Patrice Chotard,
Yao Zi, Alif Zakuan Yuslaimi, Lucien.Jheng, 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.
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
drivers/misc/fw_loader/fw_loader.c | 32 ++++++++++++++++++++++++++++++
include/fw_loader.h | 15 ++++++++++++++
2 files changed, 47 insertions(+)
diff --git a/drivers/misc/fw_loader/fw_loader.c b/drivers/misc/fw_loader/fw_loader.c
index 644b98de9f6c..c3d0de1b93e7 100644
--- a/drivers/misc/fw_loader/fw_loader.c
+++ b/drivers/misc/fw_loader/fw_loader.c
@@ -8,6 +8,7 @@
#include <blk.h>
#include <linux/types.h>
#include <dm/device.h>
+#include <dm/uclass.h>
#include <fw_loader.h>
#ifdef CONFIG_CMD_UBIFS
@@ -90,6 +91,37 @@ int generic_fw_loader_probe(struct udevice *dev)
return 0;
}
+/**
+ * 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)
+{
+ struct udevice *fw_dev;
+ int i, ret;
+
+ node = ofnode_parse_phandle(node, "firmware-loader", 0);
+ if (!ofnode_valid(node))
+ return -ENODEV;
+
+ ret = device_find_global_by_ofnode(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..7053c7bc6f05 100644
--- a/include/fw_loader.h
+++ b/include/fw_loader.h
@@ -6,8 +6,23 @@
#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.
+ *
+ * Loop over all the supported FW loader and find a matching
+ * one.
+ *
+ * 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.53.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v5 3/6] misc: fw_loader: implement generic get_fw_loader_from_node()
2026-04-03 13:52 ` [PATCH v5 3/6] misc: fw_loader: implement generic get_fw_loader_from_node() Christian Marangi
@ 2026-04-06 17:10 ` Simon Glass
0 siblings, 0 replies; 13+ messages in thread
From: Simon Glass @ 2026-04-06 17:10 UTC (permalink / raw)
To: ansuelsmth
Cc: Tom Rini, Simon Glass, Casey Connolly, Quentin Schulz, Peng Fan,
Justin Klaassen, Neha Malcom Francis, Heinrich Schuchardt,
Jamie Gibbons, Leo Yu-Chi Liang, Harsha Vardhan V M, Weijie Gao,
Marek Vasut, Patrice Chotard, Yao Zi, Alif Zakuan Yuslaimi,
Lucien.Jheng, u-boot
Hi Christian,
On 2026-04-03T13:51:57, Christian Marangi <ansuelsmth@gmail.com> wrote:
> misc: fw_loader: implement generic get_fw_loader_from_node()
>
> 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.
>
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> diff --git a/include/fw_loader.h b/include/fw_loader.h
> @@ -6,8 +6,23 @@
> + * Loop over all the supported FW loader and find a matching
> + * one.
Is there a compatible string for this node? Is it in the DT schema?
The comment says it loops over all supported FW loaders but the
implementation does a direct lookup by ofnode with
device_find_global_by_ofnode(). Please can you update the comment to
match.
Regards,
Simon
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v5 4/6] misc: fw_loader: implement request_firmware_size() OP
2026-04-03 13:51 [PATCH v5 0/6] misc: fs_loader: reorg and split to FS and FW loader + FIP loader Christian Marangi
` (2 preceding siblings ...)
2026-04-03 13:52 ` [PATCH v5 3/6] misc: fw_loader: implement generic get_fw_loader_from_node() Christian Marangi
@ 2026-04-03 13:52 ` Christian Marangi
2026-04-06 17:13 ` Simon Glass
2026-04-03 13:52 ` [PATCH v5 5/6] misc: fw_loader: introduce FIP loader driver Christian Marangi
2026-04-03 13:52 ` [PATCH v5 6/6] doc: dtbinding: Update documentation for Generic Firmware loader Christian Marangi
5 siblings, 1 reply; 13+ messages in thread
From: Christian Marangi @ 2026-04-03 13:52 UTC (permalink / raw)
To: Tom Rini, Simon Glass, Christian Marangi, Casey Connolly,
Quentin Schulz, Peng Fan, Justin Klaassen, Neha Malcom Francis,
Heinrich Schuchardt, Jamie Gibbons, Leo Yu-Chi Liang,
Harsha Vardhan V M, Weijie Gao, Marek Vasut, Patrice Chotard,
Yao Zi, Alif Zakuan Yuslaimi, Lucien.Jheng, 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.
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
drivers/misc/fw_loader/fs_loader.c | 72 +++++++++++++++++++++++++-----
drivers/misc/fw_loader/fw_loader.c | 20 +++++++++
drivers/misc/fw_loader/internal.h | 1 +
include/fw_loader.h | 9 ++++
4 files changed, 91 insertions(+), 11 deletions(-)
diff --git a/drivers/misc/fw_loader/fs_loader.c b/drivers/misc/fw_loader/fs_loader.c
index 2748d9f041e8..87e7840686f1 100644
--- a/drivers/misc/fw_loader/fs_loader.c
+++ b/drivers/misc/fw_loader/fs_loader.c
@@ -89,15 +89,8 @@ static int select_fs_dev(struct device_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_firmware_prepare(struct udevice *dev)
{
- loff_t actread = 0;
char *storage_interface, *dev_part, *ubi_mtdpart, *ubi_volume;
int ret;
@@ -122,6 +115,28 @@ static int fw_get_filesystem_firmware(struct udevice *dev)
ret = select_fs_dev(dev_get_plat(dev));
}
+ return ret;
+}
+
+static void __fw_get_filesystem_firmware_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)
+{
+ loff_t actread = 0;
+ int ret;
+
+ ret = __fw_get_filesystem_firmware_prepare(dev);
if (ret)
goto out;
@@ -143,9 +158,43 @@ static int fw_get_filesystem_firmware(struct udevice *dev)
}
out:
-#ifdef CONFIG_CMD_UBIFS
- umount_ubifs();
-#endif
+ __fw_get_filesystem_firmware_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 *firmwarep;
+ loff_t size = 0;
+ int ret;
+
+ ret = __fw_get_filesystem_firmware_prepare(dev);
+ if (ret)
+ goto out;
+
+ firmwarep = dev_get_priv(dev);
+ if (!firmwarep) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ ret = fs_size(firmwarep->name, &size);
+ if (ret) {
+ debug("Error: %d Failed to get size for %s.\n",
+ ret, firmwarep->name);
+ goto out;
+ }
+
+ ret = size;
+
+out:
+ __fw_get_filesystem_firmware_release(dev);
return ret;
}
@@ -159,6 +208,7 @@ static int fs_loader_probe(struct udevice *dev)
return ret;
plat->get_firmware = fw_get_filesystem_firmware;
+ plat->get_size = fw_get_filesystem_firmware_size;
return 0;
};
diff --git a/drivers/misc/fw_loader/fw_loader.c b/drivers/misc/fw_loader/fw_loader.c
index c3d0de1b93e7..06bd6ab9ab12 100644
--- a/drivers/misc/fw_loader/fw_loader.c
+++ b/drivers/misc/fw_loader/fw_loader.c
@@ -174,3 +174,23 @@ int request_firmware_into_buf(struct udevice *dev,
return plat->get_firmware(dev);
}
+
+int request_firmware_size(struct udevice *dev, const char *name)
+{
+ struct device_plat *plat;
+ int ret;
+
+ if (!dev)
+ return -EINVAL;
+
+ ret = _request_firmware_prepare(dev, name, NULL, 0, 0);
+ if (ret < 0) /* error */
+ return ret;
+
+ plat = dev_get_plat(dev);
+
+ if (!plat->get_size)
+ return -EOPNOTSUPP;
+
+ return plat->get_size(dev);
+}
diff --git a/drivers/misc/fw_loader/internal.h b/drivers/misc/fw_loader/internal.h
index fa006b7e6077..a94de609d0b6 100644
--- a/drivers/misc/fw_loader/internal.h
+++ b/drivers/misc/fw_loader/internal.h
@@ -34,6 +34,7 @@ struct device_plat {
char *ubivol;
int (*get_firmware)(struct udevice *dev);
+ int (*get_size)(struct udevice *dev);
};
/**
diff --git a/include/fw_loader.h b/include/fw_loader.h
index 7053c7bc6f05..1af3db385161 100644
--- a/include/fw_loader.h
+++ b/include/fw_loader.h
@@ -39,6 +39,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.53.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v5 4/6] misc: fw_loader: implement request_firmware_size() OP
2026-04-03 13:52 ` [PATCH v5 4/6] misc: fw_loader: implement request_firmware_size() OP Christian Marangi
@ 2026-04-06 17:13 ` Simon Glass
0 siblings, 0 replies; 13+ messages in thread
From: Simon Glass @ 2026-04-06 17:13 UTC (permalink / raw)
To: ansuelsmth
Cc: Tom Rini, Simon Glass, Casey Connolly, Quentin Schulz, Peng Fan,
Justin Klaassen, Neha Malcom Francis, Heinrich Schuchardt,
Jamie Gibbons, Leo Yu-Chi Liang, Harsha Vardhan V M, Weijie Gao,
Marek Vasut, Patrice Chotard, Yao Zi, Alif Zakuan Yuslaimi,
Lucien.Jheng, u-boot
Hi Christian,
On 2026-04-03T13:51:57, Christian Marangi <ansuelsmth@gmail.com> wrote:
> misc: fw_loader: implement request_firmware_size() OP
>
> 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.
>
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> diff --git a/drivers/misc/fw_loader/fs_loader.c b/drivers/misc/fw_loader/fs_loader.c
> @@ -122,6 +115,28 @@ static int fw_get_filesystem_firmware(struct udevice *dev)
> +static int __fw_get_filesystem_firmware_prepare(struct udevice *dev)
Please can you avoid double-underscore prefix for static functions?
Something like fw_fs_prepare() would be cleaner.
> diff --git a/drivers/misc/fw_loader/fs_loader.c b/drivers/misc/fw_loader/fs_loader.c
> @@ -143,9 +158,43 @@ static int fw_get_filesystem_firmware(struct udevice *dev)
> + firmwarep = dev_get_priv(dev);
> + if (!firmwarep) {
> + ret = -ENOMEM;
> + goto out;
> + }
This cannot happen, since driver model ensures the priv data exists.
Also how about using 'priv' as the variable name? We normally use a
'p' suffix to indicate a parameter which returns a value.
> diff --git a/drivers/misc/fw_loader/fw_loader.c b/drivers/misc/fw_loader/fw_loader.c
> @@ -174,3 +174,23 @@ int request_firmware_into_buf(struct udevice *dev,
> +int request_firmware_size(struct udevice *dev, const char *name)
> +{
The function needs a kerneldoc comment in the implementation.
Regards,
Simon
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v5 5/6] misc: fw_loader: introduce FIP loader driver
2026-04-03 13:51 [PATCH v5 0/6] misc: fs_loader: reorg and split to FS and FW loader + FIP loader Christian Marangi
` (3 preceding siblings ...)
2026-04-03 13:52 ` [PATCH v5 4/6] misc: fw_loader: implement request_firmware_size() OP Christian Marangi
@ 2026-04-03 13:52 ` Christian Marangi
2026-04-06 17:14 ` Simon Glass
2026-04-03 13:52 ` [PATCH v5 6/6] doc: dtbinding: Update documentation for Generic Firmware loader Christian Marangi
5 siblings, 1 reply; 13+ messages in thread
From: Christian Marangi @ 2026-04-03 13:52 UTC (permalink / raw)
To: Tom Rini, Simon Glass, Christian Marangi, Casey Connolly,
Quentin Schulz, Peng Fan, Justin Klaassen, Neha Malcom Francis,
Heinrich Schuchardt, Jamie Gibbons, Leo Yu-Chi Liang,
Harsha Vardhan V M, Weijie Gao, Marek Vasut, Patrice Chotard,
Yao Zi, Alif Zakuan Yuslaimi, Lucien.Jheng, 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 | 11 +
drivers/misc/fw_loader/Makefile | 1 +
drivers/misc/fw_loader/fip_loader.c | 578 ++++++++++++++++++++++++++++
drivers/misc/fw_loader/internal.h | 2 +
include/dm/uclass-id.h | 3 +-
5 files changed, 594 insertions(+), 1 deletion(-)
create mode 100644 drivers/misc/fw_loader/fip_loader.c
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 58f1a4c07ff5..d56513b71cbb 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -615,6 +615,17 @@ config MPC83XX_SERDES
config FW_LOADER_LIB
bool
+config FIP_LOADER
+ bool "Enable loader driver from FIP partition"
+ select LIB_UUID
+ select FW_LOADER_DRV
+ 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_DRV
diff --git a/drivers/misc/fw_loader/Makefile b/drivers/misc/fw_loader/Makefile
index 96baebede788..7854b64148e6 100644
--- a/drivers/misc/fw_loader/Makefile
+++ b/drivers/misc/fw_loader/Makefile
@@ -1,4 +1,5 @@
# SPDX-License-Identifier: GPL-2.0+
obj-y += fw_loader.o
+obj-$(CONFIG_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..dac471d72af9
--- /dev/null
+++ b/drivers/misc/fw_loader/fip_loader.c
@@ -0,0 +1,578 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2025 Christian Marangi <ansuelsmth@gmail.com>
+ *
+ */
+
+#define LOG_CATEGORY UCLASS_FIP_FIRMWARE_LOADER
+
+#include <dm.h>
+#include <div64.h>
+#include <env.h>
+#include <errno.h>
+#include <blk.h>
+#include <fs.h>
+#include <linux/kernel.h>
+#include <log.h>
+#include <mapmem.h>
+#include <malloc.h>
+#include <memalign.h>
+#include <part.h>
+#include <u-boot/uuid.h>
+
+#include "internal.h"
+
+#define TOC_HEADER_NAME 0xaa640001
+
+struct fip_toc_header {
+ u32 name;
+ u32 serial_number;
+ u64 flags;
+};
+
+struct fip_toc_entry {
+ struct uuid uuid;
+ u64 offset_address;
+ u64 size;
+ u64 flags;
+};
+
+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, u32 offset,
+ char *buf, struct fip_toc_header *hdr)
+{
+ unsigned int blkcnt = BLOCK_CNT(sizeof(*hdr), desc);
+ unsigned long to_read;
+ size_t read = 0;
+ int i, ret;
+
+ for (i = 0; i < blkcnt && read < sizeof(*hdr); i++) {
+ to_read = min(desc->blksz,
+ (unsigned long)(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, u32 offset,
+ int pos, char *buf,
+ struct fip_toc_entry *ent)
+{
+ unsigned long left, consumed, to_read, read = 0;
+ unsigned int blkstart, blkcnt;
+ int i, ret;
+
+ consumed = pos % desc->blksz;
+ left = desc->blksz - consumed;
+ to_read = min(left, (unsigned long)sizeof(*ent));
+
+ blkstart = BLOCK_CNT(pos, desc);
+ blkcnt = BLOCK_CNT(sizeof(*ent) - to_read, desc);
+
+ /* Read data from previous cached block if present */
+ if (left) {
+ memcpy(ent, buf + consumed, to_read);
+ read += to_read;
+ }
+
+ for (i = 0; i < blkcnt && read < sizeof(*ent); i++) {
+ to_read = min(desc->blksz,
+ (unsigned long)(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)
+{
+ unsigned int 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;
+}
+
+#ifdef CONFIG_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);
+#ifdef CONFIG_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;
+ size_t size = ent->size;
+ unsigned int blkcnt;
+ char *read_buf;
+ int i, ret;
+
+ read_buf = malloc(desc->blksz);
+ if (!read_buf)
+ return -ENOMEM;
+
+ blkcnt = BLOCK_CNT(size + firmwarep->offset, 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;
+}
+
+#ifdef CONFIG_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 = ent->size;
+ int ret;
+
+ ret = ubi_volume_read(ubi_vol,
+ (u8 *)firmwarep->data,
+ ent->offset_address + offset,
+ size - offset);
+ 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);
+#ifdef CONFIG_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)
+{
+ char *storage_interface, *dev_part, *ubi_mtdpart, *ubi_volume;
+ struct device_plat *plat = dev_get_plat(dev);
+ 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");
+ info->part_offset = env_get_hex("fw_partoffset", 0);
+
+ if (storage_interface && dev_part) {
+ int part;
+
+ part = part_get_info_by_dev_and_name_or_num(storage_interface,
+ dev_part,
+ &info->desc,
+ &info->part_info, 1);
+ if (part < 0)
+ return part;
+
+ info->storage_interface = FIP_STORAGE_INTERFACE_BLK;
+
+ return 0;
+ }
+
+ if (storage_interface && ubi_mtdpart && ubi_volume) {
+ if (strcmp("ubi", storage_interface))
+ return -ENODEV;
+
+ ret = generic_fw_loader_ubi_select(ubi_mtdpart);
+ if (ret)
+ return ret;
+
+ info->ubi_volume = ubi_volume;
+ info->storage_interface = FIP_STORAGE_INTERFACE_UBI;
+
+ return 0;
+ }
+
+ info->part_offset = plat->partoffset;
+
+ if (plat->phandlepart.phandle) {
+ struct udevice *disk_dev;
+ ofnode node;
+ int part;
+
+ node = ofnode_get_by_phandle(plat->phandlepart.phandle);
+
+ ret = device_get_global_by_ofnode(node, &disk_dev);
+ if (ret)
+ return ret;
+
+ info->desc = blk_get_by_device(disk_dev);
+ if (!info->desc)
+ return -ENODEV;
+
+ 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_toc_entry ent;
+ struct fip_storage_info info = { };
+ int ret;
+
+ ret = fw_parse_storage_info(dev, &info);
+ if (ret)
+ return ret;
+
+ struct firmware *firmwarep = dev_get_priv(dev);
+
+ if (!firmwarep)
+ return -EINVAL;
+
+ ret = parse_fip_firmware(firmwarep, &info, &ent);
+ if (ret)
+ return ret;
+
+ if (ent.size + firmwarep->offset > firmwarep->size) {
+ log_err("Not enough space to read firmware\n");
+ return -ENOMEM;
+ }
+
+ 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_toc_entry ent;
+ struct fip_storage_info info = { };
+ int ret;
+
+ ret = fw_parse_storage_info(dev, &info);
+ if (ret)
+ return ret;
+
+ struct firmware *firmwarep = dev_get_priv(dev);
+
+ if (!firmwarep)
+ return -EINVAL;
+
+ ret = parse_fip_firmware(firmwarep, &info, &ent);
+ if (ret)
+ return ret;
+
+ return ent.size;
+}
+
+static int fip_loader_probe(struct udevice *dev)
+{
+ struct device_plat *plat = dev_get_plat(dev);
+ int ret;
+
+ ret = generic_fw_loader_probe(dev);
+ if (ret)
+ return ret;
+
+ plat->get_firmware = fw_get_fip_firmware;
+ plat->get_size = fw_get_fip_firmware_size;
+
+ return 0;
+};
+
+static int fip_loader_of_to_plat(struct udevice *dev)
+{
+ struct device_plat *plat = dev_get_plat(dev);
+ ofnode fip_loader_node = dev_ofnode(dev);
+ int ret;
+
+ ret = generic_fw_loader_of_to_plat(dev);
+ if (ret)
+ return ret;
+
+ /* Node validation is already done by the generic function */
+ ofnode_read_u32(fip_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_FIP_FIRMWARE_LOADER,
+ .of_match = fip_loader_ids,
+ .probe = fip_loader_probe,
+ .of_to_plat = fip_loader_of_to_plat,
+ .plat_auto = sizeof(struct device_plat),
+ .priv_auto = sizeof(struct firmware),
+};
+
+UCLASS_DRIVER(fip_loader) = {
+ .id = UCLASS_FIP_FIRMWARE_LOADER,
+ .name = "fip-loader",
+};
diff --git a/drivers/misc/fw_loader/internal.h b/drivers/misc/fw_loader/internal.h
index a94de609d0b6..d823d56e53f5 100644
--- a/drivers/misc/fw_loader/internal.h
+++ b/drivers/misc/fw_loader/internal.h
@@ -25,11 +25,13 @@ struct phandle_part {
* 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 device_plat {
struct phandle_part phandlepart;
+ int partoffset;
char *mtdpart;
char *ubivol;
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
index 36b5d87c304f..641d8e1e42fa 100644
--- a/include/dm/uclass-id.h
+++ b/include/dm/uclass-id.h
@@ -69,7 +69,8 @@ enum uclass_id {
UCLASS_FIRMWARE, /* Firmware */
UCLASS_FPGA, /* FPGA device */
UCLASS_FUZZING_ENGINE, /* Fuzzing engine */
- UCLASS_FS_FIRMWARE_LOADER, /* Generic loader */
+ UCLASS_FIP_FIRMWARE_LOADER, /* FIP image loader */
+ UCLASS_FS_FIRMWARE_LOADER, /* Generic loader */
UCLASS_FWU_MDATA, /* FWU Metadata Access */
UCLASS_GPIO, /* Bank of general-purpose I/O pins */
UCLASS_HASH, /* Hash device */
--
2.53.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v5 5/6] misc: fw_loader: introduce FIP loader driver
2026-04-03 13:52 ` [PATCH v5 5/6] misc: fw_loader: introduce FIP loader driver Christian Marangi
@ 2026-04-06 17:14 ` Simon Glass
2026-04-09 13:36 ` Christian Marangi
0 siblings, 1 reply; 13+ messages in thread
From: Simon Glass @ 2026-04-06 17:14 UTC (permalink / raw)
To: ansuelsmth
Cc: Tom Rini, Simon Glass, Casey Connolly, Quentin Schulz, Peng Fan,
Justin Klaassen, Neha Malcom Francis, Heinrich Schuchardt,
Jamie Gibbons, Leo Yu-Chi Liang, Harsha Vardhan V M, Weijie Gao,
Marek Vasut, Patrice Chotard, Yao Zi, Alif Zakuan Yuslaimi,
Lucien.Jheng, u-boot
Hi Christian,
On 2026-04-03T13:51:57, Christian Marangi <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:
>
> [...]
> diff --git a/drivers/misc/fw_loader/fip_loader.c b/drivers/misc/fw_loader/fip_loader.c
> @@ -0,0 +1,578 @@
> +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)
> +{
> + ...
> + size_t size = ent->size;
> + ...
> + blkcnt = BLOCK_CNT(size + firmwarep->offset, desc);
> + blkstart = ent->offset_address + firmwarep->offset;
> + ...
> + if (pos) {
> + to_read = min(desc->blksz - pos, (unsigned long)size);
> + ...
> + }
> +
> + /* Consume all the remaining block */
> + for (i = 0; i < blkcnt && read < size; i++) {
> + to_read = min(desc->blksz, (unsigned long)(size - read));
This reads the full ent->size bytes but starts at ent->offset_address
+ firmwarep->offset. Compare with ubi_read_fip_firmware() which
correctly reads (size - offset) bytes. The blk variant should also
adjust size by firmwarep->offset so the behaviour is consistent.
> diff --git a/drivers/misc/fw_loader/fip_loader.c b/drivers/misc/fw_loader/fip_loader.c
> @@ -0,0 +1,578 @@
> +static int fw_get_fip_firmware_size(struct udevice *dev)
> +{
> + ...
> + return ent.size;
> +}
ent.size is u64 but the function returns int. This could truncate
large sizes. The same issue exists in fw_get_fip_firmware(). How about
using ulong which is the common type in U-Boot?
> diff --git a/drivers/misc/fw_loader/fip_loader.c b/drivers/misc/fw_loader/fip_loader.c
> @@ -0,0 +1,578 @@
> +static int fw_get_fip_firmware(struct udevice *dev)
> +{
> + ...
> + ret = fw_parse_storage_info(dev, &info);
> + if (ret)
> + return ret;
> +
> + struct firmware *firmwarep = dev_get_priv(dev);
Please can you move the firmwarep declaration to the top of the block.
The same pattern appears in fw_get_fip_firmware_size().
> diff --git a/drivers/misc/fw_loader/fip_loader.c b/drivers/misc/fw_loader/fip_loader.c
> @@ -0,0 +1,578 @@
> + if (ent.size + firmwarep->offset > firmwarep->size) {
> + log_err("Not enough space to read firmware\n");
> + return -ENOMEM;
> + }
I suspect this check is wrong. If firmwarep->offset is an offset into
the FIP entry, you likely want to verify that (ent.size -
firmwarep->offset) fits in the buffer, not (ent.size +
firmwarep->offset).
Also -NOSPC might be better, since we are not actually out of memory.
When people see -ENOMEM they tend to want to increase the malloc size.
> diff --git a/drivers/misc/fw_loader/internal.h b/drivers/misc/fw_loader/internal.h
> @@ -25,11 +25,13 @@ struct phandle_part {
> + int partoffset;
partoffset is declared as int but used as unsigned int in
fip_storage_info and read from DT with ofnode_read_u32(). Please can
you change this to u32 for consistency.
Regards,
Simon
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v5 5/6] misc: fw_loader: introduce FIP loader driver
2026-04-06 17:14 ` Simon Glass
@ 2026-04-09 13:36 ` Christian Marangi
0 siblings, 0 replies; 13+ messages in thread
From: Christian Marangi @ 2026-04-09 13:36 UTC (permalink / raw)
To: Simon Glass
Cc: Tom Rini, Casey Connolly, Quentin Schulz, Peng Fan,
Justin Klaassen, Neha Malcom Francis, Heinrich Schuchardt,
Jamie Gibbons, Leo Yu-Chi Liang, Harsha Vardhan V M, Weijie Gao,
Marek Vasut, Patrice Chotard, Yao Zi, Alif Zakuan Yuslaimi,
Lucien.Jheng, u-boot
On Mon, Apr 06, 2026 at 11:14:43AM -0600, Simon Glass wrote:
> Hi Christian,
>
> On 2026-04-03T13:51:57, Christian Marangi <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:
> >
> > [...]
>
> > diff --git a/drivers/misc/fw_loader/fip_loader.c b/drivers/misc/fw_loader/fip_loader.c
> > @@ -0,0 +1,578 @@
> > +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)
> > +{
> > + ...
> > + size_t size = ent->size;
> > + ...
> > + blkcnt = BLOCK_CNT(size + firmwarep->offset, desc);
> > + blkstart = ent->offset_address + firmwarep->offset;
> > + ...
> > + if (pos) {
> > + to_read = min(desc->blksz - pos, (unsigned long)size);
> > + ...
> > + }
> > +
> > + /* Consume all the remaining block */
> > + for (i = 0; i < blkcnt && read < size; i++) {
> > + to_read = min(desc->blksz, (unsigned long)(size - read));
>
> This reads the full ent->size bytes but starts at ent->offset_address
> + firmwarep->offset. Compare with ubi_read_fip_firmware() which
> correctly reads (size - offset) bytes. The blk variant should also
> adjust size by firmwarep->offset so the behaviour is consistent.
>
> > diff --git a/drivers/misc/fw_loader/fip_loader.c b/drivers/misc/fw_loader/fip_loader.c
> > @@ -0,0 +1,578 @@
> > +static int fw_get_fip_firmware_size(struct udevice *dev)
> > +{
> > + ...
> > + return ent.size;
> > +}
>
> ent.size is u64 but the function returns int. This could truncate
> large sizes. The same issue exists in fw_get_fip_firmware(). How about
> using ulong which is the common type in U-Boot?
>
Can you better explain this? We need to return int to handle the negative
error.
> > diff --git a/drivers/misc/fw_loader/fip_loader.c b/drivers/misc/fw_loader/fip_loader.c
> > @@ -0,0 +1,578 @@
> > +static int fw_get_fip_firmware(struct udevice *dev)
> > +{
> > + ...
> > + ret = fw_parse_storage_info(dev, &info);
> > + if (ret)
> > + return ret;
> > +
> > + struct firmware *firmwarep = dev_get_priv(dev);
>
> Please can you move the firmwarep declaration to the top of the block.
> The same pattern appears in fw_get_fip_firmware_size().
>
> > diff --git a/drivers/misc/fw_loader/fip_loader.c b/drivers/misc/fw_loader/fip_loader.c
> > @@ -0,0 +1,578 @@
> > + if (ent.size + firmwarep->offset > firmwarep->size) {
> > + log_err("Not enough space to read firmware\n");
> > + return -ENOMEM;
> > + }
>
> I suspect this check is wrong. If firmwarep->offset is an offset into
> the FIP entry, you likely want to verify that (ent.size -
> firmwarep->offset) fits in the buffer, not (ent.size +
> firmwarep->offset).
>
> Also -NOSPC might be better, since we are not actually out of memory.
> When people see -ENOMEM they tend to want to increase the malloc size.
>
This might be confusing but I feel -ENOMEM is correct here. In
firmwarep->size we set the maximum size where we can store the firmware
data so the random guy might be correct to increase malloc size.
I posted v6 with all the other comments addressed, these 2 are the only one
left.
> > diff --git a/drivers/misc/fw_loader/internal.h b/drivers/misc/fw_loader/internal.h
> > @@ -25,11 +25,13 @@ struct phandle_part {
> > + int partoffset;
>
> partoffset is declared as int but used as unsigned int in
> fip_storage_info and read from DT with ofnode_read_u32(). Please can
> you change this to u32 for consistency.
>
> Regards,
> Simon
--
Ansuel
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v5 6/6] doc: dtbinding: Update documentation for Generic Firmware loader
2026-04-03 13:51 [PATCH v5 0/6] misc: fs_loader: reorg and split to FS and FW loader + FIP loader Christian Marangi
` (4 preceding siblings ...)
2026-04-03 13:52 ` [PATCH v5 5/6] misc: fw_loader: introduce FIP loader driver Christian Marangi
@ 2026-04-03 13:52 ` Christian Marangi
2026-04-06 17:14 ` Simon Glass
5 siblings, 1 reply; 13+ messages in thread
From: Christian Marangi @ 2026-04-03 13:52 UTC (permalink / raw)
To: Tom Rini, Simon Glass, Christian Marangi, Casey Connolly,
Quentin Schulz, Peng Fan, Justin Klaassen, Neha Malcom Francis,
Heinrich Schuchardt, Jamie Gibbons, Leo Yu-Chi Liang,
Harsha Vardhan V M, Weijie Gao, Marek Vasut, Patrice Chotard,
Yao Zi, Alif Zakuan Yuslaimi, Lucien.Jheng, u-boot
Update documentation for Generic Firmware loader, generalize it from FS
specific and add new property and example for FIP loader.
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
.../misc/{fs_loader.txt => fw_loader.txt} | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
rename doc/device-tree-bindings/misc/{fs_loader.txt => fw_loader.txt} (77%)
diff --git a/doc/device-tree-bindings/misc/fs_loader.txt b/doc/device-tree-bindings/misc/fw_loader.txt
similarity index 77%
rename from doc/device-tree-bindings/misc/fs_loader.txt
rename to doc/device-tree-bindings/misc/fw_loader.txt
index 542be4b25a0a..44d9c076ebb8 100644
--- a/doc/device-tree-bindings/misc/fs_loader.txt
+++ b/doc/device-tree-bindings/misc/fw_loader.txt
@@ -1,9 +1,9 @@
-* File system firmware loader
+* Generic Firmware loader
Required properties:
--------------------
-- compatible: should contain "u-boot,fs-loader"
+- compatible: should contain "u-boot,fs-loader" or "u-boot,fip-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
@@ -12,6 +12,8 @@ Required properties:
required for ubi and mounting.
- ubivol: which volume of ubi the image loading from, this property is required
for ubi and mounting.
+- partoffset: valid ONLY for "u-boot,fip-loader". Offset of the partition to
+ parse the FIP parition from.
Example of storage device and partition search set for mmc, usb, sata and
ubi in device tree source as shown in below:
@@ -46,3 +48,12 @@ ubi in device tree source as shown in below:
mtdpart = "UBI",
ubivol = "ubi0";
};
+
+ Example for FIP from eMMC:
+ fs_loader4: fip-loader@4 {
+ bootph-all;
+ compatible = "u-boot,fip-loader";
+ phandlepart = <&mmc0 0>;
+ partoffset = <0x100>;
+ };
+
--
2.53.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v5 6/6] doc: dtbinding: Update documentation for Generic Firmware loader
2026-04-03 13:52 ` [PATCH v5 6/6] doc: dtbinding: Update documentation for Generic Firmware loader Christian Marangi
@ 2026-04-06 17:14 ` Simon Glass
0 siblings, 0 replies; 13+ messages in thread
From: Simon Glass @ 2026-04-06 17:14 UTC (permalink / raw)
To: ansuelsmth
Cc: Tom Rini, Simon Glass, Casey Connolly, Quentin Schulz, Peng Fan,
Justin Klaassen, Neha Malcom Francis, Heinrich Schuchardt,
Jamie Gibbons, Leo Yu-Chi Liang, Harsha Vardhan V M, Weijie Gao,
Marek Vasut, Patrice Chotard, Yao Zi, Alif Zakuan Yuslaimi,
Lucien.Jheng, u-boot
Hi Christian,
On 2026-04-03T13:51:57, Christian Marangi <ansuelsmth@gmail.com> wrote:
> doc: dtbinding: Update documentation for Generic Firmware loader
>
> Update documentation for Generic Firmware loader, generalize it from FS
> specific and add new property and example for FIP loader.
>
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> diff --git a/doc/device-tree-bindings/misc/fs_loader.txt b/doc/device-tree-bindings/misc/fw_loader.txt
> @@ -7,6 +7,8 @@ Required properties:
> +- mdtpart: which partition of ubi the image loading from, this property is
> + required for ubi and mounting.
mtdpart
> diff --git a/doc/device-tree-bindings/misc/fs_loader.txt b/doc/device-tree-bindings/misc/fw_loader.txt
> @@ -12,6 +12,8 @@ Required properties:
> +- partoffset: valid ONLY for "u-boot,fip-loader". Offset of the partition to
> + parse the FIP parition from.
partition
> diff --git a/doc/device-tree-bindings/misc/fs_loader.txt b/doc/device-tree-bindings/misc/fw_loader.txt
> @@ -1,9 +1,9 @@
> Required properties:
> --------------------
>
> -- compatible: should contain "u-boot,fs-loader"
> +- compatible: should contain "u-boot,fs-loader" or "u-boot,fip-loader"
> +- phandlepart: which block storage device and partition the image loading from,
The section says 'Required properties' but phandlepart, mtdpart and
ubivol are alternatives; only one set is required depending on the
storage type. Please can you clarify this.
> diff --git a/doc/device-tree-bindings/misc/fs_loader.txt b/doc/device-tree-bindings/misc/fw_loader.txt
> @@ -46,3 +48,12 @@ ubi in device tree source as shown in below:
> + Example for FIP from eMMC:
> + fs_loader4: fip-loader@4 {
> + bootph-all;
> + compatible = "u-boot,fip-loader";
> + phandlepart = <&mmc0 0>;
> + partoffset = <0x100>;
> + };
Since the FIP loader supports UBI storage as well, it would be good to
add an example for FIP on UBI too.
Regards,
Simon
^ permalink raw reply [flat|nested] 13+ messages in thread