Re: [PATCH v2 02/12] dmaengine: dw-edma: Add per-channel interrupt routing control

[Koichiro Den]

On Thu, Jun 04, 2026 at 04:18:02PM -0400, Frank Li wrote:
> On Mon, May 25, 2026 at 03:24:10PM +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 dw_edma_irq_config, carried through dma_slave_config, so a frontend
> > can choose default, local, or remote IRQ handling per channel. Update the
> > v0 path so linked-list interrupt generation and DONE/ABORT masking follow
> > the selected mode. If a frontend does not supply the config, keep the
> > existing behavior.
> >
> > HDMA native already uses dma_slave_config.peripheral_config as an int for
> > non-LL mode selection. Keep that interface unchanged and reject the new
> > IRQ config there until an IRQ routing model is implemented and validated.
> >
> > Signed-off-by: Koichiro Den <den@xxxxxxxxxxxxx>
> > ---
> ...
> >
> > +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)
> 
> suppose it should be pre channel config, why need check chip's informaiton?

The irq_mode alone does not tell whether this Linux instance should handle the
channel interrupt. The decision depends on both the per-channel irq_mode and
whether this dw-edma instance is the local or remote.

- For a local instance (i.e. EP Linux), interrupts that are supposed to be
  handled on a remote instance needs to be ignored even if edma_int[*] for that
  is raised.
- For a remote instance (i.e. Host Linux), interrupts that are supposed to be
  handled on a local instance needs to be ignored even if IMWr for that is
  raised.

The both ends sees the same status/clear registers, so the non-owner side must
not clear the done/abort status for a channel that is supposed to be cleared by
the other side.

> 
> > +		return chan->irq_mode == DW_EDMA_CH_IRQ_REMOTE;
> > +	else
> > +		return chan->irq_mode == DW_EDMA_CH_IRQ_LOCAL;
> > +}
> > +
> >  #endif /* _DW_EDMA_CORE_H */
> > diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> > index 69e8279adec8..08ec2bd7856e 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 (dw_edma_core_ch_ignore_irq(chan))
> > +			continue;
> > +
> >  		dw_edma_v0_core_clear_done_int(chan);
> >  		done(chan);
> > -
> >  		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 (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)
> >  				control |= DW_EDMA_V0_RIE;
> >  		}
> >
> > @@ -407,10 +412,15 @@ static void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> >  				break;
> >  			}
> >  		}
> > -		/* Interrupt unmask - done, abort */
> > +		/* Interrupt mask/unmask - done, abort */
> >  		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) {
> > +			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/include/linux/dma/edma.h b/include/linux/dma/edma.h
> > index 3e15cf83b784..2bf2298711e1 100644
> > --- a/include/linux/dma/edma.h
> > +++ b/include/linux/dma/edma.h
> > @@ -60,6 +60,43 @@ 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_DEFAULT:   keep legacy behavior
> 
> Feel like it make things complex, most likely use pcie ep probe, it should
> be use DW_EDMA_CH_IRQ_LOCAL.
> 
> If probe from dw-edma-pcie.c, it should be use DW_EDMA_CH_IRQ_REMOTE.
> 
> Add default mode, it make check logic become complex.

Sounds reasonable. I will drop the DEFAULT mode.

Thanks for reviewing,
Koichiro

> 
> Frank
> 
> > + * @DW_EDMA_CH_IRQ_LOCAL:     local interrupt only (edma_int[])
> > + * @DW_EDMA_CH_IRQ_REMOTE:    remote interrupt only (IMWr/MSI),
> > + *                            while masking local DONE/ABORT output.
> > + *
> > + * 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 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 (rather than relying
> > + * on LIE=0/RIE=1). See the DesignWare endpoint databook 5.40a, Non Linked
> > + * List Mode interrupt handling ("Hint").
> > + */
> > +enum dw_edma_ch_irq_mode {
> > +	DW_EDMA_CH_IRQ_DEFAULT	= 0,
> > +	DW_EDMA_CH_IRQ_LOCAL,
> > +	DW_EDMA_CH_IRQ_REMOTE,
> > +};
> > +
> > +/**
> > + * struct dw_edma_irq_config - dw-edma interrupt routing configuration
> > + * @irq_mode: per-channel interrupt routing control.
> > + * @reserved: must be zero.
> > + *
> > + * Pass this structure via dma_slave_config.peripheral_config and
> > + * dma_slave_config.peripheral_size.
> > + */
> > +struct dw_edma_irq_config {
> > +	enum dw_edma_ch_irq_mode irq_mode;
> > +	u32 reserved;
> > +};
> > +
> >  /**
> >   * struct dw_edma_chip - representation of DesignWare eDMA controller hardware
> >   * @dev:		 struct device of the eDMA controller
> > @@ -76,6 +113,8 @@ 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
> > + * @default_irq_mode:	 default per-channel interrupt routing when client
> > + *			 does not supply dw_edma_irq_config
> >   * @dw:			 struct dw_edma that is filled by dw_edma_probe()
> >   */
> >  struct dw_edma_chip {
> > @@ -101,6 +140,7 @@ struct dw_edma_chip {
> >  	resource_size_t		db_offset;
> >
> >  	enum dw_edma_map_format	mf;
> > +	enum dw_edma_ch_irq_mode	default_irq_mode;
> 
> suppose it is pre-channel config?
> 
> Frank
> >
> >  	struct dw_edma		*dw;
> >  	bool			cfg_non_ll;
> > --
> > 2.51.0
> >