Re: [PATCH] dmaengine: shdma: Runtime-resume device in .shutdown()

[Vinod Koul <vinod.koul@intel.com>]

On Sun, Dec 07, 2014 at 11:35:21AM +0100, Geert Uytterhoeven wrote:
> >>  static void sh_dmae_shutdown(struct platform_device *pdev)
> >>  {
> >>       struct sh_dmae_device *shdev = platform_get_drvdata(pdev);
> >> +
> >> +     pm_runtime_get_sync(&pdev->dev);
> >>       sh_dmae_ctl_stop(shdev);
> >> +     pm_runtime_put(&pdev->dev);
> > but if you are runtime_suspended, then why should you even proceed to stop
> > the clock. Why not just cleanup and return when runtime suspended
> 
> sh_dmae_ctl_stop() stops the DMA engine, not the clock.
> Accessing the DMA engine cannot be done while the module clock is stopped.
> 
> If pm_runtime_suspended() returns true, I can indeed just return (no new
> request can come in while .shutdown() is called).
> However, if pm_runtime_suspended() returns false, the device may still
> become runtime suspended in between the check for pm_runtime_suspended()
> and accessing the DMA engine registers in sh_dmae_ctl_stop(), as runtime
> suspend is an asynchronous operation, right?
> 
> So I think adding a check for pm_runtime_suspended() can only serve as
> an optimization, the pm_runtime_{get_sync,put}() has to stay to prevent a
> race condition.
You are quite right in your observations, but this solution though correct
seems bit heavy for case like shutdown. can't we can use driver lock to
prevent race.

-- 
~Vinod