diff mbox series

[-next,v2,4/4] soc: ti: knav_qmss_queue: Simplify with dev_err_probe()

Message ID 20240830063228.3519385-5-ruanjinjie@huawei.com (mailing list archive)
State New, archived
Headers show
Series soc: ti: Simplify with scoped for each OF child loop and dev_err_probe() | expand

Commit Message

Jinjie Ruan Aug. 30, 2024, 6:32 a.m. UTC 
Use the dev_err_probe() helper to simplify error handling
during probe.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
v2:
- Split into 2 patches.
- Rebased the newest next.
- Update the commit message.
---
 drivers/soc/ti/knav_qmss_queue.c | 33 +++++++++++---------------------
 1 file changed, 11 insertions(+), 22 deletions(-)

Comments

Nishanth Menon Aug. 30, 2024, 10:38 a.m. UTC | #1 
On 14:32-20240830, Jinjie Ruan wrote:
> Use the dev_err_probe() helper to simplify error handling
> during probe.
> 
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> ---
> v2:
> - Split into 2 patches.
> - Rebased the newest next.
> - Update the commit message.
> ---
>  drivers/soc/ti/knav_qmss_queue.c | 33 +++++++++++---------------------
>  1 file changed, 11 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/soc/ti/knav_qmss_queue.c b/drivers/soc/ti/knav_qmss_queue.c
> index 02983f8ba1b6..d583a86028af 100644
> --- a/drivers/soc/ti/knav_qmss_queue.c
> +++ b/drivers/soc/ti/knav_qmss_queue.c
> @@ -1091,10 +1091,8 @@ static int knav_queue_setup_regions(struct knav_device *kdev,
>  
>  	for_each_child_of_node_scoped(regions, child) {
>  		region = devm_kzalloc(dev, sizeof(*region), GFP_KERNEL);
> -		if (!region) {
> -			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");


These make no sense to me :( -> just using dev_err_probe when there is
no chance of -EPROBE_DEFER ?
>  
>  		region->name = knav_queue_find_name(child);
>  		of_property_read_u32(child, "id", &region->id);
> @@ -1407,10 +1405,8 @@ static int knav_queue_init_qmgrs(struct knav_device *kdev,
>  
>  	for_each_child_of_node_scoped(qmgrs, child) {
>  		qmgr = devm_kzalloc(dev, sizeof(*qmgr), GFP_KERNEL);
> -		if (!qmgr) {
> -			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");
>  

Neither this

>  		ret = of_property_read_u32_array(child, "managed-queues",
>  						 temp, 2);
> @@ -1505,10 +1501,8 @@ static int knav_queue_init_pdsps(struct knav_device *kdev,
>  
>  	for_each_child_of_node_scoped(pdsps, child) {
>  		pdsp = devm_kzalloc(dev, sizeof(*pdsp), GFP_KERNEL);
> -		if (!pdsp) {
> -			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");

or this

>  		pdsp->name = knav_queue_find_name(child);
>  		pdsp->iram =
>  			knav_queue_map_reg(kdev, child,
> @@ -1784,16 +1778,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");

or here.

>  
>  	if (device_get_match_data(dev))
>  		kdev->version = QMSS_66AK2G;
> @@ -1810,8 +1800,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");

Do you see -EPROBE_DEFER chance here? Please explain.

>  	}
>  
>  	if (of_property_read_u32_array(node, "queue-range", temp, 2)) {
> -- 
> 2.34.1
>
Krzysztof Kozlowski Aug. 31, 2024, 11:06 a.m. UTC | #2 
On 30/08/2024 08:32, Jinjie Ruan wrote:
> Use the dev_err_probe() helper to simplify error handling
> during probe.
> 
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> ---
> v2:
> - Split into 2 patches.
> - Rebased the newest next.
> - Update the commit message.
> ---
>  drivers/soc/ti/knav_qmss_queue.c | 33 +++++++++++---------------------
>  1 file changed, 11 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/soc/ti/knav_qmss_queue.c b/drivers/soc/ti/knav_qmss_queue.c
> index 02983f8ba1b6..d583a86028af 100644
> --- a/drivers/soc/ti/knav_qmss_queue.c
> +++ b/drivers/soc/ti/knav_qmss_queue.c
> @@ -1091,10 +1091,8 @@ static int knav_queue_setup_regions(struct knav_device *kdev,
>  
>  	for_each_child_of_node_scoped(regions, child) {
>  		region = devm_kzalloc(dev, sizeof(*region), GFP_KERNEL);
> -		if (!region) {
> -			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");

This as well does not make any sense. Please read carefully coding style
and then use coccinelle on all your contributions.

You send useless changes without understanding how this works. The tools
would help you, but it seems you do not want to use them.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/drivers/soc/ti/knav_qmss_queue.c b/drivers/soc/ti/knav_qmss_queue.c
index 02983f8ba1b6..d583a86028af 100644
--- a/drivers/soc/ti/knav_qmss_queue.c
+++ b/drivers/soc/ti/knav_qmss_queue.c
@@ -1091,10 +1091,8 @@  static int knav_queue_setup_regions(struct knav_device *kdev,
 
 	for_each_child_of_node_scoped(regions, child) {
 		region = devm_kzalloc(dev, sizeof(*region), GFP_KERNEL);
-		if (!region) {
-			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);
@@ -1407,10 +1405,8 @@  static int knav_queue_init_qmgrs(struct knav_device *kdev,
 
 	for_each_child_of_node_scoped(qmgrs, child) {
 		qmgr = devm_kzalloc(dev, sizeof(*qmgr), GFP_KERNEL);
-		if (!qmgr) {
-			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);
@@ -1505,10 +1501,8 @@  static int knav_queue_init_pdsps(struct knav_device *kdev,
 
 	for_each_child_of_node_scoped(pdsps, child) {
 		pdsp = devm_kzalloc(dev, sizeof(*pdsp), GFP_KERNEL);
-		if (!pdsp) {
-			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,
@@ -1784,16 +1778,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;
@@ -1810,8 +1800,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)) {