From: Frank Li <Frank.li@oss.nxp.com>
To: Linus Walleij <linusw@kernel.org>
Cc: Vinod Koul <vkoul@kernel.org>, Frank Li <Frank.Li@kernel.org>,
dmaengine@vger.kernel.org, phone-devel@vger.kernel.org
Subject: Re: [PATCH v5 07/23] dmaengine: ste_dma40: Handle runtime PM resume errors
Date: Tue, 22 Sep 2026 14:23:03 -0500 [thread overview]
Message-ID: <arLVl9lFO2wDEt6Q@SMW015318> (raw)
In-Reply-To: <20260920-dma40-fixes-v5-7-5c55cd7f92f9@kernel.org>
On Sun, Sep 20, 2026 at 08:59:42PM +0200, Linus Walleij wrote:
> Several channel operations use pm_runtime_get_sync() and access DMA40
> registers without checking whether runtime resume succeeded. If resume
> fails, the registers may be inaccessible. pm_runtime_get_sync() also
> increments the usage counter on failure, making error unwinding easy to
> unbalance.
>
> Use pm_runtime_resume_and_get() and avoid register access when resume
> fails. Acquire the runtime PM reference before allocating a channel so
> failure needs no channel-allocation rollback.
>
> If a queued transfer cannot be started because resume failed, retire all
> issued descriptors through the normal tasklet path. Since
> dma_async_issue_pending() cannot return an error, leaving them queued would
> make clients wait indefinitely for callbacks.
>
> Termination and channel release must also clean up software state when the
> controller cannot resume. Always release descriptors and the outstanding
> busy reference, and release channel allocation state when freeing the
> channel. Skip only the hardware stop that requires register access.
>
> Fixes: 7fb3e75e1833 ("dmaengine/ste_dma40: support pm in dma40")
> Assisted-by: LLM
> Signed-off-by: Linus Walleij <linusw@kernel.org>
> ---
> drivers/dma/ste_dma40.c | 67 ++++++++++++++++++++++++++++++++++++-------------
> 1 file changed, 50 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
> index 58128a980847..bb052d3028cc 100644
> --- a/drivers/dma/ste_dma40.c
> +++ b/drivers/dma/ste_dma40.c
> @@ -1567,11 +1567,14 @@ static int d40_pause(struct dma_chan *chan)
> return 0;
>
> spin_lock_irqsave(&d40c->lock, flags);
> - pm_runtime_get_sync(d40c->base->dev);
> + res = pm_runtime_resume_and_get(d40c->base->dev);
> + if (res < 0)
> + goto unlock;
>
> res = d40_channel_execute_command(d40c, D40_DMA_SUSPEND_REQ);
>
> pm_runtime_put_autosuspend(d40c->base->dev);
> + unlock:
> spin_unlock_irqrestore(&d40c->lock, flags);
> return res;
> }
> @@ -1591,13 +1594,16 @@ static int d40_resume(struct dma_chan *chan)
> return 0;
>
> spin_lock_irqsave(&d40c->lock, flags);
> - pm_runtime_get_sync(d40c->base->dev);
> + res = pm_runtime_resume_and_get(d40c->base->dev);
> + if (res < 0)
> + goto unlock;
>
> /* If bytes left to transfer or linked tx resume job */
> if (d40_residue(d40c) || d40_tx_is_linked(d40c))
> res = d40_channel_execute_command(d40c, D40_DMA_RUN);
>
> pm_runtime_put_autosuspend(d40c->base->dev);
> + unlock:
> spin_unlock_irqrestore(&d40c->lock, flags);
> return res;
> }
> @@ -1634,8 +1640,20 @@ static struct d40_desc *d40_queue_start(struct d40_chan *d40c)
>
> if (d40d != NULL) {
> if (!d40c->busy) {
> + err = pm_runtime_resume_and_get(d40c->base->dev);
> + if (err < 0) {
> + chan_err(d40c, "Failed to resume DMA: %d\n",
> + err);
> + do {
> + d40_desc_remove(d40d);
> + d40_desc_done(d40c, d40d);
> + d40c->pending_tx++;
> + d40d = d40_first_queued(d40c);
> + } while (d40d);
> + tasklet_schedule(&d40c->tasklet);
> + return ERR_PTR(err);
> + }
> d40c->busy = true;
> - pm_runtime_get_sync(d40c->base->dev);
> }
>
> /* Remove from queue */
> @@ -2150,11 +2168,9 @@ static int d40_free_dma(struct d40_chan *d40c)
> int res = 0;
> u32 event = D40_TYPE_TO_EVENT(d40c->dma_cfg.dev_type);
> struct d40_phy_res *phy = d40c->phy_chan;
> + bool pm_acquired = false;
> bool is_src;
>
> - /* Terminate all queued and active transfers */
> - d40_term_all(d40c);
> -
> if (phy == NULL) {
> chan_err(d40c, "phy == null\n");
> return -EINVAL;
> @@ -2176,13 +2192,21 @@ static int d40_free_dma(struct d40_chan *d40c)
> return -EINVAL;
> }
>
> - pm_runtime_get_sync(d40c->base->dev);
> + /* Terminate all queued and active transfers */
> + d40_term_all(d40c);
> +
> + res = pm_runtime_resume_and_get(d40c->base->dev);
> + if (res < 0)
> + goto release_channel;
> + pm_acquired = true;
> +
> res = d40_channel_execute_command(d40c, D40_DMA_STOP);
> if (res) {
> chan_err(d40c, "stop failed\n");
> goto mark_last_busy;
> }
>
> + release_channel:
> d40_alloc_mask_free(phy, is_src, chan_is_logical(d40c) ? event : 0);
>
> if (chan_is_logical(d40c))
> @@ -2197,7 +2221,8 @@ static int d40_free_dma(struct d40_chan *d40c)
> d40c->phy_chan = NULL;
> d40c->configured = false;
> mark_last_busy:
> - pm_runtime_put_autosuspend(d40c->base->dev);
> + if (pm_acquired)
> + pm_runtime_put_autosuspend(d40c->base->dev);
> return res;
> }
>
> @@ -2572,10 +2597,14 @@ static int d40_alloc_chan_resources(struct dma_chan *chan)
> err = d40_config_memcpy(d40c);
> if (err) {
> chan_err(d40c, "Failed to configure memcpy channel\n");
> - goto mark_last_busy;
> + goto unlock;
> }
> }
>
> + err = pm_runtime_resume_and_get(d40c->base->dev);
> + if (err < 0)
> + goto unlock;
> +
> err = d40_allocate_channel(d40c, &is_free_phy);
> if (err) {
> chan_err(d40c, "Failed to allocate channel\n");
> @@ -2583,8 +2612,6 @@ static int d40_alloc_chan_resources(struct dma_chan *chan)
> goto mark_last_busy;
> }
>
> - pm_runtime_get_sync(d40c->base->dev);
> -
> d40_set_prio_realtime(d40c);
>
> if (chan_is_logical(d40c)) {
> @@ -2616,6 +2643,7 @@ static int d40_alloc_chan_resources(struct dma_chan *chan)
> d40_config_write(d40c);
> mark_last_busy:
> pm_runtime_put_autosuspend(d40c->base->dev);
> + unlock:
> spin_unlock_irqrestore(&d40c->lock, flags);
> return err;
> }
> @@ -2767,6 +2795,7 @@ static int d40_terminate_all(struct dma_chan *chan)
> {
> unsigned long flags;
> struct d40_chan *d40c = container_of(chan, struct d40_chan, chan);
> + bool pm_acquired = false;
> int ret;
>
> if (d40c->phy_chan == NULL) {
> @@ -2776,19 +2805,23 @@ static int d40_terminate_all(struct dma_chan *chan)
>
> spin_lock_irqsave(&d40c->lock, flags);
>
> - pm_runtime_get_sync(d40c->base->dev);
> - ret = d40_channel_execute_command(d40c, D40_DMA_STOP);
> - if (ret)
> - chan_err(d40c, "Failed to stop channel\n");
> + ret = pm_runtime_resume_and_get(d40c->base->dev);
> + if (ret >= 0) {
> + pm_acquired = true;
> + ret = d40_channel_execute_command(d40c, D40_DMA_STOP);
> + if (ret)
> + chan_err(d40c, "Failed to stop channel\n");
> + }
>
> d40_term_all(d40c);
> - pm_runtime_put_autosuspend(d40c->base->dev);
> + if (pm_acquired)
> + pm_runtime_put_autosuspend(d40c->base->dev);
> if (d40c->busy)
> pm_runtime_put_autosuspend(d40c->base->dev);
> d40c->busy = false;
>
> spin_unlock_irqrestore(&d40c->lock, flags);
> - return 0;
> + return ret;
pm_acquired is not necesary
ret = pm_runtime_resume_and_get(d40c->base->dev);
if (ret >= ) {
ret = d40_channel_execute_command(d40c, D40_DMA_STOP);
if (ret)
...
pm_runtime_put_autosuspend(d40c->base->dev);
}
d40_term_all(d40c); /* In your patch, d40_term_all(d40c) can be call
without acquire runtime pm
...
Frank
> }
>
> static int
>
> --
> 2.55.0
>
next prev parent reply other threads:[~2026-09-22 19:23 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-20 18:59 [PATCH v5 00/23] dmaengine: ste_dma40: Fix numerous accumulated bugs Linus Walleij
2026-09-20 18:59 ` [PATCH v5 01/23] dmaengine: ste_dma40: Fix physical cyclic capability Linus Walleij
2026-09-21 16:39 ` Frank Li
2026-09-20 18:59 ` [PATCH v5 02/23] dmaengine: ste_dma40: Fix cyclic transfer residue Linus Walleij
2026-09-21 16:57 ` Frank Li
2026-09-20 18:59 ` [PATCH v5 03/23] dmaengine: ste_dma40: Recover coalesced cyclic callbacks Linus Walleij
2026-09-21 21:51 ` Frank Li
2026-09-22 21:13 ` Linus Walleij
2026-09-20 18:59 ` [PATCH v5 04/23] dmaengine: ste_dma40: Fix failed start cleanup Linus Walleij
2026-09-21 22:10 ` Frank Li
2026-09-20 18:59 ` [PATCH v5 05/23] dmaengine: ste_dma40: Fix probe runtime PM disable Linus Walleij
2026-09-21 22:16 ` Frank Li
2026-09-20 18:59 ` [PATCH v5 06/23] dmaengine: ste_dma40: Check runtime PM in IRQ Linus Walleij
2026-09-21 22:19 ` Frank Li
2026-09-20 18:59 ` [PATCH v5 07/23] dmaengine: ste_dma40: Handle runtime PM resume errors Linus Walleij
2026-09-22 19:23 ` Frank Li [this message]
2026-09-20 18:59 ` [PATCH v5 08/23] dmaengine: ste_dma40: Return IRQ_NONE without interrupt status Linus Walleij
2026-09-20 19:10 ` sashiko-bot
2026-09-20 21:26 ` Linus Walleij
2026-09-22 19:37 ` Frank Li
2026-09-22 23:17 ` Linus Walleij
2026-09-20 18:59 ` [PATCH v5 09/23] dmaengine: ste_dma40: Init hardware before registration Linus Walleij
2026-09-20 18:59 ` [PATCH v5 10/23] dmaengine: ste_dma40: Fix probe IRQ leak Linus Walleij
2026-09-20 18:59 ` [PATCH v5 11/23] dmaengine: ste_dma40: Fix DMA registration unwind Linus Walleij
2026-09-20 18:59 ` [PATCH v5 12/23] dmaengine: ste_dma40: Fix LCLA allocation order Linus Walleij
2026-09-20 18:59 ` [PATCH v5 13/23] dmaengine: ste_dma40: Fix probe LCLA free Linus Walleij
2026-09-20 18:59 ` [PATCH v5 14/23] dmaengine: ste_dma40: Put the LCPA SRAM node Linus Walleij
2026-09-20 18:59 ` [PATCH v5 15/23] dmaengine: ste_dma40: Fix memcpy channel parsing Linus Walleij
2026-09-20 18:59 ` [PATCH v5 16/23] dmaengine: ste_dma40: Validate disabled channel indexes Linus Walleij
2026-09-20 18:59 ` [PATCH v5 17/23] dmaengine: ste_dma40: Validate DMA specifier length Linus Walleij
2026-09-20 18:59 ` [PATCH v5 18/23] dmaengine: ste_dma40: Reject direction changes after allocation Linus Walleij
2026-09-20 18:59 ` [PATCH v5 19/23] dmaengine: ste_dma40: Fix logical channel bounds check Linus Walleij
2026-09-20 18:59 ` [PATCH v5 20/23] dmaengine: ste_dma40: Fix event group bounds Linus Walleij
2026-09-20 18:59 ` [PATCH v5 21/23] dmaengine: ste_dma40: Search all blocks for fixed logical channels Linus Walleij
2026-09-20 18:59 ` [PATCH v5 22/23] dmaengine: ste_dma40: Validate fixed physical channel indexes Linus Walleij
2026-09-20 18:59 ` [PATCH v5 23/23] dmaengine: ste_dma40: Validate memcpy configuration Linus Walleij
-- strict thread matches above, loose matches on Subject: below --
2026-09-22 23:30 [PATCH v5 00/23] dmaengine: ste_dma40: Fix numerous accumulated bugs Linus Walleij
2026-09-22 23:30 ` [PATCH v5 07/23] dmaengine: ste_dma40: Handle runtime PM resume errors Linus Walleij
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=arLVl9lFO2wDEt6Q@SMW015318 \
--to=frank.li@oss.nxp.com \
--cc=Frank.Li@kernel.org \
--cc=dmaengine@vger.kernel.org \
--cc=linusw@kernel.org \
--cc=phone-devel@vger.kernel.org \
--cc=vkoul@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox