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

"Golla, Nagendra" <[email protected]>
Newsgroups org.kernel.vger.dmaengine,dev.linux.lists.sashiko-reviews,org.kernel.vger.linux-devicetree
Message-ID <[email protected]>
Hi,

On 8/10/2026 3:58 PM, [email protected] wrote:
> 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 <[email protected]>
> 
> 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?

Thanks for catching this
Agreed will fix this in the next version

Thanks,
Nagendra
> 
>>   	return ret;
>>   }
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.