Re: [PATCH] dma: dw: Add suspend and resume handling for PCI modeDW_DMAC.

[Shevchenko, Andriy]

On Mon, 2014-01-20 at 14:55 +0530, Vinod Koul wrote:
> On Thu, Dec 19, 2013 at 12:51:29PM +0200, Andy Shevchenko wrote:
> > On Wed, 2013-12-18 at 21:19 +0530, Vinod Koul wrote:
> > > On Mon, Dec 16, 2013 at 01:51:47PM +0530, Chew, Chiau Ee wrote:
> > 
> > > > As mentioned by Andy, we are using *_noirq  verion of suspend/resume PM
> > > > callback whereby the callbacks would be executed after IRQ handlers have been
> > > > disabled. If using SET_SYSTEM_SLEEP_PM_OPS, it would be the normal
> > > > suspend/resume PM callback. Looking at the Desginware DMAC platform code
> > > > (drivers/dma/dw/platform.c), it is using the *_noirq suspend/resume PM
> > > > callback. Is it advisable to use the normal suspend/resume PM callback instead
> > > > of *_noirq suspend/PM callback? 
> > > 
> > > i dont see a reason why we need the noirq versions
> > 
> > Okay. I imagine the following use case.
> > 
> > For example we have compiled in DMA driver (dw_dmac) along with, for
> > example, SPI driver.
> > 
> > System was scheduled to go sleep.
> > 
> > An order of calling IIUC might be DMA first, then SPI (since they are
> > not in parent / child relations).
> > 
> > What was happened when SPI would like to do a DMA transfer and DMA is
> > going to sleep? I'm trying to understand if this is a case.
> In that case how does no irq version help us?

It guarantees that we have no user of DMA anymore, since there is no
interrupt going on.

> For these cases, I have been using suspend_late. Since the dmaengine driver is
> providing service to other clients (SPI), it needs to esnure that it suspends
> after SPI using suspend_late and resume using resume_early. That way dma is
> availble whenever the client is active

suspend_late is working in context that interrupt handler may be
invoked. Thus, to have DMA driver be properly shut down we have to
wait / terminate possible ongoing transfer.

It seems for me all DMA drivers that are using
system .suspend()/.resume() are potentially buggy.

> 
> --
> ~Vinod
> 
> > 
> > > > > How about SET_SYSTEM_SLEEP_PM_OPS instead?
> > > > 
> > > > So, we are using *_noirq versions of the functions here. What happened when we switch to normal ones? Any side effects?
> > 
> > 
> > 
> > -- 
> > Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> > Intel Finland Oy
> > 
> 

-- 
Andy Shevchenko <andriy.shevchenko@xxxxxxxxx>
Intel Finland Oy
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
èº{.nÇ+‰·Ÿ®‰­†+%ŠËlzwm…ébëæìr¸›zX§»®w¥Š{ayºÊÚë,j­¢f£¢·hš‹àz¹®w¥¢¸¢·¦j:+v‰¨ŠwèjØm¶Ÿÿ¾«‘êçzZ+ƒùšŽŠÝj"ú!¶iO•æ¬z·švØ^¶m§ÿðÃnÆàþY&—