linux-kernel - Re: [PATCH] remoteproc: mediatek: Use for_each_available_child_of_node

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250910115508.0000785e@huawei.com>
Date: Wed, 10 Sep 2025 11:55:08 +0100
From: Jonathan Cameron <jonathan.cameron@...wei.com>
To: Fei Shao <fshao@...omium.org>
CC: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
	Matthias Brugger <matthias.bgg@...il.com>, Bjorn Andersson
	<andersson@...nel.org>, Mathieu Poirier <mathieu.poirier@...aro.org>,
	<linux-arm-kernel@...ts.infradead.org>, <linux-kernel@...r.kernel.org>,
	<linux-mediatek@...ts.infradead.org>, <linux-remoteproc@...r.kernel.org>
Subject: Re: [PATCH] remoteproc: mediatek: Use
 for_each_available_child_of_node_scoped()

On Mon,  8 Sep 2025 12:43:25 +0800
Fei Shao <fshao@...omium.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@...omium.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;
>  		}