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

[PATCH v2 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().

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 | 41 +++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)


base-commit: e19967994d342a5986d950a1bfddf19d7e1191b7
-- 
2.25.1

[PATCH v2 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 v2 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().

Signed-off-by: Tarak Reddy <tarak.reddy@amd.com>
Signed-off-by: Tanmay Shah <tanmay.shah@amd.com>
---
 drivers/remoteproc/remoteproc_core.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index a3e7c8798381..e7e451012615 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -2560,6 +2560,19 @@ 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) {
+		if (rproc->dev.parent->driver) {
+			module_put(rproc->dev.parent->driver->owner);
+		} else {
+			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 v2 2/2] remoteproc: enhance rproc_put() for clusters

[kernel test robot]

Hi Tanmay,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on e19967994d342a5986d950a1bfddf19d7e1191b7]

url:    https://github.com/intel-lab-lkp/linux/commits/Tanmay-Shah/remoteproc-Make-rproc_get_by_phandle-work-for-clusters/20230322-121102
base:   e19967994d342a5986d950a1bfddf19d7e1191b7
patch link:    https://lore.kernel.org/r/20230322040933.924813-3-tanmay.shah%40amd.com
patch subject: [PATCH v2 2/2] remoteproc: enhance rproc_put() for clusters
config: arm-randconfig-r013-20230322 (https://download.01.org/0day-ci/archive/20230322/202303221441.cuBnpvye-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project 67409911353323ca5edf2049ef0df54132fa1ca7)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # https://github.com/intel-lab-lkp/linux/commit/573d22d13a697097d02d6c29a75fb0fb1ac6d8fe
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Tanmay-Shah/remoteproc-Make-rproc_get_by_phandle-work-for-clusters/20230322-121102
        git checkout 573d22d13a697097d02d6c29a75fb0fb1ac6d8fe
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash drivers/remoteproc/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303221441.cuBnpvye-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/remoteproc/remoteproc_core.c:2563:26: warning: mixing declarations and code is incompatible with standards before C99 [-Wdeclaration-after-statement]
           struct platform_device *cluster_pdev;
                                   ^
   1 warning generated.


vim +2563 drivers/remoteproc/remoteproc_core.c

  2550	
  2551	/**
  2552	 * rproc_put() - release rproc reference
  2553	 * @rproc: the remote processor handle
  2554	 *
  2555	 * This function decrements the rproc dev refcount.
  2556	 *
  2557	 * If no one holds any reference to rproc anymore, then its refcount would
  2558	 * now drop to zero, and it would be freed.
  2559	 */
  2560	void rproc_put(struct rproc *rproc)
  2561	{
  2562		module_put(rproc->dev.parent->driver->owner);
> 2563		struct platform_device *cluster_pdev;
  2564	
  2565		if (rproc->dev.parent) {
  2566			if (rproc->dev.parent->driver) {
  2567				module_put(rproc->dev.parent->driver->owner);
  2568			} else {
  2569				cluster_pdev = of_find_device_by_node(rproc->dev.parent->of_node->parent);
  2570				if (cluster_pdev) {
  2571					module_put(cluster_pdev->dev.driver->owner);
  2572					put_device(&cluster_pdev->dev);
  2573				}
  2574			}
  2575		}
  2576		put_device(&rproc->dev);
  2577	}
  2578	EXPORT_SYMBOL(rproc_put);
  2579	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

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

[kernel test robot]

Hi Tanmay,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on e19967994d342a5986d950a1bfddf19d7e1191b7]

url:    https://github.com/intel-lab-lkp/linux/commits/Tanmay-Shah/remoteproc-Make-rproc_get_by_phandle-work-for-clusters/20230322-121102
base:   e19967994d342a5986d950a1bfddf19d7e1191b7
patch link:    https://lore.kernel.org/r/20230322040933.924813-3-tanmay.shah%40amd.com
patch subject: [PATCH v2 2/2] remoteproc: enhance rproc_put() for clusters
config: ia64-allyesconfig (https://download.01.org/0day-ci/archive/20230322/202303221514.3xIiCbpk-lkp@intel.com/config)
compiler: ia64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/573d22d13a697097d02d6c29a75fb0fb1ac6d8fe
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Tanmay-Shah/remoteproc-Make-rproc_get_by_phandle-work-for-clusters/20230322-121102
        git checkout 573d22d13a697097d02d6c29a75fb0fb1ac6d8fe
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 SHELL=/bin/bash drivers/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303221514.3xIiCbpk-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/remoteproc/remoteproc_core.c: In function 'rproc_put':
>> drivers/remoteproc/remoteproc_core.c:2563:9: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
    2563 |         struct platform_device *cluster_pdev;
         |         ^~~~~~


vim +2563 drivers/remoteproc/remoteproc_core.c

  2550	
  2551	/**
  2552	 * rproc_put() - release rproc reference
  2553	 * @rproc: the remote processor handle
  2554	 *
  2555	 * This function decrements the rproc dev refcount.
  2556	 *
  2557	 * If no one holds any reference to rproc anymore, then its refcount would
  2558	 * now drop to zero, and it would be freed.
  2559	 */
  2560	void rproc_put(struct rproc *rproc)
  2561	{
  2562		module_put(rproc->dev.parent->driver->owner);
> 2563		struct platform_device *cluster_pdev;
  2564	
  2565		if (rproc->dev.parent) {
  2566			if (rproc->dev.parent->driver) {
  2567				module_put(rproc->dev.parent->driver->owner);
  2568			} else {
  2569				cluster_pdev = of_find_device_by_node(rproc->dev.parent->of_node->parent);
  2570				if (cluster_pdev) {
  2571					module_put(cluster_pdev->dev.driver->owner);
  2572					put_device(&cluster_pdev->dev);
  2573				}
  2574			}
  2575		}
  2576		put_device(&rproc->dev);
  2577	}
  2578	EXPORT_SYMBOL(rproc_put);
  2579	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

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

[Dan Carpenter]

Hi Tanmay,

url:    https://github.com/intel-lab-lkp/linux/commits/Tanmay-Shah/remoteproc-Make-rproc_get_by_phandle-work-for-clusters/20230322-121102
base:   e19967994d342a5986d950a1bfddf19d7e1191b7
patch link:    https://lore.kernel.org/r/20230322040933.924813-3-tanmay.shah%40amd.com
patch subject: [PATCH v2 2/2] remoteproc: enhance rproc_put() for clusters
config: powerpc-randconfig-m041-20230322 (https://download.01.org/0day-ci/archive/20230322/202303221916.LgKkr8Gk-lkp@intel.com/config)
compiler: powerpc-linux-gcc (GCC) 12.1.0

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Link: https://lore.kernel.org/r/202303221916.LgKkr8Gk-lkp@intel.com/

smatch warnings:
drivers/remoteproc/remoteproc_core.c:2565 rproc_put() warn: variable dereferenced before check 'rproc->dev.parent' (see line 2562)
drivers/remoteproc/remoteproc_core.c:2566 rproc_put() warn: variable dereferenced before check 'rproc->dev.parent->driver' (see line 2562)

vim +2565 drivers/remoteproc/remoteproc_core.c

160e7c840fe858 Ohad Ben-Cohen  2012-07-04  2560  void rproc_put(struct rproc *rproc)
400e64df6b237e Ohad Ben-Cohen  2011-10-20  2561  {
fbb6aacb078285 Bjorn Andersson 2016-10-02 @2562  	module_put(rproc->dev.parent->driver->owner);
                                                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^
Unchecked dereferences.

573d22d13a6970 Tanmay Shah     2023-03-21  2563  	struct platform_device *cluster_pdev;
573d22d13a6970 Tanmay Shah     2023-03-21  2564  
573d22d13a6970 Tanmay Shah     2023-03-21 @2565  	if (rproc->dev.parent) {
                                                            ^^^^^^^^^^^^^^^^^
Checked too late.

573d22d13a6970 Tanmay Shah     2023-03-21 @2566  		if (rproc->dev.parent->driver) {
                                                                    ^^^^^^^^^^^^^^^^^^^^^^^^^

573d22d13a6970 Tanmay Shah     2023-03-21  2567  			module_put(rproc->dev.parent->driver->owner);
573d22d13a6970 Tanmay Shah     2023-03-21  2568  		} else {
573d22d13a6970 Tanmay Shah     2023-03-21  2569  			cluster_pdev = of_find_device_by_node(rproc->dev.parent->of_node->parent);
573d22d13a6970 Tanmay Shah     2023-03-21  2570  			if (cluster_pdev) {
573d22d13a6970 Tanmay Shah     2023-03-21  2571  				module_put(cluster_pdev->dev.driver->owner);
573d22d13a6970 Tanmay Shah     2023-03-21  2572  				put_device(&cluster_pdev->dev);
573d22d13a6970 Tanmay Shah     2023-03-21  2573  			}
573d22d13a6970 Tanmay Shah     2023-03-21  2574  		}
573d22d13a6970 Tanmay Shah     2023-03-21  2575  	}
b5ab5e24e960b9 Ohad Ben-Cohen  2012-05-30  2576  	put_device(&rproc->dev);
400e64df6b237e Ohad Ben-Cohen  2011-10-20  2577  }

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

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

[Bjorn Andersson]

On Tue, Mar 21, 2023 at 09:09:32PM -0700, Tanmay Shah wrote:
> This series extends original patch and makes rproc_put() work
> for clusters along with rprog_get_by_phandle().
> 
> v1 is here: https://lore.kernel.org/all/20221214221643.1286585-1-mathieu.poirier@linaro.org/
> 

What changed since v1?

Thanks,
Bjorn

> 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 | 41 +++++++++++++++++++++++++++-
>  1 file changed, 40 insertions(+), 1 deletion(-)
> 
> 
> base-commit: e19967994d342a5986d950a1bfddf19d7e1191b7
> -- 
> 2.25.1
> 

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

[Mathieu Poirier]

Hi Tanmay,

On Tue, Mar 21, 2023 at 09:09:36PM -0700, Tanmay Shah wrote:
> This patch enhances rproc_put() to support remoteproc clusters
> with multiple child nodes as in rproc_get_by_phandle().
> 
> Signed-off-by: Tarak Reddy <tarak.reddy@amd.com>
> Signed-off-by: Tanmay Shah <tanmay.shah@amd.com>
> ---
>  drivers/remoteproc/remoteproc_core.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index a3e7c8798381..e7e451012615 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -2560,6 +2560,19 @@ EXPORT_SYMBOL(rproc_free);
>  void rproc_put(struct rproc *rproc)
>  {
>  	module_put(rproc->dev.parent->driver->owner);

There is something wrong here - this should have been removed.

> +	struct platform_device *cluster_pdev;
> +
> +	if (rproc->dev.parent) {

This condition is not needed, please remove.

> +		if (rproc->dev.parent->driver) {
> +			module_put(rproc->dev.parent->driver->owner);
> +		} else {
> +			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);
> +			}
> +		}
> +	}

Some in-lined documentation, the way I did in patch 1/2 would be appreciated.
Otherwize I think the above enhancement make sense.

Thanks,
Mathieu

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

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

[Mathieu Poirier]

On Wed, Mar 22, 2023 at 07:12:15AM -0700, Bjorn Andersson wrote:
> On Tue, Mar 21, 2023 at 09:09:32PM -0700, Tanmay Shah wrote:
> > This series extends original patch and makes rproc_put() work
> > for clusters along with rprog_get_by_phandle().
> > 
> > v1 is here: https://lore.kernel.org/all/20221214221643.1286585-1-mathieu.poirier@linaro.org/
> > 
> 
> What changed since v1?
>

Path 2/2 was not part of the original submission.  Speaking of that original
submission, you did not follow-up on my last comment, something that would be
much appreciated in order to move forward with this fix.

Thanks,
Mathieu


> Thanks,
> Bjorn
> 
> > 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 | 41 +++++++++++++++++++++++++++-
> >  1 file changed, 40 insertions(+), 1 deletion(-)
> > 
> > 
> > base-commit: e19967994d342a5986d950a1bfddf19d7e1191b7
> > -- 
> > 2.25.1
> > 

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

[Tanmay Shah]

On 3/22/23 9:05 AM, Mathieu Poirier wrote:
> Hi Tanmay,
>
> On Tue, Mar 21, 2023 at 09:09:36PM -0700, Tanmay Shah wrote:
>> This patch enhances rproc_put() to support remoteproc clusters
>> with multiple child nodes as in rproc_get_by_phandle().
>>
>> Signed-off-by: Tarak Reddy <tarak.reddy@amd.com>
>> Signed-off-by: Tanmay Shah <tanmay.shah@amd.com>
>> ---
>>   drivers/remoteproc/remoteproc_core.c | 13 +++++++++++++
>>   1 file changed, 13 insertions(+)
>>
>> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
>> index a3e7c8798381..e7e451012615 100644
>> --- a/drivers/remoteproc/remoteproc_core.c
>> +++ b/drivers/remoteproc/remoteproc_core.c
>> @@ -2560,6 +2560,19 @@ EXPORT_SYMBOL(rproc_free);
>>   void rproc_put(struct rproc *rproc)
>>   {
>>   	module_put(rproc->dev.parent->driver->owner);
> There is something wrong here - this should have been removed.


Thanks Mathieu. Sure this needs to be fixed.

This is result of manually picking up patch from my side.

I will try to find better automated way to pick-up patches not available 
on mailing list.


>
>> +	struct platform_device *cluster_pdev;
>> +
>> +	if (rproc->dev.parent) {
> This condition is not needed, please remove.
Ack.
>
>> +		if (rproc->dev.parent->driver) {
>> +			module_put(rproc->dev.parent->driver->owner);
>> +		} else {
>> +			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);

I am not sure if cluster_pdev->dev should be dropped here.

Should we drop it in platform driver after rproc_free() ?

>> +			}
>> +		}
>> +	}
> Some in-lined documentation, the way I did in patch 1/2 would be appreciated.
> Otherwize I think the above enhancement make sense.
Ack I will document in next revision.
>
> Thanks,
> Mathieu
>
>>   	put_device(&rproc->dev);


Also, if we decide to drop cluster->dev here  then,

should we drop reference of rproc->dev before cluster->dev ?


>>   }
>>   EXPORT_SYMBOL(rproc_put);
>> -- 
>> 2.25.1
>>

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

[Tanmay Shah]

On 3/22/23 7:12 AM, Bjorn Andersson wrote:
> On Tue, Mar 21, 2023 at 09:09:32PM -0700, Tanmay Shah wrote:
>> This series extends original patch and makes rproc_put() work
>> for clusters along with rprog_get_by_phandle().
>>
>> v1 is here: https://lore.kernel.org/all/20221214221643.1286585-1-mathieu.poirier@linaro.org/
>>
> What changed since v1?


Hello Bjorn,

Thanks for reviews.

rproc_put() is fixed in v2. I will add proper change-log in cover-letter 
in next revision.


Thanks,


> Thanks,
> Bjorn
>
>> 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 | 41 +++++++++++++++++++++++++++-
>>   1 file changed, 40 insertions(+), 1 deletion(-)
>>
>>
>> base-commit: e19967994d342a5986d950a1bfddf19d7e1191b7
>> -- 
>> 2.25.1
>>

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

[Mathieu Poirier]

On Wed, Mar 22, 2023 at 10:34:57AM -0700, Tanmay Shah wrote:
> 
> On 3/22/23 9:05 AM, Mathieu Poirier wrote:
> > Hi Tanmay,
> > 
> > On Tue, Mar 21, 2023 at 09:09:36PM -0700, Tanmay Shah wrote:
> > > This patch enhances rproc_put() to support remoteproc clusters
> > > with multiple child nodes as in rproc_get_by_phandle().
> > > 
> > > Signed-off-by: Tarak Reddy <tarak.reddy@amd.com>
> > > Signed-off-by: Tanmay Shah <tanmay.shah@amd.com>
> > > ---
> > >   drivers/remoteproc/remoteproc_core.c | 13 +++++++++++++
> > >   1 file changed, 13 insertions(+)
> > > 
> > > diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> > > index a3e7c8798381..e7e451012615 100644
> > > --- a/drivers/remoteproc/remoteproc_core.c
> > > +++ b/drivers/remoteproc/remoteproc_core.c
> > > @@ -2560,6 +2560,19 @@ EXPORT_SYMBOL(rproc_free);
> > >   void rproc_put(struct rproc *rproc)
> > >   {
> > >   	module_put(rproc->dev.parent->driver->owner);
> > There is something wrong here - this should have been removed.
> 
> 
> Thanks Mathieu. Sure this needs to be fixed.
> 
> This is result of manually picking up patch from my side.
> 
> I will try to find better automated way to pick-up patches not available on
> mailing list.
>

That would certainly help avoid problems such as this one.

> 
> > 
> > > +	struct platform_device *cluster_pdev;
> > > +
> > > +	if (rproc->dev.parent) {
> > This condition is not needed, please remove.
> Ack.
> > 
> > > +		if (rproc->dev.parent->driver) {
> > > +			module_put(rproc->dev.parent->driver->owner);
> > > +		} else {
> > > +			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);
> 
> I am not sure if cluster_pdev->dev should be dropped here.
>

It needs to be done here.

> Should we drop it in platform driver after rproc_free() ?
> 
> > > +			}
> > > +		}
> > > +	}
> > Some in-lined documentation, the way I did in patch 1/2 would be appreciated.
> > Otherwize I think the above enhancement make sense.
> Ack I will document in next revision.
> > 
> > Thanks,
> > Mathieu
> > 
> > >   	put_device(&rproc->dev);
> 
> 
> Also, if we decide to drop cluster->dev here  then,
> 
> should we drop reference of rproc->dev before cluster->dev ?
> 
> 
> > >   }
> > >   EXPORT_SYMBOL(rproc_put);
> > > -- 
> > > 2.25.1
> > >