[PATCH] dmaengine: dw-axi-dmac: report paused state and residue in tx_status

[PATCH] dmaengine: dw-axi-dmac: report paused state and residue in tx_status

[Rui Wang]

The driver implements device_pause/device_resume, but device_tx_status
keeps reporting DMA_IN_PROGRESS for a paused channel, so clients cannot
tell a paused channel apart from a running one. Report DMA_PAUSED when
the channel is paused and the cookie is still in flight, and clear the
stale is_paused flag in dma_chan_terminate_all(), which disables the
channel and thus implicitly cancels the paused state.

Also, the residue of an in-flight transfer is currently derived from
the number of completed LLI blocks, so it only advances in block-size
steps and stays stale for the duration of a large block. Read the
current hardware pointer (CH_SAR for MEM_TO_DEV and MEM_TO_MEM, CH_DAR
for DEV_TO_MEM) and walk the descriptor's LLIs to compute how many
bytes have actually been transferred. Queued descriptors keep
reporting their full length and cyclic descriptors keep the
block-granular accounting.

Tested on an FPGA platform.

Signed-off-by: Rui Wang <wr574332525@163.com>
---
 .../dma/dw-axi-dmac/dw-axi-dmac-platform.c    | 60 ++++++++++++++++---
 1 file changed, 53 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
index eebed2474..4e1b8654a 100644
--- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
+++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
@@ -352,19 +352,51 @@ static void vchan_desc_put(struct virt_dma_desc *vdesc)
 	axi_desc_put(vd_to_axi_desc(vdesc));
 }
 
+/*
+ * Return the number of bytes already transferred by the in-flight
+ * non-cyclic descriptor, based on the current hardware read or write
+ * position: CH_SAR for MEM_TO_DEV and MEM_TO_MEM, CH_DAR for
+ * DEV_TO_MEM. Must be called with vc.lock held and the channel
+ * running or paused.
+ */
+static u32 axi_chan_get_xferred(struct axi_dma_chan *chan,
+				struct axi_dma_desc *desc)
+{
+	struct axi_dma_hw_desc *hw_desc;
+	bool dst = chan->direction == DMA_DEV_TO_MEM;
+	u64 pos, start;
+	u32 xferred = 0;
+	int i;
+
+	pos = lo_hi_readq(chan->chan_regs + (dst ? CH_DAR : CH_SAR));
+
+	for (i = 0; i < desc->nr_hw_descs; i++) {
+		hw_desc = &desc->hw_desc[i];
+		start = le64_to_cpu(dst ? hw_desc->lli->dar : hw_desc->lli->sar);
+
+		/* Current position is inside this block: partial progress */
+		if (pos >= start && pos <= start + hw_desc->len)
+			return xferred + (u32)(pos - start);
+
+		xferred += hw_desc->len;
+	}
+
+	/* Position doesn't match any block, be conservative */
+	return 0;
+}
+
 static enum dma_status
 dma_chan_tx_status(struct dma_chan *dchan, dma_cookie_t cookie,
 		   struct dma_tx_state *txstate)
 {
 	struct axi_dma_chan *chan = dchan_to_axi_dma_chan(dchan);
 	struct virt_dma_desc *vdesc;
+	struct axi_dma_desc *desc;
 	enum dma_status status;
 	u32 completed_length;
 	unsigned long flags;
-	u32 completed_blocks;
 	size_t bytes = 0;
 	u32 length;
-	u32 len;
 
 	status = dma_cookie_status(dchan, cookie, txstate);
 	if (status == DMA_COMPLETE || !txstate)
@@ -374,13 +406,26 @@ dma_chan_tx_status(struct dma_chan *dchan, dma_cookie_t cookie,
 
 	vdesc = vchan_find_desc(&chan->vc, cookie);
 	if (vdesc) {
-		length = vd_to_axi_desc(vdesc)->length;
-		completed_blocks = vd_to_axi_desc(vdesc)->completed_blocks;
-		len = vd_to_axi_desc(vdesc)->hw_desc[0].len;
-		completed_length = completed_blocks * len;
-		bytes = length - completed_length;
+		desc = vd_to_axi_desc(vdesc);
+		length = desc->length;
+
+		if (chan->cyclic)
+			completed_length = desc->completed_blocks *
+					   desc->hw_desc[0].len;
+		else if (vdesc == vchan_next_desc(&chan->vc) &&
+			 (axi_chan_is_hw_enable(chan) || chan->is_paused))
+			/* In-flight descriptor: read progress from hardware */
+			completed_length = axi_chan_get_xferred(chan, desc);
+		else
+			/* Still queued, nothing transferred yet */
+			completed_length = 0;
+
+		bytes = length - min_t(u32, completed_length, length);
 	}
 
+	if (chan->is_paused && status == DMA_IN_PROGRESS)
+		status = DMA_PAUSED;
+
 	spin_unlock_irqrestore(&chan->vc.lock, flags);
 	dma_set_residue(txstate, bytes);
 
@@ -1206,6 +1251,7 @@ static int dma_chan_terminate_all(struct dma_chan *dchan)
 	vchan_get_all_descriptors(&chan->vc, &head);
 
 	chan->cyclic = false;
+	chan->is_paused = false;
 	spin_unlock_irqrestore(&chan->vc.lock, flags);
 
 	vchan_dma_desc_free_list(&chan->vc, &head);
-- 
2.43.0

Re: [PATCH] dmaengine: dw-axi-dmac: report paused state and residue in tx_status

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] dmaengine: dw-axi-dmac: torn read of active 64-bit counters using lo_hi_readq()
- [Medium] dmaengine: dw-axi-dmac: completed_length zeroed before IRQ handling
--

commit 00e157d9ceffe7d7b5238031118199d3b1fa0eca
Author: Rui Wang <wr574332525@163.com>

dmaengine: dw-axi-dmac: report paused state and residue in tx_status

The driver implements device_pause/device_resume, but device_tx_status
keeps reporting DMA_IN_PROGRESS for a paused channel, so clients cannot
tell a paused channel apart from a running one. Report DMA_PAUSED when
the channel is paused and the cookie is still in flight, and clear the
stale is_paused flag in dma_chan_terminate_all(), which disables the
channel and thus implicitly cancels the paused state.

Also, the residue of an in-flight transfer is currently derived from
the number of completed LLI blocks. Read the current hardware pointer
and walk the descriptor's LLIs to compute how many bytes have actually
been transferred.

> diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> index eebed2474210d..4e1b8654a332e 100644
> --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
[ ... ]
> @@ -352,19 +352,51 @@ static void vchan_desc_put(struct virt_dma_desc *vdesc)
>  	axi_desc_put(vd_to_axi_desc(vdesc));
>  }
>  
> +/*
> + * Return the number of bytes already transferred by the in-flight
> + * non-cyclic descriptor, based on the current hardware read or write
> + * position: CH_SAR for MEM_TO_DEV and MEM_TO_MEM, CH_DAR for
> + * DEV_TO_MEM. Must be called with vc.lock held and the channel
> + * running or paused.
> + */
> +static u32 axi_chan_get_xferred(struct axi_dma_chan *chan,
> +				struct axi_dma_desc *desc)
> +{
> +	struct axi_dma_hw_desc *hw_desc;
> +	bool dst = chan->direction == DMA_DEV_TO_MEM;
> +	u64 pos, start;
> +	u32 xferred = 0;
> +	int i;
> +
> +	pos = lo_hi_readq(chan->chan_regs + (dst ? CH_DAR : CH_SAR));

[Severity: Medium]
Does this reading of an active 64-bit hardware counter lead to word tearing?

lo_hi_readq() unconditionally performs two separate 32-bit readl() calls on
all architectures. If the DMA transfer crosses a 32-bit boundary (e.g.,
crossing a 4GB address boundary) between the two reads, the combined 64-bit
value will tear, creating a massively corrupted address.

This corrupted address can fail the descriptor bounds check (pos >= start &&
pos <= start + hw_desc->len) later in this function, causing
axi_chan_get_xferred() to conservatively return 0.

This would result in a spurious backward jump in the reported residue back
to the full transfer size, breaking monotonic progress expectations for any
polling client.

[ ... ]
> @@ -374,13 +406,26 @@ dma_chan_tx_status(struct dma_chan *dchan, dma_cookie_t cookie,
>  
>  	vdesc = vchan_find_desc(&chan->vc, cookie);
>  	if (vdesc) {
> -		length = vd_to_axi_desc(vdesc)->length;
> -		completed_blocks = vd_to_axi_desc(vdesc)->completed_blocks;
> -		len = vd_to_axi_desc(vdesc)->hw_desc[0].len;
> -		completed_length = completed_blocks * len;
> -		bytes = length - completed_length;
> +		desc = vd_to_axi_desc(vdesc);
> +		length = desc->length;
> +
> +		if (chan->cyclic)
> +			completed_length = desc->completed_blocks *
> +					   desc->hw_desc[0].len;
> +		else if (vdesc == vchan_next_desc(&chan->vc) &&
> +			 (axi_chan_is_hw_enable(chan) || chan->is_paused))
> +			/* In-flight descriptor: read progress from hardware */
> +			completed_length = axi_chan_get_xferred(chan, desc);
> +		else
> +			/* Still queued, nothing transferred yet */
> +			completed_length = 0;

[Severity: Medium]
Could this condition incorrectly zero out the completed length before IRQ
handling finishes, causing the residue to spike?

For non-cyclic transfers, the AXI DMA hardware automatically clears the
channel enable bit (DMAC_CHEN) when a transfer finishes.

If a client polls dma_chan_tx_status() during the small window after
hardware completion but before the interrupt handler acquires vc.lock to
reap the descriptor, vdesc == vchan_next_desc(&chan->vc) remains true.

However, axi_chan_is_hw_enable(chan) evaluates to false because the hardware
disabled the channel. The logic incorrectly falls through to the else branch,
setting completed_length = 0.

This causes the reported residue to instantaneously jump from nearly 0 back
to the full transfer length, breaking monotonic progress expectations.

> +
> +		bytes = length - min_t(u32, completed_length, length);
>  	}
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923030201.859-1-wr574332525@163.com?part=1