[PATCH V2 0/3] dmaengine: zynqmp_dma: Add per-channel reset support

[PATCH V2 0/3] dmaengine: zynqmp_dma: Add per-channel reset support

[Golla Nagendra]

This series adds per-channel reset support to the ZynqMP DMA driver using
the generic reset framework, along with the corresponding dt-bindings
update. It also adds a runtime PM guard in the IRQ handler to handle
spurious interrupts safely.

Patch 1 adds the optional 'resets' property to the ZynqMP DMA dt-binding.

Patch 2 adds reset control handling in the channel probe path to assert
and deassert the channel reset during initialization.

Patch 3 adds a pm_runtime_get_if_active() check in the IRQ handler to
avoid accessing hardware registers when the device is runtime-suspended,
which could occur on spurious interrupts.

Changes in V2:
- Added patch 3 to guard IRQ handler against spurious interrupts

Golla Nagendra (2):
  dmaengine: zynqmp_dma: Add per-channel reset support
  dmaengine: zynqmp_dma: Guard IRQ handler against spurious interrupts

Jay Buddhabhatti (1):
  dt-bindings: dma: xilinx: Add optional resets property for ZDMA

 .../bindings/dma/xilinx/xlnx,zynqmp-dma-1.0.yaml     |  3 +++
 drivers/dma/xilinx/zynqmp_dma.c                      | 12 ++++++++++++
 2 files changed, 15 insertions(+)

-- 
2.34.1

[PATCH V2 1/3] dt-bindings: dma: xilinx: Add optional resets property for ZDMA

[Golla Nagendra]

From: Jay Buddhabhatti <jay.buddhabhatti@amd.com>

Newer SoCs such as Versal Gen2 and Versal‑Net expose a reset line
for ZDMA. Older SoCs do not have this provision. Add an optional
resets property to describe this reset.

Signed-off-by: Jay Buddhabhatti <jay.buddhabhatti@amd.com>
Co-developed-by: Golla Nagendra <nagendra.golla@amd.com>
Signed-off-by: Golla Nagendra <nagendra.golla@amd.com>
---
 .../devicetree/bindings/dma/xilinx/xlnx,zynqmp-dma-1.0.yaml    | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/dma/xilinx/xlnx,zynqmp-dma-1.0.yaml b/Documentation/devicetree/bindings/dma/xilinx/xlnx,zynqmp-dma-1.0.yaml
index 2da86037ad79..dff16763e11b 100644
--- a/Documentation/devicetree/bindings/dma/xilinx/xlnx,zynqmp-dma-1.0.yaml
+++ b/Documentation/devicetree/bindings/dma/xilinx/xlnx,zynqmp-dma-1.0.yaml
@@ -56,6 +56,9 @@ properties:
   iommus:
     maxItems: 1
 
+  resets:
+    maxItems: 1
+
   power-domains:
     maxItems: 1
 
-- 
2.34.1

[PATCH V2 2/3] dmaengine: zynqmp_dma: Add per-channel reset support

[Golla Nagendra]

Versal Gen 2 and Versal Net SoCs expose a dedicated reset line per
ZDMA channel, replacing the earlier approach where a single reset
was shared across all channels. Add reset handling in the channel
probe path using device_reset_optional() to trigger a reset pulse
on the channel during initialization.

Platforms without per-channel reset continue to work unaffected
since device_reset_optional() returns 0 when no reset is specified.

add pm_runtime_put_noidle() in the probe error path before
pm_runtime_disable() to balance the usage counter incremented by
pm_runtime_resume_and_get(). This is particularly important since
device_reset_optional() can return -EPROBE_DEFER, causing the
kernel to retry probe() and leak one PM usage count per retry
without the put.

Signed-off-by: Golla Nagendra <nagendra.golla@amd.com>
---
 drivers/dma/xilinx/zynqmp_dma.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c
index f6a812e49ddc..a9dfec3c0ca3 100644
--- a/drivers/dma/xilinx/zynqmp_dma.c
+++ b/drivers/dma/xilinx/zynqmp_dma.c
@@ -18,6 +18,7 @@
 #include <linux/clk.h>
 #include <linux/io-64-nonatomic-lo-hi.h>
 #include <linux/pm_runtime.h>
+#include <linux/reset.h>
 
 #include "../dmaengine.h"
 
@@ -916,6 +917,11 @@ static int zynqmp_dma_chan_probe(struct zynqmp_dma_device *zdev,
 	if (IS_ERR(chan->regs))
 		return PTR_ERR(chan->regs);
 
+	err = device_reset_optional(&pdev->dev);
+	if (err)
+		return dev_err_probe(&pdev->dev, err,
+				     "failed to reset channel\n");
+
 	chan->bus_width = ZYNQMP_DMA_BUS_WIDTH_64;
 	chan->dst_burst_len = ZYNQMP_DMA_MAX_DST_BURST_LEN;
 	chan->src_burst_len = ZYNQMP_DMA_MAX_SRC_BURST_LEN;
@@ -1152,6 +1158,7 @@ static int zynqmp_dma_probe(struct platform_device *pdev)
 err_disable_pm:
 	if (!pm_runtime_enabled(zdev->dev))
 		zynqmp_dma_runtime_suspend(zdev->dev);
+	pm_runtime_put_noidle(zdev->dev);
 	pm_runtime_disable(zdev->dev);
 	return ret;
 }
-- 
2.34.1

Re: [PATCH V2 2/3] dmaengine: zynqmp_dma: Add per-channel reset support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Using pm_runtime_put_noidle() before pm_runtime_disable() leaves hardware active and leaks clock references on probe deferral.
--

commit 0e5952f6551c9a8d3b55c9a149969bfbe79c7c9c
Author: Golla Nagendra <nagendra.golla@amd.com>

dmaengine: zynqmp_dma: Add per-channel reset support

This commit adds per-channel reset support for Versal Gen 2 and Versal Net
SoCs using device_reset_optional() during channel probe. It also adds a call
to pm_runtime_put_noidle() in the probe error path to balance the usage
counter.

> diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c
> --- a/drivers/dma/xilinx/zynqmp_dma.c
> +++ b/drivers/dma/xilinx/zynqmp_dma.c
[ ... ]
> @@ -1152,6 +1158,7 @@ static int zynqmp_dma_probe(struct platform_device *pdev)
>  err_disable_pm:
>  	if (!pm_runtime_enabled(zdev->dev))
>  		zynqmp_dma_runtime_suspend(zdev->dev);
> +	pm_runtime_put_noidle(zdev->dev);

[Severity: High]
Does using pm_runtime_put_noidle() here instead of pm_runtime_put_sync()
cause a clock leak during probe deferral?

If device_reset_optional() returns -EPROBE_DEFER, pm_runtime_put_noidle()
decrements the usage counter but does not suspend the device. The device
status remains active. When pm_runtime_disable() is called and the probe
fails, the driver core handles the failure by forcefully setting the status
to suspended without calling the driver's runtime suspend callback.

This leaves clk_main and clk_apb enabled. On the next probe retry,
pm_runtime_resume_and_get() will double-enable the clocks.

Should pm_runtime_put_sync() be used before pm_runtime_disable() to ensure
the device is synchronously suspended and clocks are properly disabled?

>  	pm_runtime_disable(zdev->dev);
>  	return ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618071056.2024286-1-nagendra.golla@amd.com?part=2

[PATCH V2 3/3] dmaengine: zynqmp_dma: Guard IRQ handler against spurious interrupts

[Golla Nagendra]

Add pm_runtime_get_if_active() check in zynqmp_dma_irq_handler() to
safely handle spurious interrupts that may arrive while the device is
runtime-suspended. Without this guard, a spurious interrupt could cause
the handler to access hardware registers (ISR, IMR) with clocks gated,
potentially leading to a synchronous external abort and kernel crash.

When the device is not runtime-active, pm_runtime_get_if_active()
returns false without incrementing the usage counter, and the handler
returns IRQ_NONE immediately. When the device is active, it increments
the usage counter to prevent a concurrent runtime suspend during
register access, and pm_runtime_put() releases the reference afterward.

Signed-off-by: Golla Nagendra <nagendra.golla@amd.com>
---
 drivers/dma/xilinx/zynqmp_dma.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c
index a9dfec3c0ca3..ce9163138be7 100644
--- a/drivers/dma/xilinx/zynqmp_dma.c
+++ b/drivers/dma/xilinx/zynqmp_dma.c
@@ -730,6 +730,9 @@ static irqreturn_t zynqmp_dma_irq_handler(int irq, void *data)
 	u32 isr, imr, status;
 	irqreturn_t ret = IRQ_NONE;
 
+	if (pm_runtime_get_if_active(chan->dev) <= 0)
+		return IRQ_NONE;
+
 	isr = readl(chan->regs + ZYNQMP_DMA_ISR);
 	imr = readl(chan->regs + ZYNQMP_DMA_IMR);
 	status = isr & ~imr;
@@ -756,6 +759,8 @@ static irqreturn_t zynqmp_dma_irq_handler(int irq, void *data)
 		ret = IRQ_HANDLED;
 	}
 
+	pm_runtime_put(chan->dev);
+
 	return ret;
 }
 
-- 
2.34.1

Re: [PATCH V2 3/3] dmaengine: zynqmp_dma: Guard IRQ handler against spurious interrupts

[sashiko-bot]

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

New issues:
- [High] The interrupt handler drops valid interrupts when CONFIG_PM is disabled.

Pre-existing issues:
- [High] Memory and runtime PM reference leaks on error paths in zynqmp_dma_alloc_chan_resources().
- [High] Missing synchronize_irq() before tasklet_kill() allows the tasklet to run after channel resources and PM references are freed.
--

commit 62db01b8d81ad56c8fb04b0d8f2adecdae634703
Author: Golla Nagendra <nagendra.golla@amd.com>

dmaengine: zynqmp_dma: Guard IRQ handler against spurious interrupts

This commit adds a pm_runtime_get_if_active() check in the
zynqmp_dma_irq_handler() to safely handle spurious interrupts. It
returns IRQ_NONE immediately if the device is not runtime-active,
preventing potential synchronous external aborts.

> diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c
> index a9dfec3c0ca3d..ce9163138be74 100644
> --- a/drivers/dma/xilinx/zynqmp_dma.c
> +++ b/drivers/dma/xilinx/zynqmp_dma.c
> @@ -730,6 +730,9 @@ static irqreturn_t zynqmp_dma_irq_handler(int irq, void *data)
>  	u32 isr, imr, status;
>  	irqreturn_t ret = IRQ_NONE;
>  
> +	if (pm_runtime_get_if_active(chan->dev) <= 0)
> +		return IRQ_NONE;

[Severity: High]
When CONFIG_PM is disabled, the pm_runtime_get_if_active() inline stub
automatically returns -EINVAL. Does this check unconditionally drop valid
interrupts in non-PM configurations?

Because -EINVAL is less than or equal to 0, the handler might always return
IRQ_NONE. This could potentially lead to an unacknowledged interrupt storm
or the kernel permanently disabling the IRQ line.

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but do the error paths in
zynqmp_dma_alloc_chan_resources() leak the runtime PM reference?

drivers/dma/xilinx/zynqmp_dma.c:zynqmp_dma_alloc_chan_resources() {
    ...
    chan->sw_desc_pool = kzalloc_objs(*desc, ZYNQMP_DMA_NUM_DESCS);
    if (!chan->sw_desc_pool)
        return -ENOMEM;
    ...
    if (!chan->desc_pool_v)
        return -ENOMEM;
    ...
}

If pm_runtime_resume_and_get() is called earlier, returning directly
on memory allocation failures without calling pm_runtime_put() will
leak the PM reference and previously allocated memory.


[Severity: High]
This is a pre-existing issue, but does zynqmp_dma_synchronize() miss
a required IRQ serialization step?

drivers/dma/xilinx/zynqmp_dma.c:zynqmp_dma_synchronize() {
        struct zynqmp_dma_chan *chan = to_chan(dchan);

        tasklet_kill(&chan->tasklet);
}

If zynqmp_dma_irq_handler() is executing concurrently and gets preempted
just before calling tasklet_schedule(), tasklet_kill() might see no
scheduled tasklet and return immediately. 

When the IRQ handler resumes and schedules the tasklet, could it execute
asynchronously after channel resources and PM references are freed,
potentially causing a synchronous external abort? 

Would calling synchronize_irq() first resolve this?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618071056.2024286-1-nagendra.golla@amd.com?part=3