On 25/08/15 23:46, Rafael J. Wysocki wrote: > On 8/25/2015 11:37 AM, Jon Hunter wrote: [snip] >> Vinod, thinking about this some more, I am wondering if it is just >> better to get rid of the suspend/resume callbacks and simply handling >> the state in the runtime suspend/resume callbacks. I think that would be >> safe too, because once the clock has been disabled, then who knows what >> the context state will be. > > One caveat here: system suspend may be invoked at any time, so you need > to ensure that the device is properly suspended when that happens. > > I believe you at least need a ->suspend callback for that. Thanks, makes sense. On a related note, I see a few drivers, including this DMA driver doing the following in the driver ->remove callback. pm_runtime_disable(&pdev->dev); !pm_runtime_status_suspended(&pdev->dev)) tegra_dma_runtime_suspend(&pdev->dev); I understand that the code is trying to ensure that the device is suspended regardless of whether rpm is enabled or not in the kernel config. However, looking at the pm_runtime_status_suspended() function, AFAICT, it will always return false above as the disable_depth will be greater than 0. So I am concerned that the tegra_dma_runtime_suspend() is called even when not needed? However, I could also be missing something here. Cheers Jon -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html