[PATCH v4] misc: fs-loader: Use fw_storage_interface instead of storage_interface

The fs-loader driver reads env storage_interface and uses it to load firmware file into memory using the medium set by env. Update the driver to use env fw_storage_interface as this variable is only used to load firmwares. The env storage_interface will act as fallback so that the existing implementations do not break.
Also update the FS Loader documentation accordingly.
Signed-off-by: MD Danish Anwar danishanwar@ti.com --- Changes from v3 to v4: *) No functional change. Splitted the patch out of the series as suggested by Nishant. *) Droppped the RFC tag.
v3: https://lore.kernel.org/all/20240124064930.1787929-3-danishanwar@ti.com/
doc/develop/driver-model/fs_firmware_loader.rst | 5 ++++- drivers/misc/fs_loader.c | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/doc/develop/driver-model/fs_firmware_loader.rst b/doc/develop/driver-model/fs_firmware_loader.rst index 149b8b436e..410cc1442d 100644 --- a/doc/develop/driver-model/fs_firmware_loader.rst +++ b/doc/develop/driver-model/fs_firmware_loader.rst @@ -98,8 +98,11 @@ through the U-Boot environment variable during run time.
For examples:
+fw_storage_interface: + Firmware storage interface, it can be "mmc", "usb", "sata" or "ubi". storage_interface: - Storage interface, it can be "mmc", "usb", "sata" or "ubi". + Storage interface, it can be "mmc", "usb", "sata" or "ubi". This acts + as a fallback if fw_storage_interface is not set. fw_dev_part: Block device number and its partition, it can be "0:1". fw_ubi_mtdpart: diff --git a/drivers/misc/fs_loader.c b/drivers/misc/fs_loader.c index 1ffc199ba1..3798dab5b6 100644 --- a/drivers/misc/fs_loader.c +++ b/drivers/misc/fs_loader.c @@ -153,7 +153,10 @@ static int fw_get_filesystem_firmware(struct udevice *dev) char *storage_interface, *dev_part, *ubi_mtdpart, *ubi_volume; int ret;
- storage_interface = env_get("storage_interface"); + storage_interface = env_get("fw_storage_interface"); + if (!storage_interface) + 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");

On 1/30/2024 11:56 AM, MD Danish Anwar wrote:
The fs-loader driver reads env storage_interface and uses it to load firmware file into memory using the medium set by env. Update the driver to use env fw_storage_interface as this variable is only used to load firmwares. The env storage_interface will act as fallback so that the existing implementations do not break.
Also update the FS Loader documentation accordingly.
Signed-off-by: MD Danish Anwar danishanwar@ti.com
Changes from v3 to v4: *) No functional change. Splitted the patch out of the series as suggested by Nishant. *) Droppped the RFC tag.
v3: https://lore.kernel.org/all/20240124064930.1787929-3-danishanwar@ti.com/
Acked-by: Ravi Gunasekaran r-gunasekaran@ti.com

On 1/30/24 01:26, MD Danish Anwar wrote:
The fs-loader driver reads env storage_interface and uses it to load firmware file into memory using the medium set by env. Update the driver to use env fw_storage_interface as this variable is only used to load firmwares. The env storage_interface will act as fallback so that the existing implementations do not break.
Also update the FS Loader documentation accordingly.
So why do you want to do this? I don't see what the point of renaming the variable is, since you are not e.g. adding any new functionality, and we have to pay for the rename in code size.
--Sean

Hi Sean,
On 07/02/24 11:14 pm, Sean Anderson wrote:
On 1/30/24 01:26, MD Danish Anwar wrote:
The fs-loader driver reads env storage_interface and uses it to load firmware file into memory using the medium set by env. Update the driver to use env fw_storage_interface as this variable is only used to load firmwares. The env storage_interface will act as fallback so that the existing implementations do not break.
Also update the FS Loader documentation accordingly.
So why do you want to do this? I don't see what the point of renaming the variable is, since you are not e.g. adding any new functionality, and we have to pay for the rename in code size.
I am upstreaming TI's ICSSG driver for u-boot and during code review Roger Quadros rogerq@kernel.org commented asking to rename this variable [1]. I think the motive here was to keep all variables used by fs-loader driver with 'fw_' prefix. All other variables had 'fw_' prefix except for storage_interface.
[1] https://lore.kernel.org/all/4721f3b9-f823-47f3-b4f3-ed40002af31b@kernel.org/
--Sean

On 08/02/2024 07:19, MD Danish Anwar wrote:
Hi Sean,
On 07/02/24 11:14 pm, Sean Anderson wrote:
On 1/30/24 01:26, MD Danish Anwar wrote:
The fs-loader driver reads env storage_interface and uses it to load firmware file into memory using the medium set by env. Update the driver to use env fw_storage_interface as this variable is only used to load firmwares. The env storage_interface will act as fallback so that the existing implementations do not break.
Also update the FS Loader documentation accordingly.
So why do you want to do this? I don't see what the point of renaming the variable is, since you are not e.g. adding any new functionality, and we have to pay for the rename in code size.
I am upstreaming TI's ICSSG driver for u-boot and during code review Roger Quadros rogerq@kernel.org commented asking to rename this variable [1]. I think the motive here was to keep all variables used by fs-loader driver with 'fw_' prefix. All other variables had 'fw_' prefix except for storage_interface.
Can you please mention this motive in the Commit message?
[1] https://lore.kernel.org/all/4721f3b9-f823-47f3-b4f3-ed40002af31b@kernel.org/
--Sean

On 08/02/24 5:28 pm, Roger Quadros wrote:
On 08/02/2024 07:19, MD Danish Anwar wrote:
Hi Sean,
On 07/02/24 11:14 pm, Sean Anderson wrote:
On 1/30/24 01:26, MD Danish Anwar wrote:
The fs-loader driver reads env storage_interface and uses it to load firmware file into memory using the medium set by env. Update the driver to use env fw_storage_interface as this variable is only used to load firmwares. The env storage_interface will act as fallback so that the existing implementations do not break.
Also update the FS Loader documentation accordingly.
So why do you want to do this? I don't see what the point of renaming the variable is, since you are not e.g. adding any new functionality, and we have to pay for the rename in code size.
I am upstreaming TI's ICSSG driver for u-boot and during code review Roger Quadros rogerq@kernel.org commented asking to rename this variable [1]. I think the motive here was to keep all variables used by fs-loader driver with 'fw_' prefix. All other variables had 'fw_' prefix except for storage_interface.
Can you please mention this motive in the Commit message?
Sure Roger, I will resend the patch after updating commit message.
[1] https://lore.kernel.org/all/4721f3b9-f823-47f3-b4f3-ed40002af31b@kernel.org/
--Sean
participants (4)
-
MD Danish Anwar
-
Ravi Gunasekaran
-
Roger Quadros
-
Sean Anderson