[PATCH v3 0/3] dmaengine: dw-edma: Prepare channels for remote use

[PATCH v3 0/3] dmaengine: dw-edma: Prepare channels for remote use

[Koichiro Den]

Hi,

This small series contains standalone refactoring and new infrastructure
for dmaengine and dw-edma.

It prepares PCIe EPC-local DMA channels for remote use. The upcoming
vNTB-embedded DMA support [1] will be the first user and depend on this
series.

[1] https://lore.kernel.org/r/20260831182657.329614-1-den@valinux.co.jp/
    (This is v3. I will send v4 shortly.)

Best regards,
Koichiro
---
Changes in v3:
  - Rework patch 2 to use a common channel configuration, retain generic
    dma_slave_config fields, and close the shared-IRQ routing race.
    (Frank, Sashiko)
  - No code changes in patches 1 and 3.

Changes in v2:
  - Split and rework vNTB v1 patches 1, 4, and 5 into this prerequisite
    series.
  - Fold in the relevant PCI DMA EPF v7 review.

v2: https://lore.kernel.org/r/20260828163611.2691264-1-den@valinux.co.jp/
v1: https://lore.kernel.org/r/20260312165005.1148676-1-den@valinux.co.jp/


Koichiro Den (3):
  dmaengine: Allow drivers to assign static channel IDs
  dmaengine: dw-edma: Configure remote interrupt routing
  dmaengine: dw-edma: Account for the MSI vector offset

 drivers/dma/dmaengine.c            |  13 ++-
 drivers/dma/dw-edma/dw-edma-core.c | 160 +++++++++++++++++++++--------
 drivers/dma/dw-edma/dw-edma-core.h |   5 +-
 include/linux/dma/edma.h           |  19 ++++
 include/linux/dmaengine.h          |  20 ++++
 5 files changed, 170 insertions(+), 47 deletions(-)

base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
-- 
2.51.0

[PATCH v3 1/3] dmaengine: Allow drivers to assign static channel IDs

[Koichiro Den]

The dmaengine core assigns channel IDs in registration order. If a driver
skips a hardware channel, chan_id can differ from the hardware numbering
and a client cannot reliably correlate a requested channel with hardware
resources.

Let a driver request an exact channel ID before device registration.
Reserve static IDs through the existing IDA so they remain unique, while
retaining automatic IDA allocation as the default.

For example, idma32 uses chan_id to select DMA_CTL_CH() and
DMA_XBAR_SEL(), so it relies on ascending registration order to match
chan_id with the hardware channel number.

Use direction-flattened IDs for dw-edma channels. Unlike the
direction-local hardware channel number, these IDs are unique within the
DMA device.

Suggested-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v3:
  - No changes.

 drivers/dma/dmaengine.c            | 13 ++++++++-----
 drivers/dma/dw-edma/dw-edma-core.c |  1 +
 include/linux/dmaengine.h          | 20 ++++++++++++++++++++
 3 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 6ffd8bd82154..cc64a4679e6f 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -1078,6 +1078,7 @@ static int __dma_async_device_channel_register(struct dma_device *device,
 					       struct dma_chan *chan,
 					       const char *name)
 {
+	unsigned int id;
 	int rc;
 
 	chan->local = alloc_percpu(typeof(*chan->local));
@@ -1089,11 +1090,13 @@ static int __dma_async_device_channel_register(struct dma_device *device,
 		goto err_free_local;
 	}
 
-	/*
-	 * When the chan_id is a negative value, we are dynamically adding
-	 * the channel. Otherwise we are static enumerating.
-	 */
-	chan->chan_id = ida_alloc(&device->chan_ida, GFP_KERNEL);
+	if (chan->chan_id & DMA_CHAN_ID_STATIC) {
+		id = chan->chan_id & ~DMA_CHAN_ID_STATIC;
+		chan->chan_id = ida_alloc_range(&device->chan_ida, id, id,
+						GFP_KERNEL);
+	} else {
+		chan->chan_id = ida_alloc(&device->chan_ida, GFP_KERNEL);
+	}
 	if (chan->chan_id < 0) {
 		pr_err("%s: unable to alloc ida for chan: %d\n",
 		       __func__, chan->chan_id);
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index 03b2c2188351..a678c70a78fe 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -988,6 +988,7 @@ static int dw_edma_channel_setup(struct dw_edma *dw, u32 wr_alloc, u32 rd_alloc)
 					&dw->chip->dt_region_rd[chan->id];
 
 		vchan_init(&chan->vc, dma);
+		dmaengine_set_static_chan_id(&chan->vc.chan, i);
 
 		dw_edma_core_ch_config(chan);
 	}
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index fe33a20abc61..f669b79d7731 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -369,6 +369,26 @@ struct dma_chan {
 	void *private;
 };
 
+#define DMA_CHAN_ID_STATIC	BIT(30)
+
+/**
+ * dmaengine_set_static_chan_id - request an exact DMA engine channel ID
+ * @chan: DMA channel
+ * @id: channel ID, unique within the DMA device
+ *
+ * Drivers may call this after initializing @chan and before registering its
+ * DMA device. The dmaengine core reserves @id from the device IDA instead of
+ * assigning the next available ID.
+ */
+static inline void dmaengine_set_static_chan_id(struct dma_chan *chan,
+						unsigned int id)
+{
+	if (WARN_ON_ONCE(id >= DMA_CHAN_ID_STATIC))
+		return;
+
+	chan->chan_id = DMA_CHAN_ID_STATIC | id;
+}
+
 /**
  * struct dma_chan_dev - relate sysfs device node to backing channel device
  * @chan: driver channel device
-- 
2.51.0

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

[Koichiro Den]

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 <den@valinux.co.jp>
---
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/
  - Do not skip generic dma_slave_config fields when private settings are
    supplied. (Sashiko)
    https://lore.kernel.org/r/20260828164950.61AE51F000E9@smtp.kernel.org/
  - 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;
 }
 
 #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)
+#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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Overly restrictive check in dw_edma_device_config_irq_mode unconditionally rejects valid repeated dmaengine_slave_config calls.
--

commit a84fcfcb09487df4c48ca1e301c604e49afc0e26
Author: Koichiro Den <den@valinux.co.jp>

dmaengine: dw-edma: Configure remote interrupt routing

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.

> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index a678c70a78fe6..e3786d960a431 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;

[Severity: High]
Does this unconditionally reject valid repeated dmaengine_slave_config() calls?

A caller that populates dma_slave_config.peripheral_config with
DW_EDMA_CH_CONFIG_IRQ_MODE and calls dmaengine_slave_config() multiple times
to update standard fields (like src_addr or dst_addr) will face an unexpected
-EBUSY error on all calls after the first.

This happens because chan->configured is set to true at the end of
dw_edma_device_config(), so subsequent calls fail here even if the requested
IRQ mode is identical and the channel is idle.

Should this gracefully allow the call if chan->irq_mode == mode?

> +
> +	WRITE_ONCE(chan->irq_mode, mode);
> +
> +	return 0;
> +}
> +
>  static int dw_edma_device_config(struct dma_chan *dchan,
>  				 struct dma_slave_config *config)
>  {

[ ... ]

> +	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;

[Severity: High]
This is where chan->configured is set to true on the first call, triggering
the -EBUSY on all subsequent calls to dw_edma_device_config_irq_mode().

>  
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903064533.2269557-1-den@valinux.co.jp?part=2

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

[Frank Li]

On Thu, Sep 03, 2026 at 03:45:32PM +0900, Koichiro Den wrote:
> 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 <den@valinux.co.jp>
> ---
> 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/
>   - Do not skip generic dma_slave_config fields when private settings are
>     supplied. (Sashiko)
>     https://lore.kernel.org/r/20260828164950.61AE51F000E9@smtp.kernel.org/
>   - 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.

>
>  #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.

Frank

> +#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
>

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

[Koichiro Den]

On Fri, Sep 04, 2026 at 10:41:47AM -0500, Frank Li wrote:
> On Thu, Sep 03, 2026 at 03:45:32PM +0900, Koichiro Den wrote:
> > 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 <den@valinux.co.jp>
> > ---
> > 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/
> >   - Do not skip generic dma_slave_config fields when private settings are
> >     supplied. (Sashiko)
> >     https://lore.kernel.org/r/20260828164950.61AE51F000E9@smtp.kernel.org/
> >   - 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.

> 
> >
> >  #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.

Thanks for the review.

Best regards,
Koichiro

> 
> Frank
> 
> > +#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
> >

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

[Koichiro Den]

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:
> > On Thu, Sep 03, 2026 at 03:45:32PM +0900, Koichiro Den wrote:
> > > 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 <den@valinux.co.jp>
> > > ---
> > > 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/
> > >   - Do not skip generic dma_slave_config fields when private settings are
> > >     supplied. (Sashiko)
> > >     https://lore.kernel.org/r/20260828164950.61AE51F000E9@smtp.kernel.org/
> > >   - 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.
> 
> > 
> > >
> > >  #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
> 
> > 
> > Frank
> > 
> > > +#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
> > >

[PATCH v3 3/3] dmaengine: dw-edma: Account for the MSI vector offset

[Koichiro Den]

get_cached_msi_msg() returns the base message shared by a multi-MSI
descriptor. dw-edma currently derives per-channel data from its local IRQ
index and does not adjust a common IRQ at all. Both assume eDMA starts at
the descriptor's first vector.

That is not true when eDMA receives a tail subset. Compose each message
from the IRQ offset relative to the descriptor base in both paths.

While at it, avoid reading PCI MSI attributes from descriptors owned by
non-PCI devices.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v3:
  - Pick up Frank's Reviewed-by tag.
  - No other changes.

 drivers/dma/dw-edma/dw-edma-core.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index e3786d960a43..5e95b3e06933 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -7,6 +7,7 @@
  */
 
 #include <linux/module.h>
+#include <linux/pci.h>
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/kernel.h>
@@ -1107,12 +1108,23 @@ static inline void dw_edma_dec_irq_alloc(int *nr_irqs, u32 *alloc, u16 cnt)
 	}
 }
 
+static void dw_edma_compose_msi(int irq, struct msi_msg *msi)
+{
+	struct msi_desc *desc = irq_get_msi_desc(irq);
+
+	if (!desc)
+		return;
+
+	get_cached_msi_msg(irq, msi);
+	if (dev_is_pci(desc->dev) && !desc->pci.msi_attrib.is_msix)
+		msi->data += irq - desc->irq;
+}
+
 static int dw_edma_irq_request(struct dw_edma *dw,
 			       u32 *wr_alloc, u32 *rd_alloc)
 {
 	struct dw_edma_chip *chip = dw->chip;
 	struct device *dev = dw->chip->dev;
-	struct msi_desc *msi_desc;
 	int i, err = 0;
 	u32 ch_cnt;
 	int irq;
@@ -1137,8 +1149,7 @@ static int dw_edma_irq_request(struct dw_edma *dw,
 			return err;
 		}
 
-		if (irq_get_msi_desc(irq))
-			get_cached_msi_msg(irq, &dw->irq[0].msi);
+		dw_edma_compose_msi(irq, &dw->irq[0].msi);
 
 		dw->nr_irqs = 1;
 	} else {
@@ -1161,12 +1172,7 @@ static int dw_edma_irq_request(struct dw_edma *dw,
 					  &dw->irq[i]);
 			if (err)
 				goto err_irq_free;
-			msi_desc = irq_get_msi_desc(irq);
-			if (msi_desc) {
-				get_cached_msi_msg(irq, &dw->irq[i].msi);
-				if (!msi_desc->pci.msi_attrib.is_msix)
-					dw->irq[i].msi.data = dw->irq[0].msi.data + i;
-			}
+			dw_edma_compose_msi(irq, &dw->irq[i].msi);
 		}
 
 		dw->nr_irqs = i;
-- 
2.51.0