Re: [PATCH v3 01/13] dmaengine: dw-edma: Add per-channel interrupt routing control

[Frank Li <Frank.li@oss.nxp.com>]

On Sun, Jun 21, 2026 at 02:00:28AM +0900, Koichiro Den wrote:
> DesignWare eDMA can signal completion locally through edma_int[] and
> remotely through IMWr/MSI. When channels are delegated to a remote
> frontend, the local endpoint side and the remote host side must not both
> service the same DONE/ABORT status.
>
> Add channel interrupt routing state and initialize it from the
> controller instance configuration. Update the v0 eDMA and HDMA native
> paths so linked-list interrupt generation, HDMA non-linked-list
> interrupt enables, and DONE/ABORT masking follow the selected mode. For
> HDMA native non-linked-list channels, use the dedicated remote
> stop/abort enables without local stop/abort enables.
>
> Keep the existing dw-edma-pcie host-side instances in remote interrupt
> routing mode so their IMWr/MSI completion model remains unchanged after
> local routing becomes the zero value.
>
> Note that the routing mode describes where a channel should report
> completion. It does not, by itself, say whether this dw-edma instance
> owns the interrupt status. A local instance must ignore remote-only
> channels, and a remote instance must ignore local-only channels, even if
> such interrupts are unexpectedly delivered. Otherwise the non-owner side
> could steal the interrupt from the owner by clearing shared DONE/ABORT
> status.
>
> Suggested-by: Frank Li <Frank.Li@nxp.com>
> Signed-off-by: Koichiro Den <den@valinux.co.jp>
> ---
> Changes in v3:
>   - Remove DW_EDMA_CH_IRQ_DEFAULT; local routing is the zero value
>     (Frank).
>   - Set existing dw-edma-pcie host-side instances to remote interrupt
>     routing in this patch, preserving the legacy IMWr completion model.
>   - Remove an unreachable HDMA native check (Sashiko).
>   - Clarify local/remote instance ownership after Frank's question.
>   - Mark non-owner IRQ handling guard paths unlikely.
>   - Add HDMA native interrupt routing while keeping the existing non-LL
>     int config ABI.
>   - Keep HDMA native linked-list local interrupt generation enabled for
>     remote-routed channels while masking the local edma_int[] output.
>   - Use remote-only stop/abort enables for HDMA native non-LL remote-routed
>     channels.
>   - Drop the peripheral_config IRQ-routing ABI; initial routing comes from
>     chip setup and channel ownership handoff can override it.
>   - Keep dma_slave_config from resetting channel ownership routing.
>
>  drivers/dma/dw-edma/dw-edma-core.c    | 14 +++++++++
>  drivers/dma/dw-edma/dw-edma-core.h    | 13 +++++++++
>  drivers/dma/dw-edma/dw-edma-pcie.c    |  1 +
>  drivers/dma/dw-edma/dw-edma-v0-core.c | 22 ++++++++++----
>  drivers/dma/dw-edma/dw-hdma-v0-core.c | 41 ++++++++++++++++-----------
>  include/linux/dma/edma.h              | 30 ++++++++++++++++++++
>  6 files changed, 99 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 89a4c498a17b..7a24248b84e9 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -219,6 +219,17 @@ static void dw_edma_device_caps(struct dma_chan *dchan,
>  	}
>  }
>
> +static enum dw_edma_ch_irq_mode dw_edma_get_irq_mode(struct dw_edma_chan *chan)
> +{

dw_edma_get_default_irq_mode() ?

> +	struct dw_edma_chip *chip = chan->dw->chip;
> +
> +	if (chip->irq_mode == DW_EDMA_CH_IRQ_REMOTE &&
> +	    !(chip->flags & DW_EDMA_CHIP_LOCAL))
> +		return DW_EDMA_CH_IRQ_REMOTE;
> +

return chip->flags & DW_EDMA_CHIP_LOCAL ? DW_EDMA_CH_IRQ_LOCAL : DW_EDMA_CH_IRQ_LOCAL

> +	return DW_EDMA_CH_IRQ_LOCAL;
> +}
> +
>  static int dw_edma_device_config(struct dma_chan *dchan,
>  				 struct dma_slave_config *config)
>  {
> @@ -853,6 +864,8 @@ static int dw_edma_alloc_chan_resources(struct dma_chan *dchan)
>  	if (chan->status != EDMA_ST_IDLE)
>  		return -EBUSY;
>
> +	chan->irq_mode = dw_edma_get_irq_mode(chan);
> +
>  	return 0;
>  }
>
> @@ -904,6 +917,7 @@ static int dw_edma_channel_setup(struct dw_edma *dw, u32 wr_alloc, u32 rd_alloc)
>  		chan->configured = false;
>  		chan->request = EDMA_REQ_NONE;
>  		chan->status = EDMA_ST_IDLE;
> +		chan->irq_mode = dw_edma_get_irq_mode(chan);

if already set in dw_edma_alloc_chan_resources(), needn't set again?

>
>  		if (chan->dir == EDMA_DIR_WRITE)
>  			chan->ll_max = (chip->ll_region_wr[chan->id].sz / EDMA_LL_SZ);
> diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
> index 6474cacf7195..42f2f25ef377 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.h
> +++ b/drivers/dma/dw-edma/dw-edma-core.h
> @@ -81,6 +81,8 @@ struct dw_edma_chan {
>
>  	struct msi_msg			msi;
>
> +	enum dw_edma_ch_irq_mode	irq_mode;
> +
>  	enum dw_edma_request		request;
>  	enum dw_edma_status		status;
>  	u8				configured;
> @@ -224,4 +226,15 @@ dw_edma_core_db_offset(struct dw_edma *dw)
>  	return dw->core->db_offset(dw);
>  }
>
> +static inline bool
> +dw_edma_core_ch_ignore_irq(struct dw_edma_chan *chan)
> +{
> +	struct dw_edma *dw = chan->dw;
> +
> +	if (dw->chip->flags & DW_EDMA_CHIP_LOCAL)
> +		return chan->irq_mode == DW_EDMA_CH_IRQ_REMOTE;

is it okay to simple return false here?

> +	else
> +		return chan->irq_mode == DW_EDMA_CH_IRQ_LOCAL;
> +}
> +
>  #endif /* _DW_EDMA_CORE_H */
> diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
> index 791c46e8ae4c..70ea031147d1 100644
> --- a/drivers/dma/dw-edma/dw-edma-pcie.c
> +++ b/drivers/dma/dw-edma/dw-edma-pcie.c
> @@ -419,6 +419,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
>  	chip->dev = dev;
>
>  	chip->mf = vsec_data->mf;
> +	chip->irq_mode = DW_EDMA_CH_IRQ_REMOTE;
>  	chip->nr_irqs = nr_irqs;
>  	chip->ops = &dw_edma_pcie_plat_ops;
>  	chip->cfg_non_ll = non_ll;
> diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> index cfdd6463252e..1781ba4f022e 100644
> --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
> @@ -256,9 +256,11 @@ dw_edma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir,
>  	for_each_set_bit(pos, &val, total) {
>  		chan = &dw->chan[pos + off];
>
> +		if (unlikely(dw_edma_core_ch_ignore_irq(chan)))
> +			continue;
> +
>  		dw_edma_v0_core_clear_done_int(chan);
>  		done(chan);
> -

clean up these unncessary chagnes.

>  		ret = IRQ_HANDLED;
>  	}
>
> @@ -267,9 +269,11 @@ dw_edma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir,
>  	for_each_set_bit(pos, &val, total) {
>  		chan = &dw->chan[pos + off];
>
> +		if (unlikely(dw_edma_core_ch_ignore_irq(chan)))
> +			continue;
> +
>  		dw_edma_v0_core_clear_abort_int(chan);
>  		abort(chan);
> -
>  		ret = IRQ_HANDLED;
>  	}
>
> @@ -331,7 +335,8 @@ static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
>  		j--;
>  		if (!j) {
>  			control |= DW_EDMA_V0_LIE;
> -			if (!(chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL))
> +			if (!(chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL) &&
> +			    chan->irq_mode != DW_EDMA_CH_IRQ_LOCAL)

you define DW_EMDA_CH_IRQ_REMOTE, sugget don't use reverise logic.

	chan->irq_mode == chan->irq_mode

>  				control |= DW_EDMA_V0_RIE;
>  		}
>
> @@ -408,12 +413,17 @@ static void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
>  				break;
>  			}
>  		}
> -		/* Interrupt unmask - done, abort */
> +		/* Interrupt mask/unmask - done, abort */
>  		raw_spin_lock_irqsave(&dw->lock, flags);
>
>  		tmp = GET_RW_32(dw, chan->dir, int_mask);
> -		tmp &= ~FIELD_PREP(EDMA_V0_DONE_INT_MASK, BIT(chan->id));
> -		tmp &= ~FIELD_PREP(EDMA_V0_ABORT_INT_MASK, BIT(chan->id));
> +		if (chan->irq_mode == DW_EDMA_CH_IRQ_REMOTE) {

I think need also check chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL

> +			tmp |= FIELD_PREP(EDMA_V0_DONE_INT_MASK, BIT(chan->id));
> +			tmp |= FIELD_PREP(EDMA_V0_ABORT_INT_MASK, BIT(chan->id));
> +		} else {
> +			tmp &= ~FIELD_PREP(EDMA_V0_DONE_INT_MASK, BIT(chan->id));
> +			tmp &= ~FIELD_PREP(EDMA_V0_ABORT_INT_MASK, BIT(chan->id));
> +		}
>  		SET_RW_32(dw, chan->dir, int_mask, tmp);
>  		/* Linked list error */
>  		tmp = GET_RW_32(dw, chan->dir, linked_list_err_en);
> diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> index 2beec876b184..7ba6bdbffc17 100644
> --- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> @@ -49,6 +49,26 @@ __dw_ch_regs(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch)
>  		writel(value, &(__dw_ch_regs(dw, EDMA_DIR_READ, ch)->name));	\
>  	} while (0)
>
> +static u32 dw_hdma_v0_core_int_setup(struct dw_edma_chan *chan, u32 val)
> +{
> +	val &= ~(HDMA_V0_LOCAL_ABORT_INT_EN | HDMA_V0_REMOTE_ABORT_INT_EN |
> +		 HDMA_V0_LOCAL_STOP_INT_EN | HDMA_V0_REMOTE_STOP_INT_EN |
> +		 HDMA_V0_ABORT_INT_MASK | HDMA_V0_STOP_INT_MASK);
> +
> +	if (chan->irq_mode == DW_EDMA_CH_IRQ_REMOTE && chan->non_ll)
> +		return val | HDMA_V0_REMOTE_ABORT_INT_EN |
> +		       HDMA_V0_REMOTE_STOP_INT_EN;
> +
> +	if (chan->irq_mode == DW_EDMA_CH_IRQ_REMOTE)
> +		return val | HDMA_V0_LOCAL_ABORT_INT_EN |
> +		       HDMA_V0_REMOTE_ABORT_INT_EN |
> +		       HDMA_V0_LOCAL_STOP_INT_EN |
> +		       HDMA_V0_REMOTE_STOP_INT_EN | HDMA_V0_ABORT_INT_MASK |
> +		       HDMA_V0_STOP_INT_MASK;
> +
> +	return val | HDMA_V0_LOCAL_ABORT_INT_EN | HDMA_V0_LOCAL_STOP_INT_EN;
> +}
> +
>  /* HDMA management callbacks */
>  static void dw_hdma_v0_core_off(struct dw_edma *dw)
>  {
> @@ -132,6 +152,8 @@ dw_hdma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir,
>
>  	for_each_set_bit(pos, &mask, total) {
>  		chan = &dw->chan[pos + off];
> +		if (unlikely(dw_edma_core_ch_ignore_irq(chan)))
> +			continue;
>
>  		val = dw_hdma_v0_core_status_int(chan);
>  		if (FIELD_GET(HDMA_V0_STOP_INT_MASK, val)) {
> @@ -238,11 +260,7 @@ static void dw_hdma_v0_core_ll_start(struct dw_edma_chunk *chunk, bool first)
>  		SET_CH_32(dw, chan->dir, chan->id, ch_en, BIT(0));
>  		/* Interrupt unmask - stop, abort */
>  		tmp = GET_CH_32(dw, chan->dir, chan->id, int_setup);
> -		tmp &= ~(HDMA_V0_STOP_INT_MASK | HDMA_V0_ABORT_INT_MASK);
> -		/* Interrupt enable - stop, abort */
> -		tmp |= HDMA_V0_LOCAL_STOP_INT_EN | HDMA_V0_LOCAL_ABORT_INT_EN;
> -		if (!(dw->chip->flags & DW_EDMA_CHIP_LOCAL))
> -			tmp |= HDMA_V0_REMOTE_STOP_INT_EN | HDMA_V0_REMOTE_ABORT_INT_EN;
> +		tmp = dw_hdma_v0_core_int_setup(chan, tmp);

Can you use small patch to create helper dw_hdma_v0_core_int_setup() only

>  		SET_CH_32(dw, chan->dir, chan->id, int_setup, tmp);
>  		/* Channel control */
>  		SET_CH_32(dw, chan->dir, chan->id, control1, HDMA_V0_LINKLIST_EN);
> @@ -293,17 +311,8 @@ static void dw_hdma_v0_core_non_ll_start(struct dw_edma_chunk *chunk)
>  	SET_CH_32(dw, chan->dir, chan->id, transfer_size, child->sz);
>
>  	/* Interrupt setup */
> -	val = GET_CH_32(dw, chan->dir, chan->id, int_setup) |
> -			HDMA_V0_STOP_INT_MASK |
> -			HDMA_V0_ABORT_INT_MASK |
> -			HDMA_V0_LOCAL_STOP_INT_EN |
> -			HDMA_V0_LOCAL_ABORT_INT_EN;
> -
> -	if (!(dw->chip->flags & DW_EDMA_CHIP_LOCAL)) {
> -		val |= HDMA_V0_REMOTE_STOP_INT_EN |
> -		       HDMA_V0_REMOTE_ABORT_INT_EN;
> -	}
> -
> +	val = GET_CH_32(dw, chan->dir, chan->id, int_setup);
> +	val = dw_hdma_v0_core_int_setup(chan, val);
>  	SET_CH_32(dw, chan->dir, chan->id, int_setup, val);
>
>  	/* Channel control setup */
> diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
> index 1fafd5b0e315..c0906221a7c7 100644
> --- a/include/linux/dma/edma.h
> +++ b/include/linux/dma/edma.h
> @@ -60,6 +60,34 @@ enum dw_edma_chip_flags {
>  	DW_EDMA_CHIP_LOCAL	= BIT(0),
>  };
>
> +/**
> + * enum dw_edma_ch_irq_mode - per-channel interrupt routing control
> + * @DW_EDMA_CH_IRQ_LOCAL:     local interrupt only (edma_int[])
> + * @DW_EDMA_CH_IRQ_REMOTE:    remote interrupt only (IMWr/MSI), without
> + *                            delivering local edma_int[].
> + *
> + * DesignWare EP eDMA can signal interrupts locally through the edma_int[]
> + * bus, and remotely using posted memory writes (IMWr) that may be
> + * interpreted as MSI/MSI-X by the RC.
> + *
> + * For the v0 eDMA linked-list programming path, DMA_*_INT_MASK gates the local
> + * edma_int[] assertion, while there is no dedicated per-channel mask for IMWr
> + * generation. To request a remote-only interrupt, Synopsys recommends setting
> + * both LIE and RIE, and masking the local interrupt in DMA_*_INT_MASK. See the
> + * DesignWare endpoint databook 6.30a, Linked List Mode interrupt handling
> + * ("Software Programming of an Endpoint's LIE and RIE Bits for Linked List
> + * Transfers", Attention).
> + *
> + * HDMA linked-list watermark interrupts have the same LWIE/RWIE guidance. HDMA
> + * non-linked-list mode has dedicated local and remote stop/abort interrupt
> + * enables, and the remote CPU programming examples use remote enables without
> + * local enables.
> + */
> +enum dw_edma_ch_irq_mode {
> +	DW_EDMA_CH_IRQ_LOCAL	= 0,
> +	DW_EDMA_CH_IRQ_REMOTE,
> +};
> +
>  /**
>   * struct dw_edma_chip - representation of DesignWare eDMA controller hardware
>   * @dev:		 struct device of the eDMA controller
> @@ -76,6 +104,7 @@ enum dw_edma_chip_flags {
>   * @db_irq:		 Virtual IRQ dedicated to interrupt emulation
>   * @db_offset:		 Offset from DMA register base
>   * @mf:			 DMA register map format
> + * @irq_mode:		 default per-channel interrupt routing
>   * @dw:			 struct dw_edma that is filled by dw_edma_probe()
>   */
>  struct dw_edma_chip {
> @@ -101,6 +130,7 @@ struct dw_edma_chip {
>  	resource_size_t		db_offset;
>
>  	enum dw_edma_map_format	mf;
> +	enum dw_edma_ch_irq_mode	irq_mode;

This are already have flags dw_edma_chip_flags, not sure why need irq_mode,
suppose, if set DW_EDMA_CHIP_LOCAL, pre channel should be default as
DW_EDMA_CH_IRQ_LOCAL

Frank
>
>  	struct dw_edma		*dw;
>  	bool			cfg_non_ll;
> --
> 2.51.0
>