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

[Tom Rini]

On Wed, Mar 20, 2024 at 11:19:01AM +0530, MD Danish Anwar wrote:
> 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.

Yes please, thanks.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20240320/0143674e/attachment.sig>