| Message ID | 1398282724-2607-1-git-send-email-ulf.hansson@linaro.org |
|---|---|
| State | Accepted |
| Commit | 80245216ccbdb4b1dce4db714e0fdc692c81af6d |
| Headers | show |
On 04/23/2014 11:52 PM, Ulf Hansson wrote: > The runtime PM resume callback needs to be executed while holding the > spinlock, make sure to maintain this for the pause operation as well. > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> > --- > drivers/dma/ste_dma40.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c > index bf18c78..6e97cf6 100644 > --- a/drivers/dma/ste_dma40.c > +++ b/drivers/dma/ste_dma40.c > @@ -1495,8 +1495,8 @@ static int d40_pause(struct d40_chan *d40c) > if (!d40c->busy) > return 0; > > - pm_runtime_get_sync(d40c->base->dev); > spin_lock_irqsave(&d40c->lock, flags); > + pm_runtime_get_sync(d40c->base->dev); That function may sleep AFAIK, so you can't really call it with a spinlock held. Or do I miss something? WBR, Sergei
On 23 April 2014 22:39, Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote: > On 04/23/2014 11:52 PM, Ulf Hansson wrote: > >> The runtime PM resume callback needs to be executed while holding the >> spinlock, make sure to maintain this for the pause operation as well. > > >> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> >> --- >> drivers/dma/ste_dma40.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) > > >> diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c >> index bf18c78..6e97cf6 100644 >> --- a/drivers/dma/ste_dma40.c >> +++ b/drivers/dma/ste_dma40.c >> @@ -1495,8 +1495,8 @@ static int d40_pause(struct d40_chan *d40c) >> if (!d40c->busy) >> return 0; >> >> - pm_runtime_get_sync(d40c->base->dev); >> spin_lock_irqsave(&d40c->lock, flags); >> + pm_runtime_get_sync(d40c->base->dev); > > > That function may sleep AFAIK, so you can't really call it with a spinlock > held. Or do I miss something? That's the default behaviour from the runtime PM core. But, since we have invoked "pm_runtime_irq_safe()" at ->probe(), that means the pm_runtime_get_sync() function won't sleep. Kind regards Ulf Hansson > > WBR, Sergei >
On Wed, Apr 23, 2014 at 9:52 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote: > The runtime PM resume callback needs to be executed while holding the > spinlock, make sure to maintain this for the pause operation as well. > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> Acked-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
On Wed, Apr 23, 2014 at 09:52:01PM +0200, Ulf Hansson wrote: > The runtime PM resume callback needs to be executed while holding the > spinlock, make sure to maintain this for the pause operation as well. Applied, all thanks. Though we need to change the driver to use SET_LATE_SYSTEM_SLEEP_PM_OPS as all dmaengine drivers should suspend late as clients can be active while suspending
On 7 May 2014 08:22, Vinod Koul <vinod.koul@intel.com> wrote: > On Wed, Apr 23, 2014 at 09:52:01PM +0200, Ulf Hansson wrote: >> The runtime PM resume callback needs to be executed while holding the >> spinlock, make sure to maintain this for the pause operation as well. > > Applied, all thanks. > > Though we need to change the driver to use SET_LATE_SYSTEM_SLEEP_PM_OPS as > all dmaengine drivers should suspend late as clients can be active while > suspending Right, I suspected that as well - even if I did ran some tests to verify this not to happen in practice for ux500. Anyway, I will send a patch which converts to SET_LATE_SYSTEM_SLEEP_PM_OPS, since for sure it makes sense. Kind regards Ulf Hansson > > -- > ~Vinod >
diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c index bf18c78..6e97cf6 100644 --- a/drivers/dma/ste_dma40.c +++ b/drivers/dma/ste_dma40.c @@ -1495,8 +1495,8 @@ static int d40_pause(struct d40_chan *d40c) if (!d40c->busy) return 0; - pm_runtime_get_sync(d40c->base->dev); spin_lock_irqsave(&d40c->lock, flags); + pm_runtime_get_sync(d40c->base->dev); res = d40_channel_execute_command(d40c, D40_DMA_SUSPEND_REQ);
The runtime PM resume callback needs to be executed while holding the spinlock, make sure to maintain this for the pause operation as well. Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> --- drivers/dma/ste_dma40.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)