Hi Tom,
On 20/03/24 4:10 am, Tom Rini wrote:
> On Wed, Feb 28, 2024 at 05:36:45PM +0530, MD Danish Anwar wrote:
>
>> Add APIs to set a firmware_name to a rproc and boot the rproc with the
>> same firmware.
>>
>> Clients can call rproc_set_firmware() API to set firmware_name for a rproc
>> whereas rproc_boot() will load the firmware set by rproc_set_firmware() to
>> a buffer by calling request_firmware_into_buf(). rproc_boot() will then
>> load the firmware file to the remote processor and start the remote
>> processor.
>>
>> Also include "fs-loader.h" and make remoteproc driver select FS_LOADER in
>> Kconfig so that we can call request_firmware_into_buf() from remoteproc
>> driver.
>>
>> Signed-off-by: MD Danish Anwar <danishan...@ti.com>
>> Acked-by: Ravi Gunasekaran <r-gunaseka...@ti.com>
>> Reviewed-by: Roger Quadros <rog...@kernel.org>
>> ---
>> Changes from v5 to v6:
>> *) Collected Acked-by tag from Ravi Gunasekaran <r-gunaseka...@ti.com>
>> *) Fixed few typos as pointed out by Roger Quadros <rog...@kernel.org>
>> *) Added if condition to check if uc_pdata->fw_name exists and free it
>> before the strndup as suggested by Roger Quadros <rog...@kernel.org>
>>
>> Changes from v4 to v5:
>> *) Added Kconfig option REMOTEPROC_MAX_FW_SIZE to set max firmware size
>> that can be loaded to a rproc.
>> *) Added freeing of address in rproc_boot() as pointed out by Ravi.
>> *) Allocating the address at a later point in rproc_boot()
>> *) Rebased on latest u-boot/master [commit
>> 9e00b6993f724da9699ef12573307afea8c19284]
>>
>> Changes from v3 to v4:
>> *) No functional change. Splitted the patch out of the series as suggested
>> by Nishant.
>> *) Droppped the RFC tag.
>>
>> v5: https://lore.kernel.org/all/20240217122602.3402774-1-danishan...@ti.com/
>> v4: https://lore.kernel.org/all/20240130063322.2345057-1-danishan...@ti.com/
>> v3: https://lore.kernel.org/all/20240124064930.1787929-4-danishan...@ti.com/
>>
>> drivers/remoteproc/Kconfig | 8 +++
>> drivers/remoteproc/rproc-uclass.c | 102 ++++++++++++++++++++++++++++++
>> include/remoteproc.h | 34 ++++++++++
>> 3 files changed, 144 insertions(+)
>>
>> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
>> index 781de530af..9f9877931c 100644
>> --- a/drivers/remoteproc/Kconfig
>> +++ b/drivers/remoteproc/Kconfig
>> @@ -10,6 +10,7 @@ menu "Remote Processor drivers"
>> # All users should depend on DM
>> config REMOTEPROC
>> bool
>> + select FS_LOADER
>> depends on DM
>>
>> # Please keep the configuration alphabetically sorted.
>
> Can we not make the FS_LOADER portion optional? I didn't realize how
> many non-TI platforms this impacted. And even then it's possible I
> assume that custom designs will load the firmwares in other manners.
>
Yes we can. We can wrap the remoteproc APIs using FS_LOADER in #ifdef
CONFIG_FS_LOADER. And instead of REMOTEPROC driver selecting FS_LOADER,
the clinet driver (ICSSG in this case) who is calling those remoteproc
APIs will select FS_LOADER and enable it.
This will make sure that other platforms (ti or non-ti) that doesn't
support ICSSG but enables Remoteproc, will not enable FS_LOADER. This
way we are not forcing other platforms using remoteproc to enable
FS_LOADER. In this case the APIs will not get built.
Now FS_LOADER will only be enabled when there is a client driver that
uses rproc_boot() APIs. It's upto the client driver to enable FS_LOADER
below is the diff,
diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 9f9877931c..a49802c132 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -10,7 +10,6 @@ menu "Remote Processor drivers"
# All users should depend on DM
config REMOTEPROC
bool
- select FS_LOADER
depends on DM
# Please keep the configuration alphabetically sorted.
diff --git a/drivers/remoteproc/rproc-uclass.c
b/drivers/remoteproc/rproc-uclass.c
index f4f22a3851..a6a8be5009 100644
--- a/drivers/remoteproc/rproc-uclass.c
+++ b/drivers/remoteproc/rproc-uclass.c
@@ -994,6 +994,7 @@ int rproc_set_firmware(struct udevice *rproc_dev,
const char *fw_name)
return 0;
}
+#ifdef CONFIG_FS_LOADER
int rproc_boot(struct udevice *rproc_dev)
{
struct dm_rproc_uclass_pdata *uc_pdata;
@@ -1063,3 +1064,4 @@ free_buffer:
free(addr);
return ret;
}
+#endif
Let me know if this looks ok. If it's ok I will post v7 with this change.
--
Thanks and Regards,
Danish
