[PATCH] dma: fsldma: convert to platform_get_irq_optional()

[PATCH] dma: fsldma: convert to platform_get_irq_optional()

[Rosen Penev]

Replace the per-controller irq_of_parse_and_map() call with
platform_get_irq_optional(). The controller IRQ is optional — when
absent (-ENXIO) the driver falls back to per-channel IRQs. Any other
error is treated as fatal. The corresponding irq_dispose_mapping()
calls in the probe error path and remove function are removed.

The per-channel IRQ mapping in fsl_dma_chan_probe() uses a child
device_node rather than the platform device's of_node, so it is not
converted here.

Assisted-by: opencode:big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/dma/fsldma.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 98d02809ade5..08a8090178f8 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -1239,7 +1239,16 @@ static int fsldma_of_probe(struct platform_device *op)
 	}
 
 	/* map the channel IRQ if it exists, but don't hookup the handler yet */
-	fdev->irq = irq_of_parse_and_map(op->dev.of_node, 0);
+	fdev->irq = platform_get_irq_optional(op, 0);
+	if (fdev->irq < 0) {
+		if (fdev->irq != -ENXIO) {
+			err = fdev->irq;
+			iounmap(fdev->regs);
+			kfree(fdev);
+			return err;
+		}
+		fdev->irq = 0;
+	}
 
 	dma_cap_set(DMA_MEMCPY, fdev->common.cap_mask);
 	dma_cap_set(DMA_SLAVE, fdev->common.cap_mask);
@@ -1301,7 +1310,6 @@ static int fsldma_of_probe(struct platform_device *op)
 		if (fdev->chan[i])
 			fsl_dma_chan_remove(fdev->chan[i]);
 	}
-	irq_dispose_mapping(fdev->irq);
 	iounmap(fdev->regs);
 out_free:
 	kfree(fdev);
@@ -1323,7 +1331,6 @@ static void fsldma_of_remove(struct platform_device *op)
 		if (fdev->chan[i])
 			fsl_dma_chan_remove(fdev->chan[i]);
 	}
-	irq_dispose_mapping(fdev->irq);
 
 	iounmap(fdev->regs);
 	kfree(fdev);
-- 
2.54.0

Re: [PATCH] dma: fsldma: convert to platform_get_irq_optional()

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing tasklet_kill() in channel removal causes use-after-free of channel structure.
- [High] Ignored return value of dma_async_device_register() causes silent probe failure and later kernel panic.
--

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

dma: fsldma: convert to platform_get_irq_optional()

Replace the per-controller irq_of_parse_and_map() call with
platform_get_irq_optional(). The controller IRQ is optional — when
absent (-ENXIO) the driver falls back to per-channel IRQs.

> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
> index 22d62d958abd9..a98a7798fd180 100644
> --- a/drivers/dma/fsldma.c
> +++ b/drivers/dma/fsldma.c

[ ... ]

> @@ -1300,7 +1309,6 @@ static int fsldma_of_probe(struct platform_device *op)
>  		if (fdev->chan[i])
>  			fsl_dma_chan_remove(fdev->chan[i]);
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but does fsl_dma_chan_remove()
safely handle scheduled tasklets?

During driver removal, the driver calls fsldma_free_irqs() which executes
free_irq(). While free_irq() waits for currently executing interrupt
handlers to finish, it does not stop or wait for scheduled tasklets.

Immediately after, fsl_dma_chan_remove() frees the channel structure:

drivers/dma/fsldma.c:fsl_dma_chan_remove() {
	irq_dispose_mapping(chan->irq);
	list_del(&chan->common.device_node);
	iounmap(chan->regs);
	kfree(chan);
}

If an interrupt scheduled the tasklet just before free_irq() was called,
the tasklet could execute asynchronously on freed memory, leading to a
use-after-free when dma_do_tasklet() dereferences the chan pointer.

Should there be a call to tasklet_kill(&chan->tasklet) before freeing
the channel?

[Severity: High]
This is also a pre-existing issue, but is it safe to ignore the return
value of dma_async_device_register() during the probe?

Looking at fsldma_of_probe(), the driver unconditionally returns 0
after attempting registration:

drivers/dma/fsldma.c:fsldma_of_probe() {
	...
	err = fsldma_request_irqs(fdev);
	if (err) {
		dev_err(fdev->dev, "unable to request IRQs\n");
		goto out_free_fdev;
	}

	dma_async_device_register(&fdev->common);
	return 0;
}

If dma_async_device_register() fails, the global_node list head remains
zero-initialized, but the driver still successfully binds.

Later during device teardown, fsldma_of_remove() blindly unregisters:

drivers/dma/fsldma.c:fsldma_of_remove() {
	...
	dma_async_device_unregister(&fdev->common);
	...
}

Would this cause dma_async_device_unregister() to call list_del_rcu() on
a NULL pointer, resulting in a kernel panic?

> -	irq_dispose_mapping(fdev->irq);
>  	iounmap(fdev->regs);
>  out_free:
>  	kfree(fdev);

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