[PATCH] ata: Remove redundant dev_err()

[PATCH] ata: Remove redundant dev_err()

[Pan Chuang]

Since commit 55b48e23f5c4 ("genirq/devres: Add error handling in
devm_request_*_irq()"), devm_request_irq() automatically logs
detailed error messages on failure. Remove the now-redundant
driver-specific dev_err() calls.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
 drivers/ata/pata_mpc52xx.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/ata/pata_mpc52xx.c b/drivers/ata/pata_mpc52xx.c
index 210a63283f62..1739743427cf 100644
--- a/drivers/ata/pata_mpc52xx.c
+++ b/drivers/ata/pata_mpc52xx.c
@@ -769,10 +769,8 @@ static int mpc52xx_ata_probe(struct platform_device *op)
 	task_irq = bcom_get_task_irq(dmatsk);
 	rv = devm_request_irq(&op->dev, task_irq, &mpc52xx_ata_task_irq, 0,
 				"ATA task", priv);
-	if (rv) {
-		dev_err(&op->dev, "error requesting DMA IRQ\n");
+	if (rv)
 		goto err2;
-	}
 	priv->dmatsk = dmatsk;
 
 	/* Init the hw */
-- 
2.34.1

Re: [PATCH] ata: Remove redundant dev_err()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Double Free of IRQ Mapping
- [High] Memory Leak of irqaction (Devres Conflict)
--

--- Patch [1]: [PATCH] ata: Remove redundant dev_err() ---
commit c11d795c8457981d78fb45735e5126724a48c0df
Author: Pan Chuang <panchuang@vivo.com>

ata: Remove redundant dev_err()

Since devm_request_irq() automatically logs error messages on failure,
this commit removes the redundant driver-specific dev_err() call from
the probe path.

> diff --git a/drivers/ata/pata_mpc52xx.c b/drivers/ata/pata_mpc52xx.c
> index 210a63283f62b..1739743427cfc 100644
> --- a/drivers/ata/pata_mpc52xx.c
> +++ b/drivers/ata/pata_mpc52xx.c
> @@ -769,10 +769,8 @@ static int mpc52xx_ata_probe(struct platform_device *op)
>  	task_irq = bcom_get_task_irq(dmatsk);
>  	rv = devm_request_irq(&op->dev, task_irq, &mpc52xx_ata_task_irq, 0,
>  				"ATA task", priv);
> -	if (rv) {
> -		dev_err(&op->dev, "error requesting DMA IRQ\n");
> +	if (rv)
>  		goto err2;

[Severity: High]
This is a pre-existing issue, but I noticed a potential double free of the
IRQ mapping and a memory leak related to this error path.

If devm_request_irq() fails, the code jumps to the err2 label:

 err2:
	irq_dispose_mapping(task_irq);
	bcom_ata_release(dmatsk);

Does this cause a double free of the IRQ mapping? The bcom_ata_release()
function internally calls bcom_task_free(), which also unconditionally calls
irq_dispose_mapping(tsk->irq) on the same task_irq.

Additionally, can these manual irq_dispose_mapping() calls in the err1 and err2
paths (and similarly in mpc52xx_ata_remove()) cause the irqaction structures
to leak?

Because devm_request_irq() is used, the devres cleanup runs after the probe
function returns. If the IRQ mapping is already manually disposed of here,
irq_dispose_mapping() removes the descriptor. When devres later invokes
free_irq(), it will find a NULL descriptor and return early, skipping the
kfree() for the irqaction structure.

> -	}
>  	priv->dmatsk = dmatsk;
>  
>  	/* Init the hw */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710110247.426444-1-panchuang@vivo.com?part=1

Re: [PATCH] ata: Remove redundant dev_err()

[Hannes Reinecke]

On 7/10/26 1:02 PM, Pan Chuang wrote:
> Since commit 55b48e23f5c4 ("genirq/devres: Add error handling in
> devm_request_*_irq()"), devm_request_irq() automatically logs
> detailed error messages on failure. Remove the now-redundant
> driver-specific dev_err() calls.
> 
> Signed-off-by: Pan Chuang <panchuang@vivo.com>
> ---
>   drivers/ata/pata_mpc52xx.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/ata/pata_mpc52xx.c b/drivers/ata/pata_mpc52xx.c
> index 210a63283f62..1739743427cf 100644
> --- a/drivers/ata/pata_mpc52xx.c
> +++ b/drivers/ata/pata_mpc52xx.c
> @@ -769,10 +769,8 @@ static int mpc52xx_ata_probe(struct platform_device *op)
>   	task_irq = bcom_get_task_irq(dmatsk);
>   	rv = devm_request_irq(&op->dev, task_irq, &mpc52xx_ata_task_irq, 0,
>   				"ATA task", priv);
> -	if (rv) {
> -		dev_err(&op->dev, "error requesting DMA IRQ\n");
> +	if (rv)
>   		goto err2;
> -	}
>   	priv->dmatsk = dmatsk;
>   
>   	/* Init the hw */
Reviewed-by: Hannes Reinecke <hare@kernel.org>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

Re: [PATCH] ata: Remove redundant dev_err()

[Damien Le Moal]

On 7/10/26 20:02, Pan Chuang wrote:
> Since commit 55b48e23f5c4 ("genirq/devres: Add error handling in
> devm_request_*_irq()"), devm_request_irq() automatically logs
> detailed error messages on failure. Remove the now-redundant
> driver-specific dev_err() calls.
> 
> Signed-off-by: Pan Chuang <panchuang@vivo.com>

Applied to for-7.3 with some corrections to the commit title and message.
Thanks!

-- 
Damien Le Moal
Western Digital Research