Message-ID:
 <SA1PR12MB6798CC79FE8DA14ED2614771C9CCA@SA1PR12MB6798.namprd12.prod.outlook.com>
Date: Wed, 12 Nov 2025 12:22:59 +0000
From: "Gupta, Suraj" <Suraj.Gupta2@....com>
To: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>, Vinod Koul
	<vkoul@...nel.org>, "Simek, Michal" <michal.simek@....com>
CC: "dmaengine@...r.kernel.org" <dmaengine@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "Sagar, Vishal" <vishal.sagar@....com>
Subject: RE: [PATCH] dmaengine: xilinx_dma: Enable IRQs for AXIDMA in
 start_transfer

[Public]

> -----Original Message-----
> From: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
> Sent: Tuesday, November 11, 2025 8:37 PM
> To: Vinod Koul <vkoul@...nel.org>; Simek, Michal <michal.simek@....com>
> Cc: dmaengine@...r.kernel.org; linux-arm-kernel@...ts.infradead.org; linux-
> kernel@...r.kernel.org; Sagar, Vishal <vishal.sagar@....com>; Tomi
> Valkeinen <tomi.valkeinen@...asonboard.com>
> 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@...asonboard.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.

>  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 <tomi.valkeinen@...asonboard.com>
>

Powered by blists - more mailing lists