Re: [PATCH v3 2/3] dmaengine: dw-edma: Configure remote interrupt routing

On Sat, Sep 05, 2026 at 02:15:38AM +0900, Koichiro Den wrote:

On Fri, Sep 04, 2026 at 10:41:47AM -0500, Frank Li wrote:

quoted

On Thu, Sep 03, 2026 at 03:45:32PM +0900, Koichiro Den wrote:

quoted

An endpoint function can reserve an endpoint-local channel while the RC
programs it through an exposed register window. Such a channel must route
interrupts remotely and ignore them on the endpoint.

Use dma_slave_config to set per-channel interrupt routing on idle channels
of a local eDMA-compatible instance. Releasing a remote-routed channel
quiesces the hardware and drains its local IRQ before restoring the default
routing.

The eDMA quiesce may stop a complete direction. The caller must own every
channel in that direction and stop remote programming first.

Suggested-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Koichiro Den <redacted>
---
Changes in v3:
  - Add struct dw_edma_chan_config with validity flags for non-LL mode and
    interrupt routing. (Frank)
    https://lore.kernel.org/r/apWj7zzAw57U2r4F@SMW015318/ (local)
  - Do not skip generic dma_slave_config fields when private settings are
    supplied. (Sashiko)
    https://lore.kernel.org/r/20260828164950.61AE51F000E9@smtp.kernel.org/ (local)
  - Use READ_ONCE()/WRITE_ONCE() for IRQ mode accessed by shared handlers
    and drain the channel IRQ before restoring local routing. (Sashiko)

 drivers/dma/dw-edma/dw-edma-core.c | 135 ++++++++++++++++++++++-------
 drivers/dma/dw-edma/dw-edma-core.h |   5 +-
 include/linux/dma/edma.h           |  19 ++++
 3 files changed, 126 insertions(+), 33 deletions(-)

diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index a678c70a78fe..e3786d960a43 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c

@@ -177,48 +177,79 @@ dw_edma_get_default_irq_mode(struct dw_edma_chan *chan)
 						  DW_EDMA_CH_IRQ_REMOTE;
 }

+static int dw_edma_device_config_irq_mode(struct dw_edma_chan *chan,
+					  enum dw_edma_ch_irq_mode mode)
+{
+	if (!(chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL) ||
+	    (mode != DW_EDMA_CH_IRQ_LOCAL && mode != DW_EDMA_CH_IRQ_REMOTE))
+		return -EINVAL;
+
+	guard(spinlock_irqsave)(&chan->vc.lock);
+
+	if (chan->configured || chan->status != EDMA_ST_IDLE ||
+	    chan->request != EDMA_REQ_NONE)
+		return -EBUSY;
+
+	WRITE_ONCE(chan->irq_mode, mode);
+
+	return 0;
+}
+
 static int dw_edma_device_config(struct dma_chan *dchan,
 				 struct dma_slave_config *config)
 {
+	const struct dw_edma_chan_config *dw_config = config->peripheral_config;
 	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
-	bool cfg_non_ll;
-	int non_ll = 0;
-
-	chan->non_ll = false;
-	if (chan->dw->chip->mf == EDMA_MF_HDMA_NATIVE) {
-		if (config->peripheral_config &&
-		    config->peripheral_size != sizeof(int)) {
-			dev_err(dchan->device->dev,
-				"config param peripheral size mismatch\n");
+	bool non_ll = false;
+	u32 flags = 0;
+	int ret;
+
+	if (dw_config) {
+		if (config->peripheral_size != sizeof(*dw_config) ||
+		    dw_config->flags & ~(DW_EDMA_CH_CONFIG_NON_LL |
+					 DW_EDMA_CH_CONFIG_IRQ_MODE))
+			return -EINVAL;
+		flags = dw_config->flags;
+	}
+
+	/*
+	 * When there is no valid LLP base address available then the
+	 * default DMA ops will use the non-LL mode.
+	 *
+	 * When LL mode is the default, clients can request non-LL mode
+	 * through DW_EDMA_CH_CONFIG_NON_LL.
+	 */
+	non_ll = chan->dw->chip->mf == EDMA_MF_HDMA_NATIVE &&
+		 chan->dw->chip->cfg_non_ll;
+
+	if (flags & DW_EDMA_CH_CONFIG_NON_LL) {
+		if (chan->dw->chip->mf != EDMA_MF_HDMA_NATIVE)
+			return -EINVAL;
+
+		if (chan->dw->chip->cfg_non_ll && !dw_config->non_ll) {
+			dev_err(dchan->device->dev, "invalid configuration\n");
 			return -EINVAL;
 		}

-		/*
-		 * When there is no valid LLP base address available then the
-		 * default DMA ops will use the non-LL mode.
-		 *
-		 * Cases where LL mode is enabled and client wants to use the
-		 * non-LL mode then also client can do so via providing the
-		 * peripheral_config param.
-		 */
-		cfg_non_ll = chan->dw->chip->cfg_non_ll;
-		if (config->peripheral_config) {
-			non_ll = *(int *)config->peripheral_config;
+		non_ll = dw_config->non_ll;
+	}

-			if (cfg_non_ll && !non_ll) {
-				dev_err(dchan->device->dev, "invalid configuration\n");
-				return -EINVAL;
-			}
+	if (flags & DW_EDMA_CH_CONFIG_IRQ_MODE) {
+		switch (chan->dw->chip->mf) {
+		case EDMA_MF_EDMA_LEGACY:
+		case EDMA_MF_EDMA_UNROLL:
+		case EDMA_MF_HDMA_COMPAT:
+			break;
+		default:
+			return -EINVAL;
 		}

-		if (cfg_non_ll || non_ll)
-			chan->non_ll = true;
-	} else if (config->peripheral_config) {
-		dev_err(dchan->device->dev,
-			"peripheral config param applicable only for HDMA\n");
-		return -EINVAL;
+		ret = dw_edma_device_config_irq_mode(chan, dw_config->irq_mode);
+		if (ret)
+			return ret;
 	}

+	chan->non_ll = non_ll;
 	memcpy(&chan->config, config, sizeof(*config));
 	chan->configured = true;

@@ -890,11 +921,48 @@ static void dw_edma_wait_termination(struct dma_chan *dchan)
 		 "timeout waiting for channel termination\n");
 }

+static void dw_edma_synchronize_chan_irq(struct dw_edma_chan *chan)
+{
+	struct dw_edma *dw = chan->dw;
+	unsigned long *mask;
+	int i;
+
+	/*
+	 * With nr_irqs == 1, the common handler can enter for the other direction
+	 * and retain a status snapshot for the remotely owned direction across
+	 * quiesce. With multiple IRQs, the handler covering this channel can likewise
+	 * enter for another IRQ sharer. Drain the IRQ whose mask contains the channel
+	 * before restoring local routing.
+	 */
+	for (i = 0; i < dw->nr_irqs; i++) {
+		mask = chan->dir == EDMA_DIR_WRITE ? dw->irq[i].wr_mask :
+						     dw->irq[i].rd_mask;
+		if (!test_bit(chan->id, mask))
+			continue;
+
+		synchronize_irq(dw->chip->ops->irq_vector(dw->chip->dev, i));
+		return;
+	}
+}
+
 static void dw_edma_device_synchronize(struct dma_chan *dchan)
 {
 	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
+	bool remote;
+
+	scoped_guard(spinlock_irqsave, &chan->vc.lock)
+		remote = chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL &&
+			 chan->irq_mode == DW_EDMA_CH_IRQ_REMOTE;
+
+	if (remote && dw_edma_core_ch_quiesce(chan))
+		dev_warn(chan->dw->chip->dev,
+			 "failed to quiesce remote-routed %s channel %u\n",
+			 chan->dir == EDMA_DIR_WRITE ? "write" : "read",
+			 chan->id);

 	dw_edma_wait_termination(dchan);
+	if (remote)
+		dw_edma_synchronize_chan_irq(chan);
 	cancel_work_sync(&chan->irq_work);
 	atomic_set(&chan->irq_pending, 0);
 	vchan_synchronize(&chan->vc);

@@ -903,12 +971,17 @@ static void dw_edma_device_synchronize(struct dma_chan *dchan)
 static void dw_edma_free_chan_resources(struct dma_chan *dchan)
 {
 	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
+	enum dw_edma_ch_irq_mode default_mode =
+			dw_edma_get_default_irq_mode(chan);

 	dw_edma_device_terminate_all(dchan);
 	dw_edma_device_synchronize(dchan);

-	scoped_guard(spinlock_irqsave, &chan->vc.lock)
+	scoped_guard(spinlock_irqsave, &chan->vc.lock) {
 		chan->configured = false;
+		if (chan->irq_mode != default_mode)
+			WRITE_ONCE(chan->irq_mode, default_mode);
+	}

 	vchan_free_chan_resources(&chan->vc);
 }

diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
index f6a5ad317567..275d362d1805 100644
--- a/drivers/dma/dw-edma/dw-edma-core.h
+++ b/drivers/dma/dw-edma/dw-edma-core.h

@@ -280,11 +280,12 @@ static inline bool
 dw_edma_core_ch_ignore_irq(struct dw_edma_chan *chan)
 {
 	struct dw_edma *dw = chan->dw;
+	enum dw_edma_ch_irq_mode mode = READ_ONCE(chan->irq_mode);

 	if (dw->chip->flags & DW_EDMA_CHIP_LOCAL)
-		return chan->irq_mode == DW_EDMA_CH_IRQ_REMOTE;
+		return mode == DW_EDMA_CH_IRQ_REMOTE;
 	else
-		return chan->irq_mode == DW_EDMA_CH_IRQ_LOCAL;
+		return mode == DW_EDMA_CH_IRQ_LOCAL;
 }

does this tunk fix something, you make this change later.

I added this for a possible race with the shared IRQ handler, especially when
nr_irqs == 1.

... but after carefully thinking about this again, the hunk seems unnecessary.

- Before undelegating (changing the irq_mode from REMOTE to LOCAL), the client
  must stop remote programming, and the driver quiesces the direction and
  synchronizes the local IRQ.
  At that point, no-one is supposed to issue any transfer for the channel
- Conversely, remote routing is configured while the channel is idle, before the
  peer starts programming it.

If the peer continued programming during either transition, it just breaks
things. READ_ONCE()/WRITE_ONCE() would not make the handoff safe.

I will drop this hunk and the corresponding WRITE_ONCE() use.

quoted

quoted

 #endif /* _DW_EDMA_CORE_H */

diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
index 3c8e2ef9dee0..93f866d57987 100644
--- a/include/linux/dma/edma.h
+++ b/include/linux/dma/edma.h

@@ -101,6 +101,25 @@ enum dw_edma_ch_irq_mode {
 	DW_EDMA_CH_IRQ_REMOTE,
 };

+#define DW_EDMA_CH_CONFIG_NON_LL	BIT(0)

you need update other non_ll user to set DW_EDMA_CH_CONFIG_NON_LL.

Ugh, I missed updating the existing non-LL user. I will respin.

Sorry, I spoke too soon here. I now remember checking this while preparing v3
and finding no existing in-tree non-LL peripheral_config user.

I just rechecked the dmaengine remote branches (and also Devendra's recent
submissions just in case), but still could not find one.
Did you have a particular user in mind?

Best regards,
Koichiro

Thanks for the review.

Best regards,
Koichiro

quoted

Frank

quoted

+#define DW_EDMA_CH_CONFIG_IRQ_MODE	BIT(1)
+
+/**
+ * struct dw_edma_chan_config - dw-edma channel configuration
+ * @flags: fields selected by DW_EDMA_CH_CONFIG_*
+ * @non_ll: use HDMA non-linked-list mode
+ * @irq_mode: interrupt routing mode
+ *
+ * Pass this structure through dma_slave_config.peripheral_config. Before
+ * synchronizing a remote-routed channel, the client must stop remote
+ * programming and own every channel affected by the hardware quiesce.
+ */
+struct dw_edma_chan_config {
+	u32 flags;
+	bool non_ll;
+	enum dw_edma_ch_irq_mode irq_mode;
+};
+
 /**
  * struct dw_edma_chip - representation of DesignWare eDMA controller hardware
  * @dev:		 struct device of the eDMA controller
--
2.51.0