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

Hi Frank

On 8/11/2026 9:43 PM, Frank Li wrote:
> On Mon, Aug 10, 2026 at 03:34:47PM +0530, Golla Nagendra wrote:
>> 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 <[email protected]>
>> ---
>> 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))
> 
> You just enable runtime pm above few lines.  Is it for CONFIG_PM?
> 
> You should check return value of pm_runtime_enable(), or change to use
> 
> devm_pm_runtime_enable().
> 
> if devm_runtime_enable() failure, you can direct return. Need check
> pm_runtime_enabled() here again.
> 
>>   		zynqmp_dma_runtime_suspend(zdev->dev);
>> +	else
>> +		pm_runtime_put_sync(zdev->dev);
> 
> This one suggest use PM acquire to fix this.
Thanks for the review

The probe error path already holds a reference from 
pm_runtime_resume_and_get().Using PM_RUNTIME_ACQUIRE_IF_ACTIVE() here 
would acquire a second reference on top of the existing one incrementing 
the usage count to 2, and the guard releasing one at scope exit would 
still leave one leaked reference. The guard is designed for cases where 
no reference is held yet (like the IRQ handler). Here we just need to 
release the one existing reference, so i am using the 
pm_runtime_put_sync_suspend().

Thanks,
Nagendra
> 
> Frank
> 
>>   	pm_runtime_disable(zdev->dev);
>>   	return ret;
>>   }
>> --
>> 2.43.7
>>