Received-SPF: pass (google.com: domain of linux-kernel+bounces-57251-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Message-ID: <0f4905b4-9972-40ce-85f4-0880f273d7dc@amd.com>
Date: Wed, 7 Feb 2024 16:18:50 -0600
User-Agent: Mozilla Thunderbird Beta
Subject: Re: [PATCH v5] remoteproc: Make rproc_get_by_phandle() work for
 clusters
Content-Language: en-US
To: andersson@kernel.org, mathieu.poirier@linaro.org
Cc: linux-remoteproc@vger.kernel.org, linux-kernel@vger.kernel.org,
 Ben Levinsky <ben.levinsky@xilinx.com>, Tarak Reddy <tarak.reddy@amd.com>
References: <20240130154849.1018666-1-tanmay.shah@amd.com>
From: Tanmay Shah <tanmay.shah@amd.com>
In-Reply-To: <20240130154849.1018666-1-tanmay.shah@amd.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?dzFYNytzdmRXSDY2UU5DNXlKcUhxd0xuT1phaXdoVG04ZkhTNlpUTzVEdFI5?=
 =?utf-8?B?Q3FuaURDK0NXOTJMZmhueW1uWHUzZEQvQ2duNlhCdVFvbzkrQ2QyZWw5NzFi?=
 =?utf-8?B?bEV4UHkzemVDbEJMOWZyVnRKNjFwM090ZGx5MTh4WnI4THE1MyszY3BwMFlk?=
 =?utf-8?B?WnNPb3hDcjBJTmQybStWNFVxaDczL0NDcjV3ODY3Ykw0azl4QlVpdXhONVpa?=
 =?utf-8?B?cGwzRFIwZDd6SnU2UWx1TktYQlFxUDg1dDFpSHBrUXh6Q0VWOU1UU3V5VGk2?=
 =?utf-8?B?L1BKamVNd3c0UVNMZW85cG4xYmR6SnlVU2p3U2ZSblFrTGpZOWR4dEgvNTV2?=
 =?utf-8?B?Qi9LVExtckhvVkRlOVFZc3U3YUtWRE1QVm1wS2c0U2c0aTBwVTROUmlmZGla?=
 =?utf-8?B?QUpxYVZXR1NVblozOEVCcTFqUUVuZGVsaXQxcW1GL2RiQlZERkZtbVV5cTF4?=
 =?utf-8?B?Y3Jwb2FOVTUzR0lXU2dlNm1JWXZ4a1JUd2RYY1M4RjVqU2pUemN6OEJNL0RY?=
 =?utf-8?B?MDZ6ckV3dGNWWEw2VFoxYjduNnJ4YkZoODc0Sm8zSitXc1dZTEZ2dTdWMVlU?=
 =?utf-8?B?bUE4M01tdklobnRVSWhVblc4WUhhcXpYRG5iSTZvaUNRTXo0Q241bkZySUxI?=
 =?utf-8?B?OS9JUEVqZU9Vem0wUHpKTWdmMHFLY093SzQzVksrelFOTW9jRzg2SWVKREV6?=
 =?utf-8?B?dDQ3SjFoZ1d1bGJQc0F3czlZS2hDbjF5Q0VZY1lnYWJYVm1DanprM2doMnp1?=
 =?utf-8?B?d2ZCL1NzUnozbU93dnI4ZWFEMGZ0eXVVVS92WG9HelRxeXRxT0ZxSjZWWml0?=
 =?utf-8?B?aXVXOWl0KzRoNEhackdIQ1hVVFBERHppeDhkRkhLNy9LaFM0VFJHaXZCekVs?=
 =?utf-8?B?Y0tidzQ1R1BhZEZ2b1FOL0pZNEorUUVOeFNKeFp6UENXM1VoVTdmU0poN3hT?=
 =?utf-8?B?YVBlLytyNUFkZTFCYXdWaS9KaUdnL1N5QVF1YlVpeUlFT1JQaUZReHpzZGRp?=
 =?utf-8?B?SnVGbGRzM2hseUJxL3RTdUdqd1o0dTJsUmNKVXVlUW9ZOXZOb0xEcmhkaFVk?=
 =?utf-8?B?TDByNVVZMVF6dEkya3FPV2ErdGNrUlZwVXU5SnpxM1o1dkpjZXpycHdvbEdR?=
 =?utf-8?B?aDcrU3hIeXdscFpIdlhTL3N6Z2FzZTFvYUkzTjVjU09JZ0M3UVJINUdqSlRl?=
 =?utf-8?B?Z2MyODJOWjNmck5IUGdsd3RNcysrTkhYbnp6TnFDODJYUkFQRlNvUlhCOTRW?=
 =?utf-8?B?M0hmd0JtNzROTE1QWWNveVlJS1JyUnlHMjRZMEptZjZpRXFQVHZVdHphMksr?=
 =?utf-8?B?c0VQTUYyd3BoUFRRNU11aGhJR2E0emdHMjlGT3dFSVVBWTFqTVg3bHZBY3Bw?=
 =?utf-8?B?S2s1aVhad1dKbzJhUzN2YTk0dFFnUm0xV0hOZUlQQmhaWGFTbWhEa1A1RmdE?=
 =?utf-8?B?QzNmaXMzQWh6blExZlM4SkpGbStpL0Z1U0YvZ2hWdmRvWVNVNHZ6b2IwaWlI?=
 =?utf-8?B?T2o5QzZyQUNuS0xqN3pzQmpIazdSZFVxNmFzZjBlcUR1T3hpei9oWlpkZTd1?=
 =?utf-8?B?R0Zwa3ZvTlpYUFZXaENtOU51cksrc295QjRYekxucGxMREVzVVVxNzZPY3hq?=
 =?utf-8?B?bHNMMlJjbnQxQnVKKzhMbXVvUGdVcnVVaFVXNG5NalBPSWhaeFkzSEVSZVZh?=
 =?utf-8?B?dEVRcG9HNVZFWjVpSDRKUnN6ZVFVRWUxYWY4WHB1UzFRczlQenlqMFU3c0p6?=
 =?utf-8?B?OGNmWWZISWV1NUdkaXpTTlFzK1JyK1drWHNGbGlOV281dFZteElDLzczb09M?=
 =?utf-8?B?MTYzMXBKci9VaXNMamtFaGRWSFZxQkNNL3dzN0RScGp4K3JUcGtHdUVTTUNP?=
 =?utf-8?B?amZBTVJtNUwwUmkxcTFTcTNLRTZuMVFPbitzbllQdkFrL1FXY3ErcUtFZlhj?=
 =?utf-8?B?RHN1ODFSOUJQODBxSkgzQ3R5NXAraUMrU3pYZHdGVzhrbk1nL3d1cWlDa0pY?=
 =?utf-8?B?WTZFMWpDdXMzbVRQQnlySWpvZ2hSVEt3QTBUQ05HQmhwTWhrMHI5QnBJNFpE?=
 =?utf-8?B?cWFWbW9Ga25nc25Eb3ZRbFpTalZDYUlWbUJRSWs1VU9nYXpWcHZFbzNjT1lQ?=
 =?utf-8?Q?oA+cJ5GN7rmmKKzg6acSikF8E?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 78db755c-bf59-4da7-c367-08dc282ac4c5
X-MS-Exchange-CrossTenant-AuthSource: BL1PR12MB5874.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Feb 2024 22:18:53.6700
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: HTjXHuLjeauTGqKTycMQeLzDoDuswx9ewZSHc4Ocmt3u8aobajvlspvrRmVXCsHS
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH3PR12MB8935

Rejected-by: Tanmay Shah <tanmay.shah@amd.com>

I will send new v5 with change long included.

On 1/30/24 9:48 AM, Tanmay Shah wrote:
> 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>
> Co-developed-by: Tarak Reddy <tarak.reddy@amd.com>
> Signed-off-by: Tarak Reddy <tarak.reddy@amd.com>
> Co-developed-by: Tanmay Shah <tanmay.shah@amd.com>
> Signed-off-by: Tanmay Shah <tanmay.shah@amd.com>
> ---
>  drivers/remoteproc/remoteproc_core.c | 29 ++++++++++++++++++++++++++--
>  1 file changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index 695cce218e8c..f276956f2c5c 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>
> @@ -2112,6 +2113,7 @@ EXPORT_SYMBOL(rproc_detach);
>  struct rproc *rproc_get_by_phandle(phandle phandle)
>  {
>  	struct rproc *rproc = NULL, *r;
> +	struct device_driver *driver;
>  	struct device_node *np;
>  
>  	np = of_find_node_by_phandle(phandle);
> @@ -2122,7 +2124,26 @@ 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) {
> +				if (r->dev.parent->parent)
> +					driver = r->dev.parent->parent->driver;
> +				if (!driver)
> +					break;
> +			}
> +
> +			if (!try_module_get(driver->owner)) {
>  				dev_err(&r->dev, "can't get owner\n");
>  				break;
>  			}
> @@ -2533,7 +2554,11 @@ EXPORT_SYMBOL(rproc_free);
>   */
>  void rproc_put(struct rproc *rproc)
>  {
> -	module_put(rproc->dev.parent->driver->owner);
> +	if (rproc->dev.parent->driver)
> +		module_put(rproc->dev.parent->driver->owner);
> +	else
> +		module_put(rproc->dev.parent->parent->driver->owner);
> +
>  	put_device(&rproc->dev);
>  }
>  EXPORT_SYMBOL(rproc_put);
>
> base-commit: 99f59b148871dadb9104366e3d25b120a97f897b