Re: [PATCH v2 5/7] dmaengine: dw: platform: power on device on shutdown

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Thu, 2015-11-26 at 22:31 +0530, Vinod Koul wrote:
> On Thu, Nov 26, 2015 at 05:19:11PM +0200, Andy Shevchenko wrote:
> > We have to call dw_dma_disable() to stop any ongoing transfer.
> 
> Ok
> 
> > On some platforms we can't do that since DMA device is powered off.
> 
> Umm, you said we have ongoing transfer which means DMA should be
> on..!!

Yes, that's true for HSW/BDW and non-affected BYT/CHT.

Like I mentioned here is no possibility to know which platform we run
on.

Would you like to test this on a real device? We can provide you a
login.

> 
> > Moreover we have no
> > possibility at that point to check if the platform is affected or
> > not. That's
> > why we call pm_runtime_get_sync() / pm_runtime_put()
> > unconditionally. On the
> > other hand we can't use pm_runtime_suspended() because runtime PM
> > framework is
> > not fully used by the driver.
> 
> Shouldn't that be fixed?

Do you have any solution how?

Rough approach is to turn on it on channel allocation and turn off on
freeing resources. The obvious downside of this solution is power
consumption of idling device.

In any case it might be done in the future and it's not a scope of this
series.

> 
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> >  drivers/dma/dw/platform.c | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/drivers/dma/dw/platform.c b/drivers/dma/dw/platform.c
> > index 68a4815..d0734e9 100644
> > --- a/drivers/dma/dw/platform.c
> > +++ b/drivers/dma/dw/platform.c
> > @@ -239,7 +239,19 @@ static void dw_shutdown(struct platform_device
> > *pdev)
> >  {
> >  	struct dw_dma_chip *chip = platform_get_drvdata(pdev);
> >  
> > +	/*
> > +	 * We have to call dw_dma_disable() to stop any ongoing
> > transfer. On
> > +	 * some platforms we can't do that since DMA device is
> > powered off.
> > +	 * Moreover we have no possibility to check if the
> > platform is affected
> > +	 * or not. That's why we call pm_runtime_get_sync() /
> > pm_runtime_put()
> > +	 * unconditionally. On the other hand we can't use
> > +	 * pm_runtime_suspended() because runtime PM framework is
> > not fully
> > +	 * used by the driver.
> > +	 */
> > +	pm_runtime_get_sync(chip->dev);
> >  	dw_dma_disable(chip);
> > +	pm_runtime_put_sync_suspend(chip->dev);
> > +
> >  	clk_disable_unprepare(chip->clk);
> >  }
> >  
> > -- 
> > 2.6.2
> > 
> 

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy