linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] remoteproc: mediatek: Use for_each_available_child_of_node_scoped()
@ 2025-09-08  4:43 Fei Shao
  2025-09-10  9:57 ` Matthias Brugger
  2025-09-10 10:55 ` Jonathan Cameron
  0 siblings, 2 replies; 4+ messages in thread
From: Fei Shao @ 2025-09-08  4:43 UTC (permalink / raw)
  To: AngeloGioacchino Del Regno, Matthias Brugger
  Cc: Fei Shao, Bjorn Andersson, Mathieu Poirier, linux-arm-kernel,
	linux-kernel, linux-mediatek, linux-remoteproc

Use scoped for_each_available_child_of_node_scoped() to remove manual
of_node_put() calls from early returns.

Signed-off-by: Fei Shao <fshao@chromium.org>
---

 drivers/remoteproc/mtk_scp.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
index 8206a1766481..b123698feb52 100644
--- a/drivers/remoteproc/mtk_scp.c
+++ b/drivers/remoteproc/mtk_scp.c
@@ -1234,7 +1234,6 @@ static int scp_add_multi_core(struct platform_device *pdev,
 	struct device *dev = &pdev->dev;
 	struct device_node *np = dev_of_node(dev);
 	struct platform_device *cpdev;
-	struct device_node *child;
 	struct list_head *scp_list = &scp_cluster->mtk_scp_list;
 	const struct mtk_scp_of_data **cluster_of_data;
 	struct mtk_scp *scp, *temp;
@@ -1243,11 +1242,10 @@ static int scp_add_multi_core(struct platform_device *pdev,
 
 	cluster_of_data = (const struct mtk_scp_of_data **)of_device_get_match_data(dev);
 
-	for_each_available_child_of_node(np, child) {
+	for_each_available_child_of_node_scoped(np, child) {
 		if (!cluster_of_data[core_id]) {
 			ret = -EINVAL;
 			dev_err(dev, "Not support core %d\n", core_id);
-			of_node_put(child);
 			goto init_fail;
 		}
 
@@ -1255,7 +1253,6 @@ static int scp_add_multi_core(struct platform_device *pdev,
 		if (!cpdev) {
 			ret = -ENODEV;
 			dev_err(dev, "Not found platform device for core %d\n", core_id);
-			of_node_put(child);
 			goto init_fail;
 		}
 
@@ -1264,14 +1261,12 @@ static int scp_add_multi_core(struct platform_device *pdev,
 		if (IS_ERR(scp)) {
 			ret = PTR_ERR(scp);
 			dev_err(dev, "Failed to initialize core %d rproc\n", core_id);
-			of_node_put(child);
 			goto init_fail;
 		}
 
 		ret = rproc_add(scp->rproc);
 		if (ret) {
 			dev_err(dev, "Failed to add rproc of core %d\n", core_id);
-			of_node_put(child);
 			scp_free(scp);
 			goto init_fail;
 		}
-- 
2.51.0.355.g5224444f11-goog



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

* Re: [PATCH] remoteproc: mediatek: Use for_each_available_child_of_node_scoped()
  2025-09-08  4:43 [PATCH] remoteproc: mediatek: Use for_each_available_child_of_node_scoped() Fei Shao
@ 2025-09-10  9:57 ` Matthias Brugger
  2025-09-10 10:55 ` Jonathan Cameron
  1 sibling, 0 replies; 4+ messages in thread
From: Matthias Brugger @ 2025-09-10  9:57 UTC (permalink / raw)
  To: Fei Shao, AngeloGioacchino Del Regno
  Cc: Bjorn Andersson, Mathieu Poirier, linux-arm-kernel, linux-kernel,
	linux-mediatek, linux-remoteproc



On 08/09/2025 06:43, Fei Shao wrote:
> Use scoped for_each_available_child_of_node_scoped() to remove manual
> of_node_put() calls from early returns.
> 
> Signed-off-by: Fei Shao <fshao@chromium.org>

Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com>

> ---
> 
>   drivers/remoteproc/mtk_scp.c | 7 +------
>   1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
> index 8206a1766481..b123698feb52 100644
> --- a/drivers/remoteproc/mtk_scp.c
> +++ b/drivers/remoteproc/mtk_scp.c
> @@ -1234,7 +1234,6 @@ static int scp_add_multi_core(struct platform_device *pdev,
>   	struct device *dev = &pdev->dev;
>   	struct device_node *np = dev_of_node(dev);
>   	struct platform_device *cpdev;
> -	struct device_node *child;
>   	struct list_head *scp_list = &scp_cluster->mtk_scp_list;
>   	const struct mtk_scp_of_data **cluster_of_data;
>   	struct mtk_scp *scp, *temp;
> @@ -1243,11 +1242,10 @@ static int scp_add_multi_core(struct platform_device *pdev,
>   
>   	cluster_of_data = (const struct mtk_scp_of_data **)of_device_get_match_data(dev);
>   
> -	for_each_available_child_of_node(np, child) {
> +	for_each_available_child_of_node_scoped(np, child) {
>   		if (!cluster_of_data[core_id]) {
>   			ret = -EINVAL;
>   			dev_err(dev, "Not support core %d\n", core_id);
> -			of_node_put(child);
>   			goto init_fail;
>   		}
>   
> @@ -1255,7 +1253,6 @@ static int scp_add_multi_core(struct platform_device *pdev,
>   		if (!cpdev) {
>   			ret = -ENODEV;
>   			dev_err(dev, "Not found platform device for core %d\n", core_id);
> -			of_node_put(child);
>   			goto init_fail;
>   		}
>   
> @@ -1264,14 +1261,12 @@ static int scp_add_multi_core(struct platform_device *pdev,
>   		if (IS_ERR(scp)) {
>   			ret = PTR_ERR(scp);
>   			dev_err(dev, "Failed to initialize core %d rproc\n", core_id);
> -			of_node_put(child);
>   			goto init_fail;
>   		}
>   
>   		ret = rproc_add(scp->rproc);
>   		if (ret) {
>   			dev_err(dev, "Failed to add rproc of core %d\n", core_id);
> -			of_node_put(child);
>   			scp_free(scp);
>   			goto init_fail;
>   		}



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

* Re: [PATCH] remoteproc: mediatek: Use for_each_available_child_of_node_scoped()
  2025-09-08  4:43 [PATCH] remoteproc: mediatek: Use for_each_available_child_of_node_scoped() Fei Shao
  2025-09-10  9:57 ` Matthias Brugger
@ 2025-09-10 10:55 ` Jonathan Cameron
  2025-09-11  8:43   ` Fei Shao
  1 sibling, 1 reply; 4+ messages in thread
From: Jonathan Cameron @ 2025-09-10 10:55 UTC (permalink / raw)
  To: Fei Shao
  Cc: AngeloGioacchino Del Regno, Matthias Brugger, Bjorn Andersson,
	Mathieu Poirier, linux-arm-kernel, linux-kernel, linux-mediatek,
	linux-remoteproc

On Mon,  8 Sep 2025 12:43:25 +0800
Fei Shao <fshao@chromium.org> wrote:

> Use scoped for_each_available_child_of_node_scoped() to remove manual
> of_node_put() calls from early returns.

There aren't any early returns here.

This runs into some of the stuff that cleanup.h docs suggest we shouldn't
do which is combining gotos and __free() magic. 
I think this case is actually fine despite that but in general worth
thinking about the code structure and whether that can be avoided.

One option would be to factor out the loop into another function then use
and error return from that to call the stuff under the init_free label.

Jonathan


> 
> Signed-off-by: Fei Shao <fshao@chromium.org>
> ---
> 
>  drivers/remoteproc/mtk_scp.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
> index 8206a1766481..b123698feb52 100644
> --- a/drivers/remoteproc/mtk_scp.c
> +++ b/drivers/remoteproc/mtk_scp.c
> @@ -1234,7 +1234,6 @@ static int scp_add_multi_core(struct platform_device *pdev,
>  	struct device *dev = &pdev->dev;
>  	struct device_node *np = dev_of_node(dev);
>  	struct platform_device *cpdev;
> -	struct device_node *child;
>  	struct list_head *scp_list = &scp_cluster->mtk_scp_list;
>  	const struct mtk_scp_of_data **cluster_of_data;
>  	struct mtk_scp *scp, *temp;
> @@ -1243,11 +1242,10 @@ static int scp_add_multi_core(struct platform_device *pdev,
>  
>  	cluster_of_data = (const struct mtk_scp_of_data **)of_device_get_match_data(dev);
>  
> -	for_each_available_child_of_node(np, child) {
> +	for_each_available_child_of_node_scoped(np, child) {
>  		if (!cluster_of_data[core_id]) {
>  			ret = -EINVAL;
>  			dev_err(dev, "Not support core %d\n", core_id);
> -			of_node_put(child);
>  			goto init_fail;
>  		}
>  
> @@ -1255,7 +1253,6 @@ static int scp_add_multi_core(struct platform_device *pdev,
>  		if (!cpdev) {
>  			ret = -ENODEV;
>  			dev_err(dev, "Not found platform device for core %d\n", core_id);
> -			of_node_put(child);
>  			goto init_fail;
>  		}
>  
> @@ -1264,14 +1261,12 @@ static int scp_add_multi_core(struct platform_device *pdev,
>  		if (IS_ERR(scp)) {
>  			ret = PTR_ERR(scp);
>  			dev_err(dev, "Failed to initialize core %d rproc\n", core_id);
> -			of_node_put(child);
>  			goto init_fail;
>  		}
>  
>  		ret = rproc_add(scp->rproc);
>  		if (ret) {
>  			dev_err(dev, "Failed to add rproc of core %d\n", core_id);
> -			of_node_put(child);
>  			scp_free(scp);
>  			goto init_fail;
>  		}



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

* Re: [PATCH] remoteproc: mediatek: Use for_each_available_child_of_node_scoped()
  2025-09-10 10:55 ` Jonathan Cameron
@ 2025-09-11  8:43   ` Fei Shao
  0 siblings, 0 replies; 4+ messages in thread
From: Fei Shao @ 2025-09-11  8:43 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: AngeloGioacchino Del Regno, Matthias Brugger, Bjorn Andersson,
	Mathieu Poirier, linux-arm-kernel, linux-kernel, linux-mediatek,
	linux-remoteproc

On Wed, Sep 10, 2025 at 6:55 PM Jonathan Cameron
<jonathan.cameron@huawei.com> wrote:
>
> On Mon,  8 Sep 2025 12:43:25 +0800
> Fei Shao <fshao@chromium.org> wrote:
>
> > Use scoped for_each_available_child_of_node_scoped() to remove manual
> > of_node_put() calls from early returns.
>
> There aren't any early returns here.
>
> This runs into some of the stuff that cleanup.h docs suggest we shouldn't
> do which is combining gotos and __free() magic.
> I think this case is actually fine despite that but in general worth
> thinking about the code structure and whether that can be avoided.
>
> One option would be to factor out the loop into another function then use
> and error return from that to call the stuff under the init_free label.

Fair point, I can send a v2 with that.

Thanks,
Fei

>
> Jonathan
>


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

end of thread, other threads:[~2025-09-11  8:44 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-08  4:43 [PATCH] remoteproc: mediatek: Use for_each_available_child_of_node_scoped() Fei Shao
2025-09-10  9:57 ` Matthias Brugger
2025-09-10 10:55 ` Jonathan Cameron
2025-09-11  8:43   ` Fei Shao

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).