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

[Frank Li]

On Fri, Oct 16, 2026 at 10:49:31AM +0000, Mizuno Miotsuki wrote:
> Hi Frank,
>
> Thanks for the review.
>
> > Look like need lock between terminate and issue_pendings. There are should
> > other problems if interfere each others.
> >
> > terminate should wait issue_pending finish and block queue new request.
>
> Both pl330_issue_pending() and pl330_terminate_all() already use
> pch->lock to protect the channel state. However, issue_pending()
> also invokes pl330_tasklet() after releasing the lock, so this
> does not establish complete synchronization.
>
> I ran a source-based interleaving test. The original
> code reproduces the extra runtime PM put after termination, while
> the proposed one-line fix balances the references under the same
> interleaving.
>
> > This function can be called in an interrupt context
> >
> > call pm_runtime_get() is risk, which may sleep.
>
> pl330_probe() calls pm_runtime_irq_safe(&adev->dev), and the AMBA
> runtime PM callbacks use clk_enable()/clk_disable() for IRQ-safe
> devices. This allows pm_runtime_get_sync() to be called from
> atomic context, provided the relevant PM callbacks and dependencies
> satisfy the IRQ-safe requirements.
>
> I agree that the remaining synchronization and error-handling
> paths deserve further investigation.
>
> Would it be reasonable to keep this patch focused on the
> reproduced reference-count imbalance and address any confirmed
> synchronization issues separately?

This is quick fix after I read code again. but original mothed is not
good enough, which mix use check varible and runtime_pm reference-count.

If pair get() and put(), needn't pch->active.

if (list_empty(&pch->work_list)) {
                /*
                 * Warn on nothing pending. Empty submitted_list may
                 * break our pm_runtime usage counter as it is
                 * updated on work_list emptiness status.
                 */
                WARN_ON(list_empty(&pch->submitted_list));
                pch->active = true;
                pm_runtime_get_sync(pch->dmac->ddma.dev);
        }

simple call
	for_each(work_list) {
		pm_runtime_get_sync()
	}

ideally, there are PM API (pm_runtime_get_sync(, count(work_list)) to bump
more than 1 ref count. But you can use above code to proof concept.

In pl330_tasklet()

 while (!list_empty(&pch->completed_list)) {
	...
	pm_runtime_put_autosuspend()
 }

Keep pm ref count is the same as count of work_list. So needn't pch->active
at all.

Frank

>
> Thanks,
> Mizuno