[PATCH v3 0/2] remoteproc: get rproc devices for clusters

[PATCH v3 0/2] remoteproc: get rproc devices for clusters

[Tanmay Shah]

This series extends original patch and makes rproc_put() work
for clusters along with rprog_get_by_phandle().

Changes in v3:
  - remove module_put call that was introduced in the patch by mistake
  - remove redundant check in rproc_put
  - Add inline comments in rproc_put that explains functionality

Changes in v2:
  - Introduce patch to fix rproc_put as per modified rproc_get_by_phandle

v1 is here: https://lore.kernel.org/all/20221214221643.1286585-1-mathieu.poirier@linaro.org/

Mathieu Poirier (1):
  remoteproc: Make rproc_get_by_phandle() work for clusters

Tanmay Shah (1):
  remoteproc: enhance rproc_put() for clusters

 drivers/remoteproc/remoteproc_core.c | 45 ++++++++++++++++++++++++++--
 1 file changed, 43 insertions(+), 2 deletions(-)


base-commit: e19967994d342a5986d950a1bfddf19d7e1191b7
-- 
2.25.1

[PATCH v3 1/2] remoteproc: Make rproc_get_by_phandle() work for clusters

[Tanmay Shah]

From: Mathieu Poirier <mathieu.poirier@linaro.org>

Multi-cluster remoteproc designs typically have the following DT
declaration:

	remoteproc_cluster {
		compatible = "soc,remoteproc-cluster";

                core0: core0 {
			compatible = "soc,remoteproc-core"
                        memory-region;
                        sram;
                };

                core1: core1 {
			compatible = "soc,remoteproc-core"
                        memory-region;
                        sram;
                }
        };

A driver exists for the cluster rather than the individual cores
themselves so that operation mode and HW specific configurations
applicable to the cluster can be made.

Because the driver exists at the cluster level and not the individual
core level, function rproc_get_by_phandle() fails to return the
remoteproc associated with the phandled it is called for.

This patch enhances rproc_get_by_phandle() by looking for the cluster's
driver when the driver for the immediate remoteproc's parent is not
found.

Reported-by: Ben Levinsky <ben.levinsky@xilinx.com>
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Tested-by: Ben Levinsky <ben.levinsky@amd.com>
---
 drivers/remoteproc/remoteproc_core.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 80072b6b6283..a3e7c8798381 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -33,6 +33,7 @@
 #include <linux/idr.h>
 #include <linux/elf.h>
 #include <linux/crc32.h>
+#include <linux/of_platform.h>
 #include <linux/of_reserved_mem.h>
 #include <linux/virtio_ids.h>
 #include <linux/virtio_ring.h>
@@ -2111,7 +2112,9 @@ EXPORT_SYMBOL(rproc_detach);
 #ifdef CONFIG_OF
 struct rproc *rproc_get_by_phandle(phandle phandle)
 {
+	struct platform_device *cluster_pdev;
 	struct rproc *rproc = NULL, *r;
+	struct device_driver *driver;
 	struct device_node *np;
 
 	np = of_find_node_by_phandle(phandle);
@@ -2122,7 +2125,30 @@ struct rproc *rproc_get_by_phandle(phandle phandle)
 	list_for_each_entry_rcu(r, &rproc_list, node) {
 		if (r->dev.parent && device_match_of_node(r->dev.parent, np)) {
 			/* prevent underlying implementation from being removed */
-			if (!try_module_get(r->dev.parent->driver->owner)) {
+
+			/*
+			 * If the remoteproc's parent has a driver, the
+			 * remoteproc is not part of a cluster and we can use
+			 * that driver.
+			 */
+			driver = r->dev.parent->driver;
+
+			/*
+			 * If the remoteproc's parent does not have a driver,
+			 * look for the driver associated with the cluster.
+			 */
+			if (!driver) {
+				cluster_pdev = of_find_device_by_node(np->parent);
+				if (!cluster_pdev) {
+					dev_err(&r->dev, "can't get parent\n");
+					break;
+				}
+
+				driver = cluster_pdev->dev.driver;
+				put_device(&cluster_pdev->dev);
+			}
+
+			if (!try_module_get(driver->owner)) {
 				dev_err(&r->dev, "can't get owner\n");
 				break;
 			}
-- 
2.25.1

[PATCH v3 2/2] remoteproc: enhance rproc_put() for clusters

[Tanmay Shah]

This patch enhances rproc_put() to support remoteproc clusters
with multiple child nodes as in rproc_get_by_phandle().

Reported-by: kernel test robot <lkp@intel.com>
Link: https://lore.kernel.org/oe-kbuild-all/202303221441.cuBnpvye-lkp@intel.com/
Signed-off-by: Tarak Reddy <tarak.reddy@amd.com>
Signed-off-by: Tanmay Shah <tanmay.shah@amd.com>
---

Changes in v3:
  - remove module_put call that was introduced in the patch by mistake
  - remove redundant check in rproc_put
  - Add inline comments in rproc_put that explains functionality

Changes in v2:
  - Introduce this patch to fix rproc_put as per modified rproc_get_by_phandle

 drivers/remoteproc/remoteproc_core.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index a3e7c8798381..d8f28c6fe3a3 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -2559,7 +2559,22 @@ EXPORT_SYMBOL(rproc_free);
  */
 void rproc_put(struct rproc *rproc)
 {
-	module_put(rproc->dev.parent->driver->owner);
+	struct platform_device *cluster_pdev;
+
+	if (rproc->dev.parent->driver) {
+		module_put(rproc->dev.parent->driver->owner);
+	} else {
+		/*
+		 * If the remoteproc's parent does not have a driver,
+		 * driver is associated with the cluster.
+		 */
+		cluster_pdev = of_find_device_by_node(rproc->dev.parent->of_node->parent);
+		if (cluster_pdev) {
+			module_put(cluster_pdev->dev.driver->owner);
+			put_device(&cluster_pdev->dev);
+		}
+	}
+
 	put_device(&rproc->dev);
 }
 EXPORT_SYMBOL(rproc_put);
-- 
2.25.1

Re: [PATCH v3 2/2] remoteproc: enhance rproc_put() for clusters

[Mathieu Poirier]

On Tue, Mar 28, 2023 at 09:49:24AM -0700, Tanmay Shah wrote:
> This patch enhances rproc_put() to support remoteproc clusters
> with multiple child nodes as in rproc_get_by_phandle().
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Link: https://lore.kernel.org/oe-kbuild-all/202303221441.cuBnpvye-lkp@intel.com/
> Signed-off-by: Tarak Reddy <tarak.reddy@amd.com>
> Signed-off-by: Tanmay Shah <tanmay.shah@amd.com>
> ---
> 
> Changes in v3:
>   - remove module_put call that was introduced in the patch by mistake
>   - remove redundant check in rproc_put
>   - Add inline comments in rproc_put that explains functionality
> 
> Changes in v2:
>   - Introduce this patch to fix rproc_put as per modified rproc_get_by_phandle
> 
>  drivers/remoteproc/remoteproc_core.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index a3e7c8798381..d8f28c6fe3a3 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -2559,7 +2559,22 @@ EXPORT_SYMBOL(rproc_free);
>   */
>  void rproc_put(struct rproc *rproc)
>  {
> -	module_put(rproc->dev.parent->driver->owner);
> +	struct platform_device *cluster_pdev;
> +
> +	if (rproc->dev.parent->driver) {
> +		module_put(rproc->dev.parent->driver->owner);
> +	} else {
> +		/*
> +		 * If the remoteproc's parent does not have a driver,
> +		 * driver is associated with the cluster.
> +		 */
> +		cluster_pdev = of_find_device_by_node(rproc->dev.parent->of_node->parent);
> +		if (cluster_pdev) {
> +			module_put(cluster_pdev->dev.driver->owner);
> +			put_device(&cluster_pdev->dev);
> +		}
> +	}
> +

This looks good to me now.

Bjorn, please have a look at this set when you have a minute.

Thanks,
Mathieu

>  	put_device(&rproc->dev);
>  }
>  EXPORT_SYMBOL(rproc_put);
> -- 
> 2.25.1
>