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

[Mizuno Miotsuki <founder@crepuse.com>]

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?

Thanks,
Mizuno