[PATCH v6] remoteproc: uclass: Add methods to load firmware to rproc and boot rproc

[MD Danish Anwar]

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 <danishanwar at ti.com>
>> Acked-by: Ravi Gunasekaran <r-gunasekaran at ti.com>
>> Reviewed-by: Roger Quadros <rogerq at kernel.org>
>> ---
>> Changes from v5 to v6:
>> *) Collected Acked-by tag from Ravi Gunasekaran <r-gunasekaran at ti.com>
>> *) Fixed few typos as pointed out by Roger Quadros <rogerq at kernel.org>
>> *) Added if condition to check if uc_pdata->fw_name exists and free it
>>    before the strndup as suggested by Roger Quadros <rogerq at 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-danishanwar@ti.com/
>> v4: https://lore.kernel.org/all/20240130063322.2345057-1-danishanwar@ti.com/
>> v3: https://lore.kernel.org/all/20240124064930.1787929-4-danishanwar@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