Hi,

On 12/11/2025 14:22, Gupta, Suraj wrote:

[Public]

quoted

-----Original Message-----
From: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Sent: Tuesday, November 11, 2025 8:37 PM
To: Vinod Koul <vkoul@kernel.org>; Simek, Michal <michal.simek@amd.com>
Cc: dmaengine@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
kernel@vger.kernel.org; Sagar, Vishal [off-list ref]; Tomi
Valkeinen [off-list ref]
Subject: [PATCH] dmaengine: xilinx_dma: Enable IRQs for AXIDMA in
start_transfer

Caution: This message originated from an External Source. Use proper
caution when opening attachments, clicking links, or responding.


A single AXIDMA controller can have one or two channels. When it has two
channels, the reset for both are tied together: resetting one channel
resets the other as well. This creates a problem where resetting one
channel will reset the registers for both channels, including clearing
interrupt enable bits for the other channel, which can then lead  to
timeouts as the driver is waiting for an interrupt which never comes.

The driver currently has a probe-time work around for this: when a
channel is created, the driver also resets and enables the
interrupts. With two channels the reset for the second channel will
clear the interrupt enables for the first one. The work around in the
driver is just to manually enable the interrupts again in
xilinx_dma_alloc_chan_resources().

This workaround only addresses the probe-time issue. When channels are
reset at runtime (e.g., in xilinx_dma_terminate_all() or during error
recovery), there's no corresponding mechanism to restore the other
channel's interrupt enables. This leads to one channel having its
interrupts disabled while the driver expects them to work, causing
timeouts and DMA failures.

A proper fix is a complicated matter, as we should not reset the other
channel when it's operating normally. So, perhaps, there should be some
kind of synchronization for a common reset, which is not trivial to
implement. To add to the complexity, the driver also supports other DMA
types, like VDMA, CDMA and MCDMA, which don't have a shared reset.

However, when the two-channel AXIDMA is used in the (assumably) normal
use case, providing DMA for a single memory-to-memory device, the
common
reset is a bit smaller issue: when something bad happens on one channel,
or when one channel is terminated, the assumption is that we also want
to terminate the other channel. And thus resetting both at the same time
is "ok".

With that line of thinking we can implement a bit better work around
than just the current probe time work around: let's enable the
AXIDMA interrupts at xilinx_dma_start_transfer() instead.
This ensures interrupts are enabled whenever a transfer starts,
regardless of any prior resets that may have cleared them.

This approach is also more logical: enable interrupts only when needed
for a transfer, rather than at resource allocation time, and, I think,
all the other DMA types should also use this model, but I'm reluctant to
do such changes as I cannot test them.

The reset function still enables interrupts even though it's not needed
for AXIDMA anymore, but it's common code for all DMA types (VDMA, CDMA,
MCDMA), so leave it unchanged to avoid affecting other variants.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---

A warning has long been included in the PG stating that multichannel support for AXI DMA will be discontinued. Please refer 2022.1 AXIDMA PG:
 https://www.amd.com/content/dam/xilinx/support/documents/ip_documentation/axi_dma/v7_1/pg021_axi_dma.pdf : Product Specification/Multichannel DMA support.
Therefore, it's unnecessary to make changes for multiple channel support.

A warning in a document doesn't help the users who use AXI DMA with
multichannel, and have the kernel driver time-outing.

The multichannel AXI DMA is supported in the latest Vivado release. And
even if it wasn't, in my experience people often use older Vivado
releases. And even if they didn't, the IP has been out there for a long
time, so there are existing FPGA bitstreams with multichannel AXI DMA in
use.

I realized this patch should say it's a fix in the subject line, and
have a Fixes tag, so I'll send a v2.

 Tomi

quoted

 drivers/dma/xilinx/xilinx_dma.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index a34d8f0ceed8..50dd93ce6741 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c

@@ -1216,14 +1216,6 @@ static int xilinx_dma_alloc_chan_resources(struct

dma_chan *dchan)

        dma_cookie_init(dchan);

-       if (chan->xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA) {
-               /* For AXI DMA resetting once channel will reset the
-                * other channel as well so enable the interrupts here.
-                */
-               dma_ctrl_set(chan, XILINX_DMA_REG_DMACR,
-                             XILINX_DMA_DMAXR_ALL_IRQ_MASK);
-       }
-
        if ((chan->xdev->dma_config->dmatype == XDMA_TYPE_CDMA) &&
chan->has_sg)
                dma_ctrl_set(chan, XILINX_DMA_REG_DMACR,
                             XILINX_CDMA_CR_SGMODE);

@@ -1572,6 +1564,7 @@ static void xilinx_dma_start_transfer(struct

xilinx_dma_chan *chan)
                             head_desc->async_tx.phys);
        reg  &= ~XILINX_DMA_CR_DELAY_MAX;
        reg  |= chan->irq_delay << XILINX_DMA_CR_DELAY_SHIFT;
+       reg |= XILINX_DMA_DMAXR_ALL_IRQ_MASK;
        dma_ctrl_write(chan, XILINX_DMA_REG_DMACR, reg);

        xilinx_dma_start(chan);

---
base-commit: e5f0a698b34ed76002dc5cff3804a61c80233a7a
change-id: 20251111-xilinx-dma-fix-bc26a6e9be5a

Best regards,
--
Tomi Valkeinen [off-list ref]