[PATCH 0/5] dmaengine: xilinx_dma: Fix AXI DMA descriptor ring ordering and failed resets

[PATCH 0/5] dmaengine: xilinx_dma: Fix AXI DMA descriptor ring ordering and failed resets

[Jeff C. Jensen via B4 Relay]

This series fixes descriptor handling problems in the AXI DMA driver
that I ran into while streaming from programmable logic into a single
S2MM channel on a Zynq-7020.

Patches 1-4 are fixes. Patch 5 is an RFC and is not meant to be applied
with them; patches 1-4 do not depend on it.

Patches 1 and 2 are small fixes to the preparation paths. A transfer
whose entries are all zero length built a descriptor with no segments,
and the cyclic period count could be truncated on 64-bit builds.

Patch 3 is the main change. The driver hands out segments from a free
list, while the engine follows the descriptor ring in pool order, so the
two must stay in step. Commit cee9c863ee68 fixed one way they drift
apart. The Sashiko review of that commit [1] pointed out three more, and
patch 3 removes them all by allocating consecutive ring positions
instead of free list entries.

Patch 4 handles a reset that does not complete. Today the terminate path
frees the descriptors anyway, although the engine may still be using
them. With this patch the engine is withheld instead: its channels keep
their descriptors and take no new work.

Patch 5 lets a withheld channel recover: the next terminate resets the
engine, and synchronize reclaims the descriptors once the completion
tasklet has been killed. My question for this RFC is whether a completed
engine reset, followed by synchronize, is an acceptable point to reclaim
the descriptors of a withheld channel, given that the driver instance
manages no other channel of that engine.

The series is based on v7.3-rc4, the dmaengine fixes branch at
93f51579e7df, and builds on cee9c863ee68 and 7ed1e3070c9b, both of which
are in v7.3-rc4.

Testing
=======

Every patch builds for arm (multi_v7_defconfig) and arm64 (defconfig),
with the driver built in and as a module, with W=1 and sparse, and
passes checkpatch --strict. I tested the series on hardware as a
backport to the AMD vendor kernel, described below; the patches as
posted, against v7.3-rc4, are build-tested only.

The backport ran on the AMD 6.12.40 vendor kernel together with
aa99c4d1d63b, cee9c863ee68 and 7ed1e3070c9b, on a Zynq-7020 with one AXI
DMA v7.1 S2MM channel: scatter-gather, 32-bit, no DRE and the
512-segment pool. The vendor kernel has no dma_vec path and lacks
607869003479, so the dma_vec path and the transfer chaining that commit
adds are build-tested only with these changes.

 - A test client streamed 640128-byte packets at 80 per second from a
   pattern generator, refilled the channel from the completion callback
   and from a thread on the other CPU, and checked every sample. Two 60
   s runs passed, each running out of segments about 15000 times. Cases
   for pool capacity, a zero-length scatterlist, the buffer-length
   boundary and cyclic followed by slave-sg also passed.

 - Our production client streamed nine sessions, more than 160000
   packets, with every sample checked on the receiving host.

 - Failed resets were injected with fail_function on
   xilinx_dma_chan_reset(), through an ALLOW_ERROR_INJECTION() added for
   the test only. Withholding after one and two failed resets, an
   asynchronous terminate, 20 withhold and recover cycles, a failed
   recovery and streaming after recovery all passed, and a withheld
   channel released by its client kept its memory and refused the next
   allocation. The halt before each skipped reset completed, so this
   tests the state handling of patches 4 and 5 with a stopped engine,
   not an engine that cannot be stopped.

To reproduce what patch 3 fixes: without the series, the test client ran
for 10 minutes without an error, both on the vendor kernel with the
three backports and on the vendor kernel with aa99c4d1d63b alone. The
windows are small, so I added a test-only 500 us busy-wait between the
completion callback and the free in xilinx_dma_chan_desc_cleanup(). The
test client terminates the channel once at the start of each run while
packets keep arriving. On the vendor kernel with the backports, all
three 20 s runs failed, one with a DMA internal error and two with the
channel stalled. With patches 1-3 added, the same three runs passed, and
so did one with a 2000 us busy-wait. I could not reproduce the
partial-preparation case: the test client serializes its preparations
against its own callback, which leaves that window open only between the
callback and the free.

Not tested: MM2S, two-channel controllers, CDMA, VDMA and MCDMA, and an
engine that really cannot be stopped.

Known limitations
=================

 - The AXI MCDMA pool keeps its free list, so the terminate and
   partial-preparation problems remain there. My design does not use AXI
   MCDMA, and I did not test it. Extending the reservation to MCDMA
   would be possible future work.

 - xilinx_vdma_channel_set_config() with .reset set reports a failed
   reset without withholding the engine. A terminate that follows resets
   again, and withholds the engine if that reset fails too.

 - The other two issues in the Sashiko review, the chan->terminating
   race with submit and dma_run_dependencies() under chan->lock, are not
   addressed here.

This series was developed with the help of Claude Code, an LLM coding
assistant; I reviewed all of the code and text.

[1] https://lore.kernel.org/all/20260817094004.A74621F000E9@smtp.kernel.org/

---
Jeff C. Jensen (5):
      dmaengine: xilinx_dma: Reject transfers that map no data
      dmaengine: xilinx_dma: Fix cyclic period count truncation
      dmaengine: xilinx_dma: Allocate AXI DMA segments in ring order
      dmaengine: xilinx_dma: Withhold an AXI DMA engine whose reset did not complete
      dmaengine: xilinx_dma: Recover a withheld AXI DMA channel

 drivers/dma/xilinx/xilinx_dma.c | 533 +++++++++++++++++++++++++++++++++++-----
 1 file changed, 466 insertions(+), 67 deletions(-)
---
base-commit: 93f51579e7df248780214094418f205253383cc5
change-id: 20261022-xilinx-dma-work-4f2fad787d15

Best regards,
-- 
Jeff C. Jensen <kernel@nullptr.com>

[PATCH 1/5] dmaengine: xilinx_dma: Reject transfers that map no data

[Jeff C. Jensen via B4 Relay]

From: "Jeff C. Jensen" <kernel@nullptr.com>

If every entry of a scatterlist or a DMA vector array has zero length,
the preparation loops in xilinx_dma_prep_slave_sg(),
xilinx_dma_prep_peripheral_dma_vec() and xilinx_mcdma_prep_slave_sg()
allocate no segments. Each function then takes list_first_entry() of the
empty segment list anyway, so desc->async_tx.phys, which is later
programmed as the current descriptor, is derived from the list head
rather than from a segment. For DMA_MEM_TO_DEV, the SOP and EOP bits are
also set through pointers that do not point at a segment.

Fail the preparation when no segment was allocated.

Fixes: c0bba3a99f07 ("dmaengine: vdma: Add Support for Xilinx AXI Direct Memory Access Engine")
Fixes: 6ccd692bfb7f ("dmaengine: xilinx_dma: Add Xilinx AXI MCDMA Engine driver support")
Fixes: 38433a6fdfb7 ("dmaengine: xilinx_dma: Support descriptor setup from dma_vecs")
Assisted-by: Claude:claude-fable-5-1
Signed-off-by: Jeff C. Jensen <kernel@nullptr.com>
---
 drivers/dma/xilinx/xilinx_dma.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index cffe7c6fa640..5ccc9f9d58a3 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -2305,6 +2305,9 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_peripheral_dma_vec(
 		}
 	}
 
+	if (list_empty(&desc->segments))
+		goto error;
+
 	head = list_first_entry(&desc->segments, struct xilinx_axidma_tx_segment, node);
 	desc->async_tx.phys = head->phys;
 
@@ -2406,6 +2409,9 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_slave_sg(
 		}
 	}
 
+	if (list_empty(&desc->segments))
+		goto error;
+
 	segment = list_first_entry(&desc->segments,
 				   struct xilinx_axidma_tx_segment, node);
 	desc->async_tx.phys = segment->phys;
@@ -2613,6 +2619,9 @@ xilinx_mcdma_prep_slave_sg(struct dma_chan *dchan, struct scatterlist *sgl,
 		}
 	}
 
+	if (list_empty(&desc->segments))
+		goto error;
+
 	segment = list_first_entry(&desc->segments,
 				   struct xilinx_aximcdma_tx_segment, node);
 	desc->async_tx.phys = segment->phys;

-- 
2.43.0

[PATCH 2/5] dmaengine: xilinx_dma: Fix cyclic period count truncation

[Jeff C. Jensen via B4 Relay]

From: "Jeff C. Jensen" <kernel@nullptr.com>

xilinx_dma_prep_dma_cyclic() stores buf_len / period_len, a size_t
quotient, in an unsigned int. On 64-bit builds the count can be
truncated: a buffer of 2^32 + 1 bytes split into 1-byte periods becomes
a single period, and the transfer covers one byte of the buffer the
client asked for.

Compute the quotient as a size_t and reject it if it exceeds
XILINX_DMA_NUM_DESCS before narrowing it. Each period needs at least one
hardware descriptor, so a larger count could never be prepared from the
pool anyway.

Fixes: 92d794dfb699 ("dmaengine: vdma: Add support for cyclic dma mode")
Assisted-by: Claude:claude-fable-5-1
Signed-off-by: Jeff C. Jensen <kernel@nullptr.com>
---
 drivers/dma/xilinx/xilinx_dma.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index 5ccc9f9d58a3..aa714effd10d 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -2454,7 +2454,7 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_dma_cyclic(
 	struct xilinx_dma_chan *chan = to_xilinx_chan(dchan);
 	struct xilinx_dma_tx_descriptor *desc;
 	struct xilinx_axidma_tx_segment *segment, *head_segment, *prev = NULL;
-	size_t copy, sg_used;
+	size_t copy, sg_used, periods;
 	unsigned int num_periods;
 	int i;
 	u32 reg;
@@ -2462,10 +2462,11 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_dma_cyclic(
 	if (!period_len)
 		return NULL;
 
-	num_periods = buf_len / period_len;
-
-	if (!num_periods)
+	/* a period takes at least one descriptor */
+	periods = buf_len / period_len;
+	if (!periods || periods > XILINX_DMA_NUM_DESCS)
 		return NULL;
+	num_periods = periods;
 
 	if (!is_slave_direction(direction))
 		return NULL;

-- 
2.43.0

[PATCH 3/5] dmaengine: xilinx_dma: Allocate AXI DMA segments in ring order

[Jeff C. Jensen via B4 Relay]

From: "Jeff C. Jensen" <kernel@nullptr.com>

The AXI DMA driver builds a static ring of hardware descriptors
(segments) when the channel is allocated, each linked to the next in
pool order, and the engine follows those links.
xilinx_dma_prep_slave_sg() writes no next pointers within a descriptor,
and a descriptor appended while the engine is running is reached through
the next pointer of the previous tail. Segments are handed out from
free_seg_list, so this only works while the free list stays in pool
order.

Commit cee9c863ee68 ("dmaengine: xilinx_dma: Fix hardware buffer
descriptor reuse order") fixed one way the free list falls out of that
order. As the Sashiko review of that patch pointed out [1], others
remain:

 - xilinx_dma_chan_desc_cleanup() drops chan->lock while it runs a
   completion callback, and frees the descriptor afterwards. If the
   channel is terminated in that window, the newer segments are returned
   first and the oldest descriptor's segments end up at the tail of the
   free list.

 - A preparation that runs out of segments returns its partial
   allocation to the tail of the free list, possibly after a completion
   on another CPU has freed newer ones. It does so without taking
   chan->lock, so it can also corrupt the list.

 - A submission rejected because a cyclic transfer is running frees its
   segments while older ones are still in use.

Once the free list and the ring disagree, a later descriptor gets
segments that are not adjacent in the ring, and the engine follows a
next pointer into a cleaned segment with a zero length field. With a
delay added to widen the first window, a receive stream fails right
after a terminate:

  xilinx-vdma 40400000.dma: Channel 7a90d35b has errors 10, cdr 3f053780 tdr 3f059a00

Rather than keeping the free list in order on every path, allocate by
ring position. Count the segments a transaction needs, then reserve that
many consecutive positions, starting at a cursor, under chan->lock: all
of them or none. A bitmap tracks the positions in use. Allocation no
longer depends on the order in which segments are freed, so neither the
terminate path nor the completion path can disturb it. The count uses
xilinx_dma_calc_copysize(), like the preparation loops, so the two
always agree.

The slave-sg, cyclic and dma_vec paths all count and reserve before
building a descriptor, so a failed preparation leaves the channel
unchanged. A submission rejected on a busy cyclic channel cancels its
reservation and rewinds the cursor under the same lock.

This relies on commit 7ed1e3070c9b ("dmaengine: xilinx_dma: Fix hardware
buffer descriptor chain after cyclic DMA") to restore the segment links
after a cyclic transfer. The AXI MCDMA pool keeps its free list and is
not changed.

Link: https://lore.kernel.org/all/20260817094004.A74621F000E9@smtp.kernel.org/ [1]
Fixes: 23059408b6a3 ("dmaengine: xilinx_dma: Fix race condition in the driver for multiple descriptor scenario")
Fixes: c0bba3a99f07 ("dmaengine: vdma: Add Support for Xilinx AXI Direct Memory Access Engine")
Fixes: 92d794dfb699 ("dmaengine: vdma: Add support for cyclic dma mode")
Assisted-by: Claude:claude-fable-5-1
Signed-off-by: Jeff C. Jensen <kernel@nullptr.com>
---
 drivers/dma/xilinx/xilinx_dma.c | 279 ++++++++++++++++++++++++++++++++--------
 1 file changed, 224 insertions(+), 55 deletions(-)

diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index aa714effd10d..805cd3e2bc27 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -33,6 +33,7 @@
  *
  */
 
+#include <linux/bitmap.h>
 #include <linux/bitops.h>
 #include <linux/dmapool.h>
 #include <linux/dma/xilinx_dma.h>
@@ -390,7 +391,9 @@ struct xilinx_dma_tx_descriptor {
  * @pending_list: Descriptors waiting
  * @active_list: Descriptors ready to submit
  * @done_list: Complete descriptors
- * @free_seg_list: Free descriptors
+ * @free_seg_list: Free AXI MCDMA descriptors
+ * @seg_used: AXI DMA segments in use, indexed by ring position
+ * @seg_next: Ring position the next AXI DMA reservation starts at
  * @common: DMA common channel
  * @desc_pool: Descriptors pool
  * @dev: The dma device
@@ -430,6 +433,8 @@ struct xilinx_dma_chan {
 	struct list_head active_list;
 	struct list_head done_list;
 	struct list_head free_seg_list;
+	DECLARE_BITMAP(seg_used, XILINX_DMA_NUM_DESCS);
+	u32 seg_next;
 	struct dma_chan common;
 	struct dma_pool *desc_pool;
 	struct device *dev;
@@ -706,30 +711,53 @@ xilinx_cdma_alloc_tx_segment(struct xilinx_dma_chan *chan)
 }
 
 /**
- * xilinx_axidma_alloc_tx_segment - Allocate transaction segment
+ * xilinx_axidma_reserve_segments - Reserve a run of AXI DMA segments
  * @chan: Driver specific DMA channel
+ * @count: Segments needed, at most XILINX_DMA_NUM_DESCS
+ * @start: Ring position of the first reserved segment, set on success
  *
- * Return: The allocated segment on success and NULL on failure.
+ * Reserves @count consecutive ring positions starting at @chan->seg_next,
+ * wrapping at the pool end. The engine fetches descriptors in ring order, so
+ * the run cannot be placed anywhere else.
+ *
+ * Return: 0 on success, -ENOMEM when the run is not free. A failure leaves the
+ * ownership bitmap and @chan->seg_next unchanged.
  */
-static struct xilinx_axidma_tx_segment *
-xilinx_axidma_alloc_tx_segment(struct xilinx_dma_chan *chan)
+static int xilinx_axidma_reserve_segments(struct xilinx_dma_chan *chan,
+					  u32 count, u32 *start)
 {
-	struct xilinx_axidma_tx_segment *segment = NULL;
 	unsigned long flags;
+	int ret = -ENOMEM;
+	u32 i;
 
 	spin_lock_irqsave(&chan->lock, flags);
-	if (!list_empty(&chan->free_seg_list)) {
-		segment = list_first_entry(&chan->free_seg_list,
-					   struct xilinx_axidma_tx_segment,
-					   node);
-		list_del(&segment->node);
+
+	for (i = 0; i < count; i++) {
+		if (test_bit((chan->seg_next + i) % XILINX_DMA_NUM_DESCS,
+			     chan->seg_used))
+			goto out;
 	}
+
+	*start = chan->seg_next;
+	for (i = 0; i < count; i++)
+		__set_bit((chan->seg_next + i) % XILINX_DMA_NUM_DESCS,
+			  chan->seg_used);
+	chan->seg_next = (chan->seg_next + count) % XILINX_DMA_NUM_DESCS;
+	ret = 0;
+
+out:
 	spin_unlock_irqrestore(&chan->lock, flags);
 
-	if (!segment)
-		dev_dbg(chan->dev, "Could not find free tx segment\n");
+	if (ret)
+		dev_dbg(chan->dev, "no run of %u free tx segments\n", count);
 
-	return segment;
+	return ret;
+}
+
+static struct xilinx_axidma_tx_segment *
+xilinx_axidma_run_segment(struct xilinx_dma_chan *chan, u32 start, u32 i)
+{
+	return &chan->seg_v[(start + i) % XILINX_DMA_NUM_DESCS];
 }
 
 /**
@@ -792,13 +820,18 @@ static void xilinx_mcdma_clean_hw_desc(struct xilinx_aximcdma_desc_hw *hw)
  * xilinx_dma_free_tx_segment - Free transaction segment
  * @chan: Driver specific DMA channel
  * @segment: DMA transaction segment
+ *
+ * Called with chan->lock held, so no reservation sees the segment between its
+ * links being restored and its ownership being released.
  */
 static void xilinx_dma_free_tx_segment(struct xilinx_dma_chan *chan,
 				struct xilinx_axidma_tx_segment *segment)
 {
+	lockdep_assert_held(&chan->lock);
+
 	xilinx_dma_clean_hw_desc(chan, segment);
 
-	list_add_tail(&segment->node, &chan->free_seg_list);
+	__clear_bit(segment - chan->seg_v, chan->seg_used);
 }
 
 /**
@@ -902,6 +935,34 @@ xilinx_dma_free_tx_descriptor(struct xilinx_dma_chan *chan,
 	kfree(desc);
 }
 
+/**
+ * xilinx_dma_cancel_tx_descriptor - Discard a descriptor before submission
+ * @chan: Driver specific DMA channel
+ * @desc: Prepared descriptor
+ *
+ * Called with chan->lock held. On an AXI DMA channel, releases the reservation
+ * of @desc and rewinds @chan->seg_next to its first segment, so that no
+ * cleaned segment is left between two live descriptors. @desc must hold the
+ * most recent reservation on the channel.
+ */
+static void xilinx_dma_cancel_tx_descriptor(struct xilinx_dma_chan *chan,
+					    struct xilinx_dma_tx_descriptor *desc)
+{
+	struct xilinx_axidma_tx_segment *first;
+
+	lockdep_assert_held(&chan->lock);
+
+	if (chan->xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA) {
+		first = list_first_entry_or_null(&desc->segments,
+						 struct xilinx_axidma_tx_segment,
+						 node);
+		if (first)
+			chan->seg_next = first - chan->seg_v;
+	}
+
+	xilinx_dma_free_tx_descriptor(chan, desc);
+}
+
 /* Required functions */
 
 /**
@@ -951,10 +1012,6 @@ static void xilinx_dma_free_chan_resources(struct dma_chan *dchan)
 	xilinx_dma_free_descriptors(chan);
 
 	if (chan->xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA) {
-		spin_lock_irqsave(&chan->lock, flags);
-		INIT_LIST_HEAD(&chan->free_seg_list);
-		spin_unlock_irqrestore(&chan->lock, flags);
-
 		/* Free memory that is allocated for BD */
 		dma_free_coherent(chan->dev, sizeof(*chan->seg_v) *
 				  XILINX_DMA_NUM_DESCS, chan->seg_v,
@@ -1195,9 +1252,9 @@ static int xilinx_dma_alloc_chan_resources(struct dma_chan *dchan)
 				((i + 1) % XILINX_DMA_NUM_DESCS));
 			chan->seg_v[i].phys = chan->seg_p +
 				sizeof(*chan->seg_v) * i;
-			list_add_tail(&chan->seg_v[i].node,
-				      &chan->free_seg_list);
 		}
+		bitmap_zero(chan->seg_used, XILINX_DMA_NUM_DESCS);
+		chan->seg_next = 0;
 	} else if (chan->xdev->dma_config->dmatype == XDMA_TYPE_AXIMCDMA) {
 		/* Allocate the buffer descriptors. */
 		chan->seg_mv = dma_alloc_coherent(chan->dev,
@@ -1282,6 +1339,98 @@ static int xilinx_dma_calc_copysize(struct xilinx_dma_chan *chan,
 	return copy;
 }
 
+/**
+ * xilinx_axidma_count_split - Count the descriptors a length splits into
+ * @chan: Driver specific DMA channel
+ * @len: Length in bytes
+ *
+ * Splits @len with xilinx_dma_calc_copysize(), so the count equals the number
+ * of segments a preparation builds for @len. Intermediate chunks are rounded
+ * down for alignment, so the count depends on the channel alignment as well as
+ * on @len.
+ *
+ * Return: The descriptor count, which is zero for a zero length, or -EINVAL if
+ * a split makes no progress or @len needs more descriptors than the pool holds.
+ */
+static int xilinx_axidma_count_split(struct xilinx_dma_chan *chan, size_t len)
+{
+	size_t used = 0;
+	int count = 0;
+
+	while (used < len) {
+		size_t copy = xilinx_dma_calc_copysize(chan, len, used);
+
+		if (!copy || count == XILINX_DMA_NUM_DESCS)
+			return -EINVAL;
+
+		used += copy;
+		count++;
+	}
+
+	return count;
+}
+
+/**
+ * xilinx_axidma_count_sg - Count the descriptors a mapped scatterlist needs
+ * @chan: Driver specific DMA channel
+ * @sgl: Mapped scatterlist
+ * @sg_len: Number of mapped entries in @sgl
+ *
+ * Return: The descriptor count, or -EINVAL if the list splits into no
+ * descriptors at all or into more than the pool holds.
+ */
+static int xilinx_axidma_count_sg(struct xilinx_dma_chan *chan,
+				  struct scatterlist *sgl, unsigned int sg_len)
+{
+	struct scatterlist *sg;
+	unsigned int i;
+	int total = 0;
+
+	for_each_sg(sgl, sg, sg_len, i) {
+		int count = xilinx_axidma_count_split(chan, sg_dma_len(sg));
+
+		if (count < 0 || count > XILINX_DMA_NUM_DESCS - total)
+			return -EINVAL;
+
+		total += count;
+	}
+
+	if (!total)
+		return -EINVAL;
+
+	return total;
+}
+
+/**
+ * xilinx_axidma_count_vecs - Count the descriptors an array of DMA vectors needs
+ * @chan: Driver specific DMA channel
+ * @vecs: DMA vectors
+ * @nb: Entry count
+ *
+ * Return: The descriptor count, or -EINVAL if the vectors split into no
+ * descriptors at all or into more than the pool holds.
+ */
+static int xilinx_axidma_count_vecs(struct xilinx_dma_chan *chan,
+				    const struct dma_vec *vecs, size_t nb)
+{
+	size_t i;
+	int total = 0;
+
+	for (i = 0; i < nb; i++) {
+		int count = xilinx_axidma_count_split(chan, vecs[i].len);
+
+		if (count < 0 || count > XILINX_DMA_NUM_DESCS - total)
+			return -EINVAL;
+
+		total += count;
+	}
+
+	if (!total)
+		return -EINVAL;
+
+	return total;
+}
+
 /**
  * xilinx_dma_tx_status - Get DMA transaction status
  * @dchan: DMA channel
@@ -2050,6 +2199,10 @@ static void append_desc_queue(struct xilinx_dma_chan *chan,
  * xilinx_dma_tx_submit - Submit DMA transaction
  * @tx: Async transaction descriptor
  *
+ * An AXI DMA channel reserves segment runs in ring order, so a transaction must
+ * be submitted, or its rejection handled, before the next one is prepared on
+ * the same channel.
+ *
  * Return: cookie value on success and failure value on error
  */
 static dma_cookie_t xilinx_dma_tx_submit(struct dma_async_tx_descriptor *tx)
@@ -2060,8 +2213,11 @@ static dma_cookie_t xilinx_dma_tx_submit(struct dma_async_tx_descriptor *tx)
 	unsigned long flags;
 	int err;
 
+	spin_lock_irqsave(&chan->lock, flags);
+
 	if (chan->cyclic) {
-		xilinx_dma_free_tx_descriptor(chan, desc);
+		xilinx_dma_cancel_tx_descriptor(chan, desc);
+		spin_unlock_irqrestore(&chan->lock, flags);
 		return -EBUSY;
 	}
 
@@ -2070,13 +2226,13 @@ static dma_cookie_t xilinx_dma_tx_submit(struct dma_async_tx_descriptor *tx)
 		 * If reset fails, need to hard reset the system.
 		 * Channel is no longer functional
 		 */
+		spin_unlock_irqrestore(&chan->lock, flags);
 		err = xilinx_dma_chan_reset(chan);
 		if (err < 0)
 			return err;
+		spin_lock_irqsave(&chan->lock, flags);
 	}
 
-	spin_lock_irqsave(&chan->lock, flags);
-
 	cookie = dma_cookie_assign(tx);
 
 	/* Put this transaction onto the tail of the pending queue */
@@ -2254,15 +2410,26 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_peripheral_dma_vec(
 	struct xilinx_axidma_tx_segment *segment, *head, *prev = NULL;
 	size_t copy;
 	size_t sg_used;
+	u32 start = 0, taken = 0;
 	unsigned int i;
+	int needed;
 
 	if (!is_slave_direction(direction) || direction != chan->direction)
 		return NULL;
 
+	needed = xilinx_axidma_count_vecs(chan, vecs, nb);
+	if (needed < 0)
+		return NULL;
+
 	desc = xilinx_dma_alloc_tx_descriptor(chan);
 	if (!desc)
 		return NULL;
 
+	if (xilinx_axidma_reserve_segments(chan, needed, &start)) {
+		xilinx_dma_free_tx_descriptor(chan, desc);
+		return NULL;
+	}
+
 	dma_async_tx_descriptor_init(&desc->async_tx, &chan->common);
 	desc->async_tx.tx_submit = xilinx_dma_tx_submit;
 
@@ -2274,10 +2441,8 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_peripheral_dma_vec(
 		while (sg_used < vecs[i].len) {
 			struct xilinx_axidma_desc_hw *hw;
 
-			/* Get a free segment */
-			segment = xilinx_axidma_alloc_tx_segment(chan);
-			if (!segment)
-				goto error;
+			segment = xilinx_axidma_run_segment(chan, start,
+							    taken++);
 
 			/*
 			 * Calculate the maximum number of bytes to transfer,
@@ -2305,9 +2470,6 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_peripheral_dma_vec(
 		}
 	}
 
-	if (list_empty(&desc->segments))
-		goto error;
-
 	head = list_first_entry(&desc->segments, struct xilinx_axidma_tx_segment, node);
 	desc->async_tx.phys = head->phys;
 
@@ -2324,10 +2486,6 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_peripheral_dma_vec(
 		desc->async_tx.metadata_ops = &xilinx_dma_metadata_ops;
 
 	return &desc->async_tx;
-
-error:
-	xilinx_dma_free_tx_descriptor(chan, desc);
-	return NULL;
 }
 
 /**
@@ -2353,16 +2511,28 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_slave_sg(
 	struct scatterlist *sg;
 	size_t copy;
 	size_t sg_used;
+	u32 start = 0, taken = 0;
 	unsigned int i;
+	int needed;
 
 	if (!is_slave_direction(direction))
 		return NULL;
 
+	/* reserve exactly the segments the loop below builds */
+	needed = xilinx_axidma_count_sg(chan, sgl, sg_len);
+	if (needed < 0)
+		return NULL;
+
 	/* Allocate a transaction descriptor. */
 	desc = xilinx_dma_alloc_tx_descriptor(chan);
 	if (!desc)
 		return NULL;
 
+	if (xilinx_axidma_reserve_segments(chan, needed, &start)) {
+		xilinx_dma_free_tx_descriptor(chan, desc);
+		return NULL;
+	}
+
 	dma_async_tx_descriptor_init(&desc->async_tx, &chan->common);
 	desc->async_tx.tx_submit = xilinx_dma_tx_submit;
 
@@ -2374,10 +2544,8 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_slave_sg(
 		while (sg_used < sg_dma_len(sg)) {
 			struct xilinx_axidma_desc_hw *hw;
 
-			/* Get a free segment */
-			segment = xilinx_axidma_alloc_tx_segment(chan);
-			if (!segment)
-				goto error;
+			segment = xilinx_axidma_run_segment(chan, start,
+							    taken++);
 
 			/*
 			 * Calculate the maximum number of bytes to transfer,
@@ -2409,9 +2577,6 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_slave_sg(
 		}
 	}
 
-	if (list_empty(&desc->segments))
-		goto error;
-
 	segment = list_first_entry(&desc->segments,
 				   struct xilinx_axidma_tx_segment, node);
 	desc->async_tx.phys = segment->phys;
@@ -2429,10 +2594,6 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_slave_sg(
 		desc->async_tx.metadata_ops = &xilinx_dma_metadata_ops;
 
 	return &desc->async_tx;
-
-error:
-	xilinx_dma_free_tx_descriptor(chan, desc);
-	return NULL;
 }
 
 /**
@@ -2456,7 +2617,8 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_dma_cyclic(
 	struct xilinx_axidma_tx_segment *segment, *head_segment, *prev = NULL;
 	size_t copy, sg_used, periods;
 	unsigned int num_periods;
-	int i;
+	u32 start = 0, taken = 0;
+	int i, per_period;
 	u32 reg;
 
 	if (!period_len)
@@ -2471,11 +2633,24 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_dma_cyclic(
 	if (!is_slave_direction(direction))
 		return NULL;
 
+	/* the division bounds per_period * num_periods without overflow */
+	per_period = xilinx_axidma_count_split(chan, period_len);
+	if (per_period <= 0)
+		return NULL;
+	if ((u32)per_period > XILINX_DMA_NUM_DESCS / num_periods)
+		return NULL;
+
 	/* Allocate a transaction descriptor. */
 	desc = xilinx_dma_alloc_tx_descriptor(chan);
 	if (!desc)
 		return NULL;
 
+	if (xilinx_axidma_reserve_segments(chan, per_period * num_periods,
+					   &start)) {
+		xilinx_dma_free_tx_descriptor(chan, desc);
+		return NULL;
+	}
+
 	chan->direction = direction;
 	dma_async_tx_descriptor_init(&desc->async_tx, &chan->common);
 	desc->async_tx.tx_submit = xilinx_dma_tx_submit;
@@ -2486,10 +2661,8 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_dma_cyclic(
 		while (sg_used < period_len) {
 			struct xilinx_axidma_desc_hw *hw;
 
-			/* Get a free segment */
-			segment = xilinx_axidma_alloc_tx_segment(chan);
-			if (!segment)
-				goto error;
+			segment = xilinx_axidma_run_segment(chan, start,
+							    taken++);
 
 			/*
 			 * Calculate the maximum number of bytes to transfer,
@@ -2537,10 +2710,6 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_dma_cyclic(
 	}
 
 	return &desc->async_tx;
-
-error:
-	xilinx_dma_free_tx_descriptor(chan, desc);
-	return NULL;
 }
 
 /**

-- 
2.43.0

[PATCH 4/5] dmaengine: xilinx_dma: Withhold an AXI DMA engine whose reset did not complete

[Jeff C. Jensen via B4 Relay]

From: "Jeff C. Jensen" <kernel@nullptr.com>

When xilinx_dma_chan_reset() times out, all the driver knows is that it
did not see the reset complete. The engine may still be fetching
descriptors and writing data: the AXI DMA product guide (PG021, Reset
bit of MM2S_DMACR and S2MM_DMACR) says a soft reset completes or flushes
outstanding operations while the reset bit stays set. Two paths carry on
regardless:

 - xilinx_dma_terminate_all() ignores the result of the reset, frees
   every descriptor, marks the channel idle and returns success. The
   next transaction then reuses segments the engine may still fetch.
   This has been the case since AXI DMA support was added;
   commit 2575cb81a966 ("dmaengine: xilinx_dma: Reset DMA channel in
   dma_terminate_all") made the reset unconditional without checking its
   result either.

 - xilinx_dma_tx_submit() resets a channel that is in error and returns
   the error without freeing the descriptor, which leaks the descriptor
   and the segments it holds.

Withhold the engine instead. Setting the reset bit of either direction
resets the whole AXI DMA engine, so both channels of the controller are
withheld: their descriptors move to a quarantine list instead of being
freed, and neither channel takes new work. Each check is made under
chan->lock, where the resource changes hands:

 - segment reservation fails, so repeated preparation cannot drain the
   pool;
 - xilinx_dma_tx_submit() keeps a descriptor that was prepared before
   the channel was withheld, since the engine may own its segments;
 - xilinx_dma_start_transfer() starts nothing, whether it is called from
   issue_pending or from the interrupt handler;
 - xilinx_dma_alloc_chan_resources() fails, and
   xilinx_dma_free_chan_resources() keeps the descriptor memory, so the
   engine cannot write into whatever is allocated next.

Withholding sets chan->terminating before it moves the lists, so that a
completion pass running a callback stops at its current descriptor, as
it does for a terminate. xilinx_dma_tx_status() reports DMA_ERROR for
cookies of a withheld channel that have not completed; cookies that
completed earlier still report DMA_COMPLETE.

This protects descriptor ownership only. A withheld channel stays out of
service. The client keeps its buffers mapped and its callback resources
alive, because a terminate that fails has not stopped the engine, and
dmaengine_terminate_sync() does not synchronize after one. On driver
removal the withheld memory is left allocated, and
xilinx_dma_chan_remove() reports it.

CDMA, VDMA and MCDMA channels behave as before.

Fixes: c0bba3a99f07 ("dmaengine: vdma: Add Support for Xilinx AXI Direct Memory Access Engine")
Assisted-by: Claude:claude-fable-5-1
Signed-off-by: Jeff C. Jensen <kernel@nullptr.com>
---
 drivers/dma/xilinx/xilinx_dma.c | 150 +++++++++++++++++++++++++++++++++++-----
 1 file changed, 133 insertions(+), 17 deletions(-)

diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index 805cd3e2bc27..0316c6e7881f 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -391,6 +391,9 @@ struct xilinx_dma_tx_descriptor {
  * @pending_list: Descriptors waiting
  * @active_list: Descriptors ready to submit
  * @done_list: Complete descriptors
+ * @quarantine_list: Descriptors withheld after a reset that did not complete
+ * @quarantined: The reset did not complete; the channel is withheld and takes
+ *	no work
  * @free_seg_list: Free AXI MCDMA descriptors
  * @seg_used: AXI DMA segments in use, indexed by ring position
  * @seg_next: Ring position the next AXI DMA reservation starts at
@@ -432,6 +435,8 @@ struct xilinx_dma_chan {
 	struct list_head pending_list;
 	struct list_head active_list;
 	struct list_head done_list;
+	struct list_head quarantine_list;
+	bool quarantined;
 	struct list_head free_seg_list;
 	DECLARE_BITMAP(seg_used, XILINX_DMA_NUM_DESCS);
 	u32 seg_next;
@@ -720,8 +725,9 @@ xilinx_cdma_alloc_tx_segment(struct xilinx_dma_chan *chan)
  * wrapping at the pool end. The engine fetches descriptors in ring order, so
  * the run cannot be placed anywhere else.
  *
- * Return: 0 on success, -ENOMEM when the run is not free. A failure leaves the
- * ownership bitmap and @chan->seg_next unchanged.
+ * Return: 0 on success, -ENOMEM when the run is not free, -ENODEV when the
+ * channel is withheld. A failure leaves the ownership bitmap and
+ * @chan->seg_next unchanged.
  */
 static int xilinx_axidma_reserve_segments(struct xilinx_dma_chan *chan,
 					  u32 count, u32 *start)
@@ -732,6 +738,11 @@ static int xilinx_axidma_reserve_segments(struct xilinx_dma_chan *chan,
 
 	spin_lock_irqsave(&chan->lock, flags);
 
+	if (chan->quarantined) {
+		ret = -ENODEV;
+		goto out;
+	}
+
 	for (i = 0; i < count; i++) {
 		if (test_bit((chan->seg_next + i) % XILINX_DMA_NUM_DESCS,
 			     chan->seg_used))
@@ -748,7 +759,7 @@ static int xilinx_axidma_reserve_segments(struct xilinx_dma_chan *chan,
 out:
 	spin_unlock_irqrestore(&chan->lock, flags);
 
-	if (ret)
+	if (ret == -ENOMEM)
 		dev_dbg(chan->dev, "no run of %u free tx segments\n", count);
 
 	return ret;
@@ -963,6 +974,64 @@ static void xilinx_dma_cancel_tx_descriptor(struct xilinx_dma_chan *chan,
 	xilinx_dma_free_tx_descriptor(chan, desc);
 }
 
+/**
+ * xilinx_dma_quarantine_channel - Withhold a channel whose engine did not reset
+ * @chan: Driver specific DMA channel
+ *
+ * Moves every descriptor of the channel to the quarantine list. While
+ * @chan->quarantined is set, the channel refuses segment reservation,
+ * submission, transfer start and resource allocation. Segment reservations
+ * and the descriptor pool stay held.
+ */
+static void xilinx_dma_quarantine_channel(struct xilinx_dma_chan *chan)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&chan->lock, flags);
+	chan->quarantined = true;
+	chan->err = true;
+	/* a completion pass mid-callback stops at its current descriptor */
+	chan->terminating = true;
+	list_splice_tail_init(&chan->done_list, &chan->quarantine_list);
+	list_splice_tail_init(&chan->active_list, &chan->quarantine_list);
+	list_splice_tail_init(&chan->pending_list, &chan->quarantine_list);
+	chan->desc_pendingcount = 0;
+	chan->desc_submitcount = 0;
+	spin_unlock_irqrestore(&chan->lock, flags);
+
+	dev_err(chan->dev, "channel %p withheld: the engine did not reset\n", chan);
+}
+
+/**
+ * xilinx_dma_quarantine_controller - Withhold every channel of an AXI DMA engine
+ * @chan: Channel whose reset did not complete
+ *
+ * The AXI DMA soft reset acts on the whole engine, so a reset that did not
+ * complete leaves both directions in an unknown state.
+ */
+static void xilinx_dma_quarantine_controller(struct xilinx_dma_chan *chan)
+{
+	struct xilinx_dma_device *xdev = chan->xdev;
+	int i;
+
+	for (i = 0; i < xdev->dma_config->max_channels; i++) {
+		if (xdev->chan[i])
+			xilinx_dma_quarantine_channel(xdev->chan[i]);
+	}
+}
+
+static bool xilinx_dma_chan_quarantined(struct xilinx_dma_chan *chan)
+{
+	unsigned long flags;
+	bool held;
+
+	spin_lock_irqsave(&chan->lock, flags);
+	held = chan->quarantined;
+	spin_unlock_irqrestore(&chan->lock, flags);
+
+	return held;
+}
+
 /* Required functions */
 
 /**
@@ -1009,6 +1078,15 @@ static void xilinx_dma_free_chan_resources(struct dma_chan *dchan)
 
 	dev_dbg(chan->dev, "Free all channel resources.\n");
 
+	/*
+	 * Freed descriptor memory would go to the next allocation while the
+	 * engine may still fetch from it.
+	 */
+	if (xilinx_dma_chan_quarantined(chan)) {
+		dev_err(chan->dev, "channel %p withheld: keeping its descriptor memory\n", chan);
+		return;
+	}
+
 	xilinx_dma_free_descriptors(chan);
 
 	if (chan->xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA) {
@@ -1204,6 +1282,12 @@ static int xilinx_dma_alloc_chan_resources(struct dma_chan *dchan)
 	struct xilinx_dma_chan *chan = to_xilinx_chan(dchan);
 	int i;
 
+	/* a new pool would replace the one the withheld descriptors still occupy */
+	if (xilinx_dma_chan_quarantined(chan)) {
+		dev_err(chan->dev, "channel %p withheld: refusing allocation\n", chan);
+		return -EBUSY;
+	}
+
 	/* Has this channel already been allocated? */
 	if (chan->desc_pool)
 		return 0;
@@ -1450,11 +1534,13 @@ static enum dma_status xilinx_dma_tx_status(struct dma_chan *dchan,
 	u32 residue = 0;
 
 	ret = dma_cookie_status(dchan, cookie, txstate);
-	if (ret == DMA_COMPLETE || !txstate)
+	if (ret == DMA_COMPLETE)
 		return ret;
 
 	spin_lock_irqsave(&chan->lock, flags);
-	if (!list_empty(&chan->active_list)) {
+	if (chan->quarantined) {
+		ret = DMA_ERROR;
+	} else if (txstate && !list_empty(&chan->active_list)) {
 		desc = list_last_entry(&chan->active_list,
 				       struct xilinx_dma_tx_descriptor, node);
 		/*
@@ -1466,7 +1552,8 @@ static enum dma_status xilinx_dma_tx_status(struct dma_chan *dchan,
 	}
 	spin_unlock_irqrestore(&chan->lock, flags);
 
-	dma_set_residue(txstate, residue);
+	if (txstate)
+		dma_set_residue(txstate, residue);
 
 	return ret;
 }
@@ -1717,7 +1804,7 @@ static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan)
 	struct xilinx_axidma_tx_segment *tail_segment;
 	u32 reg;
 
-	if (chan->err)
+	if (chan->err || chan->quarantined)
 		return;
 
 	if (list_empty(&chan->pending_list)) {
@@ -2203,18 +2290,23 @@ static void append_desc_queue(struct xilinx_dma_chan *chan,
  * be submitted, or its rejection handled, before the next one is prepared on
  * the same channel.
  *
- * Return: cookie value on success and failure value on error
+ * Return: cookie value on success, -EBUSY while a cyclic transaction runs, or
+ * -ENODEV or the reset error on a withheld channel, which keeps the descriptor
+ * on the quarantine list.
  */
 static dma_cookie_t xilinx_dma_tx_submit(struct dma_async_tx_descriptor *tx)
 {
 	struct xilinx_dma_tx_descriptor *desc = to_dma_tx_descriptor(tx);
 	struct xilinx_dma_chan *chan = to_xilinx_chan(tx->chan);
-	dma_cookie_t cookie;
+	dma_cookie_t cookie = -ENODEV;
 	unsigned long flags;
 	int err;
 
 	spin_lock_irqsave(&chan->lock, flags);
 
+	if (chan->quarantined)
+		goto withhold;
+
 	if (chan->cyclic) {
 		xilinx_dma_cancel_tx_descriptor(chan, desc);
 		spin_unlock_irqrestore(&chan->lock, flags);
@@ -2222,15 +2314,17 @@ static dma_cookie_t xilinx_dma_tx_submit(struct dma_async_tx_descriptor *tx)
 	}
 
 	if (chan->err) {
-		/*
-		 * If reset fails, need to hard reset the system.
-		 * Channel is no longer functional
-		 */
 		spin_unlock_irqrestore(&chan->lock, flags);
 		err = xilinx_dma_chan_reset(chan);
-		if (err < 0)
-			return err;
+		if (err < 0) {
+			if (chan->xdev->dma_config->dmatype != XDMA_TYPE_AXIDMA)
+				return err;
+			xilinx_dma_quarantine_controller(chan);
+			cookie = err;
+		}
 		spin_lock_irqsave(&chan->lock, flags);
+		if (chan->quarantined)
+			goto withhold;
 	}
 
 	cookie = dma_cookie_assign(tx);
@@ -2245,6 +2339,16 @@ static dma_cookie_t xilinx_dma_tx_submit(struct dma_async_tx_descriptor *tx)
 
 	spin_unlock_irqrestore(&chan->lock, flags);
 
+	return cookie;
+
+withhold:
+	/*
+	 * The reservation predates the withholding, so its segments cannot be
+	 * told apart from ones the engine still owns.
+	 */
+	list_add_tail(&desc->node, &chan->quarantine_list);
+	spin_unlock_irqrestore(&chan->lock, flags);
+
 	return cookie;
 }
 
@@ -2817,7 +2921,8 @@ xilinx_mcdma_prep_slave_sg(struct dma_chan *dchan, struct scatterlist *sgl,
  * xilinx_dma_terminate_all - Halt the channel and free descriptors
  * @dchan: Driver specific DMA Channel pointer
  *
- * Return: '0' always.
+ * Return: 0, or the reset error when an AXI DMA reset does not complete, in
+ * which case every channel of the engine is withheld with its descriptors.
  */
 static int xilinx_dma_terminate_all(struct dma_chan *dchan)
 {
@@ -2835,7 +2940,12 @@ static int xilinx_dma_terminate_all(struct dma_chan *dchan)
 		}
 	}
 
-	xilinx_dma_chan_reset(chan);
+	err = xilinx_dma_chan_reset(chan);
+	if (err && chan->xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA) {
+		xilinx_dma_quarantine_controller(chan);
+		return err;
+	}
+
 	/* Remove and free all of the descriptors in the lists */
 	chan->terminating = true;
 	xilinx_dma_free_descriptors(chan);
@@ -2943,6 +3053,11 @@ EXPORT_SYMBOL(xilinx_vdma_channel_set_config);
  */
 static void xilinx_dma_chan_remove(struct xilinx_dma_chan *chan)
 {
+	if (xilinx_dma_chan_quarantined(chan))
+		dev_err(chan->dev,
+			"channel %p withheld: its descriptor memory is not reclaimed\n",
+			chan);
+
 	/* Disable all interrupts */
 	dma_ctrl_clr(chan, XILINX_DMA_REG_DMACR,
 		      XILINX_DMA_DMAXR_ALL_IRQ_MASK);
@@ -3171,6 +3286,7 @@ static int xilinx_dma_chan_probe(struct xilinx_dma_device *xdev,
 	INIT_LIST_HEAD(&chan->pending_list);
 	INIT_LIST_HEAD(&chan->done_list);
 	INIT_LIST_HEAD(&chan->active_list);
+	INIT_LIST_HEAD(&chan->quarantine_list);
 	INIT_LIST_HEAD(&chan->free_seg_list);
 
 	/* Retrieve the channel properties from the device tree */

-- 
2.43.0

[PATCH 5/5] dmaengine: xilinx_dma: Recover a withheld AXI DMA channel

[Jeff C. Jensen via B4 Relay]

From: "Jeff C. Jensen" <kernel@nullptr.com>

A channel withheld by the previous patch keeps its descriptors, segment
reservations and descriptor memory for as long as it exists, because
nothing has shown that the engine stopped.

Recover it in the two steps the dmaengine API already separates. A
terminate halts and resets the channel, and a reset that completes is
taken as proof that the engine no longer fetches descriptors or writes
data. The withheld descriptors are freed later, from
xilinx_dma_synchronize() after the completion tasklet has been killed,
because a completion pass may be running a callback with the lock
dropped while the terminate runs, and a cyclic completion keeps its
descriptor on the list for the whole callback. Until then the channel
stays withheld and refuses new work.

The two steps have to stay apart because xilinx_dma_chan_reset() is not
purely a hardware operation: on success it clears chan->err, marks the
channel idle and enables interrupts. It is chan->quarantined, not
chan->err, that keeps every start path closed until synchronize,
including the one in the interrupt handler.

After synchronize the quarantine list is empty, the error and cyclic
state are cleared, the counters are zero and the channel is idle. The
ownership bitmap keeps only the reservations of descriptors that still
exist, and chan->seg_next keeps its position, since the engine fetches
next from wherever the last tail pointed. A client that terminates with
dmaengine_terminate_async() and never synchronizes leaves the channel
reset but still withheld.

Recovery is refused when this driver instance also manages the other
channel of the engine. An AXI DMA soft reset acts on the whole engine,
so it would reset that channel too and clear its interrupt enables,
which the driver does not coordinate, as
commit a17ce4bc6f4f ("dmaengine: xilinx_dma: Fix reset related timeout
with two-channel AXIDMA") describes. This patch handles single-channel
controllers; a withheld two-channel controller stays withheld.

A failed halt before a reset that completes is ignored, since the reset
is what stops the engine. If the reset fails again, the channel stays
withheld with all its reservations.

Assisted-by: Claude:claude-fable-5-1
Signed-off-by: Jeff C. Jensen <kernel@nullptr.com>
---
 drivers/dma/xilinx/xilinx_dma.c | 106 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 105 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index 0316c6e7881f..c72c0fc985c5 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -394,6 +394,8 @@ struct xilinx_dma_tx_descriptor {
  * @quarantine_list: Descriptors withheld after a reset that did not complete
  * @quarantined: The reset did not complete; the channel is withheld and takes
  *	no work
+ * @quiesced: The engine completed a reset while the channel was withheld;
+ *	xilinx_dma_synchronize() reclaims the descriptors
  * @free_seg_list: Free AXI MCDMA descriptors
  * @seg_used: AXI DMA segments in use, indexed by ring position
  * @seg_next: Ring position the next AXI DMA reservation starts at
@@ -437,6 +439,7 @@ struct xilinx_dma_chan {
 	struct list_head done_list;
 	struct list_head quarantine_list;
 	bool quarantined;
+	bool quiesced;
 	struct list_head free_seg_list;
 	DECLARE_BITMAP(seg_used, XILINX_DMA_NUM_DESCS);
 	u32 seg_next;
@@ -2917,12 +2920,109 @@ xilinx_mcdma_prep_slave_sg(struct dma_chan *dchan, struct scatterlist *sgl,
 	return NULL;
 }
 
+/**
+ * xilinx_dma_reset_is_isolated - Report whether a reset disturbs only this channel
+ * @chan: Driver specific DMA channel
+ *
+ * An AXI DMA soft reset acts on the whole engine, so resetting one channel of a
+ * two-channel controller also resets the other and clears its interrupt enables.
+ *
+ * Return: true when this driver instance manages no other channel of the
+ * engine.
+ */
+static bool xilinx_dma_reset_is_isolated(struct xilinx_dma_chan *chan)
+{
+	struct xilinx_dma_device *xdev = chan->xdev;
+	int i;
+
+	for (i = 0; i < xdev->dma_config->max_channels; i++) {
+		if (xdev->chan[i] && xdev->chan[i] != chan)
+			return false;
+	}
+
+	return true;
+}
+
+/**
+ * xilinx_dma_reset_withheld_channel - Stop the engine behind a withheld channel
+ * @chan: Driver specific DMA channel
+ *
+ * A completed reset shows that the engine no longer fetches descriptors or
+ * writes payload. The channel stays withheld until xilinx_dma_recover_channel()
+ * reclaims its descriptors. A failure leaves the channel withheld with its
+ * reservations intact.
+ *
+ * Return: 0 when the reset completed, negative otherwise.
+ */
+static int xilinx_dma_reset_withheld_channel(struct xilinx_dma_chan *chan)
+{
+	unsigned long flags;
+	int err;
+
+	if (!xilinx_dma_reset_is_isolated(chan)) {
+		dev_err(chan->dev,
+			"channel %p stays withheld: a reset would disturb the other channel\n",
+			chan);
+		return -EBUSY;
+	}
+
+	err = chan->stop_transfer(chan);
+	if (err)
+		dev_dbg(chan->dev, "channel %p did not halt before its reset\n", chan);
+
+	err = xilinx_dma_chan_reset(chan);
+	if (err) {
+		dev_err(chan->dev, "channel %p stays withheld: reset failed again\n", chan);
+		return err;
+	}
+
+	spin_lock_irqsave(&chan->lock, flags);
+	chan->quiesced = true;
+	spin_unlock_irqrestore(&chan->lock, flags);
+
+	return 0;
+}
+
+/**
+ * xilinx_dma_recover_channel - Return a withheld channel to service
+ * @chan: Driver specific DMA channel
+ *
+ * Must run after the completion tasklet has been killed, so that no completion
+ * pass holds a withheld descriptor. Frees the withheld descriptors, which
+ * restores their segment links and releases their reservations, and returns
+ * the channel to idle with no error, so it takes work again. Does nothing
+ * unless the engine was reset after the channel was withheld.
+ */
+static void xilinx_dma_recover_channel(struct xilinx_dma_chan *chan)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&chan->lock, flags);
+	if (!chan->quarantined || !chan->quiesced) {
+		spin_unlock_irqrestore(&chan->lock, flags);
+		return;
+	}
+	xilinx_dma_free_desc_list(chan, &chan->quarantine_list);
+	chan->quarantined = false;
+	chan->quiesced = false;
+	chan->cyclic = false;
+	chan->err = false;
+	chan->idle = true;
+	chan->desc_pendingcount = 0;
+	chan->desc_submitcount = 0;
+	spin_unlock_irqrestore(&chan->lock, flags);
+
+	dev_info(chan->dev, "channel %p recovered\n", chan);
+}
+
 /**
  * xilinx_dma_terminate_all - Halt the channel and free descriptors
  * @dchan: Driver specific DMA Channel pointer
  *
  * Return: 0, or the reset error when an AXI DMA reset does not complete, in
- * which case every channel of the engine is withheld with its descriptors.
+ * which case every channel of the engine is withheld with its descriptors. On
+ * a channel that is already withheld, the result of resetting the engine;
+ * xilinx_dma_synchronize() then reclaims the descriptors.
  */
 static int xilinx_dma_terminate_all(struct dma_chan *dchan)
 {
@@ -2930,6 +3030,9 @@ static int xilinx_dma_terminate_all(struct dma_chan *dchan)
 	u32 reg;
 	int err;
 
+	if (xilinx_dma_chan_quarantined(chan))
+		return xilinx_dma_reset_withheld_channel(chan);
+
 	if (!chan->cyclic) {
 		err = chan->stop_transfer(chan);
 		if (err) {
@@ -2970,6 +3073,7 @@ static void xilinx_dma_synchronize(struct dma_chan *dchan)
 	struct xilinx_dma_chan *chan = to_xilinx_chan(dchan);
 
 	tasklet_kill(&chan->tasklet);
+	xilinx_dma_recover_channel(chan);
 }
 
 /**

-- 
2.43.0