Re: [PATCH v4 1/6] dmaengine: zynqmp_dma: Use PM guard for probe-time runtime PM reference

[Frank Li]

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@xxxxxxx>
> ---
>  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
>