[PATCH -next 0/8] soc: Simplify with scoped for each OF child loop and dev_err_probe()

[PATCH -next 0/8] soc: Simplify with scoped for each OF child loop and dev_err_probe()

[Jinjie Ruan]

Use for_each_child_of_node_scoped() to simplify code.

Jinjie Ruan (8):
  soc: fsl: cpm1: Simplify with scoped for each OF child loop
  soc: fsl: cpm1: Simplify with dev_err_probe()
  soc: fsl: cpm1: qmc: Simplify with scoped for each OF child
  soc: fsl: cpm1: qmc: Simplify with dev_err_probe()
  soc/tegra: pmc: Simplify with scoped for each OF child loop
  soc: dove: Simplify with scoped for each OF child loop
  soc: ti: knav_dma: Simplify with scoped for each OF child loop
  soc: ti: knav_qmss_queue: Simplify with scoped for each OF child loop

 drivers/soc/dove/pmu.c           |  9 ++--
 drivers/soc/fsl/qe/qmc.c         | 66 ++++++++---------------
 drivers/soc/fsl/qe/tsa.c         | 90 ++++++++++----------------------
 drivers/soc/tegra/pmc.c          | 12 ++---
 drivers/soc/ti/knav_dma.c        | 16 ++----
 drivers/soc/ti/knav_qmss_queue.c | 57 +++++++-------------
 6 files changed, 80 insertions(+), 170 deletions(-)

-- 
2.34.1

[PATCH -next 1/8] soc: fsl: cpm1: Simplify with scoped for each OF child loop

[Jinjie Ruan]

Use scoped for_each_available_child_of_node_scoped when iterating
over device nodes to make code a bit simpler.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
 drivers/soc/fsl/qe/tsa.c | 28 ++++------------------------
 1 file changed, 4 insertions(+), 24 deletions(-)

diff --git a/drivers/soc/fsl/qe/tsa.c b/drivers/soc/fsl/qe/tsa.c
index 6c5741cf5e9d..7fa399b7a47c 100644
--- a/drivers/soc/fsl/qe/tsa.c
+++ b/drivers/soc/fsl/qe/tsa.c
@@ -441,7 +441,6 @@ static inline int tsa_of_parse_tdm_tx_route(struct tsa *tsa,
 
 static int tsa_of_parse_tdms(struct tsa *tsa, struct device_node *np)
 {
-	struct device_node *tdm_np;
 	struct tsa_tdm *tdm;
 	struct clk *clk;
 	u32 tdm_id, val;
@@ -452,11 +451,10 @@ static int tsa_of_parse_tdms(struct tsa *tsa, struct device_node *np)
 	tsa->tdm[0].is_enable = false;
 	tsa->tdm[1].is_enable = false;
 
-	for_each_available_child_of_node(np, tdm_np) {
+	for_each_available_child_of_node_scoped(np, tdm_np) {
 		ret = of_property_read_u32(tdm_np, "reg", &tdm_id);
 		if (ret) {
 			dev_err(tsa->dev, "%pOF: failed to read reg\n", tdm_np);
-			of_node_put(tdm_np);
 			return ret;
 		}
 		switch (tdm_id) {
@@ -469,16 +467,14 @@ static int tsa_of_parse_tdms(struct tsa *tsa, struct device_node *np)
 		default:
 			dev_err(tsa->dev, "%pOF: Invalid tdm_id (%u)\n", tdm_np,
 				tdm_id);
-			of_node_put(tdm_np);
 			return -EINVAL;
 		}
 	}
 
-	for_each_available_child_of_node(np, tdm_np) {
+	for_each_available_child_of_node_scoped(np, tdm_np) {
 		ret = of_property_read_u32(tdm_np, "reg", &tdm_id);
 		if (ret) {
 			dev_err(tsa->dev, "%pOF: failed to read reg\n", tdm_np);
-			of_node_put(tdm_np);
 			return ret;
 		}
 
@@ -492,14 +488,12 @@ static int tsa_of_parse_tdms(struct tsa *tsa, struct device_node *np)
 			dev_err(tsa->dev,
 				"%pOF: failed to read fsl,rx-frame-sync-delay-bits\n",
 				tdm_np);
-			of_node_put(tdm_np);
 			return ret;
 		}
 		if (val > 3) {
 			dev_err(tsa->dev,
 				"%pOF: Invalid fsl,rx-frame-sync-delay-bits (%u)\n",
 				tdm_np, val);
-			of_node_put(tdm_np);
 			return -EINVAL;
 		}
 		tdm->simode_tdm |= TSA_SIMODE_TDM_RFSD(val);
@@ -511,14 +505,12 @@ static int tsa_of_parse_tdms(struct tsa *tsa, struct device_node *np)
 			dev_err(tsa->dev,
 				"%pOF: failed to read fsl,tx-frame-sync-delay-bits\n",
 				tdm_np);
-			of_node_put(tdm_np);
 			return ret;
 		}
 		if (val > 3) {
 			dev_err(tsa->dev,
 				"%pOF: Invalid fsl,tx-frame-sync-delay-bits (%u)\n",
 				tdm_np, val);
-			of_node_put(tdm_np);
 			return -EINVAL;
 		}
 		tdm->simode_tdm |= TSA_SIMODE_TDM_TFSD(val);
@@ -538,13 +530,11 @@ static int tsa_of_parse_tdms(struct tsa *tsa, struct device_node *np)
 		clk = of_clk_get_by_name(tdm_np, "l1rsync");
 		if (IS_ERR(clk)) {
 			ret = PTR_ERR(clk);
-			of_node_put(tdm_np);
 			goto err;
 		}
 		ret = clk_prepare_enable(clk);
 		if (ret) {
 			clk_put(clk);
-			of_node_put(tdm_np);
 			goto err;
 		}
 		tdm->l1rsync_clk = clk;
@@ -552,13 +542,11 @@ static int tsa_of_parse_tdms(struct tsa *tsa, struct device_node *np)
 		clk = of_clk_get_by_name(tdm_np, "l1rclk");
 		if (IS_ERR(clk)) {
 			ret = PTR_ERR(clk);
-			of_node_put(tdm_np);
 			goto err;
 		}
 		ret = clk_prepare_enable(clk);
 		if (ret) {
 			clk_put(clk);
-			of_node_put(tdm_np);
 			goto err;
 		}
 		tdm->l1rclk_clk = clk;
@@ -567,13 +555,11 @@ static int tsa_of_parse_tdms(struct tsa *tsa, struct device_node *np)
 			clk = of_clk_get_by_name(tdm_np, "l1tsync");
 			if (IS_ERR(clk)) {
 				ret = PTR_ERR(clk);
-				of_node_put(tdm_np);
 				goto err;
 			}
 			ret = clk_prepare_enable(clk);
 			if (ret) {
 				clk_put(clk);
-				of_node_put(tdm_np);
 				goto err;
 			}
 			tdm->l1tsync_clk = clk;
@@ -581,29 +567,23 @@ static int tsa_of_parse_tdms(struct tsa *tsa, struct device_node *np)
 			clk = of_clk_get_by_name(tdm_np, "l1tclk");
 			if (IS_ERR(clk)) {
 				ret = PTR_ERR(clk);
-				of_node_put(tdm_np);
 				goto err;
 			}
 			ret = clk_prepare_enable(clk);
 			if (ret) {
 				clk_put(clk);
-				of_node_put(tdm_np);
 				goto err;
 			}
 			tdm->l1tclk_clk = clk;
 		}
 
 		ret = tsa_of_parse_tdm_rx_route(tsa, tdm_np, tsa->tdms, tdm_id);
-		if (ret) {
-			of_node_put(tdm_np);
+		if (ret)
 			goto err;
-		}
 
 		ret = tsa_of_parse_tdm_tx_route(tsa, tdm_np, tsa->tdms, tdm_id);
-		if (ret) {
-			of_node_put(tdm_np);
+		if (ret)
 			goto err;
-		}
 
 		tdm->is_enable = true;
 	}
-- 
2.34.1

[PATCH -next 2/8] soc: fsl: cpm1: Simplify with dev_err_probe()

[Jinjie Ruan]

Use the dev_err_probe() helper to simplify error handling during probe.
This also handle scenario, when EDEFER is returned and useless error
is printed.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
 drivers/soc/fsl/qe/tsa.c | 62 +++++++++++++++-------------------------
 1 file changed, 23 insertions(+), 39 deletions(-)

diff --git a/drivers/soc/fsl/qe/tsa.c b/drivers/soc/fsl/qe/tsa.c
index 7fa399b7a47c..fc37d23b746d 100644
--- a/drivers/soc/fsl/qe/tsa.c
+++ b/drivers/soc/fsl/qe/tsa.c
@@ -453,10 +453,8 @@ static int tsa_of_parse_tdms(struct tsa *tsa, struct device_node *np)
 
 	for_each_available_child_of_node_scoped(np, tdm_np) {
 		ret = of_property_read_u32(tdm_np, "reg", &tdm_id);
-		if (ret) {
-			dev_err(tsa->dev, "%pOF: failed to read reg\n", tdm_np);
-			return ret;
-		}
+		if (ret)
+			return dev_err_probe(tsa->dev, ret, "%pOF: failed to read reg\n", tdm_np);
 		switch (tdm_id) {
 		case 0:
 			tsa->tdms |= BIT(TSA_TDMA);
@@ -465,18 +463,15 @@ static int tsa_of_parse_tdms(struct tsa *tsa, struct device_node *np)
 			tsa->tdms |= BIT(TSA_TDMB);
 			break;
 		default:
-			dev_err(tsa->dev, "%pOF: Invalid tdm_id (%u)\n", tdm_np,
-				tdm_id);
-			return -EINVAL;
+			return dev_err_probe(tsa->dev, -EINVAL, "%pOF: Invalid tdm_id (%u)\n",
+					     tdm_np, tdm_id);
 		}
 	}
 
 	for_each_available_child_of_node_scoped(np, tdm_np) {
 		ret = of_property_read_u32(tdm_np, "reg", &tdm_id);
-		if (ret) {
-			dev_err(tsa->dev, "%pOF: failed to read reg\n", tdm_np);
-			return ret;
-		}
+		if (ret)
+			return dev_err_probe(tsa->dev, ret, "%pOF: failed to read reg\n", tdm_np);
 
 		tdm = &tsa->tdm[tdm_id];
 		tdm->simode_tdm = TSA_SIMODE_TDM_SDM_NORM;
@@ -484,35 +479,26 @@ static int tsa_of_parse_tdms(struct tsa *tsa, struct device_node *np)
 		val = 0;
 		ret = of_property_read_u32(tdm_np, "fsl,rx-frame-sync-delay-bits",
 					   &val);
-		if (ret && ret != -EINVAL) {
-			dev_err(tsa->dev,
-				"%pOF: failed to read fsl,rx-frame-sync-delay-bits\n",
-				tdm_np);
-			return ret;
-		}
-		if (val > 3) {
-			dev_err(tsa->dev,
-				"%pOF: Invalid fsl,rx-frame-sync-delay-bits (%u)\n",
-				tdm_np, val);
-			return -EINVAL;
-		}
+		if (ret && ret != -EINVAL)
+			return dev_err_probe(tsa->dev, ret,
+					     "%pOF: failed to read fsl,rx-frame-sync-delay-bits\n",
+					     tdm_np);
+		if (val > 3)
+			return dev_err_probe(tsa->dev, -EINVAL,
+					     "%pOF: Invalid fsl,rx-frame-sync-delay-bits (%u)\n",
+					     tdm_np, val);
 		tdm->simode_tdm |= TSA_SIMODE_TDM_RFSD(val);
 
 		val = 0;
 		ret = of_property_read_u32(tdm_np, "fsl,tx-frame-sync-delay-bits",
 					   &val);
-		if (ret && ret != -EINVAL) {
-			dev_err(tsa->dev,
-				"%pOF: failed to read fsl,tx-frame-sync-delay-bits\n",
-				tdm_np);
-			return ret;
-		}
-		if (val > 3) {
-			dev_err(tsa->dev,
-				"%pOF: Invalid fsl,tx-frame-sync-delay-bits (%u)\n",
-				tdm_np, val);
-			return -EINVAL;
-		}
+		if (ret && ret != -EINVAL)
+			return dev_err_probe(tsa->dev, ret,
+				"%pOF: failed to read fsl,tx-frame-sync-delay-bits\n", tdm_np);
+		if (val > 3)
+			return dev_err_probe(tsa->dev, -EINVAL,
+					     "%pOF: Invalid fsl,tx-frame-sync-delay-bits (%u)\n",
+					     tdm_np, val);
 		tdm->simode_tdm |= TSA_SIMODE_TDM_TFSD(val);
 
 		if (of_property_read_bool(tdm_np, "fsl,common-rxtx-pins"))
@@ -645,10 +631,8 @@ static int tsa_probe(struct platform_device *pdev)
 		return PTR_ERR(tsa->si_regs);
 
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "si_ram");
-	if (!res) {
-		dev_err(tsa->dev, "si_ram resource missing\n");
-		return -EINVAL;
-	}
+	if (!res)
+		return dev_err_probe(tsa->dev, -EINVAL, "si_ram resource missing\n");
 	tsa->si_ram_sz = resource_size(res);
 	tsa->si_ram = devm_ioremap_resource(&pdev->dev, res);
 	if (IS_ERR(tsa->si_ram))
-- 
2.34.1

Re: [PATCH -next 2/8] soc: fsl: cpm1: Simplify with dev_err_probe()

[Krzysztof Kozlowski]

On 27/08/2024 13:46, Jinjie Ruan wrote:
> Use the dev_err_probe() helper to simplify error handling during probe.
> This also handle scenario, when EDEFER is returned and useless error
> is printed.

? Sorry, this cannot happen. Please point to below code which can defer.

> 
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> ---
>  drivers/soc/fsl/qe/tsa.c | 62 +++++++++++++++-------------------------
>  1 file changed, 23 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/soc/fsl/qe/tsa.c b/drivers/soc/fsl/qe/tsa.c
> index 7fa399b7a47c..fc37d23b746d 100644
> --- a/drivers/soc/fsl/qe/tsa.c
> +++ b/drivers/soc/fsl/qe/tsa.c
> @@ -453,10 +453,8 @@ static int tsa_of_parse_tdms(struct tsa *tsa, struct device_node *np)
>  
>  	for_each_available_child_of_node_scoped(np, tdm_np) {
>  		ret = of_property_read_u32(tdm_np, "reg", &tdm_id);
> -		if (ret) {
> -			dev_err(tsa->dev, "%pOF: failed to read reg\n", tdm_np);
> -			return ret;
> -		}
> +		if (ret)
> +			return dev_err_probe(tsa->dev, ret, "%pOF: failed to read reg\n", tdm_np);
>  		switch (tdm_id) {
>  		case 0:
>  			tsa->tdms |= BIT(TSA_TDMA);
> @@ -465,18 +463,15 @@ static int tsa_of_parse_tdms(struct tsa *tsa, struct device_node *np)
>  			tsa->tdms |= BIT(TSA_TDMB);
>  			break;
>  		default:
> -			dev_err(tsa->dev, "%pOF: Invalid tdm_id (%u)\n", tdm_np,
> -				tdm_id);
> -			return -EINVAL;
> +			return dev_err_probe(tsa->dev, -EINVAL, "%pOF: Invalid tdm_id (%u)\n",
> +					     tdm_np, tdm_id);
>  		}
>  	}
>  
>  	for_each_available_child_of_node_scoped(np, tdm_np) {
>  		ret = of_property_read_u32(tdm_np, "reg", &tdm_id);
> -		if (ret) {
> -			dev_err(tsa->dev, "%pOF: failed to read reg\n", tdm_np);
> -			return ret;
> -		}
> +		if (ret)
> +			return dev_err_probe(tsa->dev, ret, "%pOF: failed to read reg\n", tdm_np);
>  
>  		tdm = &tsa->tdm[tdm_id];
>  		tdm->simode_tdm = TSA_SIMODE_TDM_SDM_NORM;
> @@ -484,35 +479,26 @@ static int tsa_of_parse_tdms(struct tsa *tsa, struct device_node *np)
>  		val = 0;
>  		ret = of_property_read_u32(tdm_np, "fsl,rx-frame-sync-delay-bits",
>  					   &val);
> -		if (ret && ret != -EINVAL) {
> -			dev_err(tsa->dev,
> -				"%pOF: failed to read fsl,rx-frame-sync-delay-bits\n",
> -				tdm_np);
> -			return ret;
> -		}
> -		if (val > 3) {
> -			dev_err(tsa->dev,
> -				"%pOF: Invalid fsl,rx-frame-sync-delay-bits (%u)\n",
> -				tdm_np, val);
> -			return -EINVAL;
> -		}
> +		if (ret && ret != -EINVAL)
> +			return dev_err_probe(tsa->dev, ret,
> +					     "%pOF: failed to read fsl,rx-frame-sync-delay-bits\n",
> +					     tdm_np);
> +		if (val > 3)
> +			return dev_err_probe(tsa->dev, -EINVAL,
> +					     "%pOF: Invalid fsl,rx-frame-sync-delay-bits (%u)\n",
> +					     tdm_np, val);
>  		tdm->simode_tdm |= TSA_SIMODE_TDM_RFSD(val);
>  
>  		val = 0;
>  		ret = of_property_read_u32(tdm_np, "fsl,tx-frame-sync-delay-bits",
>  					   &val);
> -		if (ret && ret != -EINVAL) {
> -			dev_err(tsa->dev,
> -				"%pOF: failed to read fsl,tx-frame-sync-delay-bits\n",
> -				tdm_np);
> -			return ret;
> -		}
> -		if (val > 3) {
> -			dev_err(tsa->dev,
> -				"%pOF: Invalid fsl,tx-frame-sync-delay-bits (%u)\n",
> -				tdm_np, val);
> -			return -EINVAL;
> -		}
> +		if (ret && ret != -EINVAL)
> +			return dev_err_probe(tsa->dev, ret,
> +				"%pOF: failed to read fsl,tx-frame-sync-delay-bits\n", tdm_np);
> +		if (val > 3)
> +			return dev_err_probe(tsa->dev, -EINVAL,
> +					     "%pOF: Invalid fsl,tx-frame-sync-delay-bits (%u)\n",
> +					     tdm_np, val);
>  		tdm->simode_tdm |= TSA_SIMODE_TDM_TFSD(val);
>  
>  		if (of_property_read_bool(tdm_np, "fsl,common-rxtx-pins"))
> @@ -645,10 +631,8 @@ static int tsa_probe(struct platform_device *pdev)
>  		return PTR_ERR(tsa->si_regs);
>  
>  	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "si_ram");
> -	if (!res) {
> -		dev_err(tsa->dev, "si_ram resource missing\n");
> -		return -EINVAL;
> -	}
> +	if (!res)
> +		return dev_err_probe(tsa->dev, -EINVAL, "si_ram resource missing\n");
>  	tsa->si_ram_sz = resource_size(res);
>  	tsa->si_ram = devm_ioremap_resource(&pdev->dev, res);
>  	if (IS_ERR(tsa->si_ram))

Best regards,
Krzysztof

Re: [PATCH -next 2/8] soc: fsl: cpm1: Simplify with dev_err_probe()

[Jinjie Ruan]

On 2024/8/27 21:50, Krzysztof Kozlowski wrote:
> On 27/08/2024 13:46, Jinjie Ruan wrote:
>> Use the dev_err_probe() helper to simplify error handling during probe.
>> This also handle scenario, when EDEFER is returned and useless error
>> is printed.
> 
> ? Sorry, this cannot happen. Please point to below code which can defer.
> 

Thank you!

This is not referring to a specific one, but rather the benefits it
offers，simplify code is the main purpose, if necessary, it will be
removed in next version.

>>
>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>> ---
>>  drivers/soc/fsl/qe/tsa.c | 62 +++++++++++++++-------------------------
>>  1 file changed, 23 insertions(+), 39 deletions(-)
>>
>> diff --git a/drivers/soc/fsl/qe/tsa.c b/drivers/soc/fsl/qe/tsa.c
>> index 7fa399b7a47c..fc37d23b746d 100644
>> --- a/drivers/soc/fsl/qe/tsa.c
>> +++ b/drivers/soc/fsl/qe/tsa.c
>> @@ -453,10 +453,8 @@ static int tsa_of_parse_tdms(struct tsa *tsa, struct device_node *np)
>>  
>>  	for_each_available_child_of_node_scoped(np, tdm_np) {
>>  		ret = of_property_read_u32(tdm_np, "reg", &tdm_id);
>> -		if (ret) {
>> -			dev_err(tsa->dev, "%pOF: failed to read reg\n", tdm_np);
>> -			return ret;
>> -		}
>> +		if (ret)
>> +			return dev_err_probe(tsa->dev, ret, "%pOF: failed to read reg\n", tdm_np);
>>  		switch (tdm_id) {
>>  		case 0:
>>  			tsa->tdms |= BIT(TSA_TDMA);
>> @@ -465,18 +463,15 @@ static int tsa_of_parse_tdms(struct tsa *tsa, struct device_node *np)
>>  			tsa->tdms |= BIT(TSA_TDMB);
>>  			break;
>>  		default:
>> -			dev_err(tsa->dev, "%pOF: Invalid tdm_id (%u)\n", tdm_np,
>> -				tdm_id);
>> -			return -EINVAL;
>> +			return dev_err_probe(tsa->dev, -EINVAL, "%pOF: Invalid tdm_id (%u)\n",
>> +					     tdm_np, tdm_id);
>>  		}
>>  	}
>>  
>>  	for_each_available_child_of_node_scoped(np, tdm_np) {
>>  		ret = of_property_read_u32(tdm_np, "reg", &tdm_id);
>> -		if (ret) {
>> -			dev_err(tsa->dev, "%pOF: failed to read reg\n", tdm_np);
>> -			return ret;
>> -		}
>> +		if (ret)
>> +			return dev_err_probe(tsa->dev, ret, "%pOF: failed to read reg\n", tdm_np);
>>  
>>  		tdm = &tsa->tdm[tdm_id];
>>  		tdm->simode_tdm = TSA_SIMODE_TDM_SDM_NORM;
>> @@ -484,35 +479,26 @@ static int tsa_of_parse_tdms(struct tsa *tsa, struct device_node *np)
>>  		val = 0;
>>  		ret = of_property_read_u32(tdm_np, "fsl,rx-frame-sync-delay-bits",
>>  					   &val);
>> -		if (ret && ret != -EINVAL) {
>> -			dev_err(tsa->dev,
>> -				"%pOF: failed to read fsl,rx-frame-sync-delay-bits\n",
>> -				tdm_np);
>> -			return ret;
>> -		}
>> -		if (val > 3) {
>> -			dev_err(tsa->dev,
>> -				"%pOF: Invalid fsl,rx-frame-sync-delay-bits (%u)\n",
>> -				tdm_np, val);
>> -			return -EINVAL;
>> -		}
>> +		if (ret && ret != -EINVAL)
>> +			return dev_err_probe(tsa->dev, ret,
>> +					     "%pOF: failed to read fsl,rx-frame-sync-delay-bits\n",
>> +					     tdm_np);
>> +		if (val > 3)
>> +			return dev_err_probe(tsa->dev, -EINVAL,
>> +					     "%pOF: Invalid fsl,rx-frame-sync-delay-bits (%u)\n",
>> +					     tdm_np, val);
>>  		tdm->simode_tdm |= TSA_SIMODE_TDM_RFSD(val);
>>  
>>  		val = 0;
>>  		ret = of_property_read_u32(tdm_np, "fsl,tx-frame-sync-delay-bits",
>>  					   &val);
>> -		if (ret && ret != -EINVAL) {
>> -			dev_err(tsa->dev,
>> -				"%pOF: failed to read fsl,tx-frame-sync-delay-bits\n",
>> -				tdm_np);
>> -			return ret;
>> -		}
>> -		if (val > 3) {
>> -			dev_err(tsa->dev,
>> -				"%pOF: Invalid fsl,tx-frame-sync-delay-bits (%u)\n",
>> -				tdm_np, val);
>> -			return -EINVAL;
>> -		}
>> +		if (ret && ret != -EINVAL)
>> +			return dev_err_probe(tsa->dev, ret,
>> +				"%pOF: failed to read fsl,tx-frame-sync-delay-bits\n", tdm_np);
>> +		if (val > 3)
>> +			return dev_err_probe(tsa->dev, -EINVAL,
>> +					     "%pOF: Invalid fsl,tx-frame-sync-delay-bits (%u)\n",
>> +					     tdm_np, val);
>>  		tdm->simode_tdm |= TSA_SIMODE_TDM_TFSD(val);
>>  
>>  		if (of_property_read_bool(tdm_np, "fsl,common-rxtx-pins"))
>> @@ -645,10 +631,8 @@ static int tsa_probe(struct platform_device *pdev)
>>  		return PTR_ERR(tsa->si_regs);
>>  
>>  	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "si_ram");
>> -	if (!res) {
>> -		dev_err(tsa->dev, "si_ram resource missing\n");
>> -		return -EINVAL;
>> -	}
>> +	if (!res)
>> +		return dev_err_probe(tsa->dev, -EINVAL, "si_ram resource missing\n");
>>  	tsa->si_ram_sz = resource_size(res);
>>  	tsa->si_ram = devm_ioremap_resource(&pdev->dev, res);
>>  	if (IS_ERR(tsa->si_ram))
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH -next 2/8] soc: fsl: cpm1: Simplify with dev_err_probe()

[Krzysztof Kozlowski]

On 28/08/2024 03:58, Jinjie Ruan wrote:
> 
> 
> On 2024/8/27 21:50, Krzysztof Kozlowski wrote:
>> On 27/08/2024 13:46, Jinjie Ruan wrote:
>>> Use the dev_err_probe() helper to simplify error handling during probe.
>>> This also handle scenario, when EDEFER is returned and useless error
>>> is printed.
>>
>> ? Sorry, this cannot happen. Please point to below code which can defer.
>>
> 
> Thank you!
> 
> This is not referring to a specific one, but rather the benefits it
> offers，simplify code is the main purpose, if necessary, it will be
> removed in next version.

It just feels like you are doing these in machine way without actually
knowing concepts behind dev_err_probe().

Best regards,
Krzysztof

Re: [PATCH -next 2/8] soc: fsl: cpm1: Simplify with dev_err_probe()

[Jinjie Ruan]

On 2024/8/28 21:08, Krzysztof Kozlowski wrote:
> On 28/08/2024 03:58, Jinjie Ruan wrote:
>>
>>
>> On 2024/8/27 21:50, Krzysztof Kozlowski wrote:
>>> On 27/08/2024 13:46, Jinjie Ruan wrote:
>>>> Use the dev_err_probe() helper to simplify error handling during probe.
>>>> This also handle scenario, when EDEFER is returned and useless error
>>>> is printed.
>>>
>>> ? Sorry, this cannot happen. Please point to below code which can defer.
>>>
>>
>> Thank you!
>>
>> This is not referring to a specific one, but rather the benefits it
>> offers，simplify code is the main purpose, if necessary, it will be
>> removed in next version.
> 
> It just feels like you are doing these in machine way without actually
> knowing concepts behind dev_err_probe().

Just try my best to make the code as clean as possible and do it by the way.

> 
> Best regards,
> Krzysztof
> 

[PATCH -next 3/8] soc: fsl: cpm1: qmc: Simplify with scoped for each OF child

[Jinjie Ruan]

Use scoped for_each_available_child_of_node_scoped() when iterating
over device nodes to make code a bit simpler.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
 drivers/soc/fsl/qe/qmc.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/soc/fsl/qe/qmc.c b/drivers/soc/fsl/qe/qmc.c
index 76bb496305a0..d5937b5b5f15 100644
--- a/drivers/soc/fsl/qe/qmc.c
+++ b/drivers/soc/fsl/qe/qmc.c
@@ -1174,31 +1174,26 @@ static unsigned int qmc_nb_chans(struct qmc *qmc)
 
 static int qmc_of_parse_chans(struct qmc *qmc, struct device_node *np)
 {
-	struct device_node *chan_np;
 	struct qmc_chan *chan;
 	const char *mode;
 	u32 chan_id;
 	u64 ts_mask;
 	int ret;
 
-	for_each_available_child_of_node(np, chan_np) {
+	for_each_available_child_of_node_scoped(np, chan_np) {
 		ret = of_property_read_u32(chan_np, "reg", &chan_id);
 		if (ret) {
 			dev_err(qmc->dev, "%pOF: failed to read reg\n", chan_np);
-			of_node_put(chan_np);
 			return ret;
 		}
 		if (chan_id > 63) {
 			dev_err(qmc->dev, "%pOF: Invalid chan_id\n", chan_np);
-			of_node_put(chan_np);
 			return -EINVAL;
 		}
 
 		chan = devm_kzalloc(qmc->dev, sizeof(*chan), GFP_KERNEL);
-		if (!chan) {
-			of_node_put(chan_np);
+		if (!chan)
 			return -ENOMEM;
-		}
 
 		chan->id = chan_id;
 		spin_lock_init(&chan->ts_lock);
@@ -1209,7 +1204,6 @@ static int qmc_of_parse_chans(struct qmc *qmc, struct device_node *np)
 		if (ret) {
 			dev_err(qmc->dev, "%pOF: failed to read fsl,tx-ts-mask\n",
 				chan_np);
-			of_node_put(chan_np);
 			return ret;
 		}
 		chan->tx_ts_mask_avail = ts_mask;
@@ -1219,7 +1213,6 @@ static int qmc_of_parse_chans(struct qmc *qmc, struct device_node *np)
 		if (ret) {
 			dev_err(qmc->dev, "%pOF: failed to read fsl,rx-ts-mask\n",
 				chan_np);
-			of_node_put(chan_np);
 			return ret;
 		}
 		chan->rx_ts_mask_avail = ts_mask;
@@ -1230,7 +1223,6 @@ static int qmc_of_parse_chans(struct qmc *qmc, struct device_node *np)
 		if (ret && ret != -EINVAL) {
 			dev_err(qmc->dev, "%pOF: failed to read fsl,operational-mode\n",
 				chan_np);
-			of_node_put(chan_np);
 			return ret;
 		}
 		if (!strcmp(mode, "transparent")) {
@@ -1240,7 +1232,6 @@ static int qmc_of_parse_chans(struct qmc *qmc, struct device_node *np)
 		} else {
 			dev_err(qmc->dev, "%pOF: Invalid fsl,operational-mode (%s)\n",
 				chan_np, mode);
-			of_node_put(chan_np);
 			return -EINVAL;
 		}
 
-- 
2.34.1

[PATCH -next 4/8] soc: fsl: cpm1: qmc: Simplify with dev_err_probe()

[Jinjie Ruan]

Use the dev_err_probe() helper to simplify error handling during probe.
This also handle scenario, when EDEFER is returned and useless error
is printed.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
 drivers/soc/fsl/qe/qmc.c | 53 ++++++++++++++++------------------------
 1 file changed, 21 insertions(+), 32 deletions(-)

diff --git a/drivers/soc/fsl/qe/qmc.c b/drivers/soc/fsl/qe/qmc.c
index d5937b5b5f15..4315af115b8e 100644
--- a/drivers/soc/fsl/qe/qmc.c
+++ b/drivers/soc/fsl/qe/qmc.c
@@ -1182,14 +1182,10 @@ static int qmc_of_parse_chans(struct qmc *qmc, struct device_node *np)
 
 	for_each_available_child_of_node_scoped(np, chan_np) {
 		ret = of_property_read_u32(chan_np, "reg", &chan_id);
-		if (ret) {
-			dev_err(qmc->dev, "%pOF: failed to read reg\n", chan_np);
-			return ret;
-		}
-		if (chan_id > 63) {
-			dev_err(qmc->dev, "%pOF: Invalid chan_id\n", chan_np);
-			return -EINVAL;
-		}
+		if (ret)
+			return dev_err_probe(qmc->dev, ret, "%pOF: failed to read reg\n", chan_np);
+		if (chan_id > 63)
+			return dev_err_probe(qmc->dev, -EINVAL, "%pOF: Invalid chan_id\n", chan_np);
 
 		chan = devm_kzalloc(qmc->dev, sizeof(*chan), GFP_KERNEL);
 		if (!chan)
@@ -1201,39 +1197,34 @@ static int qmc_of_parse_chans(struct qmc *qmc, struct device_node *np)
 		spin_lock_init(&chan->tx_lock);
 
 		ret = of_property_read_u64(chan_np, "fsl,tx-ts-mask", &ts_mask);
-		if (ret) {
-			dev_err(qmc->dev, "%pOF: failed to read fsl,tx-ts-mask\n",
-				chan_np);
-			return ret;
-		}
+		if (ret)
+			return dev_err_probe(qmc->dev, ret,
+					     "%pOF: failed to read fsl,tx-ts-mask\n",
+					     chan_np);
 		chan->tx_ts_mask_avail = ts_mask;
 		chan->tx_ts_mask = chan->tx_ts_mask_avail;
 
 		ret = of_property_read_u64(chan_np, "fsl,rx-ts-mask", &ts_mask);
-		if (ret) {
-			dev_err(qmc->dev, "%pOF: failed to read fsl,rx-ts-mask\n",
-				chan_np);
-			return ret;
-		}
+		if (ret)
+			dev_err_probe(qmc->dev, ret, "%pOF: failed to read fsl,rx-ts-mask\n",
+				      chan_np);
 		chan->rx_ts_mask_avail = ts_mask;
 		chan->rx_ts_mask = chan->rx_ts_mask_avail;
 
 		mode = "transparent";
 		ret = of_property_read_string(chan_np, "fsl,operational-mode", &mode);
-		if (ret && ret != -EINVAL) {
-			dev_err(qmc->dev, "%pOF: failed to read fsl,operational-mode\n",
-				chan_np);
-			return ret;
-		}
+		if (ret && ret != -EINVAL)
+			return dev_err_probe(qmc->dev, ret,
+					     "%pOF: failed to read fsl,operational-mode\n",
+					     chan_np);
 		if (!strcmp(mode, "transparent")) {
 			chan->mode = QMC_TRANSPARENT;
 		} else if (!strcmp(mode, "hdlc")) {
 			chan->mode = QMC_HDLC;
-		} else {
-			dev_err(qmc->dev, "%pOF: Invalid fsl,operational-mode (%s)\n",
-				chan_np, mode);
-			return -EINVAL;
-		}
+		} else
+			return dev_err_probe(qmc->dev, -EINVAL,
+					     "%pOF: Invalid fsl,operational-mode (%s)\n",
+					     chan_np, mode);
 
 		chan->is_reverse_data = of_property_read_bool(chan_np,
 							      "fsl,reverse-data");
@@ -1590,10 +1581,8 @@ static int qmc_probe(struct platform_device *pdev)
 
 	/* Connect the serial (SCC) to TSA */
 	ret = tsa_serial_connect(qmc->tsa_serial);
-	if (ret) {
-		dev_err(qmc->dev, "Failed to connect TSA serial\n");
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(qmc->dev, ret, "Failed to connect TSA serial\n");
 
 	/* Parse channels informationss */
 	ret = qmc_of_parse_chans(qmc, np);
-- 
2.34.1

[PATCH -next 5/8] soc/tegra: pmc: Simplify with scoped for each OF child loop

[Jinjie Ruan]

Use scoped for_each_child_of_node_scoped() when iterating over device
nodes to make code a bit simpler.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
 drivers/soc/tegra/pmc.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index 6c37d6eb8b49..a08c377933c5 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -1438,7 +1438,7 @@ static int tegra_powergate_init(struct tegra_pmc *pmc,
 				struct device_node *parent)
 {
 	struct of_phandle_args child_args, parent_args;
-	struct device_node *np, *child;
+	struct device_node *np;
 	int err = 0;
 
 	/*
@@ -1457,12 +1457,10 @@ static int tegra_powergate_init(struct tegra_pmc *pmc,
 	if (!np)
 		return 0;
 
-	for_each_child_of_node(np, child) {
+	for_each_child_of_node_scoped(np, child) {
 		err = tegra_powergate_add(pmc, child);
-		if (err < 0) {
-			of_node_put(child);
+		if (err < 0)
 			break;
-		}
 
 		if (of_parse_phandle_with_args(child, "power-domains",
 					       "#power-domain-cells",
@@ -1474,10 +1472,8 @@ static int tegra_powergate_init(struct tegra_pmc *pmc,
 
 		err = of_genpd_add_subdomain(&parent_args, &child_args);
 		of_node_put(parent_args.np);
-		if (err) {
-			of_node_put(child);
+		if (err)
 			break;
-		}
 	}
 
 	of_node_put(np);
-- 
2.34.1

[PATCH -next 6/8] soc: dove: Simplify with scoped for each OF child loop

[Jinjie Ruan]

Use scoped for_each_child_of_node_scoped() when iterating over device
nodes to make code a bit simpler.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
 drivers/soc/dove/pmu.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/soc/dove/pmu.c b/drivers/soc/dove/pmu.c
index 6202dbcd20a8..75f5fc3cb2c6 100644
--- a/drivers/soc/dove/pmu.c
+++ b/drivers/soc/dove/pmu.c
@@ -372,7 +372,7 @@ int __init dove_init_pmu_legacy(const struct dove_pmu_initdata *initdata)
  */
 int __init dove_init_pmu(void)
 {
-	struct device_node *np_pmu, *domains_node, *np;
+	struct device_node *np_pmu, *domains_node;
 	struct pmu_data *pmu;
 	int ret, parent_irq;
 
@@ -405,21 +405,18 @@ int __init dove_init_pmu(void)
 
 	pmu_reset_init(pmu);
 
-	for_each_available_child_of_node(domains_node, np) {
+	for_each_available_child_of_node_scoped(domains_node, np) {
 		struct of_phandle_args args;
 		struct pmu_domain *domain;
 
 		domain = kzalloc(sizeof(*domain), GFP_KERNEL);
-		if (!domain) {
-			of_node_put(np);
+		if (!domain)
 			break;
-		}
 
 		domain->pmu = pmu;
 		domain->base.name = kasprintf(GFP_KERNEL, "%pOFn", np);
 		if (!domain->base.name) {
 			kfree(domain);
-			of_node_put(np);
 			break;
 		}
 
-- 
2.34.1

[PATCH -next 7/8] soc: ti: knav_dma: Simplify with scoped for each OF child loop

[Jinjie Ruan]

Use scoped for_each_child_of_node_scoped() when iterating over device
nodes to make code a bit simpler.

And use the dev_err_probe() helper to simplify error handling during
probe. This also handle scenario, when EDEFER is returned and useless
error is printed.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
 drivers/soc/ti/knav_dma.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/soc/ti/knav_dma.c b/drivers/soc/ti/knav_dma.c
index fb0746d8caad..eeec422a46f0 100644
--- a/drivers/soc/ti/knav_dma.c
+++ b/drivers/soc/ti/knav_dma.c
@@ -706,20 +706,15 @@ static int knav_dma_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct device_node *node = pdev->dev.of_node;
-	struct device_node *child;
 	int ret = 0;
 
-	if (!node) {
-		dev_err(&pdev->dev, "could not find device info\n");
-		return -EINVAL;
-	}
+	if (!node)
+		return dev_err_probe(&pdev->dev, -EINVAL, "could not find device info\n");
 
 	kdev = devm_kzalloc(dev,
 			sizeof(struct knav_dma_pool_device), GFP_KERNEL);
-	if (!kdev) {
-		dev_err(dev, "could not allocate driver mem\n");
-		return -ENOMEM;
-	}
+	if (!kdev)
+		return dev_err_probe(dev, -ENOMEM, "could not allocate driver mem\n");
 
 	kdev->dev = dev;
 	INIT_LIST_HEAD(&kdev->list);
@@ -732,10 +727,9 @@ static int knav_dma_probe(struct platform_device *pdev)
 	}
 
 	/* Initialise all packet dmas */
-	for_each_child_of_node(node, child) {
+	for_each_child_of_node_scoped(node, child) {
 		ret = dma_init(node, child);
 		if (ret) {
-			of_node_put(child);
 			dev_err(&pdev->dev, "init failed with %d\n", ret);
 			break;
 		}
-- 
2.34.1

[PATCH -next 8/8] soc: ti: knav_qmss_queue: Simplify with scoped for each OF child loop

[Jinjie Ruan]

Use scoped for_each_child_of_node_scoped() when iterating over device
nodes to make code a bit simpler.

And use the dev_err_probe() helper to simplify error handling during
probe. This also handle scenario, when EDEFER is returned and useless
error is printed.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
 drivers/soc/ti/knav_qmss_queue.c | 57 ++++++++++----------------------
 1 file changed, 18 insertions(+), 39 deletions(-)

diff --git a/drivers/soc/ti/knav_qmss_queue.c b/drivers/soc/ti/knav_qmss_queue.c
index a15eaa1900ab..5126863ed647 100644
--- a/drivers/soc/ti/knav_qmss_queue.c
+++ b/drivers/soc/ti/knav_qmss_queue.c
@@ -1080,17 +1080,13 @@ static int knav_queue_setup_regions(struct knav_device *kdev,
 {
 	struct device *dev = kdev->dev;
 	struct knav_region *region;
-	struct device_node *child;
 	u32 temp[2];
 	int ret;
 
-	for_each_child_of_node(regions, child) {
+	for_each_child_of_node_scoped(regions, child) {
 		region = devm_kzalloc(dev, sizeof(*region), GFP_KERNEL);
-		if (!region) {
-			of_node_put(child);
-			dev_err(dev, "out of memory allocating region\n");
-			return -ENOMEM;
-		}
+		if (!region)
+			return dev_err_probe(dev, -ENOMEM, "out of memory allocating region\n");
 
 		region->name = knav_queue_find_name(child);
 		of_property_read_u32(child, "id", &region->id);
@@ -1116,10 +1112,8 @@ static int knav_queue_setup_regions(struct knav_device *kdev,
 		INIT_LIST_HEAD(&region->pools);
 		list_add_tail(&region->list, &kdev->regions);
 	}
-	if (list_empty(&kdev->regions)) {
-		dev_err(dev, "no valid region information found\n");
-		return -ENODEV;
-	}
+	if (list_empty(&kdev->regions))
+		return dev_err_probe(dev, -ENODEV, "no valid region information found\n");
 
 	/* Next, we run through the regions and set things up */
 	for_each_region(kdev, region)
@@ -1313,10 +1307,8 @@ static int knav_setup_queue_pools(struct knav_device *kdev,
 	}
 
 	/* ... and barf if they all failed! */
-	if (list_empty(&kdev->queue_ranges)) {
-		dev_err(kdev->dev, "no valid queue range found\n");
-		return -ENODEV;
-	}
+	if (list_empty(&kdev->queue_ranges))
+		return dev_err_probe(kdev->dev, -ENODEV, "no valid queue range found\n");
 	return 0;
 }
 
@@ -1388,17 +1380,13 @@ static int knav_queue_init_qmgrs(struct knav_device *kdev,
 {
 	struct device *dev = kdev->dev;
 	struct knav_qmgr_info *qmgr;
-	struct device_node *child;
 	u32 temp[2];
 	int ret;
 
-	for_each_child_of_node(qmgrs, child) {
+	for_each_child_of_node_scoped(qmgrs, child) {
 		qmgr = devm_kzalloc(dev, sizeof(*qmgr), GFP_KERNEL);
-		if (!qmgr) {
-			of_node_put(child);
-			dev_err(dev, "out of memory allocating qmgr\n");
-			return -ENOMEM;
-		}
+		if (!qmgr)
+			return dev_err_probe(dev, -ENOMEM, "out of memory allocating qmgr\n");
 
 		ret = of_property_read_u32_array(child, "managed-queues",
 						 temp, 2);
@@ -1490,15 +1478,11 @@ static int knav_queue_init_pdsps(struct knav_device *kdev,
 {
 	struct device *dev = kdev->dev;
 	struct knav_pdsp_info *pdsp;
-	struct device_node *child;
 
-	for_each_child_of_node(pdsps, child) {
+	for_each_child_of_node_scoped(pdsps, child) {
 		pdsp = devm_kzalloc(dev, sizeof(*pdsp), GFP_KERNEL);
-		if (!pdsp) {
-			of_node_put(child);
-			dev_err(dev, "out of memory allocating pdsp\n");
-			return -ENOMEM;
-		}
+		if (!pdsp)
+			return dev_err_probe(dev, -ENOMEM, "out of memory allocating pdsp\n");
 		pdsp->name = knav_queue_find_name(child);
 		pdsp->iram =
 			knav_queue_map_reg(kdev, child,
@@ -1755,16 +1739,12 @@ static int knav_queue_probe(struct platform_device *pdev)
 	u32 temp[2];
 	int ret;
 
-	if (!node) {
-		dev_err(dev, "device tree info unavailable\n");
-		return -ENODEV;
-	}
+	if (!node)
+		return dev_err_probe(dev, -ENODEV, "device tree info unavailable\n");
 
 	kdev = devm_kzalloc(dev, sizeof(struct knav_device), GFP_KERNEL);
-	if (!kdev) {
-		dev_err(dev, "memory allocation failed\n");
-		return -ENOMEM;
-	}
+	if (!kdev)
+		return dev_err_probe(dev, -ENOMEM, "memory allocation failed\n");
 
 	if (device_get_match_data(dev))
 		kdev->version = QMSS_66AK2G;
@@ -1781,8 +1761,7 @@ static int knav_queue_probe(struct platform_device *pdev)
 	ret = pm_runtime_resume_and_get(&pdev->dev);
 	if (ret < 0) {
 		pm_runtime_disable(&pdev->dev);
-		dev_err(dev, "Failed to enable QMSS\n");
-		return ret;
+		return dev_err_probe(dev, ret, "Failed to enable QMSS\n");
 	}
 
 	if (of_property_read_u32_array(node, "queue-range", temp, 2)) {
-- 
2.34.1

Re: [PATCH -next 8/8] soc: ti: knav_qmss_queue: Simplify with scoped for each OF child loop

[Kousik Sanagavarapu]

Jinjie Ruan <ruanjinjie@huawei.com> writes:
> @@ -1080,17 +1080,13 @@ static int knav_queue_setup_regions(struct knav_device *kdev,
>  {
>  	struct device *dev = kdev->dev;
>  	struct knav_region *region;
> -	struct device_node *child;
>  	u32 temp[2];
>  	int ret;
>  
> -	for_each_child_of_node(regions, child) {
> +	for_each_child_of_node_scoped(regions, child) {

Are you sure using *_scoped() is better here?  Since it seems that we
need the memory pointed to by "child" in cases where we don't go into an
error path.

>  		region = devm_kzalloc(dev, sizeof(*region), GFP_KERNEL);
> -		if (!region) {
> -			of_node_put(child);
> -			dev_err(dev, "out of memory allocating region\n");
> -			return -ENOMEM;
> -		}
> +		if (!region)
> +			return dev_err_probe(dev, -ENOMEM, "out of memory allocating region\n");
>  
>  		region->name = knav_queue_find_name(child);
>  		of_property_read_u32(child, "id", &region->id);

Similarly in most of the other cases in this series where a similar
change is done.

Also FYI, as for dev_err_probe(), I think I covered all of them in this
file and a patch for it is currently sitting in ti-drivers-soc-next.

Thanks

Re: [PATCH -next 8/8] soc: ti: knav_qmss_queue: Simplify with scoped for each OF child loop

[Nishanth Menon]

On 21:28-20240829, Kousik Sanagavarapu wrote:
> Jinjie Ruan <ruanjinjie@huawei.com> writes:
> > @@ -1080,17 +1080,13 @@ static int knav_queue_setup_regions(struct knav_device *kdev,
> >  {
> >  	struct device *dev = kdev->dev;
> >  	struct knav_region *region;
> > -	struct device_node *child;
> >  	u32 temp[2];
> >  	int ret;
> >  
> > -	for_each_child_of_node(regions, child) {
> > +	for_each_child_of_node_scoped(regions, child) {
> 
> Are you sure using *_scoped() is better here?  Since it seems that we
> need the memory pointed to by "child" in cases where we don't go into an
> error path.
> 
> >  		region = devm_kzalloc(dev, sizeof(*region), GFP_KERNEL);
> > -		if (!region) {
> > -			of_node_put(child);
> > -			dev_err(dev, "out of memory allocating region\n");
> > -			return -ENOMEM;
> > -		}
> > +		if (!region)
> > +			return dev_err_probe(dev, -ENOMEM, "out of memory allocating region\n");
> >  
> >  		region->name = knav_queue_find_name(child);
> >  		of_property_read_u32(child, "id", &region->id);
> 
> Similarly in most of the other cases in this series where a similar
> change is done.
> 
> Also FYI, as for dev_err_probe(), I think I covered all of them in this
> file and a patch for it is currently sitting in ti-drivers-soc-next.
> 

Thanks Kousik. yeah - it will probably help with a rebase to latest
next.

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [PATCH -next 8/8] soc: ti: knav_qmss_queue: Simplify with scoped for each OF child loop

[Jinjie Ruan]

On 2024/8/29 23:58, Kousik Sanagavarapu wrote:
> Jinjie Ruan <ruanjinjie@huawei.com> writes:
>> @@ -1080,17 +1080,13 @@ static int knav_queue_setup_regions(struct knav_device *kdev,
>>  {
>>  	struct device *dev = kdev->dev;
>>  	struct knav_region *region;
>> -	struct device_node *child;
>>  	u32 temp[2];
>>  	int ret;
>>  
>> -	for_each_child_of_node(regions, child) {
>> +	for_each_child_of_node_scoped(regions, child) {
> 
> Are you sure using *_scoped() is better here?  Since it seems that we
> need the memory pointed to by "child" in cases where we don't go into an
> error path.

Hi, Jonathan, could you help review this code?

> 
>>  		region = devm_kzalloc(dev, sizeof(*region), GFP_KERNEL);
>> -		if (!region) {
>> -			of_node_put(child);
>> -			dev_err(dev, "out of memory allocating region\n");
>> -			return -ENOMEM;
>> -		}
>> +		if (!region)
>> +			return dev_err_probe(dev, -ENOMEM, "out of memory allocating region\n");
>>  
>>  		region->name = knav_queue_find_name(child);
>>  		of_property_read_u32(child, "id", &region->id);
> 
> Similarly in most of the other cases in this series where a similar
> change is done.
> 
> Also FYI, as for dev_err_probe(), I think I covered all of them in this
> file and a patch for it is currently sitting in ti-drivers-soc-next.
> 
> Thanks

Re: [PATCH -next 8/8] soc: ti: knav_qmss_queue: Simplify with scoped for each OF child loop

[Jonathan Cameron]

On Fri, 30 Aug 2024 11:24:14 +0800
Jinjie Ruan <ruanjinjie@huawei.com> wrote:

> On 2024/8/29 23:58, Kousik Sanagavarapu wrote:
> > Jinjie Ruan <ruanjinjie@huawei.com> writes:  
> >> @@ -1080,17 +1080,13 @@ static int knav_queue_setup_regions(struct knav_device *kdev,
> >>  {
> >>  	struct device *dev = kdev->dev;
> >>  	struct knav_region *region;
> >> -	struct device_node *child;
> >>  	u32 temp[2];
> >>  	int ret;
> >>  
> >> -	for_each_child_of_node(regions, child) {
> >> +	for_each_child_of_node_scoped(regions, child) {  
> > 
> > Are you sure using *_scoped() is better here?  Since it seems that we
> > need the memory pointed to by "child" in cases where we don't go into an
> > error path.  
> 
> Hi, Jonathan, could you help review this code?

I don't understand the review comment.
The reference counting before and after this patch is the same, just
with the error path handled in a simpler fashion and the scope of
the child variable reduced.


> 
> >   
> >>  		region = devm_kzalloc(dev, sizeof(*region), GFP_KERNEL);
> >> -		if (!region) {
> >> -			of_node_put(child);
> >> -			dev_err(dev, "out of memory allocating region\n");
> >> -			return -ENOMEM;
> >> -		}
> >> +		if (!region)
> >> +			return dev_err_probe(dev, -ENOMEM, "out of memory allocating region\n");
> >>  
> >>  		region->name = knav_queue_find_name(child);
> >>  		of_property_read_u32(child, "id", &region->id);  
> > 
> > Similarly in most of the other cases in this series where a similar
> > change is done.
> > 
> > Also FYI, as for dev_err_probe(), I think I covered all of them in this
> > file and a patch for it is currently sitting in ti-drivers-soc-next.
> > 
> > Thanks  

Re: (subset) [PATCH -next 0/8] soc: Simplify with scoped for each OF child loop and dev_err_probe()

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>


On Tue, 27 Aug 2024 19:45:59 +0800, Jinjie Ruan wrote:
> Use for_each_child_of_node_scoped() to simplify code.
> 
> Jinjie Ruan (8):
>   soc: fsl: cpm1: Simplify with scoped for each OF child loop
>   soc: fsl: cpm1: Simplify with dev_err_probe()
>   soc: fsl: cpm1: qmc: Simplify with scoped for each OF child
>   soc: fsl: cpm1: qmc: Simplify with dev_err_probe()
>   soc/tegra: pmc: Simplify with scoped for each OF child loop
>   soc: dove: Simplify with scoped for each OF child loop
>   soc: ti: knav_dma: Simplify with scoped for each OF child loop
>   soc: ti: knav_qmss_queue: Simplify with scoped for each OF child loop
> 
> [...]

Applied, thanks!

[5/8] soc/tegra: pmc: Simplify with scoped for each OF child loop
      commit: 4d57a840560c3ff04fed07a06b3aec7cbac4bff0

Best regards,
-- 
Thierry Reding <treding@nvidia.com>