On 17/02/2024 14:26, 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>
> ---
> 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.
>
> 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 | 100 ++++++++++++++++++++++++++++++
> include/remoteproc.h | 34 ++++++++++
> 3 files changed, 142 insertions(+)
>
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index 781de530af..759d21437a 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.
> @@ -102,4 +103,11 @@ config REMOTEPROC_TI_IPU
> help
> Say 'y' here to add support for TI' K3 remoteproc driver.
>
> +config REMOTEPROC_MAX_FW_SIZE
> + hex "Maximum size of firmware that needs to be loaded to rproc"
firmware file?
/rproc/the remote processor
> + default 0x10000
> + help
> + Maximum size of the firmware file (elf, binary) that needs to be
> + loaded to th rproc core.
s/th/the
s/rproc/remote processor
> +
> endmenu
> diff --git a/drivers/remoteproc/rproc-uclass.c
> b/drivers/remoteproc/rproc-uclass.c
> index 28b362c887..784361cef9 100644
> --- a/drivers/remoteproc/rproc-uclass.c
> +++ b/drivers/remoteproc/rproc-uclass.c
> @@ -13,6 +13,7 @@
> #include <log.h>
> #include <malloc.h>
> #include <virtio_ring.h>
> +#include <fs_loader.h>
> #include <remoteproc.h>
> #include <asm/io.h>
> #include <dm/device-internal.h>
> @@ -961,3 +962,102 @@ unsigned long rproc_parse_resource_table(struct udevice
> *dev, struct rproc *cfg)
>
> return 1;
> }
> +
> +int rproc_set_firmware(struct udevice *rproc_dev, const char *fw_name)
> +{
> + struct dm_rproc_uclass_pdata *uc_pdata;
> + int len;
> + char *p;
> +
> + if (!rproc_dev || !fw_name)
> + return -EINVAL;
> +
> + uc_pdata = dev_get_uclass_plat(rproc_dev);
> + if (!uc_pdata)
> + return -EINVAL;
> +
> + len = strcspn(fw_name, "\n");
> + if (!len) {
> + debug("invalid firmware name\n");
> + return -EINVAL;
> + }
> +
> + p = strndup(fw_name, len);
> + if (!p)
> + return -ENOMEM;
Aren't we leaking memory if rproc_set_firmware() is called multiple times?
Can we check if uc_pdata->fw_name exists and free it before the strndup?
> +
> + uc_pdata->fw_name = p;
> +
> + return 0;
> +}
> +
> +int rproc_boot(struct udevice *rproc_dev)
> +{
> + struct dm_rproc_uclass_pdata *uc_pdata;
> + struct udevice *fs_loader;
> + int core_id, ret = 0;
> + char *firmware;
> + void *addr;
> +
> + if (!rproc_dev)
> + return -EINVAL;
> +
> + uc_pdata = dev_get_uclass_plat(rproc_dev);
> + if (!uc_pdata)
> + return -EINVAL;
> +
> + core_id = dev_seq(rproc_dev);
> + firmware = uc_pdata->fw_name;
> +
unnecessary blank line.
> + if (!firmware) {
> + debug("No firmware set for rproc core %d\n", core_id);
No firmware name...
> + return -EINVAL;
> + }
> +
> + /* Initialize all rproc cores */
> + if (!rproc_is_initialized()) {
> + ret = rproc_init();
> + if (ret) {
> + debug("rproc_init() failed: %d\n", ret);
> + return ret;
> + }
> + }
> +
> + /* Loading firmware to a given address */
> + ret = get_fs_loader(&fs_loader);
> + if (ret) {
> + debug("could not get fs loader: %d\n", ret);
> + return ret;
> + }
> +
> + if (CONFIG_REMOTEPROC_MAX_FW_SIZE) {
if (defined(CONFIG_REMOTEPROC_MAX_FW_SIZE)) {
> + addr = malloc(CONFIG_REMOTEPROC_MAX_FW_SIZE);
> + if (!addr)
> + return -ENOMEM;
> + } else {
> + debug("CONFIG_REMOTEPROC_MAX_FW_SIZE not defined\n");
> + return -EINVAL;
> + }
> +
> + ret = request_firmware_into_buf(fs_loader, firmware, addr,
> CONFIG_REMOTEPROC_MAX_FW_SIZE,
> + 0);
> + if (ret < 0) {
> + debug("could not request %s: %d\n", firmware, ret);
> + goto free_buffer;
> + }
> +
> + ret = rproc_load(core_id, (ulong)addr, ret);
> + if (ret) {
> + debug("failed to load %s to rproc core %d from addr 0x%08lX err
> %d\n",
> + uc_pdata->fw_name, core_id, (ulong)addr, ret);
> + goto free_buffer;
> + }
> +
> + ret = rproc_start(core_id);
> + if (ret)
> + debug("failed to start rproc core %d\n", core_id);
> +
> +free_buffer:
> + free(addr);
> + return ret;
> +}
> diff --git a/include/remoteproc.h b/include/remoteproc.h
> index 91a88791a4..6f8068e149 100644
> --- a/include/remoteproc.h
> +++ b/include/remoteproc.h
> @@ -403,6 +403,7 @@ enum rproc_mem_type {
> * @name: Platform-specific way of naming the Remote proc
> * @mem_type: one of 'enum rproc_mem_type'
> * @driver_plat_data: driver specific platform data that may be needed.
> + * @fw_name: firmware name
> *
> * This can be accessed with dev_get_uclass_plat() for any UCLASS_REMOTEPROC
> * device.
> @@ -412,6 +413,7 @@ struct dm_rproc_uclass_pdata {
> const char *name;
> enum rproc_mem_type mem_type;
> void *driver_plat_data;
> + char *fw_name;
> };
>
> /**
> @@ -705,6 +707,34 @@ unsigned long rproc_parse_resource_table(struct udevice
> *dev,
> struct resource_table *rproc_find_resource_table(struct udevice *dev,
> unsigned int addr,
> int *tablesz);
> +/**
> + * rproc_set_firmware() - assign a new firmware name
> + * @rproc_dev: device for which new firmware name is being assigned
> + * @fw_name: new firmware name to be assigned
> + *
> + * This function allows remoteproc drivers or clients to configure a custom
> + * firmware name. The function does not trigger a remote processor boot,
> + * only sets the firmware name used for a subsequent boot.
> + *
> + * This function sets the fw_name field in uclass pdata of the Remote proc
> + *
> + * Return: 0 on success or a negative value upon failure
> + */
> +int rproc_set_firmware(struct udevice *rproc_dev, const char *fw_name);
> +
> +/**
> + * rproc_boot() - boot a remote processor
> + * @rproc_dev: rproc device to boot
> + *
> + * Boot a remote processor (i.e. load its firmware, power it on, ...).
> + *
> + * This function first loads the firmware set in the uclass pdata of Remote
> + * processor to a buffer and then loads firmware to the remote processor
> + * using rproc_load().
> + *
> + * Return: 0 on success, and an appropriate error value otherwise
> + */
> +int rproc_boot(struct udevice *rproc_dev);
> #else
> static inline int rproc_init(void) { return -ENOSYS; }
> static inline int rproc_dev_init(int id) { return -ENOSYS; }
> @@ -744,6 +774,10 @@ static inline int rproc_elf_load_rsc_table(struct
> udevice *dev, ulong fw_addr,
> ulong fw_size, ulong *rsc_addr,
> ulong *rsc_size)
> { return -ENOSYS; }
> +static inline int rproc_set_firmware(struct udevice *rproc_dev, const char
> *fw_name)
> +{ return -ENOSYS; }
> +static inline int rproc_boot(struct udevice *rproc_dev)
> +{ return -ENOSYS; }
> #endif
>
> #endif /* _RPROC_H_ */
--
cheers,
-roger
