Re: [PATCH v8 6/7] dmaengine: dw-edma: Dynamically append requests while running

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] dmaengine: dw-edma: stranded or aborted queued descriptors on
  LL reset
--

commit a60bb88e6010e30a59b85799859be8d9e37e8b49
Author: Frank Li <Frank.Li@nxp.com>

dmaengine: dw-edma: Dynamically append requests while running

This commit allows dw_edma_device_issue_pending() to append new linked-list
entries while a channel is running and aborts in-flight descriptors when
resetting the ring.

> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 1d13a25663eff..bcf4fca345914 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -111,8 +111,22 @@ static void dw_edma_complete_vdesc(struct virt_dma_desc *vd,
>  
>  static void dw_edma_core_reset_ll(struct dw_edma_chan *chan)
>  {
> +	struct virt_dma_desc *vd, *tmp;
>  	u32 i;
>  
> +	/*
> +	 * We cannot tell which pending entries completed before the reset.
> +	 * Abort their descriptors to avoid repeating a transfer.
> +	 */
> +	list_for_each_entry_safe(vd, tmp, &chan->vc.desc_issued, node) {
> +		struct dw_edma_desc *desc = vd2dw_edma_desc(vd);
> +
> +		if (desc->start_burst == desc->done_burst)
> +			continue;
> +
> +		dw_edma_complete_vdesc(vd, DMA_TRANS_ABORTED);
> +	}

[Severity: Medium]
Does this leave unstarted descriptors in chan->vc.desc_issued stranded on an
idle channel after a hardware abort?

When issued descriptors exceed the free LL ring capacity, dw_edma_start_transfer()
stops once dw_edma_core_get_free_num(chan) reaches zero, leaving trailing
descriptors in chan->vc.desc_issued with start_burst == 0 and done_burst == 0.
When dw_edma_abort_interrupt() runs, dw_edma_core_reset_ll() skips those
unstarted descriptors, and dw_edma_abort_interrupt() then sets chan->status to
EDMA_ST_IDLE without starting them:

drivers/dma/dw-edma/dw-edma-core.c:dw_edma_abort_interrupt() {
    ...
	if (!chan->non_ll)
		dw_edma_core_reset_ll(chan);
	dw_edma_set_request(chan, EDMA_REQ_NONE);
	chan->status = EDMA_ST_IDLE;
	spin_unlock_irqrestore(&chan->vc.lock, flags);
}

Because the client already called dma_async_issue_pending() for those
descriptors, they never execute or complete unless another transfer is issued
later.

Conversely, if both DW_EDMA_DEFERRED_DONE and DW_EDMA_DEFERRED_ABORT are
pending in dw_edma_irq_work(), dw_edma_ll_interrupt() runs first and calls
dw_edma_ll_advance() -> dw_edma_start_transfer(), which publishes those
waiting descriptors into the freed LL slots (setting start_burst > 0) right
before dw_edma_abort_interrupt() -> dw_edma_core_reset_ll() aborts them with
DMA_TRANS_ABORTED.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261107133027.364237-1-den@valinux.co.jp?part=6