[PATCH v3 0/3] dmaengine: xilinx_dma: Fixes and optimizations for AXIDMA and MCDMA channel management

[PATCH v3 0/3] dmaengine: xilinx_dma: Fixes and optimizations for AXIDMA and MCDMA channel management

[Suraj Gupta]

This patch series addresses issues and optimizations in the Xilinx
AXI DMA and MCDMA drivers:
1. Fix channel idle state management in the interrupt handlers.
2. Enable transfer chaining by removing unnecessary idle restrictions.
3. Optimize control register writes and channel start logic.

Note: The patches in this series were part of following IRQ coalescing
series which is under discussion:
https://lore.kernel.org/all/20250710101229.804183-1-suraj.gupta2@amd.com/

Changes in V3:
- Patch 2: Restrict the idle-check removal to scatter-gather mode. Direct
  (non-SG) mode has no descriptor queue, so writing the BTT register while
  a transfer is in flight would corrupt the active transfer; keep those
  transfers serialized by retaining the idle check on the non-SG path.
  MCDMA always operates in scatter-gather mode and is unaffected. Update
  the commit description accordingly.

Changes in V2:
- Apply similar fixes and optimizations to MCDMA as well.
- Expand the 1/3 commit description with when the described issue occurs.

Suraj Gupta (3):
  dmaengine: xilinx_dma: Fix channel idle state management in AXIDMA and
    MCDMA interrupt handlers
  dmaengine: xilinx_dma: Enable transfer chaining for AXIDMA and MCDMA
    by removing idle restriction
  dmaengine: xilinx_dma: Optimize control register write and channel
    start logic for AXIDMA and MCDMA in corresponding start_transfer()

 drivers/dma/xilinx/xilinx_dma.c | 38 +++++++++++++++++++++------------
 1 file changed, 24 insertions(+), 14 deletions(-)

-- 
2.25.1

[PATCH v3 1/3] dmaengine: xilinx_dma: Fix channel idle state management in AXIDMA and MCDMA interrupt handlers

[Suraj Gupta]

Fix a race condition in AXIDMA and MCDMA irq handlers where the channel
could be incorrectly marked as idle and attempt spurious transfers when
descriptors are still being processed.

The issue occurs when:
1. Multiple descriptors are queued and active.
2. An interrupt fires after completing some descriptors.
3. xilinx_dma_complete_descriptor() moves completed descriptors to
done_list.
4. Channel is marked idle and start_transfer() is called even though
   active_list still contains unprocessed descriptors.
5. This leads to premature transfer attempts and potential descriptor
   corruption or missed completions.

Only mark the channel as idle and start new transfers when the active list
is actually empty, ensuring proper channel state management and avoiding
spurious transfer attempts.

Fixes: c0bba3a99f07 ("dmaengine: vdma: Add Support for Xilinx AXI Direct Memory Access Engine")
Tested-by: Folker Schwesinger <dev@folker-schwesinger.de>
Signed-off-by: Suraj Gupta <suraj.gupta2@amd.com>
Co-developed-by: Srinivas Neeli <srinivas.neeli@amd.com>
Signed-off-by: Srinivas Neeli <srinivas.neeli@amd.com>
---
 drivers/dma/xilinx/xilinx_dma.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index 404235c17353..ca396b709742 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -1893,8 +1893,10 @@ static irqreturn_t xilinx_mcdma_irq_handler(int irq, void *data)
 	if (status & XILINX_MCDMA_IRQ_IOC_MASK) {
 		spin_lock(&chan->lock);
 		xilinx_dma_complete_descriptor(chan);
-		chan->idle = true;
-		chan->start_transfer(chan);
+		if (list_empty(&chan->active_list)) {
+			chan->idle = true;
+			chan->start_transfer(chan);
+		}
 		spin_unlock(&chan->lock);
 	}
 
@@ -1950,8 +1952,10 @@ static irqreturn_t xilinx_dma_irq_handler(int irq, void *data)
 		      XILINX_DMA_DMASR_DLY_CNT_IRQ)) {
 		spin_lock(&chan->lock);
 		xilinx_dma_complete_descriptor(chan);
-		chan->idle = true;
-		chan->start_transfer(chan);
+		if (list_empty(&chan->active_list)) {
+			chan->idle = true;
+			chan->start_transfer(chan);
+		}
 		spin_unlock(&chan->lock);
 	}
 
-- 
2.25.1

[PATCH v3 2/3] dmaengine: xilinx_dma: Enable transfer chaining for AXIDMA and MCDMA by removing idle restriction

[Suraj Gupta]

Relax the idle check in xilinx_dma_start_transfer() and
xilinx_mcdma_start_transfer() that prevented new transfers from being
queued when the channel was busy, so scatter-gather transfers can be
chained onto an in-flight transfer.

In scatter-gather mode, only update the CURDESC register when the active
list is empty to avoid interfering with transfers already in progress.
When the active list contains transfers, the hardware tail pointer
extension mechanism handles chaining automatically via the descriptor
next pointer chain, which is set up at channel allocation and preserved
across descriptor recycling.

Direct (non-SG) mode has no descriptor queue: writing the BTT register
launches a transfer immediately, so a new transfer must not be programmed
while one is in flight. Keep those transfers serialized by retaining the
idle check on the non-SG path. MCDMA always operates in scatter-gather
mode, so it is unaffected.

Signed-off-by: Suraj Gupta <suraj.gupta2@amd.com>
Co-developed-by: Srinivas Neeli <srinivas.neeli@amd.com>
Signed-off-by: Srinivas Neeli <srinivas.neeli@amd.com>
---
 drivers/dma/xilinx/xilinx_dma.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index ca396b709742..6e7b183cb499 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -1580,7 +1580,14 @@ static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan)
 		return;
 	}
 
-	if (!chan->idle)
+	/*
+	 * Direct (non-SG) mode has no descriptor queue: writing the BTT
+	 * register launches a transfer immediately, so a new transfer must
+	 * not be programmed while one is in flight. Keep such transfers
+	 * serialized. SG mode supports chaining onto a running transfer via
+	 * tail-pointer extension, so it is allowed to proceed when busy.
+	 */
+	if (!chan->has_sg && !chan->idle)
 		return;
 
 	head_desc = list_first_entry(&chan->pending_list,
@@ -1599,7 +1606,7 @@ static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan)
 		dma_ctrl_write(chan, XILINX_DMA_REG_DMACR, reg);
 	}
 
-	if (chan->has_sg)
+	if (chan->has_sg && list_empty(&chan->active_list))
 		xilinx_write(chan, XILINX_DMA_REG_CURDESC,
 			     head_desc->async_tx.phys);
 	reg  &= ~XILINX_DMA_CR_DELAY_MAX;
@@ -1660,9 +1667,6 @@ static void xilinx_mcdma_start_transfer(struct xilinx_dma_chan *chan)
 	if (chan->err)
 		return;
 
-	if (!chan->idle)
-		return;
-
 	if (list_empty(&chan->pending_list))
 		return;
 
@@ -1685,8 +1689,9 @@ static void xilinx_mcdma_start_transfer(struct xilinx_dma_chan *chan)
 	dma_ctrl_write(chan, XILINX_MCDMA_CHAN_CR_OFFSET(chan->tdest), reg);
 
 	/* Program current descriptor */
-	xilinx_write(chan, XILINX_MCDMA_CHAN_CDESC_OFFSET(chan->tdest),
-		     head_desc->async_tx.phys);
+	if (chan->has_sg && list_empty(&chan->active_list))
+		xilinx_write(chan, XILINX_MCDMA_CHAN_CDESC_OFFSET(chan->tdest),
+			     head_desc->async_tx.phys);
 
 	/* Program channel enable register */
 	reg = dma_ctrl_read(chan, XILINX_MCDMA_CHEN_OFFSET);
-- 
2.25.1

[PATCH v3 3/3] dmaengine: xilinx_dma: Optimize control register write and channel start logic for AXIDMA and MCDMA in corresponding start_transfer()

[Suraj Gupta]

Optimize AXI DMA control register programming by consolidating
coalesce count and delay configuration into a single register write.
Previously, the coalesce count was written separately from the delay
configuration, resulting in two register writes. Combine these into
one write operation to reduce bus overhead.
Additionally, avoid redundant channel starts in xilinx_dma_start_transfer()
and xilinx_mcdma_start_transfer() by only calling xilinx_dma_start() when
the channel is actually idle.

Tested-by: Folker Schwesinger <dev@folker-schwesinger.de>
Signed-off-by: Suraj Gupta <suraj.gupta2@amd.com>
Co-developed-by: Srinivas Neeli <srinivas.neeli@amd.com>
Signed-off-by: Srinivas Neeli <srinivas.neeli@amd.com>
---
 drivers/dma/xilinx/xilinx_dma.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index 6e7b183cb499..829601d8a16f 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -1603,7 +1603,6 @@ static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan)
 		reg &= ~XILINX_DMA_CR_COALESCE_MAX;
 		reg |= chan->desc_pendingcount <<
 				  XILINX_DMA_CR_COALESCE_SHIFT;
-		dma_ctrl_write(chan, XILINX_DMA_REG_DMACR, reg);
 	}
 
 	if (chan->has_sg && list_empty(&chan->active_list))
@@ -1614,7 +1613,8 @@ static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan)
 	reg |= XILINX_DMA_DMAXR_ALL_IRQ_MASK;
 	dma_ctrl_write(chan, XILINX_DMA_REG_DMACR, reg);
 
-	xilinx_dma_start(chan);
+	if (chan->idle)
+		xilinx_dma_start(chan);
 
 	if (chan->err)
 		return;
@@ -1703,7 +1703,8 @@ static void xilinx_mcdma_start_transfer(struct xilinx_dma_chan *chan)
 	reg |= XILINX_MCDMA_CR_RUNSTOP_MASK;
 	dma_ctrl_write(chan, XILINX_MCDMA_CHAN_CR_OFFSET(chan->tdest), reg);
 
-	xilinx_dma_start(chan);
+	if (chan->idle)
+		xilinx_dma_start(chan);
 
 	if (chan->err)
 		return;
-- 
2.25.1

Re: [PATCH v3 1/3] dmaengine: xilinx_dma: Fix channel idle state management in AXIDMA and MCDMA interrupt handlers

[sashiko-bot]

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

Pre-existing issues:
- [High] The patch claims to fix a race condition for MCDMA channels by conditionally marking the channel as idle only when `active_list` is empty. However, this fix is entirely ineffective because `xilinx_dma_complete_descriptor()` unconditionally empties `active_list` for MCDMA.
--

commit fef85fe49f3788eecfb93a71efa6536e69b799c0
Author: Suraj Gupta <suraj.gupta2@amd.com>

dmaengine: xilinx_dma: Fix channel idle state management in AXIDMA and MCDMA interrupt handlers

This patch fixes a race condition in AXIDMA and MCDMA irq handlers by only
marking the channel idle and starting a new transfer when the active list is
empty.

> diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
> index 404235c173538..ca396b7097427 100644
> --- a/drivers/dma/xilinx/xilinx_dma.c
> +++ b/drivers/dma/xilinx/xilinx_dma.c
> @@ -1893,8 +1893,10 @@ static irqreturn_t xilinx_mcdma_irq_handler(int irq, void *data)
>  	if (status & XILINX_MCDMA_IRQ_IOC_MASK) {
>  		spin_lock(&chan->lock);
>  		xilinx_dma_complete_descriptor(chan);
> -		chan->idle = true;
> -		chan->start_transfer(chan);
> +		if (list_empty(&chan->active_list)) {

[Severity: High]
This is a pre-existing issue, but does this check actually prevent the race
condition for MCDMA channels?

Looking at xilinx_dma_complete_descriptor(), the hardware completion check
is restricted to AXIDMA:

drivers/dma/xilinx/xilinx_dma.c:xilinx_dma_complete_descriptor() {
	list_for_each_entry_safe(desc, next, &chan->active_list, node) {
		if (chan->xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA) {
			...
			if (!(seg->hw.status & XILINX_DMA_BD_COMP_MASK) && chan->has_sg)
				break;
		}
		...
		list_del(&desc->node);
		...
	}
}

Because MCDMA descriptors are unconditionally unlinked from active_list
regardless of their actual hardware completion status, active_list will
always be empty after xilinx_dma_complete_descriptor() returns. 

Will this cause the new idle check to always evaluate to true for MCDMA,
leaving the race condition unaddressed for those channels?

> +			chan->idle = true;
> +			chan->start_transfer(chan);
> +		}
>  		spin_unlock(&chan->lock);
>  	}

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260626092656.1563871-1-suraj.gupta2@amd.com?part=1

Re: [PATCH v3 2/3] dmaengine: xilinx_dma: Enable transfer chaining for AXIDMA and MCDMA by removing idle restriction

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Bypassing the idle check allows new descriptors to be appended to a running transfer, but the driver fails to link the active transfer's hardware `next_desc` pointer to the newly appended descriptors, leading to wild DMA fetches.
- [High] Overwriting the DMA Coalesce Threshold register while a channel is running resets the hardware coalescing counter, causing the channel to stall indefinitely.
--

commit 06494ab31953d1c39c050abca192d725ee06375d
Author: Suraj Gupta <suraj.gupta2@amd.com>

dmaengine: xilinx_dma: Enable transfer chaining for AXIDMA and MCDMA by removing idle restriction

This commit relaxes the idle checks in xilinx_dma_start_transfer() and
xilinx_mcdma_start_transfer() to allow scatter-gather transfers to be chained
onto an in-flight transfer. It relies on the hardware tail pointer extension
mechanism to handle chaining automatically.

> diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
> index ca396b7097427..6e7b183cb4994 100644
> --- a/drivers/dma/xilinx/xilinx_dma.c
> +++ b/drivers/dma/xilinx/xilinx_dma.c
> @@ -1580,7 +1580,14 @@ static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan)
>  		return;
>  	}
>  
> -	if (!chan->idle)
> +	/*
> +	 * Direct (non-SG) mode has no descriptor queue: writing the BTT
> +	 * register launches a transfer immediately, so a new transfer must
> +	 * not be programmed while one is in flight. Keep such transfers
> +	 * serialized. SG mode supports chaining onto a running transfer via
> +	 * tail-pointer extension, so it is allowed to proceed when busy.
> +	 */
> +	if (!chan->has_sg && !chan->idle)
>  		return;

[Severity: Critical]
Does bypassing this idle check for SG mode result in wild DMA fetches when a new
transfer is queued on a busy channel?

If a new transfer is queued while the channel is busy, the pending_list will
be empty when processed by append_desc_queue():

drivers/dma/xilinx/xilinx_dma.c:append_desc_queue() {
    ...
    if (list_empty(&chan->pending_list))
        goto append;
    ...
}

Because of this, append_desc_queue() explicitly skips hardware chaining entirely
and fails to link the active transfer's tail descriptor to the newly submitted
descriptor.

Since out-of-order frees can scramble the ring at runtime, won't the hardware
finish the active transfer and follow a stale next_desc pointer to an unrelated,
uninitialized, or freed descriptor segment?

[ ... ]
> @@ -1599,7 +1606,7 @@ static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan)
>  		dma_ctrl_write(chan, XILINX_DMA_REG_DMACR, reg);
>  	}
>  
> -	if (chan->has_sg)
> +	if (chan->has_sg && list_empty(&chan->active_list))
>  		xilinx_write(chan, XILINX_DMA_REG_CURDESC,
>  			     head_desc->async_tx.phys);
>  	reg  &= ~XILINX_DMA_CR_DELAY_MAX;

[Severity: High]
Does updating the Coalesce Threshold register while a channel is running stall
the channel indefinitely?

Just before this CURDESC check in xilinx_dma_start_transfer(), the threshold
is updated:

    if (chan->desc_pendingcount <= XILINX_DMA_COALESCE_MAX) {
        reg &= ~XILINX_DMA_CR_COALESCE_MAX;
        reg |= chan->desc_pendingcount <<
                  XILINX_DMA_CR_COALESCE_SHIFT;
    }
    ...
    dma_ctrl_write(chan, XILINX_DMA_REG_DMACR, reg);
    
According to the Xilinx AXI DMA specification (PG021), writing this threshold
automatically resets the hardware coalescing counter to zero.

If there are already active transfers running, writing the new threshold will
reset the counter. As a result, the total number of remaining transfers
(active + pending) will no longer align with the new threshold multiple. The
hardware will finish the transfers and stop with a counter value strictly below
the threshold, never triggering the completion interrupt.

[ ... ]
> @@ -1685,8 +1689,9 @@ static void xilinx_mcdma_start_transfer(struct xilinx_dma_chan *chan)
>  	dma_ctrl_write(chan, XILINX_MCDMA_CHAN_CR_OFFSET(chan->tdest), reg);
>  
>  	/* Program current descriptor */
> -	xilinx_write(chan, XILINX_MCDMA_CHAN_CDESC_OFFSET(chan->tdest),
> -		     head_desc->async_tx.phys);
> +	if (chan->has_sg && list_empty(&chan->active_list))
> +		xilinx_write(chan, XILINX_MCDMA_CHAN_CDESC_OFFSET(chan->tdest),
> +			     head_desc->async_tx.phys);
>  
>  	/* Program channel enable register */
>  	reg = dma_ctrl_read(chan, XILINX_MCDMA_CHEN_OFFSET);

[Severity: High]
This appears to introduce the same coalescing regression as seen in
xilinx_dma_start_transfer().

Earlier in xilinx_mcdma_start_transfer(), the CR register is overwritten
with the new pending count:

    if (chan->desc_pendingcount <= XILINX_MCDMA_COALESCE_MAX) {
        reg &= ~XILINX_MCDMA_COALESCE_MASK;
        reg |= chan->desc_pendingcount <<
            XILINX_MCDMA_COALESCE_SHIFT;
    }
    ...
    dma_ctrl_write(chan, XILINX_MCDMA_CHAN_CR_OFFSET(chan->tdest), reg);
    
Won't this similarly reset the MCDMA hardware coalescing counter while the
channel is active, causing it to stall indefinitely?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260626092656.1563871-1-suraj.gupta2@amd.com?part=2

Re: [PATCH v3 1/3] dmaengine: xilinx_dma: Fix channel idle state management in AXIDMA and MCDMA interrupt handlers

[Gupta, Suraj]

On 6/26/2026 3:17 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] The patch claims to fix a race condition for MCDMA channels by conditionally marking the channel as idle only when `active_list` is empty. However, this fix is entirely ineffective because `xilinx_dma_complete_descriptor()` unconditionally empties `active_list` for MCDMA.
> --
> 

I agree, MCDMA unconditionally empties active_list. It is being 
addressed as part of separate series:
https://lore.kernel.org/all/20260313062533.421249-3-srinivas.neeli@amd.com/


Regards,
Suraj

> commit fef85fe49f3788eecfb93a71efa6536e69b799c0
> Author: Suraj Gupta <suraj.gupta2@amd.com>
> 
> dmaengine: xilinx_dma: Fix channel idle state management in AXIDMA and MCDMA interrupt handlers
> 
> This patch fixes a race condition in AXIDMA and MCDMA irq handlers by only
> marking the channel idle and starting a new transfer when the active list is
> empty.
> 
>> diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
>> index 404235c173538..ca396b7097427 100644
>> --- a/drivers/dma/xilinx/xilinx_dma.c
>> +++ b/drivers/dma/xilinx/xilinx_dma.c
>> @@ -1893,8 +1893,10 @@ static irqreturn_t xilinx_mcdma_irq_handler(int irq, void *data)
>>   	if (status & XILINX_MCDMA_IRQ_IOC_MASK) {
>>   		spin_lock(&chan->lock);
>>   		xilinx_dma_complete_descriptor(chan);
>> -		chan->idle = true;
>> -		chan->start_transfer(chan);
>> +		if (list_empty(&chan->active_list)) {
> 
> [Severity: High]
> This is a pre-existing issue, but does this check actually prevent the race
> condition for MCDMA channels?
> 
> Looking at xilinx_dma_complete_descriptor(), the hardware completion check
> is restricted to AXIDMA:
> 
> drivers/dma/xilinx/xilinx_dma.c:xilinx_dma_complete_descriptor() {
> 	list_for_each_entry_safe(desc, next, &chan->active_list, node) {
> 		if (chan->xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA) {
> 			...
> 			if (!(seg->hw.status & XILINX_DMA_BD_COMP_MASK) && chan->has_sg)
> 				break;
> 		}
> 		...
> 		list_del(&desc->node);
> 		...
> 	}
> }
> 
> Because MCDMA descriptors are unconditionally unlinked from active_list
> regardless of their actual hardware completion status, active_list will
> always be empty after xilinx_dma_complete_descriptor() returns.
> 
> Will this cause the new idle check to always evaluate to true for MCDMA,
> leaving the race condition unaddressed for those channels?
> 
>> +			chan->idle = true;
>> +			chan->start_transfer(chan);
>> +		}
>>   		spin_unlock(&chan->lock);
>>   	}
> 
> [ ... ]
> 

Re: [PATCH v3 2/3] dmaengine: xilinx_dma: Enable transfer chaining for AXIDMA and MCDMA by removing idle restriction

[Gupta, Suraj]

On 6/26/2026 3:18 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Critical] Bypassing the idle check allows new descriptors to be appended to a running transfer, but the driver fails to link the active transfer's hardware `next_desc` pointer to the newly appended descriptors, leading to wild DMA fetches.
> - [High] Overwriting the DMA Coalesce Threshold register while a channel is running resets the hardware coalescing counter, causing the channel to stall indefinitely.
> --

I replied for same comments in V2 here:

https://lore.kernel.org/all/0f00a643-8aa0-48c0-b6e8-d1956e623cc4@amd.com/

Regards,
Suraj

> 
> commit 06494ab31953d1c39c050abca192d725ee06375d
> Author: Suraj Gupta <suraj.gupta2@amd.com>
> 
> dmaengine: xilinx_dma: Enable transfer chaining for AXIDMA and MCDMA by removing idle restriction
> 
> This commit relaxes the idle checks in xilinx_dma_start_transfer() and
> xilinx_mcdma_start_transfer() to allow scatter-gather transfers to be chained
> onto an in-flight transfer. It relies on the hardware tail pointer extension
> mechanism to handle chaining automatically.
> 
>> diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
>> index ca396b7097427..6e7b183cb4994 100644
>> --- a/drivers/dma/xilinx/xilinx_dma.c
>> +++ b/drivers/dma/xilinx/xilinx_dma.c
>> @@ -1580,7 +1580,14 @@ static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan)
>>   		return;
>>   	}
>>   
>> -	if (!chan->idle)
>> +	/*
>> +	 * Direct (non-SG) mode has no descriptor queue: writing the BTT
>> +	 * register launches a transfer immediately, so a new transfer must
>> +	 * not be programmed while one is in flight. Keep such transfers
>> +	 * serialized. SG mode supports chaining onto a running transfer via
>> +	 * tail-pointer extension, so it is allowed to proceed when busy.
>> +	 */
>> +	if (!chan->has_sg && !chan->idle)
>>   		return;
> 
> [Severity: Critical]
> Does bypassing this idle check for SG mode result in wild DMA fetches when a new
> transfer is queued on a busy channel?
> 
> If a new transfer is queued while the channel is busy, the pending_list will
> be empty when processed by append_desc_queue():
> 
> drivers/dma/xilinx/xilinx_dma.c:append_desc_queue() {
>      ...
>      if (list_empty(&chan->pending_list))
>          goto append;
>      ...
> }
> 
> Because of this, append_desc_queue() explicitly skips hardware chaining entirely
> and fails to link the active transfer's tail descriptor to the newly submitted
> descriptor.
> 
> Since out-of-order frees can scramble the ring at runtime, won't the hardware
> finish the active transfer and follow a stale next_desc pointer to an unrelated,
> uninitialized, or freed descriptor segment?
> 
> [ ... ]
>> @@ -1599,7 +1606,7 @@ static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan)
>>   		dma_ctrl_write(chan, XILINX_DMA_REG_DMACR, reg);
>>   	}
>>   
>> -	if (chan->has_sg)
>> +	if (chan->has_sg && list_empty(&chan->active_list))
>>   		xilinx_write(chan, XILINX_DMA_REG_CURDESC,
>>   			     head_desc->async_tx.phys);
>>   	reg  &= ~XILINX_DMA_CR_DELAY_MAX;
> 
> [Severity: High]
> Does updating the Coalesce Threshold register while a channel is running stall
> the channel indefinitely?
> 
> Just before this CURDESC check in xilinx_dma_start_transfer(), the threshold
> is updated:
> 
>      if (chan->desc_pendingcount <= XILINX_DMA_COALESCE_MAX) {
>          reg &= ~XILINX_DMA_CR_COALESCE_MAX;
>          reg |= chan->desc_pendingcount <<
>                    XILINX_DMA_CR_COALESCE_SHIFT;
>      }
>      ...
>      dma_ctrl_write(chan, XILINX_DMA_REG_DMACR, reg);
>      
> According to the Xilinx AXI DMA specification (PG021), writing this threshold
> automatically resets the hardware coalescing counter to zero.
> 
> If there are already active transfers running, writing the new threshold will
> reset the counter. As a result, the total number of remaining transfers
> (active + pending) will no longer align with the new threshold multiple. The
> hardware will finish the transfers and stop with a counter value strictly below
> the threshold, never triggering the completion interrupt.
> 
> [ ... ]
>> @@ -1685,8 +1689,9 @@ static void xilinx_mcdma_start_transfer(struct xilinx_dma_chan *chan)
>>   	dma_ctrl_write(chan, XILINX_MCDMA_CHAN_CR_OFFSET(chan->tdest), reg);
>>   
>>   	/* Program current descriptor */
>> -	xilinx_write(chan, XILINX_MCDMA_CHAN_CDESC_OFFSET(chan->tdest),
>> -		     head_desc->async_tx.phys);
>> +	if (chan->has_sg && list_empty(&chan->active_list))
>> +		xilinx_write(chan, XILINX_MCDMA_CHAN_CDESC_OFFSET(chan->tdest),
>> +			     head_desc->async_tx.phys);
>>   
>>   	/* Program channel enable register */
>>   	reg = dma_ctrl_read(chan, XILINX_MCDMA_CHEN_OFFSET);
> 
> [Severity: High]
> This appears to introduce the same coalescing regression as seen in
> xilinx_dma_start_transfer().
> 
> Earlier in xilinx_mcdma_start_transfer(), the CR register is overwritten
> with the new pending count:
> 
>      if (chan->desc_pendingcount <= XILINX_MCDMA_COALESCE_MAX) {
>          reg &= ~XILINX_MCDMA_COALESCE_MASK;
>          reg |= chan->desc_pendingcount <<
>              XILINX_MCDMA_COALESCE_SHIFT;
>      }
>      ...
>      dma_ctrl_write(chan, XILINX_MCDMA_CHAN_CR_OFFSET(chan->tdest), reg);
>      
> Won't this similarly reset the MCDMA hardware coalescing counter while the
> channel is active, causing it to stall indefinitely?
> 

Re: [PATCH v3 2/3] dmaengine: xilinx_dma: Enable transfer chaining for AXIDMA and MCDMA by removing idle restriction

[Folker Schwesinger]

On Fri Jun 26, 2026 at 11:26 AM CEST, Suraj Gupta wrote:
> Relax the idle check in xilinx_dma_start_transfer() and
> xilinx_mcdma_start_transfer() that prevented new transfers from being
> queued when the channel was busy, so scatter-gather transfers can be
> chained onto an in-flight transfer.
>
> In scatter-gather mode, only update the CURDESC register when the active
> list is empty to avoid interfering with transfers already in progress.
> When the active list contains transfers, the hardware tail pointer
> extension mechanism handles chaining automatically via the descriptor
> next pointer chain, which is set up at channel allocation and preserved
> across descriptor recycling.
>
> Direct (non-SG) mode has no descriptor queue: writing the BTT register
> launches a transfer immediately, so a new transfer must not be programmed
> while one is in flight. Keep those transfers serialized by retaining the
> idle check on the non-SG path. MCDMA always operates in scatter-gather
> mode, so it is unaffected.
>
> Signed-off-by: Suraj Gupta <suraj.gupta2@amd.com>
> Co-developed-by: Srinivas Neeli <srinivas.neeli@amd.com>
> Signed-off-by: Srinivas Neeli <srinivas.neeli@amd.com>

For the AXIDMA SG-path:

Tested-by: Folker Schwesinger <dev@folker-schwesinger.de>

> ---
>  drivers/dma/xilinx/xilinx_dma.c | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
> index ca396b709742..6e7b183cb499 100644
> --- a/drivers/dma/xilinx/xilinx_dma.c
> +++ b/drivers/dma/xilinx/xilinx_dma.c
> @@ -1580,7 +1580,14 @@ static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan)
>  		return;
>  	}
>  
> -	if (!chan->idle)
> +	/*
> +	 * Direct (non-SG) mode has no descriptor queue: writing the BTT
> +	 * register launches a transfer immediately, so a new transfer must
> +	 * not be programmed while one is in flight. Keep such transfers
> +	 * serialized. SG mode supports chaining onto a running transfer via
> +	 * tail-pointer extension, so it is allowed to proceed when busy.
> +	 */
> +	if (!chan->has_sg && !chan->idle)
>  		return;
>  
>  	head_desc = list_first_entry(&chan->pending_list,
> @@ -1599,7 +1606,7 @@ static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan)
>  		dma_ctrl_write(chan, XILINX_DMA_REG_DMACR, reg);
>  	}
>  
> -	if (chan->has_sg)
> +	if (chan->has_sg && list_empty(&chan->active_list))
>  		xilinx_write(chan, XILINX_DMA_REG_CURDESC,
>  			     head_desc->async_tx.phys);
>  	reg  &= ~XILINX_DMA_CR_DELAY_MAX;
> @@ -1660,9 +1667,6 @@ static void xilinx_mcdma_start_transfer(struct xilinx_dma_chan *chan)
>  	if (chan->err)
>  		return;
>  
> -	if (!chan->idle)
> -		return;
> -
>  	if (list_empty(&chan->pending_list))
>  		return;
>  
> @@ -1685,8 +1689,9 @@ static void xilinx_mcdma_start_transfer(struct xilinx_dma_chan *chan)
>  	dma_ctrl_write(chan, XILINX_MCDMA_CHAN_CR_OFFSET(chan->tdest), reg);
>  
>  	/* Program current descriptor */
> -	xilinx_write(chan, XILINX_MCDMA_CHAN_CDESC_OFFSET(chan->tdest),
> -		     head_desc->async_tx.phys);
> +	if (chan->has_sg && list_empty(&chan->active_list))
> +		xilinx_write(chan, XILINX_MCDMA_CHAN_CDESC_OFFSET(chan->tdest),
> +			     head_desc->async_tx.phys);
>  
>  	/* Program channel enable register */
>  	reg = dma_ctrl_read(chan, XILINX_MCDMA_CHEN_OFFSET);

Re: [PATCH v3 1/3] dmaengine: xilinx_dma: Fix channel idle state management in AXIDMA and MCDMA interrupt handlers

[Pandey, Radhey Shyam]

> Fix a race condition in AXIDMA and MCDMA irq handlers where the channel
> could be incorrectly marked as idle and attempt spurious transfers when
> descriptors are still being processed.
> 
> The issue occurs when:
> 1. Multiple descriptors are queued and active.
> 2. An interrupt fires after completing some descriptors.
> 3. xilinx_dma_complete_descriptor() moves completed descriptors to
> done_list.
> 4. Channel is marked idle and start_transfer() is called even though
>     active_list still contains unprocessed descriptors.
> 5. This leads to premature transfer attempts and potential descriptor
>     corruption or missed completions.
> 
> Only mark the channel as idle and start new transfers when the active list
> is actually empty, ensuring proper channel state management and avoiding
> spurious transfer attempts.
> 
> Fixes: c0bba3a99f07 ("dmaengine: vdma: Add Support for Xilinx AXI Direct Memory Access Engine")
> Tested-by: Folker Schwesinger <dev@folker-schwesinger.de>
> Signed-off-by: Suraj Gupta <suraj.gupta2@amd.com>
> Co-developed-by: Srinivas Neeli <srinivas.neeli@amd.com>
> Signed-off-by: Srinivas Neeli <srinivas.neeli@amd.com>

Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
Thanks!

> ---
>   drivers/dma/xilinx/xilinx_dma.c | 12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
> index 404235c17353..ca396b709742 100644
> --- a/drivers/dma/xilinx/xilinx_dma.c
> +++ b/drivers/dma/xilinx/xilinx_dma.c
> @@ -1893,8 +1893,10 @@ static irqreturn_t xilinx_mcdma_irq_handler(int irq, void *data)
>   	if (status & XILINX_MCDMA_IRQ_IOC_MASK) {
>   		spin_lock(&chan->lock);
>   		xilinx_dma_complete_descriptor(chan);
> -		chan->idle = true;
> -		chan->start_transfer(chan);
> +		if (list_empty(&chan->active_list)) {
> +			chan->idle = true;
> +			chan->start_transfer(chan);
> +		}
>   		spin_unlock(&chan->lock);
>   	}
>   
> @@ -1950,8 +1952,10 @@ static irqreturn_t xilinx_dma_irq_handler(int irq, void *data)
>   		      XILINX_DMA_DMASR_DLY_CNT_IRQ)) {
>   		spin_lock(&chan->lock);
>   		xilinx_dma_complete_descriptor(chan);
> -		chan->idle = true;
> -		chan->start_transfer(chan);
> +		if (list_empty(&chan->active_list)) {
> +			chan->idle = true;
> +			chan->start_transfer(chan);
> +		}
>   		spin_unlock(&chan->lock);
>   	}
>   

Re: [PATCH v3 2/3] dmaengine: xilinx_dma: Enable transfer chaining for AXIDMA and MCDMA by removing idle restriction

[Pandey, Radhey Shyam]

> Relax the idle check in xilinx_dma_start_transfer() and
> xilinx_mcdma_start_transfer() that prevented new transfers from being
> queued when the channel was busy, so scatter-gather transfers can be
> chained onto an in-flight transfer.
> 
> In scatter-gather mode, only update the CURDESC register when the active
> list is empty to avoid interfering with transfers already in progress.
> When the active list contains transfers, the hardware tail pointer
> extension mechanism handles chaining automatically via the descriptor
> next pointer chain, which is set up at channel allocation and preserved
> across descriptor recycling.
> 
> Direct (non-SG) mode has no descriptor queue: writing the BTT register
> launches a transfer immediately, so a new transfer must not be programmed
> while one is in flight. Keep those transfers serialized by retaining the
> idle check on the non-SG path. MCDMA always operates in scatter-gather
> mode, so it is unaffected.
> 
> Signed-off-by: Suraj Gupta <suraj.gupta2@amd.com>
> Co-developed-by: Srinivas Neeli <srinivas.neeli@amd.com>
> Signed-off-by: Srinivas Neeli <srinivas.neeli@amd.com>

Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
Thanks!

> ---
>   drivers/dma/xilinx/xilinx_dma.c | 19 ++++++++++++-------
>   1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
> index ca396b709742..6e7b183cb499 100644
> --- a/drivers/dma/xilinx/xilinx_dma.c
> +++ b/drivers/dma/xilinx/xilinx_dma.c
> @@ -1580,7 +1580,14 @@ static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan)
>   		return;
>   	}
>   
> -	if (!chan->idle)
> +	/*
> +	 * Direct (non-SG) mode has no descriptor queue: writing the BTT
> +	 * register launches a transfer immediately, so a new transfer must
> +	 * not be programmed while one is in flight. Keep such transfers
> +	 * serialized. SG mode supports chaining onto a running transfer via
> +	 * tail-pointer extension, so it is allowed to proceed when busy.
> +	 */
> +	if (!chan->has_sg && !chan->idle)
>   		return;
>   
>   	head_desc = list_first_entry(&chan->pending_list,
> @@ -1599,7 +1606,7 @@ static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan)
>   		dma_ctrl_write(chan, XILINX_DMA_REG_DMACR, reg);
>   	}
>   
> -	if (chan->has_sg)
> +	if (chan->has_sg && list_empty(&chan->active_list))
>   		xilinx_write(chan, XILINX_DMA_REG_CURDESC,
>   			     head_desc->async_tx.phys);
>   	reg  &= ~XILINX_DMA_CR_DELAY_MAX;
> @@ -1660,9 +1667,6 @@ static void xilinx_mcdma_start_transfer(struct xilinx_dma_chan *chan)
>   	if (chan->err)
>   		return;
>   
> -	if (!chan->idle)
> -		return;
> -
>   	if (list_empty(&chan->pending_list))
>   		return;
>   
> @@ -1685,8 +1689,9 @@ static void xilinx_mcdma_start_transfer(struct xilinx_dma_chan *chan)
>   	dma_ctrl_write(chan, XILINX_MCDMA_CHAN_CR_OFFSET(chan->tdest), reg);
>   
>   	/* Program current descriptor */
> -	xilinx_write(chan, XILINX_MCDMA_CHAN_CDESC_OFFSET(chan->tdest),
> -		     head_desc->async_tx.phys);
> +	if (chan->has_sg && list_empty(&chan->active_list))
> +		xilinx_write(chan, XILINX_MCDMA_CHAN_CDESC_OFFSET(chan->tdest),
> +			     head_desc->async_tx.phys);
>   
>   	/* Program channel enable register */
>   	reg = dma_ctrl_read(chan, XILINX_MCDMA_CHEN_OFFSET);

Re: [PATCH v3 3/3] dmaengine: xilinx_dma: Optimize control register write and channel start logic for AXIDMA and MCDMA in corresponding start_transfer()

[Pandey, Radhey Shyam]

> Optimize AXI DMA control register programming by consolidating
> coalesce count and delay configuration into a single register write.
> Previously, the coalesce count was written separately from the delay
> configuration, resulting in two register writes. Combine these into
> one write operation to reduce bus overhead.
> Additionally, avoid redundant channel starts in xilinx_dma_start_transfer()
> and xilinx_mcdma_start_transfer() by only calling xilinx_dma_start() when
> the channel is actually idle.
> 
> Tested-by: Folker Schwesinger <dev@folker-schwesinger.de>
> Signed-off-by: Suraj Gupta <suraj.gupta2@amd.com>
> Co-developed-by: Srinivas Neeli <srinivas.neeli@amd.com>
> Signed-off-by: Srinivas Neeli <srinivas.neeli@amd.com>

Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
Thanks!

> ---
>   drivers/dma/xilinx/xilinx_dma.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
> index 6e7b183cb499..829601d8a16f 100644
> --- a/drivers/dma/xilinx/xilinx_dma.c
> +++ b/drivers/dma/xilinx/xilinx_dma.c
> @@ -1603,7 +1603,6 @@ static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan)
>   		reg &= ~XILINX_DMA_CR_COALESCE_MAX;
>   		reg |= chan->desc_pendingcount <<
>   				  XILINX_DMA_CR_COALESCE_SHIFT;
> -		dma_ctrl_write(chan, XILINX_DMA_REG_DMACR, reg);
>   	}
>   
>   	if (chan->has_sg && list_empty(&chan->active_list))
> @@ -1614,7 +1613,8 @@ static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan)
>   	reg |= XILINX_DMA_DMAXR_ALL_IRQ_MASK;
>   	dma_ctrl_write(chan, XILINX_DMA_REG_DMACR, reg);
>   
> -	xilinx_dma_start(chan);
> +	if (chan->idle)
> +		xilinx_dma_start(chan);
>   
>   	if (chan->err)
>   		return;
> @@ -1703,7 +1703,8 @@ static void xilinx_mcdma_start_transfer(struct xilinx_dma_chan *chan)
>   	reg |= XILINX_MCDMA_CR_RUNSTOP_MASK;
>   	dma_ctrl_write(chan, XILINX_MCDMA_CHAN_CR_OFFSET(chan->tdest), reg);
>   
> -	xilinx_dma_start(chan);
> +	if (chan->idle)
> +		xilinx_dma_start(chan);
>   
>   	if (chan->err)
>   		return;

Re: [PATCH v3 0/3] dmaengine: xilinx_dma: Fixes and optimizations for AXIDMA and MCDMA channel management

[Vinod Koul]

On Fri, 26 Jun 2026 14:56:53 +0530, Suraj Gupta wrote:
> This patch series addresses issues and optimizations in the Xilinx
> AXI DMA and MCDMA drivers:
> 1. Fix channel idle state management in the interrupt handlers.
> 2. Enable transfer chaining by removing unnecessary idle restrictions.
> 3. Optimize control register writes and channel start logic.
> 
> Note: The patches in this series were part of following IRQ coalescing
> series which is under discussion:
> https://lore.kernel.org/all/20250710101229.804183-1-suraj.gupta2@amd.com/
> 
> [...]

Applied, thanks!

[1/3] dmaengine: xilinx_dma: Fix channel idle state management in AXIDMA and MCDMA interrupt handlers
      commit: 0b6d055edb55ecadadf54e930c2b4fab76fa9a5a
[2/3] dmaengine: xilinx_dma: Enable transfer chaining for AXIDMA and MCDMA by removing idle restriction
      commit: 6078690034790131b9a59081bdf30e26de2254af
[3/3] dmaengine: xilinx_dma: Optimize control register write and channel start logic for AXIDMA and MCDMA in corresponding start_transfer()
      commit: 887b3119380cde56f648130029062c223341a1b3

Best regards,
-- 
~Vinod