| Message ID | 20240124064930.1787929-3-danishanwar@ti.com |
|---|---|
| State | RFC |
| Delegated to: | Ramon Fried |
| Headers | show |
| Series | Introduce ICSSG Ethernet driver | expand |
On 12:19-20240124, 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> > --- > 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"); > -- > 2.34.1 > You should move these specific patches out of the series and debate on their merits seperately. mixing things like these in a single series that needs to go to multiple u-boot custodians just creates problems for everyone.
Hi Nishant, On 1/24/2024 11:40 PM, Nishanth Menon wrote: > On 12:19-20240124, 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> >> --- >> 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"); >> -- >> 2.34.1 >> > > You should move these specific patches out of the series and debate on > their merits seperately. mixing things like these in a single series > that needs to go to multiple u-boot custodians just creates problems for > everyone. > I have kept these patches in the series because ICSSG driver depends on these patches. I didn't want to post multiple series and map dependencies so I posted all patches in one series. Patch 1 to 3 are dependency for ICSSG driver. Patch 4 to 9 are ICSSG driver patches. Patch 10 to 15 are AM65x-EVM specific patches (DTS and configs). The first 3 patches will also go to different u-boot custodians as first one is dma related, second one is related to fs-loader driver and the third one modifies the remoteproc driver. So instead of posting all these separately, I wanted to post it along with the ICSSG driver series so that folks can understand why these three patches are needed.
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");
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> --- doc/develop/driver-model/fs_firmware_loader.rst | 5 ++++- drivers/misc/fs_loader.c | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-)