Re: [U-Boot, v1, 1/1] misc: fs_loader: Add request_firmware_into_buf_via_env API for environment-based firmware loading

[Frank Wunderlich]

Am 21. Juli 2025 17:33:48 MESZ schrieb Tom Rini <trini at konsulko.com>:
>On Sun, Jul 20, 2025 at 10:47:50PM +0200, Marek Vasut wrote:
>> On 7/20/25 5:04 PM, Lucien.Jheng wrote:
>> > This commit adds request_firmware_into_buf_via_env() to the fs_loader driver,
>> > enabling firmware loading into a buffer
>> > using environment variables for path and size.
>> > It supports multiple entries via indexed variable names
>> > (e.g., "fw_dir0", "fw_size0").
>> > This provides flexible, scriptable firmware loading without hardcoded details.
>> > 
>> > Signed-off-by: Lucien.Jheng <lucienzx159 at gmail.com>
>> > ---
>> > Change log:
>> > - Re-sending as previous version and add more maintainers to review.
>> > 
>> >   drivers/misc/fs_loader.c | 59 ++++++++++++++++++++++++++++++++++++++++
>> >   include/fs_loader.h      | 18 ++++++++++++
>> >   2 files changed, 77 insertions(+)
>> > 
>> > diff --git a/drivers/misc/fs_loader.c b/drivers/misc/fs_loader.c
>> > index 66803f4b997..a4f4448f5cc 100644
>> > --- a/drivers/misc/fs_loader.c
>> > +++ b/drivers/misc/fs_loader.c
>> > @@ -228,6 +228,65 @@ int request_firmware_into_buf(struct udevice *dev,
>> >   	return ret;
>> >   }
>> > 
>> > +/**
>> > + * request_firmware_into_buf_via_env - Load firmware using environment variables.
>> > + * @dev: An instance of a driver.
>> > + * @buf: Address of buffer to load firmware into.
>> > + * @offset: Offset of a file for start reading into buffer.
>> > + * @fw_index: Index of the firmware entry to load.
>> > + *
>> > + * This function loads firmware into the provided buffer using environment
>> > + * variables to determine the firmware file path, address, and size. It supports
>> > + * multiple firmware entries by using indexed environment variable names such as
>> > + * "fw_dir0", "fw_size0", etc.
>> 
>> To make this really scriptable, this code has to run a script which does the
>> firmware loading and then points the U-Boot code to the buffer with data in
>> it.
>> 
>> Instead of "fw_index", use "char *script_name" or some such. Then, run the
>> script. The script should set up two environment variables, call them e.g.
>> "fw_addr" and "fw_size" or some such . Then, this code should read the two
>> environment variables, validate them, and possibly copy their content into
>> $buf . Finally, the code should always unset fw_addr and fw_size .
>> 
>> Note that request_firmware_into_buf_via_env() should also have 'size'
>> parameter which describes the maximum size of loaded firmware, so the code
>> could verify whether the copy of firmware into $buf would write past the
>> buffer and prevent that.
>
>And for v4, can we please put this patch and the patch using it in the
>same series? I see there's some feedback right now from Frank on the
>other patch about how maybe this part should be implemented.

The airoha phy patch does not yet use the fs_loader,it uses only similar concept to be
replaced by fw_loader. Currently it uses own
vars and loads directly without fs_loader as far as i see.

For fs loader it must be more generic,so basedir and vendor and filename maybe need to be separated.
Or vendor is part of filename...but in this case it is again redundant in env (could be
defined in driver),but not that easy to determine if file is in rootdir of e.g. usb. Then
basedir and vendor is empty,but subsystem
and part set,filename from driver.

Possibly vendor can be optional in fsloader so it can check with or without. So driver can set it but fs_loader tries first with and as fallback without vendor string if file is not found there.

There are much complex subpaths (e.g.
mediatek/mt7996, i know, wifi firmware not
related for bootloader, but imho we should think about it that this can happen)

regards Frank