[PATCH v6 5/6] misc: fw_loader: introduce FIP loader driver

[Simon Glass]

Hi Christian,

On 2026-04-09T13:32:56, Christian Marangi <ansuelsmth at 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                      |  11 +
>  drivers/misc/fw_loader/Makefile           |   1 +
>  drivers/misc/fw_loader/fip_loader.c       | 544 ++++++++++++++++++++++++++++++
>  drivers/misc/fw_loader/fw_loader-uclass.c |   3 +
>  drivers/misc/fw_loader/internal.h         |   2 +
>  5 files changed, 561 insertions(+)

> diff --git a/drivers/misc/fw_loader/fip_loader.c b/drivers/misc/fw_loader/fip_loader.c
> @@ -0,0 +1,544 @@
> +#include <fs.h>

Is this header actually used?

> diff --git a/drivers/misc/fw_loader/fip_loader.c b/drivers/misc/fw_loader/fip_loader.c
> @@ -0,0 +1,544 @@
> +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;
> +     }

I suspect this condition is wrong. When pos is exactly at a block
boundary (e.g. pos = 512 on a 512-byte block device), consumed is 0
and left is 512. The code then enters the branch and copies stale data
from the buffer. The condition should probably be if (consumed) to
match the logic in blk_read_fip_firmware() which correctly uses if
(pos)

Regards,
Simon