Re: [PATCH v3 5/5] dmaengine: mcf-edma: Use devm for per-channel IRQ registration

[Frank Li]

On Thu, Jun 25, 2026 at 10:59:41AM +0200, Jean-Michel Hautbois wrote:
> Register each eDMA transfer interrupt with a per-channel name
> ("eDMA-<n>") so /proc/interrupts and debugging tools can identify the
> channel behind each line, and switch the whole IRQ setup to
> devm_request_irq().
>
> Using the managed API lets devres release the handlers on probe
> failure or device removal, which removes the manual mcf_edma_irq_free()
> teardown and the IRQ leak / dangling irqaction that the previous error
> paths left behind. The devm_kasprintf() result is now checked for NULL
> before being used as the IRQ name.
>
> Because devres only frees the handlers after mcf_edma_remove() returns,
> the controller must be quiesced at the start of remove(): disable every
> channel's request and acknowledge any pending interrupt before tearing
> down the virtual channels. Otherwise an interrupt could fire into a
> partially torn-down state while the handlers are still registered.
>
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@xxxxxxxxxx>
> ---

Reviewed-by: Frank Li <Frank.Li@xxxxxxx>

>  drivers/dma/mcf-edma-main.c | 84 ++++++++++++++++++++++-----------------------
>  1 file changed, 41 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/dma/mcf-edma-main.c b/drivers/dma/mcf-edma-main.c
> index 3dab5d475d1b..119d49c829fb 100644
> --- a/drivers/dma/mcf-edma-main.c
> +++ b/drivers/dma/mcf-edma-main.c
> @@ -66,7 +66,7 @@ static irqreturn_t mcf_edma_err_handler(int irq, void *dev_id)
>  static int mcf_edma_irq_init(struct platform_device *pdev,
>  				struct fsl_edma_engine *mcf_edma)
>  {
> -	int ret = 0, i;
> +	int ret, i, chan = 0;
>  	struct resource *res;
>
>  	res = platform_get_resource_byname(pdev,
> @@ -74,33 +74,47 @@ static int mcf_edma_irq_init(struct platform_device *pdev,
>  	if (!res)
>  		return -1;
>
> -	for (ret = 0, i = res->start; i <= res->end; ++i)
> -		ret |= request_irq(i, mcf_edma_tx_handler, 0, "eDMA", mcf_edma);
> -	if (ret)
> -		return ret;
> +	for (i = res->start; i <= res->end; ++i) {
> +		char *irq_name;
> +
> +		irq_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "eDMA-%d", chan++);
> +		if (!irq_name)
> +			return -ENOMEM;
> +		ret = devm_request_irq(&pdev->dev, i, mcf_edma_tx_handler, 0,
> +				       irq_name, mcf_edma);
> +		if (ret)
> +			return ret;
> +	}
>
>  	res = platform_get_resource_byname(pdev,
>  			IORESOURCE_IRQ, "edma-tx-16-55");
>  	if (!res)
>  		return -1;
>
> -	for (ret = 0, i = res->start; i <= res->end; ++i)
> -		ret |= request_irq(i, mcf_edma_tx_handler, 0, "eDMA", mcf_edma);
> -	if (ret)
> -		return ret;
> +	for (i = res->start; i <= res->end; ++i) {
> +		char *irq_name;
> +
> +		irq_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "eDMA-%d", chan++);
> +		if (!irq_name)
> +			return -ENOMEM;
> +		ret = devm_request_irq(&pdev->dev, i, mcf_edma_tx_handler, 0,
> +				       irq_name, mcf_edma);
> +		if (ret)
> +			return ret;
> +	}
>
>  	ret = platform_get_irq_byname(pdev, "edma-tx-56-63");
>  	if (ret != -ENXIO) {
> -		ret = request_irq(ret, mcf_edma_tx_handler,
> -				  0, "eDMA", mcf_edma);
> +		ret = devm_request_irq(&pdev->dev, ret, mcf_edma_tx_handler, 0,
> +				       "eDMA-tx-56-63", mcf_edma);
>  		if (ret)
>  			return ret;
>  	}
>
>  	ret = platform_get_irq_byname(pdev, "edma-err");
>  	if (ret != -ENXIO) {
> -		ret = request_irq(ret, mcf_edma_err_handler,
> -				  0, "eDMA", mcf_edma);
> +		ret = devm_request_irq(&pdev->dev, ret, mcf_edma_err_handler, 0,
> +				       "eDMA-err", mcf_edma);
>  		if (ret)
>  			return ret;
>  	}
> @@ -108,35 +122,6 @@ static int mcf_edma_irq_init(struct platform_device *pdev,
>  	return 0;
>  }
>
> -static void mcf_edma_irq_free(struct platform_device *pdev,
> -				struct fsl_edma_engine *mcf_edma)
> -{
> -	int irq;
> -	struct resource *res;
> -
> -	res = platform_get_resource_byname(pdev,
> -			IORESOURCE_IRQ, "edma-tx-00-15");
> -	if (res) {
> -		for (irq = res->start; irq <= res->end; irq++)
> -			free_irq(irq, mcf_edma);
> -	}
> -
> -	res = platform_get_resource_byname(pdev,
> -			IORESOURCE_IRQ, "edma-tx-16-55");
> -	if (res) {
> -		for (irq = res->start; irq <= res->end; irq++)
> -			free_irq(irq, mcf_edma);
> -	}
> -
> -	irq = platform_get_irq_byname(pdev, "edma-tx-56-63");
> -	if (irq != -ENXIO)
> -		free_irq(irq, mcf_edma);
> -
> -	irq = platform_get_irq_byname(pdev, "edma-err");
> -	if (irq != -ENXIO)
> -		free_irq(irq, mcf_edma);
> -}
> -
>  static struct fsl_edma_drvdata mcf_data = {
>  	.flags = FSL_EDMA_DRV_EDMA64 | FSL_EDMA_DRV_MCF,
>  	.setup_irq = mcf_edma_irq_init,
> @@ -249,8 +234,21 @@ static int mcf_edma_probe(struct platform_device *pdev)
>  static void mcf_edma_remove(struct platform_device *pdev)
>  {
>  	struct fsl_edma_engine *mcf_edma = platform_get_drvdata(pdev);
> +	struct edma_regs *regs = &mcf_edma->regs;
> +	int i;
> +
> +	/*
> +	 * The per-channel interrupts are requested with devm and are only
> +	 * freed after this function returns.  Quiesce the controller first so
> +	 * that no interrupt can fire while the virtual channels are torn down:
> +	 * disable every channel's request and acknowledge any pending
> +	 * interrupt.
> +	 */
> +	for (i = 0; i < mcf_edma->n_chans; i++)
> +		fsl_edma_disable_request(&mcf_edma->chans[i]);
> +	iowrite32(~0, regs->inth);
> +	iowrite32(~0, regs->intl);
>
> -	mcf_edma_irq_free(pdev, mcf_edma);
>  	fsl_edma_cleanup_vchan(&mcf_edma->dma_dev);
>  	dma_async_device_unregister(&mcf_edma->dma_dev);
>  }
>
> --
> 2.39.5
>