
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@ti.com Acked-by: Ravi Gunasekaran r-gunasekaran@ti.com Reviewed-by: Roger Quadros rogerq@kernel.org
Changes from v5 to v6: *) Collected Acked-by tag from Ravi Gunasekaran r-gunasekaran@ti.com *) Fixed few typos as pointed out by Roger Quadros rogerq@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@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.