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?
> | |
>
> 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
>
> Signed-off-by: Huibin Hong <huibin.hong@xxxxxxxxxxxxxx>
> Signed-off-by: Sugar Zhang <sugar.zhang@xxxxxxxxxxxxxx>
> ---
>
> drivers/dma/pl330.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
> index 858400e..ccd430e 100644
> --- a/drivers/dma/pl330.c
> +++ b/drivers/dma/pl330.c
> @@ -2084,7 +2084,7 @@ static void pl330_tasklet(struct tasklet_struct *t)
> spin_lock(&pch->thread->dmac->lock);
> _stop(pch->thread);
> spin_unlock(&pch->thread->dmac->lock);
> - power_down = true;
> + power_down = pch->active;
> pch->active = false;
> } else {
> /* Make sure the PL330 Channel thread is active */
> --
> 2.7.4
--
~Vinod
