[PATCH V3 0/6] dmaengine: zynqmp_dma: Per-channel reset, IRQ guard improvements, and PM fixes

[PATCH V3 0/6] dmaengine: zynqmp_dma: Per-channel reset, IRQ guard improvements, and PM fixes

[Golla Nagendra]

This series extends the ZynqMP DMA driver with per-channel reset support
for Versal Gen2 and Versal Net, improves the runtime PM guard in the IRQ
handler, and fixes a PM usage count leak in the probe error path.

Patch 1 fixes a PM usage count leak in the probe error path where
pm_runtime_resume_and_get() succeeds but a later step (e.g., channel
probe) fails, leaving the usage count incremented and clocks enabled.

Patch 2 adds a PM_RUNTIME_ACQUIRE_IF_ACTIVE() guard macro to pm_runtime.h
so drivers can conditionally acquire a runtime PM reference only when the
device is already active, with automatic release on scope exit.

Patch 3 uses PM_RUNTIME_ACQUIRE_IF_ACTIVE() in zynqmp_dma_irq_handler()
to safely guard against spurious interrupts arriving while the device is
runtime-suspended. When runtime PM is disabled (probe fallback path) the
handler falls through to normal processing unchanged.

Patch 4 updates the dt-binding to add the amd,versal-net-dma-1.0
compatible string and restricts the resets property per-compatible using
allOf:if:then:, making it required for Versal Gen2 and Versal Net and
explicitly absent for all other platforms.

Patch 5 adds the amd,versal-net-dma-1.0 entry to the driver match table
with a dedicated versal_net_dma_config carrying has_reset=true.

Patch 6 adds reset control handling in the channel probe path, using the
has_reset field in zynqmp_dma_config to conditionally call device_reset()
on Versal Gen2 and Versal Net channels.

Changes in V3:
- Patch 1: New patch - split PM usage count fix into a separate patch as
- Patch 2: New patch - add PM_RUNTIME_ACQUIRE_IF_ACTIVE() guard and its
  backing infrastructure to pm_runtime.h 
- Patch 3: Use PM_RUNTIME_ACQUIRE_IF_ACTIVE() guard instead of the manual
  pm_runtime_get_if_active()/pm_runtime_put() pair from V2
- Patch 4: Use allOf:if:then: to restrict the resets property per-
  compatible with resets: false for non-matching platforms 
  also expand to include amd,versal-net-dma-1.0
- Patch 5: New patch - add amd,versal-net-dma-1.0 compatible to driver
- Patch 6: Introduce has_reset flag in zynqmp_dma_config to gate the
  reset path; PM usage count fix split into patch 1/6

Golla Nagendra (5):
  dmaengine: zynqmp_dma: Fix PM usage count handling in probe error path
  PM: runtime: Add pm_runtime_if_active guard and conditional variant
  dmaengine: zynqmp_dma: Guard IRQ handler against spurious interrupts
  dmaengine: zynqmp_dma: Add new compatible string for Versal Net
  dmaengine: zynqmp_dma: Add per-channel reset support

Jay Buddhabhatti (1):
  dt-bindings: dma: xlnx,zynqmp-dma: Add Versal Net compatible support

 .../dma/xilinx/xlnx,zynqmp-dma-1.0.yaml       | 25 ++++++++++++++++---
 drivers/dma/xilinx/zynqmp_dma.c               | 25 ++++++++++++++++++-
 include/linux/pm_runtime.h                    | 12 +++++++++
 3 files changed, 58 insertions(+), 4 deletions(-)

-- 
2.43.7

[PATCH V3 1/6] dmaengine: zynqmp_dma: Fix PM usage count handling in probe error path

[Golla Nagendra]

When pm_runtime_resume_and_get() succeeds during probe, it increments
the usage count and enables clocks. If a later step (e.g., channel
probe) fails, the error path only called pm_runtime_disable() without
a matching pm_runtime_put_sync(), leaking the reference and the clock
enable.

Add an else branch to drop the usage count acquired by
pm_runtime_resume_and_get() when runtime PM is still enabled, letting
the PM core suspend/disable clocks on the success-then-later-failure
path.

Fixes: 64c6f7da8c2c ("dmaengine: zynqmp_dma: Add runtime pm support")
Signed-off-by: Golla Nagendra <nagendra.golla@amd.com>
---
Changes in V3:
- New patch: split from V2 2/3 per Frank Li's review to keep the PM usage
  count fix separate from the per-channel reset changes
---
 drivers/dma/xilinx/zynqmp_dma.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c
index f6a812e49ddc..87986f65d24c 100644
--- a/drivers/dma/xilinx/zynqmp_dma.c
+++ b/drivers/dma/xilinx/zynqmp_dma.c
@@ -1152,6 +1152,8 @@ static int zynqmp_dma_probe(struct platform_device *pdev)
 err_disable_pm:
 	if (!pm_runtime_enabled(zdev->dev))
 		zynqmp_dma_runtime_suspend(zdev->dev);
+	else
+		pm_runtime_put_sync(zdev->dev);
 	pm_runtime_disable(zdev->dev);
 	return ret;
 }
-- 
2.43.7

Re: [PATCH V3 1/6] dmaengine: zynqmp_dma: Fix PM usage count handling in probe error path

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The introduced pm_runtime_put_sync() fails to synchronously suspend the device because autosuspend is enabled, leaving clocks active.
--

--- Patch [1]: [PATCH V3 1/6] dmaengine: zynqmp_dma: Fix PM usage count handling in probe error path ---
commit 8bb5754b8f17c08d0144740ac308428274806a68
Author: Golla Nagendra <nagendra.golla@amd.com>

dmaengine: zynqmp_dma: Fix PM usage count handling in probe error path

This commit attempts to fix a reference leak on the probe error path. When
probe fails after successfully waking up the device, it adds a missing
runtime PM put call to drop the usage count and allow the device to suspend.

> 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 +1152,8 @@ static int zynqmp_dma_probe(struct platform_device *pdev)
>  err_disable_pm:
>  	if (!pm_runtime_enabled(zdev->dev))
>  		zynqmp_dma_runtime_suspend(zdev->dev);
> +	else
> +		pm_runtime_put_sync(zdev->dev);
>  	pm_runtime_disable(zdev->dev);

[Severity: Medium]
Does this actually suspend the device and disable the clocks?

Since autosuspend is enabled for this device earlier in zynqmp_dma_probe():
pm_runtime_use_autosuspend(zdev->dev);

Calling pm_runtime_put_sync() will only schedule an asynchronous suspend
timer. The immediately following pm_runtime_disable() will cancel this
pending timer before the suspend callback ever runs, leaving the clocks
permanently enabled.

Should this use pm_runtime_put_sync_suspend() instead to force a synchronous
suspend and bypass the autosuspend delay before disabling runtime PM?

>  	return ret;
>  }

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

[PATCH V3 2/6] PM: runtime: Add pm_runtime_if_active guard and conditional variant

[Golla Nagendra]

Add a pm_runtime_if_active base guard and its _try conditional variant
to pm_runtime.h for drivers that need to conditionally acquire a runtime
PM reference only when the device is already active.

The base guard must not be used directly via guard()/scoped_guard()
because pm_runtime_get_if_active() only acquires a reference when it
returns 1; the destructor unconditionally calls pm_runtime_put(), which
would underflow usage_count on a suspended or RPM-disabled device. The
base guard exists solely to back the DEFINE_GUARD_COND _try variant.

The _try variant (used via PM_RUNTIME_ACQUIRE_IF_ACTIVE) checks the
return value and only runs the destructor when the reference was
actually acquired. This is useful in interrupt handlers where the
device may be runtime-suspended and MMIO accesses must be avoided.

Signed-off-by: Golla Nagendra <nagendra.golla@amd.com>
---
Changes in V3:
- New patch: add PM_RUNTIME_ACQUIRE_IF_ACTIVE() guard and backing
  infrastructure to pm_runtime.h so that drivers
  can use a structured guard instead of an open-coded
  pm_runtime_get_if_active()/pm_runtime_put() pair
---
 include/linux/pm_runtime.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
index 64921b10ac74..bc6de97e6111 100644
--- a/include/linux/pm_runtime.h
+++ b/include/linux/pm_runtime.h
@@ -609,6 +609,13 @@ DEFINE_GUARD(pm_runtime_active, struct device *,
 	     pm_runtime_get_sync(_T), pm_runtime_put(_T));
 DEFINE_GUARD(pm_runtime_active_auto, struct device *,
 	     pm_runtime_get_sync(_T), pm_runtime_put_autosuspend(_T));
+/*
+ * Do not use directly -- the destructor calls pm_runtime_put()
+ * unconditionally, which underflows if no reference was acquired.
+ * Use only via the _try variant below.
+ */
+DEFINE_GUARD(pm_runtime_if_active, struct device *,
+	     pm_runtime_get_if_active(_T), pm_runtime_put(_T));
 /*
  * Use the following guards with ACQUIRE()/ACQUIRE_ERR().
  *
@@ -624,6 +631,8 @@ DEFINE_GUARD_COND(pm_runtime_active_auto, _try,
 		  pm_runtime_get_active(_T, RPM_TRANSPARENT), _RET == 0)
 DEFINE_GUARD_COND(pm_runtime_active_auto, _try_enabled,
 		  pm_runtime_resume_and_get(_T), _RET == 0)
+DEFINE_GUARD_COND(pm_runtime_if_active, _try,
+		  pm_runtime_get_if_active(_T) ?: -EAGAIN, _RET == 1)
 
 /* ACQUIRE() wrapper macros for the guards defined above. */
 
@@ -639,6 +648,9 @@ DEFINE_GUARD_COND(pm_runtime_active_auto, _try_enabled,
 #define PM_RUNTIME_ACQUIRE_IF_ENABLED_AUTOSUSPEND(_dev, _var)	\
 	ACQUIRE(pm_runtime_active_auto_try_enabled, _var)(_dev)
 
+#define PM_RUNTIME_ACQUIRE_IF_ACTIVE(_dev, _var)	\
+	ACQUIRE(pm_runtime_if_active_try, _var)(_dev)
+
 /*
  * ACQUIRE_ERR() wrapper macro for guard pm_runtime_active.
  *
-- 
2.43.7

Re: [PATCH V3 2/6] PM: runtime: Add pm_runtime_if_active guard and conditional variant

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The `pm_runtime_if_active_try` guard evaluates to an error when runtime PM is disabled, violating the API contract and breaking IRQ handlers.
- [Medium] The conditionally defined base guard `guard(pm_runtime_if_active)` silently corrupts the runtime PM usage counter if used directly.
--

--- Patch [2]: [PATCH V3 2/6] PM: runtime: Add pm_runtime_if_active guard and conditional variant ---
commit 57173006d8cc8596d1863023936dfd41e4325af1
Author: Golla Nagendra <nagendra.golla@amd.com>

PM: runtime: Add pm_runtime_if_active guard and conditional variant

Add a pm_runtime_if_active base guard and its _try conditional variant
to pm_runtime.h for drivers that need to conditionally acquire a runtime
PM reference only when the device is already active.

> diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
> --- a/include/linux/pm_runtime.h
> +++ b/include/linux/pm_runtime.h
> @@ -609,6 +609,13 @@ DEFINE_GUARD(pm_runtime_active, struct device *,
>  	     pm_runtime_get_sync(_T), pm_runtime_put(_T));
>  DEFINE_GUARD(pm_runtime_active_auto, struct device *,
>  	     pm_runtime_get_sync(_T), pm_runtime_put_autosuspend(_T));
> +/*
> + * Do not use directly -- the destructor calls pm_runtime_put()
> + * unconditionally, which underflows if no reference was acquired.
> + * Use only via the _try variant below.
> + */
> +DEFINE_GUARD(pm_runtime_if_active, struct device *,
> +	     pm_runtime_get_if_active(_T), pm_runtime_put(_T));

[Severity: Medium]
While the comment warns against direct usage, does exposing this base
guard via DEFINE_GUARD in include/linux/pm_runtime.h create a risky API
footgun?

If a developer uses guard(pm_runtime_if_active)(&dev) directly and the
device is not active, pm_runtime_get_if_active() returns 0 without
incrementing the usage counter. At scope exit, the destructor
unconditionally calls pm_runtime_put(), which would underflow the usage
counter and could prevent the device from suspending.

Could a lock-tracking guard like DEFINE_LOCK_GUARD_1 be used instead to
avoid defining an unsafe base guard?

>  /*
>   * Use the following guards with ACQUIRE()/ACQUIRE_ERR().
>   *
> @@ -624,6 +631,8 @@ DEFINE_GUARD_COND(pm_runtime_active_auto, _try,
>  		  pm_runtime_get_active(_T, RPM_TRANSPARENT), _RET == 0)
>  DEFINE_GUARD_COND(pm_runtime_active_auto, _try_enabled,
>  		  pm_runtime_resume_and_get(_T), _RET == 0)
> +DEFINE_GUARD_COND(pm_runtime_if_active, _try,
> +		  pm_runtime_get_if_active(_T) ?: -EAGAIN, _RET == 1)

[Severity: High]
Does this strict condition break interrupt handlers when runtime PM is
disabled?

If CONFIG_PM is disabled (or runtime PM is not enabled for the device),
pm_runtime_get_if_active() returns -EINVAL. With the _RET == 1 check
in DEFINE_GUARD_COND, the guard condition fails and evaluates to
ERR_PTR(-EINVAL).

If a driver uses PM_RUNTIME_ACQUIRE_IF_ACTIVE() in an interrupt handler
on such a system, the handler will bail out instead of servicing the valid
interrupt. This can lead to an unhandled IRQ storm that permanently
disables the device's interrupt line.

The documented API contract for _try guards states they do not produce
an error when runtime PM is disabled. Should this handle the -EINVAL
case to safely bypass the guard when PM is disabled?

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

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

[Golla Nagendra]

Use the PM_RUNTIME_ACQUIRE_IF_ACTIVE() guard in
zynqmp_dma_irq_handler() to safely handle spurious interrupts that may
arrive while the device is runtime-suspended.

The guard acquires a runtime PM reference only if the device is active,
returning IRQ_NONE when the device is genuinely runtime-suspended. When
runtime PM is disabled for the device (probe fallback path), the handler
falls through to normal processing since the hardware is still powered.
The reference is released automatically on function exit via the cleanup
infrastructure.

Fixes: 64c6f7da8c2c ("dmaengine: zynqmp_dma: Add runtime pm support")
Signed-off-by: Golla Nagendra <nagendra.golla@amd.com>
---
Changes in V3:
- Replace open-coded pm_runtime_get_if_active()/pm_runtime_put() pair
  with the PM_RUNTIME_ACQUIRE_IF_ACTIVE() guard macro
---
 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 87986f65d24c..b03171e37e27 100644
--- a/drivers/dma/xilinx/zynqmp_dma.c
+++ b/drivers/dma/xilinx/zynqmp_dma.c
@@ -729,6 +729,11 @@ static irqreturn_t zynqmp_dma_irq_handler(int irq, void *data)
 	u32 isr, imr, status;
 	irqreturn_t ret = IRQ_NONE;
 
+	PM_RUNTIME_ACQUIRE_IF_ACTIVE(chan->dev, pm);
+	if (IS_ENABLED(CONFIG_PM) && pm_runtime_enabled(chan->dev) &&
+	    PM_RUNTIME_ACQUIRE_ERR(&pm))
+		return IRQ_NONE;
+
 	isr = readl(chan->regs + ZYNQMP_DMA_ISR);
 	imr = readl(chan->regs + ZYNQMP_DMA_IMR);
 	status = isr & ~imr;
-- 
2.43.7

[PATCH V3 4/6] dt-bindings: dma: xlnx,zynqmp-dma: Add Versal Net compatible support

[Golla Nagendra]

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

Introduce a new compatible string amd,versal-net-dma-1.0 for
Versal Net ZDMA.The new compatible requires xlnx,zynqmp-dma-1.0 as a
fallback.

Add a reset property to describe the per-channel reset line exposed
on Versal Gen2 and Versal Net. Mark the resets property as required
for both Versal Gen2 and Versal Net, and not applicable for other
platforms.

Note that making resets required for Versal Gen2 results in an ABI
break but this is unavoidable. When DMA is used by two software components
back to back (e.g., firmware before Linux boot), DMA functionality fails
due to configuration differences or the DMA being left in a bad state by
the previous user. The channel can only be cleanly re-configured after
a DMA channel reset,making it essential.

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>
---
Changes in V3:
- Use allOf:if:then: to restrict the resets property per-compatible,
  making it required for amd,versal2-dma-1.0 and amd,versal-net-dma-1.0
  and explicitly setting resets: false for all other platforms
- Add amd,versal-net-dma-1.0 compatible string with xlnx,zynqmp-dma-1.0
  as fallback
- Switch compatible from enum to oneOf to accommodate the new multi-item
  entry for Versal Net
- Add ABI break justification
---
 .../dma/xilinx/xlnx,zynqmp-dma-1.0.yaml       | 25 ++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

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..87c65aef5c7d 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
@@ -18,15 +18,31 @@ maintainers:
 
 allOf:
   - $ref: ../dma-controller.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - amd,versal2-dma-1.0
+              - amd,versal-net-dma-1.0
+    then:
+      required:
+        - resets
+    else:
+      properties:
+        resets: false
 
 properties:
   "#dma-cells":
     const: 1
 
   compatible:
-    enum:
-      - amd,versal2-dma-1.0
-      - xlnx,zynqmp-dma-1.0
+    oneOf:
+      - const: amd,versal2-dma-1.0
+      - const: xlnx,zynqmp-dma-1.0
+      - items:
+          - const: amd,versal-net-dma-1.0
+          - const: xlnx,zynqmp-dma-1.0
 
   reg:
     description: memory map for gdma/adma module access
@@ -59,6 +75,9 @@ properties:
   power-domains:
     maxItems: 1
 
+  resets:
+    maxItems: 1
+
   dma-coherent: true
 
 required:
-- 
2.43.7

Re: [PATCH V3 4/6] dt-bindings: dma: xlnx,zynqmp-dma: Add Versal Net compatible support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Invalid fallback compatible string `xlnx,zynqmp-dma-1.0` for `amd,versal-net-dma-1.0`.
--

--- Patch [4]: [PATCH V3 4/6] dt-bindings: dma: xlnx,zynqmp-dma: Add Versal Net compatible support ---
commit c94f71cbb3b4d274afbe8968d6bb108158803292
Author: Jay Buddhabhatti <jay.buddhabhatti@amd.com>

dt-bindings: dma: xlnx,zynqmp-dma: Add Versal Net compatible support

This commit introduces the amd,versal-net-dma-1.0 compatible string with a
fallback to xlnx,zynqmp-dma-1.0. It also adds a required per-channel reset
property for Versal Gen2 and Versal Net because the DMA channel must be
reset to clear the state left by firmware.

> 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 2da86037ad79e..87c65aef5c7d3 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

[ ... ]

> @@ -18,15 +18,31 @@ maintainers:
>  properties:
>    "#dma-cells":
>      const: 1
>  
>    compatible:
> -    enum:
> -      - amd,versal2-dma-1.0
> -      - xlnx,zynqmp-dma-1.0
> +    oneOf:
> +      - const: amd,versal2-dma-1.0
> +      - const: xlnx,zynqmp-dma-1.0
> +      - items:
> +          - const: amd,versal-net-dma-1.0
> +          - const: xlnx,zynqmp-dma-1.0

[Severity: Medium]
Is this fallback compatible string appropriate here?

The commit message explicitly states that a DMA channel reset is essential
because the device is left in a bad state by firmware, and DMA functionality
fails without it. 

If a device tree is parsed by an older OS using the fallback
xlnx,zynqmp-dma-1.0 driver, it will attempt to operate the hardware without
knowing about the new resets property. Since the older driver cannot perform
the required channel reset, won't the DMA functionality fail just as
described?

Should the fallback be removed if the older driver cannot reliably operate
the device?

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

[PATCH V3 5/6] dmaengine: zynqmp_dma: Add new compatible string for Versal Net

[Golla Nagendra]

Versal Net previously used the legacy xlnx,zynqmp-dma-1.0 compatible
string. Add amd,versal-net-dma-1.0 to the of_device_id match table
with SoC-specific match data via versal_net_dma_config.

Introduce a has_reset field in zynqmp_dma_config to indicate whether
the compatible requires a per-channel reset during probe.

Signed-off-by: Golla Nagendra <nagendra.golla@amd.com>
---
Changes in V3:
- New patch: add amd,versal-net-dma-1.0 to the driver match table with
  versal_net_dma_config (has_reset=true, offset=0) 
---

 drivers/dma/xilinx/zynqmp_dma.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c
index b03171e37e27..7561416b8e62 100644
--- a/drivers/dma/xilinx/zynqmp_dma.c
+++ b/drivers/dma/xilinx/zynqmp_dma.c
@@ -259,13 +259,20 @@ struct zynqmp_dma_device {
 };
 
 struct zynqmp_dma_config {
+	bool has_reset;
 	u32 offset;
 };
 
 static const struct zynqmp_dma_config versal2_dma_config = {
+	.has_reset = true,
 	.offset = IRQ_REG_OFFSET,
 };
 
+/* offset = 0: Versal Net uses base IRQ register address */
+static const struct zynqmp_dma_config versal_net_dma_config = {
+	.has_reset = true,
+};
+
 static inline void zynqmp_dma_writeq(struct zynqmp_dma_chan *chan, u32 reg,
 				     u64 value)
 {
@@ -1184,6 +1191,7 @@ static void zynqmp_dma_remove(struct platform_device *pdev)
 
 static const struct of_device_id zynqmp_dma_of_match[] = {
 	{ .compatible = "amd,versal2-dma-1.0", .data = &versal2_dma_config },
+	{ .compatible = "amd,versal-net-dma-1.0", .data = &versal_net_dma_config },
 	{ .compatible = "xlnx,zynqmp-dma-1.0", },
 	{}
 };
-- 
2.43.7

Re: [PATCH V3 5/6] dmaengine: zynqmp_dma: Add new compatible string for Versal Net

[sashiko-bot]

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

New issues:
- [High] The `has_reset` field is introduced but completely unused, missing the required hardware reset logic.

Pre-existing issues:
- [High] `zynqmp_dma_chan_probe()` hardcodes the `platform_get_irq()` error return to `-ENXIO`, breaking probe deferral.
- [High] `zynqmp_dma_reset()` silently drops active and pending DMA descriptors without invoking their callbacks, causing deadlocks for client drivers.
- [High] `zynqmp_dma_device_terminate_all()` does not command the hardware to stop, leading to potential memory corruption.
--

--- Patch [5]: [PATCH V3 5/6] dmaengine: zynqmp_dma: Add new compatible string for Versal Net ---
commit 93c6838fba5eee0dddf024579d8474fed6b7dee5
Author: Golla Nagendra <nagendra.golla@amd.com>

dmaengine: zynqmp_dma: Add new compatible string for Versal Net

This patch introduces the amd,versal-net-dma-1.0 compatible string and its
SoC-specific match data. It also adds a has_reset field to zynqmp_dma_config
to indicate whether the compatible requires a per-channel reset during probe.

> diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c
> index b03171e37e278..7561416b8e626 100644
> --- a/drivers/dma/xilinx/zynqmp_dma.c
> +++ b/drivers/dma/xilinx/zynqmp_dma.c
> @@ -259,13 +259,20 @@ struct zynqmp_dma_device {
>  };
>  
>  struct zynqmp_dma_config {
> +	bool has_reset;
>  	u32 offset;
>  };
>  
>  static const struct zynqmp_dma_config versal2_dma_config = {
> +	.has_reset = true,
>  	.offset = IRQ_REG_OFFSET,
>  };
>  
> +/* offset = 0: Versal Net uses base IRQ register address */
> +static const struct zynqmp_dma_config versal_net_dma_config = {
> +	.has_reset = true,
> +};
> +

[Severity: High]
Does this configuration actually perform a hardware reset?

The patch introduces the has_reset flag and sets it to true for versal2 and
versal-net, but looking at zynqmp_dma_chan_probe(), the match data is
retrieved without ever checking this flag or invoking any reset functions:

drivers/dma/xilinx/zynqmp_dma.c:zynqmp_dma_chan_probe() {
    ...
	match_data = of_device_get_match_data(&pdev->dev);
	if (match_data)
		chan->irq_offset = match_data->offset;
    ...
}

Does the omission of the reset implementation leave the DMA hardware in an
uninitialized state during probe, potentially causing back-to-back usage
failures?

While reviewing this patch, I also noticed a few pre-existing issues in the
same file:

[Severity: High]
This isn't a bug introduced by this patch, but zynqmp_dma_chan_probe()
hardcodes the platform_get_irq() error return to -ENXIO, which discards
probe deferral errors like -EPROBE_DEFER:

drivers/dma/xilinx/zynqmp_dma.c:zynqmp_dma_chan_probe() {
    ...
	chan->irq = platform_get_irq(pdev, 0);
	if (chan->irq < 0)
		return -ENXIO;
    ...
}

Does this permanent probe failure prevent the DMA controller from working when
the interrupt controller is not yet ready during boot?

[Severity: High]
This is a pre-existing issue, but zynqmp_dma_reset() silently drops active and
pending DMA descriptors without invoking their callbacks:

drivers/dma/xilinx/zynqmp_dma.c:zynqmp_dma_reset() {
    ...
	spin_lock_irqsave(&chan->lock, irqflags);
	zynqmp_dma_complete_descriptor(chan);
	spin_unlock_irqrestore(&chan->lock, irqflags);
	zynqmp_dma_chan_desc_cleanup(chan);
	zynqmp_dma_free_descriptors(chan);
    ...
}

Because zynqmp_dma_complete_descriptor() only completes the first descriptor in
the active_list, the remaining active and pending descriptors are silently
recycled by zynqmp_dma_free_descriptors(). Could this strand waiting client
drivers and cause deadlocks when a hardware error occurs?

[Severity: High]
This is a pre-existing issue, but zynqmp_dma_device_terminate_all() does not
command the hardware to stop:

drivers/dma/xilinx/zynqmp_dma.c:zynqmp_dma_device_terminate_all() {
    ...
	writel(ZYNQMP_DMA_IDS_DEFAULT_MASK, chan->regs + ZYNQMP_DMA_IDS);
	zynqmp_dma_free_descriptors(chan);

	return 0;
}

The function merely masks interrupts and frees software descriptors, but never
clears the ZYNQMP_DMA_ENABLE bit or issues a hardware stop command. If a client
driver calls dmaengine_terminate_all() and frees the DMA buffers, could the DMA
controller continue to read/write the in-flight physical memory in the
background and corrupt memory?

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

[PATCH V3 6/6] dmaengine: zynqmp_dma: Add per-channel reset support

[Golla Nagendra]

Versal Gen2 and Versal Net expose a dedicated reset line per ZDMA
channel. Use device_reset() to trigger a mandatory reset pulse on the
channel during probe when match data indicates reset is supported.

The has_reset field in zynqmp_dma_config controls whether reset is
performed for a given compatible. For Versal Net and Versal Gen2,
has_reset is set to true, so device_reset() is called and probe fails
if the resets property is missing in the device tree. For legacy SoCs,
match_data is NULL and has_reset defaults to false,so the reset path
is skipped entirely.

Signed-off-by: Golla Nagendra <nagendra.golla@amd.com>
---
Changes in V3:
- Introduce has_reset flag in zynqmp_dma_config to gate the reset path
  per-compatible instead of calling device_reset() unconditionally
- Move the of_device_get_match_data() call before the reset block so
  has_reset can be checked in the same code path that reads irq_offset
---
 drivers/dma/xilinx/zynqmp_dma.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c
index 7561416b8e62..7ec6598ea587 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"
 
@@ -928,6 +929,14 @@ static int zynqmp_dma_chan_probe(struct zynqmp_dma_device *zdev,
 	if (IS_ERR(chan->regs))
 		return PTR_ERR(chan->regs);
 
+	match_data = of_device_get_match_data(&pdev->dev);
+	if (match_data && match_data->has_reset) {
+		err = device_reset(&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;
@@ -943,7 +952,6 @@ static int zynqmp_dma_chan_probe(struct zynqmp_dma_device *zdev,
 		return -EINVAL;
 	}
 
-	match_data = of_device_get_match_data(&pdev->dev);
 	if (match_data)
 		chan->irq_offset = match_data->offset;
 
-- 
2.43.7