Re: [PATCH v3 1/4] dmaengine: dw: Fix XFER bit set, but channel not idle error

[Frank Li]

On Fri, Oct 23, 2026 at 10:27:22AM -0600, James Hilliard wrote:
> From: Serge Semin <fancer.lancer@xxxxxxxxx>
>
> If a client driver gets to use the DW DMAC engine device tougher
> than usual, with occasional DMA-transfers termination and restart, then
> the next error can be randomly spotted in the system log:
>
> > dma dma0chan0: BUG: XFER bit set, but channel not idle!
>
> For instance that happens in case of the 8250 UART port driver handling
> the looped back high-speed traffic (in my case > 1.5Mbaud) by means of the
> DMA-engine interface.
>
> The error happens due to the two-staged nature of the DW DMAC IRQs
> handling procedure and due to the critical section break in the meantime.
> In particular in case if the DMA-transfer is terminated and restarted:
> 1. after the IRQ-handler submitted the tasklet but before the tasklet
>    started handling the DMA-descriptors in dwc_scan_descriptors();
> 2. after the XFER completion flag was detected in the
>    dwc_scan_descriptors() method, but before the dwc_complete_all() method
>    is called
> the error denoted above is printed due to the overlap of the last transfer
> completion and the new transfer execution stages.
>
> Clear the channel interrupt status when stopping it, and keep the XFER
> check, descriptor detachment and queued-transfer restart in one locked
> section. An empty active list needs no completion processing.
>
> When the locked XFER recheck finds no event, only update residue. A stale
> tasklet snapshot may belong to a terminated transfer, and a zero residue
> only means the source has been read, not that destination writes have
> finished. Requiring RAW.XFER also prevents the fallback scan from freeing
> a descriptor still linked on active_list.
>
> Fixes: 69cea5a00d31 ("dmaengine/dw_dmac: Replace spin_lock* with irqsave variants and enable submission from callback")
> Fixes: 3bfb1d20b547 ("dmaengine: Driver for the Synopsys DesignWare DMA controller")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Serge Semin <fancer.lancer@xxxxxxxxx>
> [james: require a locked XFER event and handle an empty active list]
> Signed-off-by: James Hilliard <james.hilliard1@xxxxxxxxx>
> ---
>  drivers/dma/dw/core.c | 70 ++++++++++++++++++++-------------------------------
>  1 file changed, 27 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
> index dd75f97a33b3..07a5f5105787 100644
> --- a/drivers/dma/dw/core.c
> +++ b/drivers/dma/dw/core.c
> @@ -141,6 +141,12 @@ static inline void dwc_chan_disable(struct dw_dma *dw, struct dw_dma_chan *dwc)
>  	channel_clear_bit(dw, CH_EN, dwc->mask);
>  	while (dma_readl(dw, CH_EN) & dwc->mask)
>  		cpu_relax();
> +
> +	dma_writel(dw, CLEAR.XFER, dwc->mask);
> +	dma_writel(dw, CLEAR.BLOCK, dwc->mask);
> +	dma_writel(dw, CLEAR.SRC_TRAN, dwc->mask);
> +	dma_writel(dw, CLEAR.DST_TRAN, dwc->mask);
> +	dma_writel(dw, CLEAR.ERROR, dwc->mask);
>  }
>
>  /*----------------------------------------------------------------------*/
> @@ -257,34 +263,6 @@ dwc_descriptor_complete(struct dw_dma_chan *dwc, struct dw_desc *desc,
>  	dmaengine_desc_callback_invoke(&cb, NULL);
>  }
>
> -static void dwc_complete_all(struct dw_dma *dw, struct dw_dma_chan *dwc)
> -{
> -	struct dw_desc *desc, *_desc;
> -	LIST_HEAD(list);
> -	unsigned long flags;
> -
> -	spin_lock_irqsave(&dwc->lock, flags);
> -	if (dma_readl(dw, CH_EN) & dwc->mask) {
> -		dev_err(chan2dev(&dwc->chan),
> -			"BUG: XFER bit set, but channel not idle!\n");
> -
> -		/* Try to continue after resetting the channel... */
> -		dwc_chan_disable(dw, dwc);
> -	}
> -
> -	/*
> -	 * Submit queued descriptors ASAP, i.e. before we go through
> -	 * the completed ones.
> -	 */
> -	list_splice_init(&dwc->active_list, &list);
> -	dwc_dostart_first_queued(dwc);
> -
> -	spin_unlock_irqrestore(&dwc->lock, flags);
> -
> -	list_for_each_entry_safe(desc, _desc, &list, desc_node)
> -		dwc_descriptor_complete(dwc, desc, true);
> -}
> -
>  /* Returns how many bytes were already received from source */
>  static inline u32 dwc_get_sent(struct dw_dma_chan *dwc)
>  {
> @@ -302,6 +280,7 @@ static void dwc_scan_descriptors(struct dw_dma *dw, struct dw_dma_chan *dwc)
>  	struct dw_desc *child;
>  	u32 status_xfer;
>  	unsigned long flags;
> +	LIST_HEAD(list);
>
>  	spin_lock_irqsave(&dwc->lock, flags);
>  	llp = channel_readl(dwc, LLP);
> @@ -310,6 +289,10 @@ static void dwc_scan_descriptors(struct dw_dma *dw, struct dw_dma_chan *dwc)
>  	if (status_xfer & dwc->mask) {
>  		/* Everything we've submitted is done */
>  		dma_writel(dw, CLEAR.XFER, dwc->mask);
> +		if (list_empty(&dwc->active_list)) {
> +			spin_unlock_irqrestore(&dwc->lock, flags);
> +			return;
> +		}
>
>  		if (test_bit(DW_DMA_IS_SOFT_LLP, &dwc->flags)) {
>  			struct list_head *head, *active = dwc->tx_node_active;
> @@ -341,9 +324,23 @@ static void dwc_scan_descriptors(struct dw_dma *dw, struct dw_dma_chan *dwc)
>  			clear_bit(DW_DMA_IS_SOFT_LLP, &dwc->flags);
>  		}
>
> +		if (dma_readl(dw, CH_EN) & dwc->mask) {
> +			dev_err(chan2dev(&dwc->chan),
> +				"BUG: XFER bit set, but channel not idle!\n");
> +
> +			/* Try to continue after resetting the channel... */
> +			dwc_chan_disable(dw, dwc);
> +		}
> +
> +		/* Detach the completed transfer before starting queued work. */
> +		list_splice_init(&dwc->active_list, &list);
> +		dwc_dostart_first_queued(dwc);
>  		spin_unlock_irqrestore(&dwc->lock, flags);
>
> -		dwc_complete_all(dw, dwc);
> +		list_for_each_entry_safe(desc, _desc, &list, desc_node) {
> +			list_del_init(&desc->desc_node);
> +			dwc_descriptor_complete(dwc, desc, true);
> +		}

what's purpose move dwc_descriptor_complete() implement into here?

Frank

>  		return;
>  	}
>
> @@ -389,22 +386,9 @@ static void dwc_scan_descriptors(struct dw_dma *dw, struct dw_dma_chan *dwc)
>  			desc->residue -= child->len;
>  		}
>
> -		/*
> -		 * No descriptors so far seem to be in progress, i.e.
> -		 * this one must be done.
> -		 */
> -		spin_unlock_irqrestore(&dwc->lock, flags);
> -		dwc_descriptor_complete(dwc, desc, true);
> -		spin_lock_irqsave(&dwc->lock, flags);
>  	}
>
> -	dev_err(chan2dev(&dwc->chan),
> -		"BUG: All descriptors done, but channel not idle!\n");
> -
> -	/* Try to continue after resetting the channel... */
> -	dwc_chan_disable(dw, dwc);
> -
> -	dwc_dostart_first_queued(dwc);
> +	/* Residue describes source reads; only RAW.XFER completes a transfer. */
>  	spin_unlock_irqrestore(&dwc->lock, flags);
>  }
>
>
> --
> 2.53.0
>