On 17-04-22, 21:53, sugar zhang wrote:
> Hi Vinod,
>
> 在 2022/4/11 21:48, Vinod Koul 写道:
> > On 26-03-22, 20:16, Sugar Zhang wrote:
> > > This driver use runtime PM autosuspend mechanism to manager clk.
> > >
> > > pm_runtime_use_autosuspend(&adev->dev);
> > > pm_runtime_set_autosuspend_delay(&adev->dev, PL330_AUTOSUSPEND_DELAY);
> > >
> > > So, after ref count reached to zero, it will enter suspend
> > > after the delay time elapsed.
> > >
> > > The unbalanced PM:
> > >
> > > * May cause dmac the next start failed.
> > > * May cause dmac read unexpected state.
> > > * May cause dmac stall if power down happen at the middle of the transfer.
> > > e.g. may lose ack from AXI bus and stall.
> > >
> > > Considering the following situation:
> > >
> > > DMA TERMINATE TASKLET ROUTINE
> > > | |
> > > | issue_pending
> > > | |
> > > | pch->active = true
> > > | pm_runtime_get
> > > pm_runtime_put(if active) |
> > > pch->active = false |
> > > | work_list empty
> > > | |
> > > | pm_runtime_put(force)
> > maybe unconditional is a better word than force here?
> okay, will do in v2.
> >
> > > | |
> > >
> > > At this point, it's unbalanced(1 get / 2 put).
> > >
> > > After this patch:
> > >
> > > DMA TERMINATE TASKLET ROUTINE
> > > | |
> > > | issue_pending
> > > | |
> > > | pch->active = true
> > > | pm_runtime_get
> > > pm_runtime_put(if active) |
> > > pch->active = false |
> > > | work_list empty
> > > | |
> > > | pm_runtime_put(if active)
> > > | |
> > >
> > > Now, it's balanced(1 get / 1 put).
> > >
> > > Fixes:
> > > commit 5c9e6c2b2ba3 ("dmaengine: pl330: Fix runtime PM support for terminated transfers")
> > > commit ae43b3289186 ("ARM: 8202/1: dmaengine: pl330: Add runtime Power Management support v12")
> > That is not the right way for Fixes tag
>
> like this?
>
> Fixes: 5c9e6c2b2ba3 ("dmaengine: pl330: Fix runtime PM support for terminated transfers")
> Fixes: ae43b3289186 ("ARM: 8202/1: dmaengine: pl330: Add runtime Power Management support v12")
yes, this is the correct way
--
~Vinod
