Re: [PATCH] dmaengine: ppc4xx: convert irq_of_parse_and_map to platform_get_irq

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Invalid free_irq() call on error path when request_irq() fails.
--

commit b94f2fec69c0e655f6872194e27d4dbff577af31
Author: Rosen Penev <rosenp@gmail.com>

dmaengine: ppc4xx: convert irq_of_parse_and_map to platform_get_irq

This commit replaces irq_of_parse_and_map() with platform_get_irq() for
obtaining IRQ resources. It also eliminates the corresponding
irq_dispose_mapping() calls and adjusts the error handling paths.

> diff --git a/drivers/dma/ppc4xx/adma.c b/drivers/dma/ppc4xx/adma.c
> index 279a431ccae38..42fc1791662ba 100644
> --- a/drivers/dma/ppc4xx/adma.c
> +++ b/drivers/dma/ppc4xx/adma.c

[ ... ]

> @@ -3956,14 +3954,13 @@ static int ppc440spe_adma_setup_irqs(struct ppc440spe_adma_device *adev,
>  	return 0;
>  
>  err_req2:
> +	if (adev->err_irq > 0)
> +		free_irq(adev->err_irq, chan);

[Severity: Medium]
Will this added free_irq() trigger a kernel warning ("Trying to free
already-free IRQ") if we reach this path because the error IRQ request
itself failed?

In ppc440spe_adma_setup_irqs(), if request_irq() fails for adev->err_irq,
the code jumps directly to err_req2:

	ret = request_irq(adev->err_irq, ppc440spe_adma_err_handler, ...);
	if (ret) {
		dev_err(adev->dev, "can't request irq %d\n", adev->err_irq);
		*initcode = PPC_ADMA_INIT_IRQ2;
		ret = -EIO;
		goto err_req2;
	}

Since the IRQ was not successfully requested when this failure path is taken,
unconditionally calling free_irq() here will attempt to free an unallocated
IRQ.

Should we track whether the IRQ was successfully requested before freeing it,
or perhaps introduce a separate error label to avoid this?

>  	free_irq(adev->irq, chan);
>  err_req1:
> -	irq_dispose_mapping(adev->irq);
>  err_irq_map:
> -	if (adev->err_irq > 0) {
> -		if (atomic_dec_and_test(&ppc440spe_adma_err_irq_ref))
> -			irq_dispose_mapping(adev->err_irq);
> -	}
> +	if (adev->err_irq > 0)
> +		atomic_dec(&ppc440spe_adma_err_irq_ref);
>  	return ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910212720.70721-1-rosenp@gmail.com?part=1