[PATCH v3 0/8] dmaengine: ti: omap-dma: probe/remove bug fixes and cleanup

[PATCH v3 0/8] dmaengine: ti: omap-dma: probe/remove bug fixes and cleanup

[Rosen Penev]

Fix several bugs in the omap-dma driver's probe error and remove paths:
missing return after failure, CPU PM notifier leaks and missing RCU
synchronization, channels freed without stopping hardware, IRQs left
enabled during teardown, descriptor pool destroyed too early, wrong
interrupt register used in remove, and a flexible array conversion.

v3: Address remaining review comments:
  - Split CPU PM notifier fix into leak fix + RCU sync
  - Add missing return in probe error path
  - Guard IRQENABLE_L1 accesses for legacy platforms
v2: Fix sashiko comments and add extra patch

Rosen Penev (8):
  dmaengine: ti: omap-dma: fix missing return in probe error path
  dmaengine: ti: omap-dma: synchronize CPU PM notifier removal
  dmaengine: ti: omap-dma: fix CPU PM notifier leak
  dmaengine: ti: omap-dma: stop channels during teardown
  dmaengine: ti: omap-dma: disable IRQs on probe failure
  dmaengine: ti: omap-dma: destroy descriptor pool last
  dmaengine: ti: omap-dma: fix interrupt handling in remove
  dmaengine: ti: omap-dma: turn lch_map into a flexible array

 drivers/dma/ti/omap-dma.c | 122 +++++++++++++++++++++++---------------
 1 file changed, 74 insertions(+), 48 deletions(-)

--
2.54.0

[PATCHv3 1/8] dmaengine: ti: omap-dma: fix missing return in probe error path

[Rosen Penev]

If of_dma_controller_register() fails, the error path omits the return
statement, causing probe to continue (and eventually succeed) despite
the DMA controller not being registered. Add the missing return rc;.

Fixes: 2e1136acf8a8 ("dmaengine: omap-dma: fix dma_pool resource leak in error paths")
Cc: stable@vger.kernel.org
Assisted-by: Opencode:BigPickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/dma/ti/omap-dma.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c
index 55ece7fd0d99..0f6dd6b0a301 100644
--- a/drivers/dma/ti/omap-dma.c
+++ b/drivers/dma/ti/omap-dma.c
@@ -1828,6 +1828,7 @@ static int omap_dma_probe(struct platform_device *pdev)
 			if (od->ll123_supported)
 				dma_pool_destroy(od->desc_pool);
 			omap_dma_free(od);
+			return rc;
 		}
 	}
 
-- 
2.54.0

[PATCHv3 2/8] dmaengine: ti: omap-dma: synchronize CPU PM notifier removal

[Rosen Penev]

cpu_pm_notify() walks the raw notifier chain under rcu_read_lock(),
while cpu_pm_unregister_notifier() only unlinks the notifier block.

The controller is devres allocated and can be freed shortly after
remove returns. Wait for an RCU grace period after unregistering the
CPU PM notifier so concurrent CPU PM readers cannot dereference a
freed notifier block.

Fixes: 4c74ecf79227 ("dmaengine: ti: omap-dma: Add device tree match data and use it for cpu_pm")
Cc: stable@vger.kernel.org
Assisted-by: Codex:GPT-5
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/dma/ti/omap-dma.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c
index 0f6dd6b0a301..15be3c90440a 100644
--- a/drivers/dma/ti/omap-dma.c
+++ b/drivers/dma/ti/omap-dma.c
@@ -14,6 +14,7 @@
 #include <linux/module.h>
 #include <linux/omap-dma.h>
 #include <linux/platform_device.h>
+#include <linux/rcupdate.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 #include <linux/of.h>
@@ -1853,8 +1854,10 @@ static void omap_dma_remove(struct platform_device *pdev)
 	struct omap_dmadev *od = platform_get_drvdata(pdev);
 	int irq;
 
-	if (od->cfg->may_lose_context)
+	if (od->cfg->may_lose_context) {
 		cpu_pm_unregister_notifier(&od->nb);
+		synchronize_rcu();
+	}
 
 	if (pdev->dev.of_node)
 		of_dma_controller_free(pdev->dev.of_node);
-- 
2.54.0

[PATCHv3 3/8] dmaengine: ti: omap-dma: fix CPU PM notifier leak

[Rosen Penev]

The CPU PM notifier may be registered for needs_busy_check on omap2
rather than may_lose_context on omap3. The remove path only checked
may_lose_context, leaving the omap2 notifier registered during driver
removal.

Check both configuration flags before unregistering the notifier.

Fixes: f4cfa36dab67 ("dmaengine: ti: omap-dma: Use cpu notifier to block idle for omap2")
Cc: stable@vger.kernel.org
Assisted-by: Codex:GPT-5
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/dma/ti/omap-dma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c
index 15be3c90440a..0ad8da8b35f8 100644
--- a/drivers/dma/ti/omap-dma.c
+++ b/drivers/dma/ti/omap-dma.c
@@ -1854,7 +1854,7 @@ static void omap_dma_remove(struct platform_device *pdev)
 	struct omap_dmadev *od = platform_get_drvdata(pdev);
 	int irq;
 
-	if (od->cfg->may_lose_context) {
+	if (od->cfg->needs_busy_check || od->cfg->may_lose_context) {
 		cpu_pm_unregister_notifier(&od->nb);
 		synchronize_rcu();
 	}
-- 
2.54.0

[PATCHv3 4/8] dmaengine: ti: omap-dma: stop channels during teardown

[Rosen Penev]

omap_dma_free() removes channels and frees their storage without
first stopping an active transfer. A channel may have moved the
active descriptor out of the virt-dma lists into c->desc, so freeing
only the list state can leave hardware running against descriptor
memory that is about to disappear.

Terminate each channel before removing it, then drain the virt-dma
resource lists before freeing the channel structure.

Fixes: 7bedaa553760 ("dmaengine: add OMAP DMA engine driver")
Cc: stable@vger.kernel.org
Assisted-by: Codex:GPT-5
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/dma/ti/omap-dma.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c
index 0ad8da8b35f8..cef4e3a38b04 100644
--- a/drivers/dma/ti/omap-dma.c
+++ b/drivers/dma/ti/omap-dma.c
@@ -1521,8 +1521,10 @@ static void omap_dma_free(struct omap_dmadev *od)
 		struct omap_chan *c = list_first_entry(&od->ddev.channels,
 			struct omap_chan, vc.chan.device_node);
 
+		omap_dma_terminate_all(&c->vc.chan);
 		list_del(&c->vc.chan.device_node);
 		tasklet_kill(&c->vc.task);
+		vchan_free_chan_resources(&c->vc);
 		kfree(c);
 	}
 }
-- 
2.54.0

[PATCHv3 5/8] dmaengine: ti: omap-dma: disable IRQs on probe failure

[Rosen Penev]

The probe failure paths after IRQ setup free channel state while
hardware interrupts can still be enabled. A concurrent interrupt can
then walk lch_map[] and access channel memory that teardown is
releasing.

Disable IRQENABLE_L1 and clear irq_enable_mask under irq_lock before
teardown, then read IRQENABLE_L1 back to flush the posted write. Guard
the L1 accesses for legacy platforms where that register is not
mapped.

Fixes: 7bedaa553760 ("dmaengine: add OMAP DMA engine driver")
Cc: stable@vger.kernel.org
Assisted-by: Codex:GPT-5
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/dma/ti/omap-dma.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c
index cef4e3a38b04..61a935660341 100644
--- a/drivers/dma/ti/omap-dma.c
+++ b/drivers/dma/ti/omap-dma.c
@@ -1811,6 +1811,13 @@ static int omap_dma_probe(struct platform_device *pdev)
 	if (rc) {
 		pr_warn("OMAP-DMA: failed to register slave DMA engine device: %d\n",
 			rc);
+		if (!omap_dma_legacy(od)) {
+			spin_lock_irq(&od->irq_lock);
+			od->irq_enable_mask = 0;
+			omap_dma_glbl_write(od, IRQENABLE_L1, 0);
+			spin_unlock_irq(&od->irq_lock);
+			omap_dma_glbl_read(od, IRQENABLE_L1);
+		}
 		if (od->ll123_supported)
 			dma_pool_destroy(od->desc_pool);
 		omap_dma_free(od);
@@ -1828,6 +1835,13 @@ static int omap_dma_probe(struct platform_device *pdev)
 		if (rc) {
 			pr_warn("OMAP-DMA: failed to register DMA controller\n");
 			dma_async_device_unregister(&od->ddev);
+			if (!omap_dma_legacy(od)) {
+				spin_lock_irq(&od->irq_lock);
+				od->irq_enable_mask = 0;
+				omap_dma_glbl_write(od, IRQENABLE_L1, 0);
+				spin_unlock_irq(&od->irq_lock);
+				omap_dma_glbl_read(od, IRQENABLE_L1);
+			}
 			if (od->ll123_supported)
 				dma_pool_destroy(od->desc_pool);
 			omap_dma_free(od);
-- 
2.54.0

[PATCHv3 6/8] dmaengine: ti: omap-dma: destroy descriptor pool last

[Rosen Penev]

Linked-list descriptors are allocated from desc_pool and can be
released from omap_dma_free() through the channel descriptor cleanup
path. Destroying desc_pool before freeing channels leaves descriptor
cleanup with a dangling pool pointer.

Free the channels before destroying desc_pool in probe failure paths
and in remove.

Fixes: 1c2e8e6b6429 ("dmaengine: omap-dma: Support for LinkedList transfer of slave_sg")
Fixes: 2e1136acf8a8 ("dmaengine: omap-dma: fix dma_pool resource leak in error paths")
Cc: stable@vger.kernel.org
Assisted-by: Codex:GPT-5
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/dma/ti/omap-dma.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c
index 61a935660341..c0890d8c43ba 100644
--- a/drivers/dma/ti/omap-dma.c
+++ b/drivers/dma/ti/omap-dma.c
@@ -1818,9 +1818,9 @@ static int omap_dma_probe(struct platform_device *pdev)
 			spin_unlock_irq(&od->irq_lock);
 			omap_dma_glbl_read(od, IRQENABLE_L1);
 		}
+		omap_dma_free(od);
 		if (od->ll123_supported)
 			dma_pool_destroy(od->desc_pool);
-		omap_dma_free(od);
 		return rc;
 	}
 
@@ -1842,9 +1842,9 @@ static int omap_dma_probe(struct platform_device *pdev)
 				spin_unlock_irq(&od->irq_lock);
 				omap_dma_glbl_read(od, IRQENABLE_L1);
 			}
+			omap_dma_free(od);
 			if (od->ll123_supported)
 				dma_pool_destroy(od->desc_pool);
-			omap_dma_free(od);
 			return rc;
 		}
 	}
@@ -1888,10 +1888,10 @@ static void omap_dma_remove(struct platform_device *pdev)
 		omap_dma_glbl_write(od, IRQENABLE_L0, 0);
 	}
 
+	omap_dma_free(od);
+
 	if (od->ll123_supported)
 		dma_pool_destroy(od->desc_pool);
-
-	omap_dma_free(od);
 }
 
 static const struct omap_dma_config omap2420_data = {
-- 
2.54.0

[PATCHv3 7/8] dmaengine: ti: omap-dma: fix interrupt handling in remove

[Rosen Penev]

The remove path had several pre-existing bugs:

1. Interrupts are enabled via IRQENABLE_L1 in probe and
   alloc_chan_resources, but the remove path writes to IRQENABLE_L0,
   which has no effect on the L1 interrupt line. The DMA engine can
   continue asserting its IRQ during removal. Write to IRQENABLE_L1
   instead.

2. devm_free_irq() was called before disabling hardware interrupts.
   With IRQF_SHARED, the hardware may still assert the IRQ line after
   the handler is freed, causing unhandled interrupts that can lead to
   the kernel permanently disabling the shared IRQ line. Disable
   interrupts first.

3. platform_get_irq() return value was not checked before
   devm_free_irq(). If it returns an error code (<= 0), passing it to
   devm_free_irq() is incorrect. Add a guard.

4. Clearing od->irq_enable_mask and writing to IRQENABLE_L1 raced with
   the interrupt handler, which reads irq_enable_mask under the
   spinlock. Hold irq_lock around the disable.

5. The posted write to IRQENABLE_L1 used _relaxed accessors with no
   readback to drain the write buffer. Add a readback flush before
   devm_free_irq() to ensure the hardware has actually disabled the
   interrupt line.

Fixes: 2e1136acf8a8 ("dmaengine: omap-dma: fix dma_pool resource leak in error paths")
Cc: stable@vger.kernel.org
Assisted-by: Opencode:BigPickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/dma/ti/omap-dma.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c
index c0890d8c43ba..7343325ce2b1 100644
--- a/drivers/dma/ti/omap-dma.c
+++ b/drivers/dma/ti/omap-dma.c
@@ -1517,9 +1517,11 @@ static int omap_dma_chan_init(struct omap_dmadev *od)
 
 static void omap_dma_free(struct omap_dmadev *od)
 {
+	struct omap_chan *c;
+
 	while (!list_empty(&od->ddev.channels)) {
-		struct omap_chan *c = list_first_entry(&od->ddev.channels,
-			struct omap_chan, vc.chan.device_node);
+		c = list_first_entry(&od->ddev.channels,
+				     struct omap_chan, vc.chan.device_node);
 
 		omap_dma_terminate_all(&c->vc.chan);
 		list_del(&c->vc.chan.device_node);
@@ -1878,16 +1880,20 @@ static void omap_dma_remove(struct platform_device *pdev)
 	if (pdev->dev.of_node)
 		of_dma_controller_free(pdev->dev.of_node);
 
-	irq = platform_get_irq(pdev, 1);
-	devm_free_irq(&pdev->dev, irq, od);
-
 	dma_async_device_unregister(&od->ddev);
 
 	if (!omap_dma_legacy(od)) {
-		/* Disable all interrupts */
-		omap_dma_glbl_write(od, IRQENABLE_L0, 0);
+		spin_lock_irq(&od->irq_lock);
+		od->irq_enable_mask = 0;
+		omap_dma_glbl_write(od, IRQENABLE_L1, 0);
+		spin_unlock_irq(&od->irq_lock);
+		omap_dma_glbl_read(od, IRQENABLE_L1);
 	}
 
+	irq = platform_get_irq(pdev, 1);
+	if (irq > 0)
+		devm_free_irq(&pdev->dev, irq, od);
+
 	omap_dma_free(od);
 
 	if (od->ll123_supported)
-- 
2.54.0

[PATCHv3 8/8] dmaengine: ti: omap-dma: turn lch_map into a flexible array

[Rosen Penev]

Convert the separately-allocated lch_map pointer array to a C99
flexible array member at the end of struct omap_dmadev and annotate it
with __counted_by(lch_count). The probe is reordered so platform_data
lookup and the lch_count determination happen before the parent
allocation, letting struct_size() size the FAM and the dedicated
devm_kcalloc() for lch_map go away.

Two allocations collapse into one and the runtime bounds checks from
__counted_by now apply to every lch_map[] access.

Assisted-by: Opencode:BigPickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/dma/ti/omap-dma.c | 72 +++++++++++++++++++--------------------
 1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c
index 7343325ce2b1..dab600604572 100644
--- a/drivers/dma/ti/omap-dma.c
+++ b/drivers/dma/ti/omap-dma.c
@@ -49,7 +49,7 @@ struct omap_dmadev {
 	const struct omap_dma_config *cfg;
 	struct notifier_block nb;
 	struct omap_dma_context context;
-	int lch_count;
+	u32 lch_count;
 	DECLARE_BITMAP(lch_bitmap, OMAP_SDMA_CHANNELS);
 	struct mutex lch_lock;		/* for assigning logical channels */
 	bool legacy;
@@ -58,7 +58,7 @@ struct omap_dmadev {
 	unsigned dma_requests;
 	spinlock_t irq_lock;
 	uint32_t irq_enable_mask;
-	struct omap_chan **lch_map;
+	struct omap_chan *lch_map[] __counted_by(lch_count);
 };
 
 struct omap_chan {
@@ -1661,36 +1661,55 @@ static const struct omap_dma_config default_cfg;
 static int omap_dma_probe(struct platform_device *pdev)
 {
 	const struct omap_dma_config *conf;
+	struct omap_system_dma_plat_info *plat;
 	struct omap_dmadev *od;
+	u32 lch_count;
 	int rc, i, irq;
 	u32 val;
 
-	od = devm_kzalloc(&pdev->dev, sizeof(*od), GFP_KERNEL);
-	if (!od)
-		return -ENOMEM;
-
-	od->base = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(od->base))
-		return PTR_ERR(od->base);
-
 	conf = of_device_get_match_data(&pdev->dev);
 	if (conf) {
-		od->cfg = conf;
-		od->plat = dev_get_platdata(&pdev->dev);
-		if (!od->plat) {
+		plat = dev_get_platdata(&pdev->dev);
+		if (!plat) {
 			dev_err(&pdev->dev, "omap_system_dma_plat_info is missing");
 			return -ENODEV;
 		}
 	} else if (IS_ENABLED(CONFIG_ARCH_OMAP1)) {
-		od->cfg = &default_cfg;
-
-		od->plat = omap_get_plat_info();
-		if (!od->plat)
+		plat = omap_get_plat_info();
+		if (!plat)
 			return -EPROBE_DEFER;
 	} else {
 		return -ENODEV;
 	}
 
+	/* Number of available logical channels */
+	if (!pdev->dev.of_node) {
+		lch_count = plat->dma_attr->lch_count;
+		if (unlikely(!lch_count))
+			lch_count = OMAP_SDMA_CHANNELS;
+	} else if (of_property_read_u32(pdev->dev.of_node, "dma-channels", &lch_count)) {
+		dev_info(&pdev->dev, "Missing dma-channels property, using %u.\n",
+			 OMAP_SDMA_CHANNELS);
+		lch_count = OMAP_SDMA_CHANNELS;
+	}
+
+	if (lch_count > OMAP_SDMA_CHANNELS) {
+		dev_err(&pdev->dev, "invalid dma-channels value %u\n", lch_count);
+		return -EINVAL;
+	}
+
+	od = devm_kzalloc(&pdev->dev, struct_size(od, lch_map, lch_count), GFP_KERNEL);
+	if (!od)
+		return -ENOMEM;
+
+	od->lch_count = lch_count;
+	od->plat = plat;
+	od->cfg = conf ? conf : &default_cfg;
+
+	od->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(od->base))
+		return PTR_ERR(od->base);
+
 	od->reg_map = od->plat->reg_map;
 
 	dma_cap_set(DMA_SLAVE, od->ddev.cap_mask);
@@ -1735,19 +1754,6 @@ static int omap_dma_probe(struct platform_device *pdev)
 			 OMAP_SDMA_REQUESTS);
 	}
 
-	/* Number of available logical channels */
-	if (!pdev->dev.of_node) {
-		od->lch_count = od->plat->dma_attr->lch_count;
-		if (unlikely(!od->lch_count))
-			od->lch_count = OMAP_SDMA_CHANNELS;
-	} else if (of_property_read_u32(pdev->dev.of_node, "dma-channels",
-					&od->lch_count)) {
-		dev_info(&pdev->dev,
-			 "Missing dma-channels property, using %u.\n",
-			 OMAP_SDMA_CHANNELS);
-		od->lch_count = OMAP_SDMA_CHANNELS;
-	}
-
 	/* Mask of allowed logical channels */
 	if (pdev->dev.of_node && !of_property_read_u32(pdev->dev.of_node,
 						       "dma-channel-mask",
@@ -1759,12 +1765,6 @@ static int omap_dma_probe(struct platform_device *pdev)
 	if (od->plat->dma_attr->dev_caps & HS_CHANNELS_RESERVED)
 		bitmap_set(od->lch_bitmap, 0, 2);
 
-	od->lch_map = devm_kcalloc(&pdev->dev, od->lch_count,
-				   sizeof(*od->lch_map),
-				   GFP_KERNEL);
-	if (!od->lch_map)
-		return -ENOMEM;
-
 	for (i = 0; i < od->dma_requests; i++) {
 		rc = omap_dma_chan_init(od);
 		if (rc) {
-- 
2.54.0