linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] remoteproc: Add a rproc_set_firmware() API
@ 2020-11-21  3:20 Suman Anna
  2020-11-21  3:33 ` Bjorn Andersson
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Suman Anna @ 2020-11-21  3:20 UTC (permalink / raw)
  To: Bjorn Andersson, Mathieu Poirier
  Cc: Grzegorz Jaszczyk, linux-remoteproc, linux-omap, linux-kernel,
	Suman Anna

A new API, rproc_set_firmware() is added to allow the remoteproc platform
drivers and remoteproc client drivers to be able to configure a custom
firmware name that is different from the default name used during
remoteproc registration. This function is being introduced to provide
a kernel-level equivalent of the current sysfs interface to remoteproc
client drivers, and can only change firmwares when the remoteproc is
offline. This allows some remoteproc drivers to choose different firmwares
at runtime based on the functionality the remote processor is providing.
The TI PRU Ethernet driver will be an example of such usage as it
requires to use different firmwares for different supported protocols.

Also, update the firmware_store() function used by the sysfs interface
to reuse this function to avoid code duplication.

Signed-off-by: Suman Anna <s-anna@ti.com>
---
 drivers/remoteproc/remoteproc_core.c  | 63 +++++++++++++++++++++++++++
 drivers/remoteproc/remoteproc_sysfs.c | 33 +-------------
 include/linux/remoteproc.h            |  1 +
 3 files changed, 66 insertions(+), 31 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index dab2c0f5caf0..46c2937ebea9 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -1934,6 +1934,69 @@ struct rproc *rproc_get_by_phandle(phandle phandle)
 #endif
 EXPORT_SYMBOL(rproc_get_by_phandle);
 
+/**
+ * rproc_set_firmware() - assign a new firmware
+ * @rproc: rproc handle to which the new firmware is being assigned
+ * @fw_name: new firmware name to be assigned
+ *
+ * This function allows remoteproc drivers or clients to configure a custom
+ * firmware name that is different from the default name used during remoteproc
+ * registration. The function does not trigger a remote processor boot,
+ * only sets the firmware name used for a subsequent boot. This function
+ * should also be called only when the remote processor is offline.
+ *
+ * This allows either the userspace to configure a different name through
+ * sysfs or a kernel-level remoteproc or a remoteproc client driver to set
+ * a specific firmware when it is controlling the boot and shutdown of the
+ * remote processor.
+ *
+ * Return: 0 on success or a negative value upon failure
+ */
+int rproc_set_firmware(struct rproc *rproc, const char *fw_name)
+{
+	struct device *dev;
+	int ret, len;
+	char *p;
+
+	if (!rproc || !fw_name)
+		return -EINVAL;
+
+	dev = rproc->dev.parent;
+
+	ret = mutex_lock_interruptible(&rproc->lock);
+	if (ret) {
+		dev_err(dev, "can't lock rproc %s: %d\n", rproc->name, ret);
+		return -EINVAL;
+	}
+
+	if (rproc->state != RPROC_OFFLINE) {
+		dev_err(dev, "can't change firmware while running\n");
+		ret = -EBUSY;
+		goto out;
+	}
+
+	len = strcspn(fw_name, "\n");
+	if (!len) {
+		dev_err(dev, "can't provide empty string for firmware name\n");
+		ret = -EINVAL;
+		goto out;
+	}
+
+	p = kstrndup(fw_name, len, GFP_KERNEL);
+	if (!p) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	kfree(rproc->firmware);
+	rproc->firmware = p;
+
+out:
+	mutex_unlock(&rproc->lock);
+	return ret;
+}
+EXPORT_SYMBOL(rproc_set_firmware);
+
 static int rproc_validate(struct rproc *rproc)
 {
 	switch (rproc->state) {
diff --git a/drivers/remoteproc/remoteproc_sysfs.c b/drivers/remoteproc/remoteproc_sysfs.c
index 3fd18a71c188..cf846caf2e1a 100644
--- a/drivers/remoteproc/remoteproc_sysfs.c
+++ b/drivers/remoteproc/remoteproc_sysfs.c
@@ -159,42 +159,13 @@ static ssize_t firmware_store(struct device *dev,
 			      const char *buf, size_t count)
 {
 	struct rproc *rproc = to_rproc(dev);
-	char *p;
-	int err, len = count;
+	int err;
 
 	/* restrict sysfs operations if not allowed by remoteproc drivers */
 	if (rproc->deny_sysfs_ops)
 		return -EPERM;
 
-	err = mutex_lock_interruptible(&rproc->lock);
-	if (err) {
-		dev_err(dev, "can't lock rproc %s: %d\n", rproc->name, err);
-		return -EINVAL;
-	}
-
-	if (rproc->state != RPROC_OFFLINE) {
-		dev_err(dev, "can't change firmware while running\n");
-		err = -EBUSY;
-		goto out;
-	}
-
-	len = strcspn(buf, "\n");
-	if (!len) {
-		dev_err(dev, "can't provide a NULL firmware\n");
-		err = -EINVAL;
-		goto out;
-	}
-
-	p = kstrndup(buf, len, GFP_KERNEL);
-	if (!p) {
-		err = -ENOMEM;
-		goto out;
-	}
-
-	kfree(rproc->firmware);
-	rproc->firmware = p;
-out:
-	mutex_unlock(&rproc->lock);
+	err = rproc_set_firmware(rproc, buf);
 
 	return err ? err : count;
 }
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index dbc3767f7d0e..6e04b99413f8 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -655,6 +655,7 @@ rproc_of_resm_mem_entry_init(struct device *dev, u32 of_resm_idx, size_t len,
 
 int rproc_boot(struct rproc *rproc);
 void rproc_shutdown(struct rproc *rproc);
+int rproc_set_firmware(struct rproc *rproc, const char *fw_name);
 void rproc_report_crash(struct rproc *rproc, enum rproc_crash_type type);
 int rproc_coredump_add_segment(struct rproc *rproc, dma_addr_t da, size_t size);
 int rproc_coredump_add_custom_segment(struct rproc *rproc,
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] remoteproc: Add a rproc_set_firmware() API
  2020-11-21  3:20 [PATCH] remoteproc: Add a rproc_set_firmware() API Suman Anna
@ 2020-11-21  3:33 ` Bjorn Andersson
  2020-11-25 18:12 ` rishabhb
  2020-11-26 22:56 ` Mathieu Poirier
  2 siblings, 0 replies; 4+ messages in thread
From: Bjorn Andersson @ 2020-11-21  3:33 UTC (permalink / raw)
  To: Suman Anna
  Cc: Mathieu Poirier, Grzegorz Jaszczyk, linux-remoteproc, linux-omap,
	linux-kernel

On Fri 20 Nov 21:20 CST 2020, Suman Anna wrote:

> A new API, rproc_set_firmware() is added to allow the remoteproc platform
> drivers and remoteproc client drivers to be able to configure a custom
> firmware name that is different from the default name used during
> remoteproc registration. This function is being introduced to provide
> a kernel-level equivalent of the current sysfs interface to remoteproc
> client drivers, and can only change firmwares when the remoteproc is
> offline. This allows some remoteproc drivers to choose different firmwares
> at runtime based on the functionality the remote processor is providing.
> The TI PRU Ethernet driver will be an example of such usage as it
> requires to use different firmwares for different supported protocols.
> 
> Also, update the firmware_store() function used by the sysfs interface
> to reuse this function to avoid code duplication.
> 
> Signed-off-by: Suman Anna <s-anna@ti.com>

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

> ---
>  drivers/remoteproc/remoteproc_core.c  | 63 +++++++++++++++++++++++++++
>  drivers/remoteproc/remoteproc_sysfs.c | 33 +-------------
>  include/linux/remoteproc.h            |  1 +
>  3 files changed, 66 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index dab2c0f5caf0..46c2937ebea9 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -1934,6 +1934,69 @@ struct rproc *rproc_get_by_phandle(phandle phandle)
>  #endif
>  EXPORT_SYMBOL(rproc_get_by_phandle);
>  
> +/**
> + * rproc_set_firmware() - assign a new firmware
> + * @rproc: rproc handle to which the new firmware is being assigned
> + * @fw_name: new firmware name to be assigned
> + *
> + * This function allows remoteproc drivers or clients to configure a custom
> + * firmware name that is different from the default name used during remoteproc
> + * registration. The function does not trigger a remote processor boot,
> + * only sets the firmware name used for a subsequent boot. This function
> + * should also be called only when the remote processor is offline.
> + *
> + * This allows either the userspace to configure a different name through
> + * sysfs or a kernel-level remoteproc or a remoteproc client driver to set
> + * a specific firmware when it is controlling the boot and shutdown of the
> + * remote processor.
> + *
> + * Return: 0 on success or a negative value upon failure
> + */
> +int rproc_set_firmware(struct rproc *rproc, const char *fw_name)
> +{
> +	struct device *dev;
> +	int ret, len;
> +	char *p;
> +
> +	if (!rproc || !fw_name)
> +		return -EINVAL;
> +
> +	dev = rproc->dev.parent;
> +
> +	ret = mutex_lock_interruptible(&rproc->lock);
> +	if (ret) {
> +		dev_err(dev, "can't lock rproc %s: %d\n", rproc->name, ret);
> +		return -EINVAL;
> +	}
> +
> +	if (rproc->state != RPROC_OFFLINE) {
> +		dev_err(dev, "can't change firmware while running\n");
> +		ret = -EBUSY;
> +		goto out;
> +	}
> +
> +	len = strcspn(fw_name, "\n");
> +	if (!len) {
> +		dev_err(dev, "can't provide empty string for firmware name\n");
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	p = kstrndup(fw_name, len, GFP_KERNEL);
> +	if (!p) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +
> +	kfree(rproc->firmware);
> +	rproc->firmware = p;
> +
> +out:
> +	mutex_unlock(&rproc->lock);
> +	return ret;
> +}
> +EXPORT_SYMBOL(rproc_set_firmware);
> +
>  static int rproc_validate(struct rproc *rproc)
>  {
>  	switch (rproc->state) {
> diff --git a/drivers/remoteproc/remoteproc_sysfs.c b/drivers/remoteproc/remoteproc_sysfs.c
> index 3fd18a71c188..cf846caf2e1a 100644
> --- a/drivers/remoteproc/remoteproc_sysfs.c
> +++ b/drivers/remoteproc/remoteproc_sysfs.c
> @@ -159,42 +159,13 @@ static ssize_t firmware_store(struct device *dev,
>  			      const char *buf, size_t count)
>  {
>  	struct rproc *rproc = to_rproc(dev);
> -	char *p;
> -	int err, len = count;
> +	int err;
>  
>  	/* restrict sysfs operations if not allowed by remoteproc drivers */
>  	if (rproc->deny_sysfs_ops)
>  		return -EPERM;
>  
> -	err = mutex_lock_interruptible(&rproc->lock);
> -	if (err) {
> -		dev_err(dev, "can't lock rproc %s: %d\n", rproc->name, err);
> -		return -EINVAL;
> -	}
> -
> -	if (rproc->state != RPROC_OFFLINE) {
> -		dev_err(dev, "can't change firmware while running\n");
> -		err = -EBUSY;
> -		goto out;
> -	}
> -
> -	len = strcspn(buf, "\n");
> -	if (!len) {
> -		dev_err(dev, "can't provide a NULL firmware\n");
> -		err = -EINVAL;
> -		goto out;
> -	}
> -
> -	p = kstrndup(buf, len, GFP_KERNEL);
> -	if (!p) {
> -		err = -ENOMEM;
> -		goto out;
> -	}
> -
> -	kfree(rproc->firmware);
> -	rproc->firmware = p;
> -out:
> -	mutex_unlock(&rproc->lock);
> +	err = rproc_set_firmware(rproc, buf);
>  
>  	return err ? err : count;
>  }
> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
> index dbc3767f7d0e..6e04b99413f8 100644
> --- a/include/linux/remoteproc.h
> +++ b/include/linux/remoteproc.h
> @@ -655,6 +655,7 @@ rproc_of_resm_mem_entry_init(struct device *dev, u32 of_resm_idx, size_t len,
>  
>  int rproc_boot(struct rproc *rproc);
>  void rproc_shutdown(struct rproc *rproc);
> +int rproc_set_firmware(struct rproc *rproc, const char *fw_name);
>  void rproc_report_crash(struct rproc *rproc, enum rproc_crash_type type);
>  int rproc_coredump_add_segment(struct rproc *rproc, dma_addr_t da, size_t size);
>  int rproc_coredump_add_custom_segment(struct rproc *rproc,
> -- 
> 2.28.0
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] remoteproc: Add a rproc_set_firmware() API
  2020-11-21  3:20 [PATCH] remoteproc: Add a rproc_set_firmware() API Suman Anna
  2020-11-21  3:33 ` Bjorn Andersson
@ 2020-11-25 18:12 ` rishabhb
  2020-11-26 22:56 ` Mathieu Poirier
  2 siblings, 0 replies; 4+ messages in thread
From: rishabhb @ 2020-11-25 18:12 UTC (permalink / raw)
  To: Suman Anna
  Cc: Bjorn Andersson, Mathieu Poirier, Grzegorz Jaszczyk,
	linux-remoteproc, linux-omap, linux-kernel

On 2020-11-20 19:20, Suman Anna wrote:
> A new API, rproc_set_firmware() is added to allow the remoteproc 
> platform
> drivers and remoteproc client drivers to be able to configure a custom
> firmware name that is different from the default name used during
> remoteproc registration. This function is being introduced to provide
> a kernel-level equivalent of the current sysfs interface to remoteproc
> client drivers, and can only change firmwares when the remoteproc is
> offline. This allows some remoteproc drivers to choose different 
> firmwares
> at runtime based on the functionality the remote processor is 
> providing.
> The TI PRU Ethernet driver will be an example of such usage as it
> requires to use different firmwares for different supported protocols.
> 
> Also, update the firmware_store() function used by the sysfs interface
> to reuse this function to avoid code duplication.
> 
> Signed-off-by: Suman Anna <s-anna@ti.com>
> ---
>  drivers/remoteproc/remoteproc_core.c  | 63 +++++++++++++++++++++++++++
>  drivers/remoteproc/remoteproc_sysfs.c | 33 +-------------
>  include/linux/remoteproc.h            |  1 +
>  3 files changed, 66 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/remoteproc/remoteproc_core.c
> b/drivers/remoteproc/remoteproc_core.c
> index dab2c0f5caf0..46c2937ebea9 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -1934,6 +1934,69 @@ struct rproc *rproc_get_by_phandle(phandle 
> phandle)
>  #endif
>  EXPORT_SYMBOL(rproc_get_by_phandle);
> 
> +/**
> + * rproc_set_firmware() - assign a new firmware
> + * @rproc: rproc handle to which the new firmware is being assigned
> + * @fw_name: new firmware name to be assigned
> + *
> + * This function allows remoteproc drivers or clients to configure a 
> custom
> + * firmware name that is different from the default name used during 
> remoteproc
> + * registration. The function does not trigger a remote processor 
> boot,
> + * only sets the firmware name used for a subsequent boot. This 
> function
> + * should also be called only when the remote processor is offline.
> + *
> + * This allows either the userspace to configure a different name 
> through
> + * sysfs or a kernel-level remoteproc or a remoteproc client driver to 
> set
> + * a specific firmware when it is controlling the boot and shutdown of 
> the
> + * remote processor.
> + *
> + * Return: 0 on success or a negative value upon failure
> + */
> +int rproc_set_firmware(struct rproc *rproc, const char *fw_name)
> +{
> +	struct device *dev;
> +	int ret, len;
> +	char *p;
> +
> +	if (!rproc || !fw_name)
> +		return -EINVAL;
> +
> +	dev = rproc->dev.parent;
> +
> +	ret = mutex_lock_interruptible(&rproc->lock);
> +	if (ret) {
> +		dev_err(dev, "can't lock rproc %s: %d\n", rproc->name, ret);
> +		return -EINVAL;
> +	}
> +
> +	if (rproc->state != RPROC_OFFLINE) {
> +		dev_err(dev, "can't change firmware while running\n");
> +		ret = -EBUSY;
> +		goto out;
> +	}
> +
> +	len = strcspn(fw_name, "\n");
> +	if (!len) {
> +		dev_err(dev, "can't provide empty string for firmware name\n");
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	p = kstrndup(fw_name, len, GFP_KERNEL);
> +	if (!p) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +
> +	kfree(rproc->firmware);
> +	rproc->firmware = p;
> +
> +out:
> +	mutex_unlock(&rproc->lock);
> +	return ret;
> +}
> +EXPORT_SYMBOL(rproc_set_firmware);
> +
>  static int rproc_validate(struct rproc *rproc)
>  {
>  	switch (rproc->state) {
> diff --git a/drivers/remoteproc/remoteproc_sysfs.c
> b/drivers/remoteproc/remoteproc_sysfs.c
> index 3fd18a71c188..cf846caf2e1a 100644
> --- a/drivers/remoteproc/remoteproc_sysfs.c
> +++ b/drivers/remoteproc/remoteproc_sysfs.c
> @@ -159,42 +159,13 @@ static ssize_t firmware_store(struct device *dev,
>  			      const char *buf, size_t count)
>  {
>  	struct rproc *rproc = to_rproc(dev);
> -	char *p;
> -	int err, len = count;
> +	int err;
> 
>  	/* restrict sysfs operations if not allowed by remoteproc drivers */
>  	if (rproc->deny_sysfs_ops)
>  		return -EPERM;
> 
> -	err = mutex_lock_interruptible(&rproc->lock);
> -	if (err) {
> -		dev_err(dev, "can't lock rproc %s: %d\n", rproc->name, err);
> -		return -EINVAL;
> -	}
> -
> -	if (rproc->state != RPROC_OFFLINE) {
> -		dev_err(dev, "can't change firmware while running\n");
> -		err = -EBUSY;
> -		goto out;
> -	}
> -
> -	len = strcspn(buf, "\n");
> -	if (!len) {
> -		dev_err(dev, "can't provide a NULL firmware\n");
> -		err = -EINVAL;
> -		goto out;
> -	}
> -
> -	p = kstrndup(buf, len, GFP_KERNEL);
> -	if (!p) {
> -		err = -ENOMEM;
> -		goto out;
> -	}
> -
> -	kfree(rproc->firmware);
> -	rproc->firmware = p;
> -out:
> -	mutex_unlock(&rproc->lock);
> +	err = rproc_set_firmware(rproc, buf);
> 
>  	return err ? err : count;
>  }
> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
> index dbc3767f7d0e..6e04b99413f8 100644
> --- a/include/linux/remoteproc.h
> +++ b/include/linux/remoteproc.h
> @@ -655,6 +655,7 @@ rproc_of_resm_mem_entry_init(struct device *dev,
> u32 of_resm_idx, size_t len,
> 
>  int rproc_boot(struct rproc *rproc);
>  void rproc_shutdown(struct rproc *rproc);
> +int rproc_set_firmware(struct rproc *rproc, const char *fw_name);
>  void rproc_report_crash(struct rproc *rproc, enum rproc_crash_type 
> type);
>  int rproc_coredump_add_segment(struct rproc *rproc, dma_addr_t da,
> size_t size);
>  int rproc_coredump_add_custom_segment(struct rproc *rproc,

Reviewed-by: Rishabh Bhatnagar <rishabhb@codeaurora.org>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] remoteproc: Add a rproc_set_firmware() API
  2020-11-21  3:20 [PATCH] remoteproc: Add a rproc_set_firmware() API Suman Anna
  2020-11-21  3:33 ` Bjorn Andersson
  2020-11-25 18:12 ` rishabhb
@ 2020-11-26 22:56 ` Mathieu Poirier
  2 siblings, 0 replies; 4+ messages in thread
From: Mathieu Poirier @ 2020-11-26 22:56 UTC (permalink / raw)
  To: Suman Anna
  Cc: Bjorn Andersson, Grzegorz Jaszczyk, linux-remoteproc, linux-omap,
	linux-kernel

On Fri, Nov 20, 2020 at 09:20:42PM -0600, Suman Anna wrote:
> A new API, rproc_set_firmware() is added to allow the remoteproc platform
> drivers and remoteproc client drivers to be able to configure a custom
> firmware name that is different from the default name used during
> remoteproc registration. This function is being introduced to provide
> a kernel-level equivalent of the current sysfs interface to remoteproc
> client drivers, and can only change firmwares when the remoteproc is
> offline. This allows some remoteproc drivers to choose different firmwares
> at runtime based on the functionality the remote processor is providing.
> The TI PRU Ethernet driver will be an example of such usage as it
> requires to use different firmwares for different supported protocols.
> 
> Also, update the firmware_store() function used by the sysfs interface
> to reuse this function to avoid code duplication.
> 
> Signed-off-by: Suman Anna <s-anna@ti.com>
> ---
>  drivers/remoteproc/remoteproc_core.c  | 63 +++++++++++++++++++++++++++
>  drivers/remoteproc/remoteproc_sysfs.c | 33 +-------------
>  include/linux/remoteproc.h            |  1 +
>  3 files changed, 66 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index dab2c0f5caf0..46c2937ebea9 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -1934,6 +1934,69 @@ struct rproc *rproc_get_by_phandle(phandle phandle)
>  #endif
>  EXPORT_SYMBOL(rproc_get_by_phandle);
>  
> +/**
> + * rproc_set_firmware() - assign a new firmware
> + * @rproc: rproc handle to which the new firmware is being assigned
> + * @fw_name: new firmware name to be assigned
> + *
> + * This function allows remoteproc drivers or clients to configure a custom
> + * firmware name that is different from the default name used during remoteproc
> + * registration. The function does not trigger a remote processor boot,
> + * only sets the firmware name used for a subsequent boot. This function
> + * should also be called only when the remote processor is offline.
> + *
> + * This allows either the userspace to configure a different name through
> + * sysfs or a kernel-level remoteproc or a remoteproc client driver to set
> + * a specific firmware when it is controlling the boot and shutdown of the
> + * remote processor.
> + *
> + * Return: 0 on success or a negative value upon failure
> + */
> +int rproc_set_firmware(struct rproc *rproc, const char *fw_name)
> +{
> +	struct device *dev;
> +	int ret, len;
> +	char *p;
> +
> +	if (!rproc || !fw_name)
> +		return -EINVAL;
> +
> +	dev = rproc->dev.parent;

Since rproc->dev is available might as well use it.  This is what the current
implementation does.  The side effect are only cosmetic though so with or
without the change:

Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>

> +
> +	ret = mutex_lock_interruptible(&rproc->lock);
> +	if (ret) {
> +		dev_err(dev, "can't lock rproc %s: %d\n", rproc->name, ret);
> +		return -EINVAL;
> +	}
> +
> +	if (rproc->state != RPROC_OFFLINE) {
> +		dev_err(dev, "can't change firmware while running\n");
> +		ret = -EBUSY;
> +		goto out;
> +	}
> +
> +	len = strcspn(fw_name, "\n");
> +	if (!len) {
> +		dev_err(dev, "can't provide empty string for firmware name\n");
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	p = kstrndup(fw_name, len, GFP_KERNEL);
> +	if (!p) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +
> +	kfree(rproc->firmware);
> +	rproc->firmware = p;
> +
> +out:
> +	mutex_unlock(&rproc->lock);
> +	return ret;
> +}
> +EXPORT_SYMBOL(rproc_set_firmware);
> +
>  static int rproc_validate(struct rproc *rproc)
>  {
>  	switch (rproc->state) {
> diff --git a/drivers/remoteproc/remoteproc_sysfs.c b/drivers/remoteproc/remoteproc_sysfs.c
> index 3fd18a71c188..cf846caf2e1a 100644
> --- a/drivers/remoteproc/remoteproc_sysfs.c
> +++ b/drivers/remoteproc/remoteproc_sysfs.c
> @@ -159,42 +159,13 @@ static ssize_t firmware_store(struct device *dev,
>  			      const char *buf, size_t count)
>  {
>  	struct rproc *rproc = to_rproc(dev);
> -	char *p;
> -	int err, len = count;
> +	int err;
>  
>  	/* restrict sysfs operations if not allowed by remoteproc drivers */
>  	if (rproc->deny_sysfs_ops)
>  		return -EPERM;
>  
> -	err = mutex_lock_interruptible(&rproc->lock);
> -	if (err) {
> -		dev_err(dev, "can't lock rproc %s: %d\n", rproc->name, err);
> -		return -EINVAL;
> -	}
> -
> -	if (rproc->state != RPROC_OFFLINE) {
> -		dev_err(dev, "can't change firmware while running\n");
> -		err = -EBUSY;
> -		goto out;
> -	}
> -
> -	len = strcspn(buf, "\n");
> -	if (!len) {
> -		dev_err(dev, "can't provide a NULL firmware\n");
> -		err = -EINVAL;
> -		goto out;
> -	}
> -
> -	p = kstrndup(buf, len, GFP_KERNEL);
> -	if (!p) {
> -		err = -ENOMEM;
> -		goto out;
> -	}
> -
> -	kfree(rproc->firmware);
> -	rproc->firmware = p;
> -out:
> -	mutex_unlock(&rproc->lock);
> +	err = rproc_set_firmware(rproc, buf);
>  
>  	return err ? err : count;
>  }
> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
> index dbc3767f7d0e..6e04b99413f8 100644
> --- a/include/linux/remoteproc.h
> +++ b/include/linux/remoteproc.h
> @@ -655,6 +655,7 @@ rproc_of_resm_mem_entry_init(struct device *dev, u32 of_resm_idx, size_t len,
>  
>  int rproc_boot(struct rproc *rproc);
>  void rproc_shutdown(struct rproc *rproc);
> +int rproc_set_firmware(struct rproc *rproc, const char *fw_name);
>  void rproc_report_crash(struct rproc *rproc, enum rproc_crash_type type);
>  int rproc_coredump_add_segment(struct rproc *rproc, dma_addr_t da, size_t size);
>  int rproc_coredump_add_custom_segment(struct rproc *rproc,
> -- 
> 2.28.0
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-11-26 22:57 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-21  3:20 [PATCH] remoteproc: Add a rproc_set_firmware() API Suman Anna
2020-11-21  3:33 ` Bjorn Andersson
2020-11-25 18:12 ` rishabhb
2020-11-26 22:56 ` Mathieu Poirier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).