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 <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.
Yes please, thanks. -- Tom
signature.asc
Description: PGP signature
