Re: [PATCH v5 19/24] dmaengine: dw-edma: Drain LL entries for STOP and PAUSE

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] `dw_edma_wait_termination()` loops infinitely when the hardware is undergoing teardown (e.g., driver unbind) because interrupts and rechecks are suppressed and the stop request is never completed.
--

--- Patch [19]: [PATCH v5 19/24] dmaengine: dw-edma: Drain LL entries for STOP and PAUSE ---
commit 1e80fd5741b43db059b6d11175c2e35c684806f3
Author: Koichiro Den <[email protected]>

dmaengine: dw-edma: Drain LL entries for STOP and PAUSE

An EDMA_REQ_STOP or EDMA_REQ_PAUSE can arrive while hardware still owns
published LL entries. Keep the request pending until hardware consumes
them instead of discarding the ring state immediately.

> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index eb0fc73a4f475..1cc9fa4866b9d 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c

[ ... ]

> @@ -1598,19 +1646,27 @@ static void dw_edma_wait_termination(struct dma_chan *dchan)
>  	/*
>  	 * A STOP may be deferred to a later interrupt while the channel is still
>  	 * running. Wait until that handler completes the termination.
> +	 *
> +	 * dmaengine_synchronize() cannot report an error. Returning while
> +	 * EDMA_REQ_STOP remains pending would let callers release transfer
> +	 * resources even though hardware access cannot be ruled out. Warn
> +	 * periodically, but keep waiting.
>  	 */
> -	while (time_before(jiffies, timeout)) {
> +	for (;;) {
>  		scoped_guard(spinlock_irqsave, &chan->vc.lock)
>  			stopping = chan->request == EDMA_REQ_STOP;
>  
>  		if (!stopping)
>  			return;

[Severity: High]
Could this infinite loop cause a soft lockup during device teardown?

If a client calls dmaengine_terminate_sync() concurrently with or just
before the DMA controller is unbound, dw_edma_remove() sets
chan->dw->teardown = true.

When teardown is set, background workers and IRQ handlers exit early without
clearing the channel status:

drivers/dma/dw-edma/dw-edma-core.c:dw_edma_queue_irq_work() {
        if (unlikely(READ_ONCE(chan->dw->teardown)))
                return;
        ...
}

Since these handlers are the only paths that clear EDMA_REQ_STOP,
the stop request is never cleared and this loop might never exit.

> +
> +		if (time_after_eq(jiffies, timeout)) {
> +			dev_warn(chan->dw->chip->dev,
> +				 "timeout waiting for channel termination; still waiting\n");
> +			timeout = jiffies + msecs_to_jiffies(5000);
> +		}
> +
>  		fsleep(1000);
>  	}
> -
> -	dev_warn(chan->dw->chip->dev,
> -		 "timeout waiting for channel termination\n");
>  }
>  
>  static void dw_edma_device_synchronize(struct dma_chan *dchan)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=19