* [PATCH v4 1/6] dmaengine: zynqmp_dma: Use PM guard for probe-time runtime PM reference
2026-10-09 4:00 [PATCH v4 0/6] dmaengine: zynqmp_dma: Per-channel reset, IRQ guard improvements, and PM fixes Golla Nagendra
@ 2026-10-09 4:00 ` Golla Nagendra
2026-10-10 3:09 ` Frank Li
2026-10-09 4:00 ` [PATCH v4 2/6] PM: runtime: Add pm_runtime_if_active guard and conditional variant Golla Nagendra
` (4 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Golla Nagendra @ 2026-10-09 4:00 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Michal Simek, Rafael J. Wysocki,
Kedareswara rao Appana, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Michael Tretter, Harini Katakam, Shyam Pandey
Cc: dmaengine, linux-arm-kernel, linux-kernel, linux-pm, devicetree,
git, Golla Nagendra
Replace pm_runtime_resume_and_get() with the PM_RUNTIME_ACQUIRE_AUTOSUSPEND
guard to manage the probe-time runtime PM reference lifecycle. If the guard
acquire fails, fail probe immediately so a broken device is never
registered.
When runtime PM is not enabled for the device, call
zynqmp_dma_runtime_resume() directly after pm_runtime_enable() to bring the
clocks up for probe.
On probe failure after a successful guard acquire, drop the reference with
zynqmp_dma_probe_pm_put(), inhibit the guard destructor with
retain_and_null_ptr(), and gate the clocks through
zynqmp_dma_probe_fail(). When runtime PM is enabled,
zynqmp_dma_probe_pm_put() uses pm_runtime_put_sync_suspend(). When runtime
PM is not enabled for the device, balance the guard usage counter with
pm_runtime_put_noidle() and gate the clocks with
zynqmp_dma_runtime_suspend(). Runtime PM does not suspend a device that
is already disabled, so pair every direct zynqmp_dma_runtime_suspend()
call with pm_runtime_set_suspended() to keep the runtime PM status in
sync with the clocks.
On probe success, release the probe-time reference with
pm_runtime_put_sync_autosuspend() when runtime PM is enabled for the
device. When runtime PM is not enabled for the device, balance the guard
usage counter with pm_runtime_put_noidle() only and leave the clocks
enabled so that later channel allocation can use the hardware. Inhibit the
guard destructor with retain_and_null_ptr(pm).
Signed-off-by: Golla Nagendra <nagendra.golla@amd.com>
---
drivers/dma/xilinx/zynqmp_dma.c | 68 ++++++++++++++++++++++++++++++-----------
1 file changed, 50 insertions(+), 18 deletions(-)
diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c
index 3b7dfa53f972..a48106e404e7 100644
--- a/drivers/dma/xilinx/zynqmp_dma.c
+++ b/drivers/dma/xilinx/zynqmp_dma.c
@@ -18,6 +18,7 @@
#include <linux/slab.h>
#include <linux/clk.h>
#include <linux/io-64-nonatomic-lo-hi.h>
+#include <linux/cleanup.h>
#include <linux/pm_runtime.h>
#include "../dmaengine.h"
@@ -1077,6 +1078,29 @@ static const struct dev_pm_ops zynqmp_dma_dev_pm_ops = {
zynqmp_dma_runtime_resume, NULL)
};
+static void zynqmp_dma_probe_pm_put(struct device *dev)
+{
+ if (pm_runtime_enabled(dev))
+ pm_runtime_put_sync_suspend(dev);
+ else
+ pm_runtime_put_noidle(dev);
+}
+
+static int zynqmp_dma_probe_fail(struct zynqmp_dma_device *zdev, int ret)
+{
+ bool rpm_disabled = !pm_runtime_enabled(zdev->dev);
+
+ zynqmp_dma_chan_remove(zdev->chan);
+ zynqmp_dma_probe_pm_put(zdev->dev);
+ pm_runtime_disable(zdev->dev);
+ if (rpm_disabled || !pm_runtime_status_suspended(zdev->dev)) {
+ zynqmp_dma_runtime_suspend(zdev->dev);
+ pm_runtime_set_suspended(zdev->dev);
+ }
+
+ return ret;
+}
+
/**
* zynqmp_dma_probe - Driver probe function
* @pdev: Pointer to the platform_device structure
@@ -1087,7 +1111,7 @@ static int zynqmp_dma_probe(struct platform_device *pdev)
{
struct zynqmp_dma_device *zdev;
struct dma_device *p;
- int ret;
+ int ret, pm_err;
zdev = devm_kzalloc(&pdev->dev, sizeof(*zdev), GFP_KERNEL);
if (!zdev)
@@ -1128,21 +1152,30 @@ static int zynqmp_dma_probe(struct platform_device *pdev)
pm_runtime_set_autosuspend_delay(zdev->dev, ZDMA_PM_TIMEOUT);
pm_runtime_use_autosuspend(zdev->dev);
pm_runtime_enable(zdev->dev);
- ret = pm_runtime_resume_and_get(zdev->dev);
- if (ret < 0) {
- dev_err(&pdev->dev, "device wakeup failed.\n");
- pm_runtime_disable(zdev->dev);
- }
if (!pm_runtime_enabled(zdev->dev)) {
ret = zynqmp_dma_runtime_resume(zdev->dev);
- if (ret)
+ if (ret) {
+ pm_runtime_disable(zdev->dev);
return ret;
+ }
+ }
+ PM_RUNTIME_ACQUIRE_AUTOSUSPEND(zdev->dev, pm);
+ pm_err = PM_RUNTIME_ACQUIRE_ERR(&pm);
+ if (pm_err) {
+ retain_and_null_ptr(pm);
+ pm_runtime_disable(zdev->dev);
+ if (!pm_runtime_status_suspended(zdev->dev)) {
+ zynqmp_dma_runtime_suspend(zdev->dev);
+ pm_runtime_set_suspended(zdev->dev);
+ }
+ return dev_err_probe(&pdev->dev, pm_err, "device wakeup failed\n");
}
ret = zynqmp_dma_chan_probe(zdev, pdev);
if (ret) {
dev_err_probe(&pdev->dev, ret, "Probing channel failed\n");
- goto free_chan_resources;
+ retain_and_null_ptr(pm);
+ return zynqmp_dma_probe_fail(zdev, ret);
}
p->dst_addr_widths = BIT(zdev->chan->bus_width / 8);
@@ -1151,7 +1184,8 @@ static int zynqmp_dma_probe(struct platform_device *pdev)
ret = dma_async_device_register(&zdev->common);
if (ret) {
dev_err(zdev->dev, "failed to register the dma device\n");
- goto free_chan_resources;
+ retain_and_null_ptr(pm);
+ return zynqmp_dma_probe_fail(zdev, ret);
}
ret = of_dma_controller_register(pdev->dev.of_node,
@@ -1159,19 +1193,17 @@ static int zynqmp_dma_probe(struct platform_device *pdev)
if (ret) {
dev_err_probe(&pdev->dev, ret, "Unable to register DMA to DT\n");
dma_async_device_unregister(&zdev->common);
- goto free_chan_resources;
+ retain_and_null_ptr(pm);
+ return zynqmp_dma_probe_fail(zdev, ret);
}
- pm_runtime_put_sync_autosuspend(zdev->dev);
+ if (pm_runtime_enabled(zdev->dev))
+ pm_runtime_put_sync_autosuspend(zdev->dev);
+ else
+ pm_runtime_put_noidle(zdev->dev);
+ retain_and_null_ptr(pm);
return 0;
-
-free_chan_resources:
- zynqmp_dma_chan_remove(zdev->chan);
- if (!pm_runtime_enabled(zdev->dev))
- zynqmp_dma_runtime_suspend(zdev->dev);
- pm_runtime_disable(zdev->dev);
- return ret;
}
/**
--
2.43.7
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v4 1/6] dmaengine: zynqmp_dma: Use PM guard for probe-time runtime PM reference
2026-10-09 4:00 ` [PATCH v4 1/6] dmaengine: zynqmp_dma: Use PM guard for probe-time runtime PM reference Golla Nagendra
@ 2026-10-10 3:09 ` Frank Li
0 siblings, 0 replies; 14+ messages in thread
From: Frank Li @ 2026-10-10 3:09 UTC (permalink / raw)
To: Golla Nagendra
Cc: Vinod Koul, Frank Li, Michal Simek, Rafael J. Wysocki,
Kedareswara rao Appana, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Michael Tretter, Harini Katakam, Shyam Pandey,
dmaengine, linux-arm-kernel, linux-kernel, linux-pm, devicetree,
git
On Fri, Oct 09, 2026 at 09:30:10AM +0530, Golla Nagendra wrote:
> Replace pm_runtime_resume_and_get() with the PM_RUNTIME_ACQUIRE_AUTOSUSPEND
> guard to manage the probe-time runtime PM reference lifecycle. If the guard
> acquire fails, fail probe immediately so a broken device is never
> registered.
>
> When runtime PM is not enabled for the device, call
> zynqmp_dma_runtime_resume() directly after pm_runtime_enable() to bring the
> clocks up for probe.
>
> On probe failure after a successful guard acquire, drop the reference with
> zynqmp_dma_probe_pm_put(), inhibit the guard destructor with
> retain_and_null_ptr(), and gate the clocks through
> zynqmp_dma_probe_fail(). When runtime PM is enabled,
> zynqmp_dma_probe_pm_put() uses pm_runtime_put_sync_suspend(). When runtime
> PM is not enabled for the device, balance the guard usage counter with
> pm_runtime_put_noidle() and gate the clocks with
> zynqmp_dma_runtime_suspend(). Runtime PM does not suspend a device that
> is already disabled, so pair every direct zynqmp_dma_runtime_suspend()
> call with pm_runtime_set_suspended() to keep the runtime PM status in
> sync with the clocks.
>
> On probe success, release the probe-time reference with
> pm_runtime_put_sync_autosuspend() when runtime PM is enabled for the
> device. When runtime PM is not enabled for the device, balance the guard
> usage counter with pm_runtime_put_noidle() only and leave the clocks
> enabled so that later channel allocation can use the hardware. Inhibit the
> guard destructor with retain_and_null_ptr(pm).
>
> Signed-off-by: Golla Nagendra <nagendra.golla@amd.com>
> ---
> drivers/dma/xilinx/zynqmp_dma.c | 68 ++++++++++++++++++++++++++++++-----------
> 1 file changed, 50 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c
> index 3b7dfa53f972..a48106e404e7 100644
> --- a/drivers/dma/xilinx/zynqmp_dma.c
> +++ b/drivers/dma/xilinx/zynqmp_dma.c
> @@ -18,6 +18,7 @@
> #include <linux/slab.h>
> #include <linux/clk.h>
> #include <linux/io-64-nonatomic-lo-hi.h>
> +#include <linux/cleanup.h>
> #include <linux/pm_runtime.h>
>
> #include "../dmaengine.h"
> @@ -1077,6 +1078,29 @@ static const struct dev_pm_ops zynqmp_dma_dev_pm_ops = {
> zynqmp_dma_runtime_resume, NULL)
> };
>
> +static void zynqmp_dma_probe_pm_put(struct device *dev)
> +{
> + if (pm_runtime_enabled(dev))
> + pm_runtime_put_sync_suspend(dev);
> + else
> + pm_runtime_put_noidle(dev);
> +}
> +
> +static int zynqmp_dma_probe_fail(struct zynqmp_dma_device *zdev, int ret)
> +{
> + bool rpm_disabled = !pm_runtime_enabled(zdev->dev);
> +
> + zynqmp_dma_chan_remove(zdev->chan);
> + zynqmp_dma_probe_pm_put(zdev->dev);
> + pm_runtime_disable(zdev->dev);
> + if (rpm_disabled || !pm_runtime_status_suspended(zdev->dev)) {
> + zynqmp_dma_runtime_suspend(zdev->dev);
> + pm_runtime_set_suspended(zdev->dev);
> + }
> +
> + return ret;
> +}
> +
> /**
> * zynqmp_dma_probe - Driver probe function
> * @pdev: Pointer to the platform_device structure
> @@ -1087,7 +1111,7 @@ static int zynqmp_dma_probe(struct platform_device *pdev)
> {
> struct zynqmp_dma_device *zdev;
> struct dma_device *p;
> - int ret;
> + int ret, pm_err;
>
> zdev = devm_kzalloc(&pdev->dev, sizeof(*zdev), GFP_KERNEL);
> if (!zdev)
> @@ -1128,21 +1152,30 @@ static int zynqmp_dma_probe(struct platform_device *pdev)
> pm_runtime_set_autosuspend_delay(zdev->dev, ZDMA_PM_TIMEOUT);
> pm_runtime_use_autosuspend(zdev->dev);
> pm_runtime_enable(zdev->dev);
> - ret = pm_runtime_resume_and_get(zdev->dev);
> - if (ret < 0) {
> - dev_err(&pdev->dev, "device wakeup failed.\n");
> - pm_runtime_disable(zdev->dev);
> - }
> if (!pm_runtime_enabled(zdev->dev)) {
> ret = zynqmp_dma_runtime_resume(zdev->dev);
> - if (ret)
> + if (ret) {
> + pm_runtime_disable(zdev->dev);
> return ret;
> + }
> + }
> + PM_RUNTIME_ACQUIRE_AUTOSUSPEND(zdev->dev, pm);
> + pm_err = PM_RUNTIME_ACQUIRE_ERR(&pm);
> + if (pm_err) {
> + retain_and_null_ptr(pm);
Not sure why need call retain_and_null_ptr() here, generally only need
call retain_and_null_ptr() at success pass if you use want to keep
runtime_pm active. at fail case, needn't do that.
> + pm_runtime_disable(zdev->dev);
If use devm_pm_runtime_enable(), needn't this one.
> + if (!pm_runtime_status_suspended(zdev->dev)) {
> + zynqmp_dma_runtime_suspend(zdev->dev);
> + pm_runtime_set_suspended(zdev->dev);
> + }
> + return dev_err_probe(&pdev->dev, pm_err, "device wakeup failed\n");
> }
>
> ret = zynqmp_dma_chan_probe(zdev, pdev);
> if (ret) {
> dev_err_probe(&pdev->dev, ret, "Probing channel failed\n");
> - goto free_chan_resources;
> + retain_and_null_ptr(pm);
> + return zynqmp_dma_probe_fail(zdev, ret);
> }
>
> p->dst_addr_widths = BIT(zdev->chan->bus_width / 8);
> @@ -1151,7 +1184,8 @@ static int zynqmp_dma_probe(struct platform_device *pdev)
> ret = dma_async_device_register(&zdev->common);
> if (ret) {
> dev_err(zdev->dev, "failed to register the dma device\n");
> - goto free_chan_resources;
> + retain_and_null_ptr(pm);
> + return zynqmp_dma_probe_fail(zdev, ret);
> }
>
> ret = of_dma_controller_register(pdev->dev.of_node,
> @@ -1159,19 +1193,17 @@ static int zynqmp_dma_probe(struct platform_device *pdev)
> if (ret) {
> dev_err_probe(&pdev->dev, ret, "Unable to register DMA to DT\n");
> dma_async_device_unregister(&zdev->common);
> - goto free_chan_resources;
> + retain_and_null_ptr(pm);
> + return zynqmp_dma_probe_fail(zdev, ret);
> }
>
> - pm_runtime_put_sync_autosuspend(zdev->dev);
> + if (pm_runtime_enabled(zdev->dev))
> + pm_runtime_put_sync_autosuspend(zdev->dev);
> + else
> + pm_runtime_put_noidle(zdev->dev);
> + retain_and_null_ptr(pm);
You mix use auto cleanup and pm_runtime_put_sync_autosuspen() and
pm_runtime_put_noidle() to make code mess up.
Frank
>
> return 0;
> -
> -free_chan_resources:
> - zynqmp_dma_chan_remove(zdev->chan);
> - if (!pm_runtime_enabled(zdev->dev))
> - zynqmp_dma_runtime_suspend(zdev->dev);
> - pm_runtime_disable(zdev->dev);
> - return ret;
> }
>
> /**
>
> --
> 2.43.7
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v4 2/6] PM: runtime: Add pm_runtime_if_active guard and conditional variant
2026-10-09 4:00 [PATCH v4 0/6] dmaengine: zynqmp_dma: Per-channel reset, IRQ guard improvements, and PM fixes Golla Nagendra
2026-10-09 4:00 ` [PATCH v4 1/6] dmaengine: zynqmp_dma: Use PM guard for probe-time runtime PM reference Golla Nagendra
@ 2026-10-09 4:00 ` Golla Nagendra
2026-10-09 4:10 ` sashiko-bot
` (2 more replies)
2026-10-09 4:00 ` [PATCH v4 3/6] dmaengine: zynqmp_dma: Guard IRQ handler against spurious interrupts Golla Nagendra
` (3 subsequent siblings)
5 siblings, 3 replies; 14+ messages in thread
From: Golla Nagendra @ 2026-10-09 4:00 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Michal Simek, Rafael J. Wysocki,
Kedareswara rao Appana, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Michael Tretter, Harini Katakam, Shyam Pandey
Cc: dmaengine, linux-arm-kernel, linux-kernel, linux-pm, devicetree,
git, 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 device. The base guard
exists solely to back the DEFINE_GUARD_COND _try variant.
The _try variant (used via PM_RUNTIME_ACQUIRE_IF_ACTIVE) satisfies the
_try API contract that guards must not produce an error when runtime PM
is disabled for the device. When pm_runtime_get_if_active() returns
-EINVAL (runtime PM disabled), the expression calls
pm_runtime_get_noresume() to pre-balance the destructor's unconditional
pm_runtime_put(), then returns 1 so the guard condition succeeds
(bypass). This mirrors the behaviour of pm_runtime_active_try which
uses RPM_TRANSPARENT for the same purpose. When the device is genuinely
runtime-suspended (return value 0), the guard condition fails and
ACQUIRE_ERR returns true, allowing callers such as interrupt handlers to
skip MMIO accesses.
Signed-off-by: Golla Nagendra <nagendra.golla@amd.com>
---
include/linux/pm_runtime.h | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
index 64921b10ac74..ae2a8740ffb3 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,23 @@ 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)
+/*
+ * The _try contract is that no error is produced when runtime PM is disabled,
+ * so bypass the guard when pm_runtime_get_if_active() returns -EINVAL: take a
+ * reference with pm_runtime_get_noresume() to pre-balance the destructor's
+ * pm_runtime_put() and return 1 so the guard condition succeeds. Interrupt
+ * handlers then keep servicing interrupts when runtime PM is disabled for the
+ * device or CONFIG_PM is not enabled.
+ */
+DEFINE_GUARD_COND(pm_runtime_if_active, _try,
+ ({
+ int _r = pm_runtime_get_if_active(_T);
+
+ if (_r == -EINVAL)
+ pm_runtime_get_noresume(_T);
+ _r == -EINVAL ? 1 : (_r ?: -EAGAIN);
+ }),
+ _RET == 1)
/* ACQUIRE() wrapper macros for the guards defined above. */
@@ -639,6 +663,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
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v4 2/6] PM: runtime: Add pm_runtime_if_active guard and conditional variant
2026-10-09 4:00 ` [PATCH v4 2/6] PM: runtime: Add pm_runtime_if_active guard and conditional variant Golla Nagendra
@ 2026-10-09 4:10 ` sashiko-bot
2026-10-10 2:57 ` Frank Li
2026-10-12 18:44 ` Rafael J. Wysocki (Intel)
2 siblings, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-10-09 4:10 UTC (permalink / raw)
To: Golla Nagendra; +Cc: robh, conor+dt, devicetree, vkoul, Frank.Li, dmaengine
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] pm: TOCTOU race in pm_runtime_if_active_try allows hardware access on suspended device
- [Medium] pm: pm_runtime_if_active base guard causes usage_count underflow if used directly
--
commit 90c398317818a4245091ccf1d776dfe02e35d94e
Author: Golla Nagendra <nagendra.golla@amd.com>
PM: runtime: Add pm_runtime_if_active guard and conditional variant
This commit introduces a pm_runtime_if_active base guard and its _try
conditional variant to allow drivers 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
> index 322e3b17f9871..41804e8480427 100644
> --- a/include/linux/pm_runtime.h
> +++ b/include/linux/pm_runtime.h
[ ... ]
> @@ -645,6 +645,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]
Is there a risk that this base guard will be used directly despite the
comment?
If used via guard(pm_runtime_if_active), pm_runtime_get_if_active() does
not acquire a reference when the device is suspended, but the destructor
unconditionally calls pm_runtime_put().
Can this underflow the usage_count and permanently break the device's
power management state?
Since the _try variant overrides the lock expression, could the base guard
be safely defined using pm_runtime_get_noresume() instead of
pm_runtime_get_if_active() to prevent this?
[ ... ]
> @@ -660,6 +667,23 @@ 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)
> +/*
> + * The _try contract is that no error is produced when runtime PM is disabled,
> + * so bypass the guard when pm_runtime_get_if_active() returns -EINVAL: take a
> + * reference with pm_runtime_get_noresume() to pre-balance the destructor's
> + * pm_runtime_put() and return 1 so the guard condition succeeds. Interrupt
> + * handlers then keep servicing interrupts when runtime PM is disabled for the
> + * device or CONFIG_PM is not enabled.
> + */
> +DEFINE_GUARD_COND(pm_runtime_if_active, _try,
> + ({
> + int _r = pm_runtime_get_if_active(_T);
> +
> + if (_r == -EINVAL)
> + pm_runtime_get_noresume(_T);
> + _r == -EINVAL ? 1 : (_r ?: -EAGAIN);
> + }),
> + _RET == 1)
[Severity: High]
Does this regression allow a race condition that leads to hardware access
on a suspended device?
pm_runtime_get_if_active() checks the state under dev->power.lock and then
drops the lock before returning -EINVAL.
Could a concurrent thread enable and suspend the device immediately after
the lock is dropped:
Thread 1
_r = pm_runtime_get_if_active(_T); // returns -EINVAL, drops lock
Thread 2
pm_runtime_enable(dev); // disable_depth becomes 0
pm_runtime_suspend(dev); // device is now suspended
Thread 1 continues
if (_r == -EINVAL)
pm_runtime_get_noresume(_T); // increments usage_count outside lock
The macro then evaluates to 1, indicating success.
Does this trick the caller into treating the device as active, causing
interrupt handlers to perform MMIO accesses on powered-down hardware?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-adma-pm-fix-and-per-channel-reset-v4-0-55f5fe409c55@amd.com?part=2
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v4 2/6] PM: runtime: Add pm_runtime_if_active guard and conditional variant
2026-10-09 4:00 ` [PATCH v4 2/6] PM: runtime: Add pm_runtime_if_active guard and conditional variant Golla Nagendra
2026-10-09 4:10 ` sashiko-bot
@ 2026-10-10 2:57 ` Frank Li
2026-10-12 18:44 ` Rafael J. Wysocki (Intel)
2 siblings, 0 replies; 14+ messages in thread
From: Frank Li @ 2026-10-10 2:57 UTC (permalink / raw)
To: Golla Nagendra
Cc: Vinod Koul, Frank Li, Michal Simek, Rafael J. Wysocki,
Kedareswara rao Appana, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Michael Tretter, Harini Katakam, Shyam Pandey,
dmaengine, linux-arm-kernel, linux-kernel, linux-pm, devicetree,
git
On Fri, Oct 09, 2026 at 09:30:11AM +0530, Golla Nagendra wrote:
> 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 device. The base guard
> exists solely to back the DEFINE_GUARD_COND _try variant.
>
> The _try variant (used via PM_RUNTIME_ACQUIRE_IF_ACTIVE) satisfies the
> _try API contract that guards must not produce an error when runtime PM
> is disabled for the device. When pm_runtime_get_if_active() returns
> -EINVAL (runtime PM disabled),
If only resolve PM disabled case, I suggest define DEFINE_GUARD as no-ops
Frank
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 2/6] PM: runtime: Add pm_runtime_if_active guard and conditional variant
2026-10-09 4:00 ` [PATCH v4 2/6] PM: runtime: Add pm_runtime_if_active guard and conditional variant Golla Nagendra
2026-10-09 4:10 ` sashiko-bot
2026-10-10 2:57 ` Frank Li
@ 2026-10-12 18:44 ` Rafael J. Wysocki (Intel)
2 siblings, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki (Intel) @ 2026-10-12 18:44 UTC (permalink / raw)
To: Golla Nagendra
Cc: Vinod Koul, Frank Li, Michal Simek, Rafael J. Wysocki,
Kedareswara rao Appana, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Michael Tretter, Harini Katakam, Shyam Pandey,
dmaengine, linux-arm-kernel, linux-kernel, linux-pm, devicetree,
git
On Fri, Oct 9, 2026 at 6:00 AM Golla Nagendra <nagendra.golla@amd.com> wrote:
>
> 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 device. The base guard
> exists solely to back the DEFINE_GUARD_COND _try variant.
>
> The _try variant (used via PM_RUNTIME_ACQUIRE_IF_ACTIVE) satisfies the
> _try API contract that guards must not produce an error when runtime PM
> is disabled for the device. When pm_runtime_get_if_active() returns
> -EINVAL (runtime PM disabled), the expression calls
> pm_runtime_get_noresume() to pre-balance the destructor's unconditional
> pm_runtime_put(), then returns 1 so the guard condition succeeds
> (bypass). This mirrors the behaviour of pm_runtime_active_try which
> uses RPM_TRANSPARENT for the same purpose. When the device is genuinely
> runtime-suspended (return value 0), the guard condition fails and
> ACQUIRE_ERR returns true, allowing callers such as interrupt handlers to
> skip MMIO accesses.
>
> Signed-off-by: Golla Nagendra <nagendra.golla@amd.com>
> ---
> include/linux/pm_runtime.h | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
> index 64921b10ac74..ae2a8740ffb3 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,23 @@ 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)
> +/*
> + * The _try contract is that no error is produced when runtime PM is disabled,
> + * so bypass the guard when pm_runtime_get_if_active() returns -EINVAL: take a
> + * reference with pm_runtime_get_noresume() to pre-balance the destructor's
> + * pm_runtime_put() and return 1 so the guard condition succeeds. Interrupt
> + * handlers then keep servicing interrupts when runtime PM is disabled for the
> + * device or CONFIG_PM is not enabled.
> + */
> +DEFINE_GUARD_COND(pm_runtime_if_active, _try,
> + ({
> + int _r = pm_runtime_get_if_active(_T);
> +
> + if (_r == -EINVAL)
> + pm_runtime_get_noresume(_T);
> + _r == -EINVAL ? 1 : (_r ?: -EAGAIN);
> + }),
> + _RET == 1)
>
> /* ACQUIRE() wrapper macros for the guards defined above. */
>
> @@ -639,6 +663,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.
> *
>
> --
For the record, I'm still unconvinced about the guard definitions above.
Please try to reimplement the series without them for the time being.
Thanks!
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v4 3/6] dmaengine: zynqmp_dma: Guard IRQ handler against spurious interrupts
2026-10-09 4:00 [PATCH v4 0/6] dmaengine: zynqmp_dma: Per-channel reset, IRQ guard improvements, and PM fixes Golla Nagendra
2026-10-09 4:00 ` [PATCH v4 1/6] dmaengine: zynqmp_dma: Use PM guard for probe-time runtime PM reference Golla Nagendra
2026-10-09 4:00 ` [PATCH v4 2/6] PM: runtime: Add pm_runtime_if_active guard and conditional variant Golla Nagendra
@ 2026-10-09 4:00 ` Golla Nagendra
2026-10-10 2:59 ` Frank Li
2026-10-09 4:00 ` [PATCH v4 4/6] dt-bindings: dma: xlnx,zynqmp-dma: Add Versal Net compatible support Golla Nagendra
` (2 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Golla Nagendra @ 2026-10-09 4:00 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Michal Simek, Rafael J. Wysocki,
Kedareswara rao Appana, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Michael Tretter, Harini Katakam, Shyam Pandey
Cc: dmaengine, linux-arm-kernel, linux-kernel, linux-pm, devicetree,
git, 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>
---
drivers/dma/xilinx/zynqmp_dma.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c
index a48106e404e7..b5a5e93e42d5 100644
--- a/drivers/dma/xilinx/zynqmp_dma.c
+++ b/drivers/dma/xilinx/zynqmp_dma.c
@@ -738,6 +738,10 @@ 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 (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
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v4 3/6] dmaengine: zynqmp_dma: Guard IRQ handler against spurious interrupts
2026-10-09 4:00 ` [PATCH v4 3/6] dmaengine: zynqmp_dma: Guard IRQ handler against spurious interrupts Golla Nagendra
@ 2026-10-10 2:59 ` Frank Li
0 siblings, 0 replies; 14+ messages in thread
From: Frank Li @ 2026-10-10 2:59 UTC (permalink / raw)
To: Golla Nagendra
Cc: Vinod Koul, Frank Li, Michal Simek, Rafael J. Wysocki,
Kedareswara rao Appana, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Michael Tretter, Harini Katakam, Shyam Pandey,
dmaengine, linux-arm-kernel, linux-kernel, linux-pm, devicetree,
git
On Fri, Oct 09, 2026 at 09:30:12AM +0530, Golla Nagendra wrote:
> 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>
> ---
Reviewed-by: Frank Li <Frank.Li@nxp.com>
> drivers/dma/xilinx/zynqmp_dma.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c
> index a48106e404e7..b5a5e93e42d5 100644
> --- a/drivers/dma/xilinx/zynqmp_dma.c
> +++ b/drivers/dma/xilinx/zynqmp_dma.c
> @@ -738,6 +738,10 @@ 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 (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
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v4 4/6] dt-bindings: dma: xlnx,zynqmp-dma: Add Versal Net compatible support
2026-10-09 4:00 [PATCH v4 0/6] dmaengine: zynqmp_dma: Per-channel reset, IRQ guard improvements, and PM fixes Golla Nagendra
` (2 preceding siblings ...)
2026-10-09 4:00 ` [PATCH v4 3/6] dmaengine: zynqmp_dma: Guard IRQ handler against spurious interrupts Golla Nagendra
@ 2026-10-09 4:00 ` Golla Nagendra
2026-10-09 4:15 ` sashiko-bot
2026-10-09 15:33 ` Conor Dooley
2026-10-09 4:00 ` [PATCH v4 5/6] dmaengine: zynqmp_dma: Add new compatible string for Versal Net Golla Nagendra
2026-10-09 4:00 ` [PATCH v4 6/6] dmaengine: zynqmp_dma: Add per-channel reset support Golla Nagendra
5 siblings, 2 replies; 14+ messages in thread
From: Golla Nagendra @ 2026-10-09 4:00 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Michal Simek, Rafael J. Wysocki,
Kedareswara rao Appana, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Michael Tretter, Harini Katakam, Shyam Pandey
Cc: dmaengine, linux-arm-kernel, linux-kernel, linux-pm, devicetree,
git, Golla Nagendra, Jay Buddhabhatti
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 and Versal Net results
in an ABI break, but this is unavoidable. While the driver works in many
scenarios when no other firmware has used the DMA prior to Linux boot,
there is no guarantee the channel is left in a clean state. In observed
error scenarios, a reset is necessary before Linux can reconfigure the IP
after previous use by other firmware components. Due to configuration
differences or the DMA being left in a bad state, DMA functionality fails
when sequential software components use the channel (e.g., firmware before
Linux boot). The channel can only be cleanly re-configured after a
per-channel reset, making it essential. This ABI break is only applied
to Versal Gen2 and Versal Net where the per-channel reset capability is
available.
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>
---
.../bindings/dma/xilinx/xlnx,zynqmp-dma-1.0.yaml | 34 +++++++++++++++++-----
1 file changed, 27 insertions(+), 7 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..ff8c9a0e5559 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
@@ -16,17 +16,17 @@ maintainers:
- Harini Katakam <harini.katakam@amd.com>
- Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
-allOf:
- - $ref: ../dma-controller.yaml#
-
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 +59,9 @@ properties:
power-domains:
maxItems: 1
+ resets:
+ maxItems: 1
+
dma-coherent: true
required:
@@ -70,13 +73,29 @@ required:
- clock-names
- xlnx,bus-width
+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
+
additionalProperties: false
examples:
- |
fpd_dma_chan1: dma-controller@fd500000 {
- compatible = "xlnx,zynqmp-dma-1.0";
+ compatible = "amd,versal-net-dma-1.0", "xlnx,zynqmp-dma-1.0";
reg = <0xfd500000 0x1000>;
interrupt-parent = <&gic>;
interrupts = <0 117 0x4>;
@@ -84,5 +103,6 @@ examples:
clock-names = "clk_main", "clk_apb";
clocks = <&zynqmp_clk 19>, <&zynqmp_clk 31>;
xlnx,bus-width = <128>;
+ resets = <&reset 0>;
dma-coherent;
};
--
2.43.7
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v4 4/6] dt-bindings: dma: xlnx,zynqmp-dma: Add Versal Net compatible support
2026-10-09 4:00 ` [PATCH v4 4/6] dt-bindings: dma: xlnx,zynqmp-dma: Add Versal Net compatible support Golla Nagendra
@ 2026-10-09 4:15 ` sashiko-bot
2026-10-09 15:33 ` Conor Dooley
1 sibling, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-10-09 4:15 UTC (permalink / raw)
To: Golla Nagendra; +Cc: vkoul, Frank.Li, devicetree, dmaengine, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dt-bindings: invalid fallback compatible due to mandatory reset requirement
--
commit 9c2ab7ce56ba7e0b6e011ec582d150236f1eafe6
Author: Jay Buddhabhatti <jay.buddhabhatti@amd.com>
dt-bindings: dma: xlnx,zynqmp-dma: Add Versal Net compatible support
This commit adds a new compatible string amd,versal-net-dma-1.0 and marks
the resets property as required for Versal Net. It also configures
xlnx,zynqmp-dma-1.0 as a fallback for the new compatible.
> 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..ff8c9a0e5559a 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
[ ... ]
> @@ -16,17 +16,17 @@ maintainers:
> - Harini Katakam <harini.katakam@amd.com>
> - Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
>
> -allOf:
> - - $ref: ../dma-controller.yaml#
> -
> 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 it safe to provide a fallback compatible string here when the hardware
requires a mandatory reset?
If an older kernel encounters a device tree using this fallback, it will bind
to xlnx,zynqmp-dma-1.0. The older driver will then attempt to operate the
hardware without parsing or asserting the newly mandated resets property.
Since the commit message explicitly states the reset is essential to recover
from a bad firmware state, providing this fallback allows older drivers to
attempt to use the hardware in an unrecoverable state, contradicting the
purpose of the ABI break.
(Notice that amd,versal2-dma-1.0, which shares the same reset requirement,
correctly omits the fallback.)
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-adma-pm-fix-and-per-channel-reset-v4-0-55f5fe409c55@amd.com?part=4
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 4/6] dt-bindings: dma: xlnx,zynqmp-dma: Add Versal Net compatible support
2026-10-09 4:00 ` [PATCH v4 4/6] dt-bindings: dma: xlnx,zynqmp-dma: Add Versal Net compatible support Golla Nagendra
2026-10-09 4:15 ` sashiko-bot
@ 2026-10-09 15:33 ` Conor Dooley
1 sibling, 0 replies; 14+ messages in thread
From: Conor Dooley @ 2026-10-09 15:33 UTC (permalink / raw)
To: Golla Nagendra
Cc: Vinod Koul, Frank Li, Michal Simek, Rafael J. Wysocki,
Kedareswara rao Appana, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Michael Tretter, Harini Katakam, Shyam Pandey,
dmaengine, linux-arm-kernel, linux-kernel, linux-pm, devicetree,
git, Jay Buddhabhatti
[-- Attachment #1: Type: text/plain, Size: 4128 bytes --]
On Fri, Oct 09, 2026 at 09:30:13AM +0530, Golla Nagendra wrote:
> 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 and Versal Net results
> in an ABI break, but this is unavoidable. While the driver works in many
> scenarios when no other firmware has used the DMA prior to Linux boot,
> there is no guarantee the channel is left in a clean state. In observed
> error scenarios, a reset is necessary before Linux can reconfigure the IP
> after previous use by other firmware components. Due to configuration
> differences or the DMA being left in a bad state, DMA functionality fails
> when sequential software components use the channel (e.g., firmware before
> Linux boot). The channel can only be cleanly re-configured after a
> per-channel reset, making it essential. This ABI break is only applied
> to Versal Gen2 and Versal Net where the per-channel reset capability is
> available.
>
> 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>
> ---
> .../bindings/dma/xilinx/xlnx,zynqmp-dma-1.0.yaml | 34 +++++++++++++++++-----
> 1 file changed, 27 insertions(+), 7 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..ff8c9a0e5559 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
> @@ -16,17 +16,17 @@ maintainers:
> - Harini Katakam <harini.katakam@amd.com>
> - Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
>
> -allOf:
> - - $ref: ../dma-controller.yaml#
> -
> properties:
> "#dma-cells":
> const: 1
>
> compatible:
> - enum:
> - - amd,versal2-dma-1.0
> - - xlnx,zynqmp-dma-1.0
This used to be an enum, so please preserve it as one.
> + 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 +59,9 @@ properties:
> power-domains:
> maxItems: 1
>
> + resets:
> + maxItems: 1
> +
> dma-coherent: true
>
> required:
> @@ -70,13 +73,29 @@ required:
> - clock-names
> - xlnx,bus-width
>
> +allOf:
> + - $ref: ../dma-controller.yaml#
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - amd,versal2-dma-1.0
> + - amd,versal-net-dma-1.0
> + then:
> + required:
> + - resets
The change here for versal2 is an ABI break (new required property) and
needs to be split out into a standalone patch. The new support can go on
top.
pw-bot: changes-requested
Thanks,
Conor.
> + else:
> + properties:
> + resets: false
> +
> additionalProperties: false
>
> examples:
> - |
>
> fpd_dma_chan1: dma-controller@fd500000 {
> - compatible = "xlnx,zynqmp-dma-1.0";
> + compatible = "amd,versal-net-dma-1.0", "xlnx,zynqmp-dma-1.0";
> reg = <0xfd500000 0x1000>;
> interrupt-parent = <&gic>;
> interrupts = <0 117 0x4>;
> @@ -84,5 +103,6 @@ examples:
> clock-names = "clk_main", "clk_apb";
> clocks = <&zynqmp_clk 19>, <&zynqmp_clk 31>;
> xlnx,bus-width = <128>;
> + resets = <&reset 0>;
> dma-coherent;
> };
>
> --
> 2.43.7
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v4 5/6] dmaengine: zynqmp_dma: Add new compatible string for Versal Net
2026-10-09 4:00 [PATCH v4 0/6] dmaengine: zynqmp_dma: Per-channel reset, IRQ guard improvements, and PM fixes Golla Nagendra
` (3 preceding siblings ...)
2026-10-09 4:00 ` [PATCH v4 4/6] dt-bindings: dma: xlnx,zynqmp-dma: Add Versal Net compatible support Golla Nagendra
@ 2026-10-09 4:00 ` Golla Nagendra
2026-10-09 4:00 ` [PATCH v4 6/6] dmaengine: zynqmp_dma: Add per-channel reset support Golla Nagendra
5 siblings, 0 replies; 14+ messages in thread
From: Golla Nagendra @ 2026-10-09 4:00 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Michal Simek, Rafael J. Wysocki,
Kedareswara rao Appana, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Michael Tretter, Harini Katakam, Shyam Pandey
Cc: dmaengine, linux-arm-kernel, linux-kernel, linux-pm, devicetree,
git, 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>
---
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 b5a5e93e42d5..6881dc19435c 100644
--- a/drivers/dma/xilinx/zynqmp_dma.c
+++ b/drivers/dma/xilinx/zynqmp_dma.c
@@ -258,13 +258,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)
{
@@ -1229,6 +1236,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
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v4 6/6] dmaengine: zynqmp_dma: Add per-channel reset support
2026-10-09 4:00 [PATCH v4 0/6] dmaengine: zynqmp_dma: Per-channel reset, IRQ guard improvements, and PM fixes Golla Nagendra
` (4 preceding siblings ...)
2026-10-09 4:00 ` [PATCH v4 5/6] dmaengine: zynqmp_dma: Add new compatible string for Versal Net Golla Nagendra
@ 2026-10-09 4:00 ` Golla Nagendra
5 siblings, 0 replies; 14+ messages in thread
From: Golla Nagendra @ 2026-10-09 4:00 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Michal Simek, Rafael J. Wysocki,
Kedareswara rao Appana, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Michael Tretter, Harini Katakam, Shyam Pandey
Cc: dmaengine, linux-arm-kernel, linux-kernel, linux-pm, devicetree,
git, 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>
---
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 6881dc19435c..047a85342824 100644
--- a/drivers/dma/xilinx/zynqmp_dma.c
+++ b/drivers/dma/xilinx/zynqmp_dma.c
@@ -20,6 +20,7 @@
#include <linux/io-64-nonatomic-lo-hi.h>
#include <linux/cleanup.h>
#include <linux/pm_runtime.h>
+#include <linux/reset.h>
#include "../dmaengine.h"
@@ -942,6 +943,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;
@@ -957,7 +966,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
^ permalink raw reply related [flat|nested] 14+ messages in thread