Re: [PATCH v5 15/24] dmaengine: dw-edma: Recheck stopped LL channels before restart

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Silent hardware stop during channel termination causes a 5-second timeout and descriptor leak.
--

--- Patch [15]: [PATCH v5 15/24] dmaengine: dw-edma: Recheck stopped LL channels before restart ---
commit aec4dbaf9567f7c6e772cd5d67fe89362acafbbf
Author: Koichiro Den <[email protected]>

dmaengine: dw-edma: Recheck stopped LL channels before restart

This commit introduces a mechanism to recheck stopped LL channels before
restarting them to handle races between DMA_LLP samples and updates. It also
gates eDMA kicks with a one-shot credit and serializes the stopped-state
check against IRQ capture.

> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 13c45549e08be..8325e296cb2a4 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
[ ... ]
>  /* Must be called with vc.lock held. */
>  static void
>  dw_edma_set_request(struct dw_edma_chan *chan, enum dw_edma_request request)
>  {
> +	if (!chan->non_ll && chan->request != request)
> +		dw_edma_ll_recheck_cancel(chan);
>  	chan->request = request;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but does cancelling the recheck
work when setting EDMA_REQ_STOP remove the mechanism that detects a silent
hardware stop?

If the client calls dmaengine_terminate_all(), it sets
chan->request = EDMA_REQ_STOP and cancels the recheck work here. If the
hardware stops silently without an interrupt, dw_edma_wait_termination()
will spin for 5 seconds waiting for the interrupt to clear EDMA_REQ_STOP,
leading to a timeout and leaking the pending descriptors.

[ ... ]
> -/* Must be called with vc.lock held. */
> -static void dw_edma_core_ch_maybe_doorbell(struct dw_edma_chan *chan)
> +/*
> + * Must be called with vc.lock held. Return true when published work is still
> + * running and may need one later stop recheck.
> + */
> +static bool dw_edma_core_ch_maybe_doorbell(struct dw_edma_chan *chan)
>  {
>  	if (chan->non_ll || chan->request != EDMA_REQ_NONE ||
> -	    chan->status != EDMA_ST_BUSY || !dw_edma_ll_pending(chan))
> -		return;
> +	    chan->status != EDMA_ST_BUSY || !dw_edma_ll_pending(chan) ||
> +	    dw_edma_abort_is_pending(chan))
> +		return false;

[Severity: High]
This is a pre-existing issue, but because this early return checks
chan->request != EDMA_REQ_NONE, doesn't this explicitly prevent
dw_edma_ll_reconcile_and_refill() from running when chan->request ==
EDMA_REQ_STOP?

Combined with the cancelled recheck work above, this means there is no
mechanism to clear EDMA_REQ_STOP if the hardware stops silently during
termination.

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