[PATCH v3] dmaengine: pl330: Fix unbalanced runtime PM

[Mizuno Miotsuki via B4 Relay]

From: Sugar Zhang <sugar.zhang@xxxxxxxxxxxxxx>

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(unconditional)
            |                            |

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: 5c9e6c2b2ba3 ("dmaengine: pl330: Fix runtime PM support for terminated transfers")
Fixes: ae43b3289186 ("ARM: 8202/1: dmaengine: pl330: Add runtime Power Management support v12")

Link: https://lore.kernel.org/r/1650450456-34351-1-git-send-email-sugar.zhang@xxxxxxxxxxxxxx
Signed-off-by: Huibin Hong <huibin.hong@xxxxxxxxxxxxxx>
Signed-off-by: Sugar Zhang <sugar.zhang@xxxxxxxxxxxxxx>
Signed-off-by: Mizuno Miotsuki <founder@xxxxxxxxxxx>
---
Hi Vinod, Sugar and Huibin,

I am forwarding Sugar's April 2022 v2 as v3, with the same one-line
change, original authorship and original Signed-off-by chain. The
original submission is linked in the commit message. I found no mainline
merge or revert of this change; current dmaengine fixes, next and master
still use the unconditional assignment.

Our SaneBSP investigation on an RK3308B UNIHIKER M10 independently
rediscovered the same accounting issue. The experimental G-only kernel
logged:

  dma-pl330 ff2c0000.dma-controller: Runtime PM usage count underflow!

The subsequent H kernel, which included this gate, did not report the
warning during a four-rate 62.625 MiB qualification and a bounded serdev
regression. H also contains other changes, and G/H were experimental
kernels. This is additional runtime evidence, not an isolated hardware
proof of this one-line fix on upstream. None of the experimental cyclic
DMA modifications is included here.

A source-anchored reference-count model reproduces the extra put after
termination in the old path and balances the active-gated path, including
cyclic requeue. That is a model, not a compiled-kernel race reproducer.

Validation of this rebased patch:
- ARM64 W=1 drivers/dma/pl330.o build on dmaengine/fixes.
- Strict checkpatch, including the two historical Fixes IDs.
- Clean application on current dmaengine fixes/next/master and mainline.

Would you reconsider the outstanding patch? Please let us know if there
was an objection or an alternative fix outside the archived discussion.

Changes in v3:
- Rebase the unchanged code change onto dmaengine/fixes, Linux 7.3-rc4.
- Add an archival Link to the original v2.
- Preserve the original author, commit explanation, Fixes and DCO trailers.
- Add the bounded new evidence and validation here, outside the commit.

Changes in v2 (Sugar, 2022):
- Update the commit message following Vinod's suggestions.

Regards,
Mizuno Miotsuki
---
 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 6214d9000..17264a05d 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -2090,7 +2090,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 */

---
base-commit: 93f51579e7df248780214094418f205253383cc5
change-id: 20261015-review-pl330-pm-original-v3-be881d29c3bf

Best regards,
--  
Mizuno Miotsuki <founder@xxxxxxxxxxx>