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

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

On Sun, Aug 30, 2026 at 03:13:23AM +0900, Koichiro Den wrote:
> On Fri, Aug 28, 2026 at 01:41:57PM -0500, Frank Li wrote:
> > On Sat, Aug 29, 2026 at 01:36:10AM +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 enum dw_edma_ch_irq_mode on idle channels of a
> > > local eDMA-compatible instance. Synchronizing a remote-routed channel
> > > quiesces the hardware, after which the caller can restore its routing and
> > > release it.
> > >
> > > 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 v2:
> > >   - Rework the channel routing from PCI DMA EPF v7 patches 5 and 6.
> > >     https://lore.kernel.org/r/20260813063757.3131865-6-den@valinux.co.jp/
> > >     https://lore.kernel.org/r/20260813063757.3131865-7-den@valinux.co.jp/
> > >   - Use dma_slave_config instead of private delegation helpers. (Frank)
> > >     https://lore.kernel.org/r/ao2nHoCwfTEEiFSr@SMW015318/
> > >
> > >  drivers/dma/dw-edma/dw-edma-core.c | 51 +++++++++++++++++++++++++++---
> > >  include/linux/dma/edma.h           |  6 ++++
> > >  2 files changed, 52 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> > > index a678c70a78fe..a8c6bd508fcd 100644
> > > --- a/drivers/dma/dw-edma/dw-edma-core.c
> > > +++ b/drivers/dma/dw-edma/dw-edma-core.c
> > > @@ -177,6 +177,30 @@ 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,
> > > +					  struct dma_slave_config *config)
> > > +{
> > > +	enum dw_edma_ch_irq_mode mode;
> > > +
> > > +	if (!(chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL) ||
> > > +	    config->peripheral_size != sizeof(mode))
> > > +		return -EINVAL;
> > > +
> > > +	mode = *(enum dw_edma_ch_irq_mode *)config->peripheral_config;
> >
> > existing code use peripheral_config indicate non_ll mode, is it compatible
> > with irq mode?
>
> Yes. dw_edma_device_config() currently distinguishes them by map format.
>
> As a small first step, the remote-channel use introduced by this prep series and
> used by the main vNTB series [1] does not cover HDMA. See the cover letter of
> [1].
>
> Would you prefer introducing a common config structure at this point?
> Requiring every caller to initialize both fields may not be ideal, so perhaps it
> could carry a bitmap indicating which fields are valid:
>
>   #define DW_EDMA_CH_CONFIG_NON_LL       BIT(0)
>   #define DW_EDMA_CH_CONFIG_IRQ_MODE     BIT(1)
>
>   struct dw_edma_chan_config {
>           u32 flags;
>           bool non_ll;
>           enum dw_edma_ch_irq_mode irq_mode;
>   };
>
> What do you think?

Okay, it avoid need init all field.

Frank

>
> [1] https://lore.kernel.org/r/20260828170932.2735807-1-den@valinux.co.jp/
>
> Thanks,
> Koichiro
>
> >
> > Frank
> >
> > > +	if (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;
> > > +
> > > +	chan->irq_mode = mode;
> > > +
> > > +	return 0;
> > > +}
> > > +
> > >  static int dw_edma_device_config(struct dma_chan *dchan,
> > >  				 struct dma_slave_config *config)
> > >  {
> > > @@ -184,6 +208,10 @@ static int dw_edma_device_config(struct dma_chan *dchan,
> > >  	bool cfg_non_ll;
> > >  	int non_ll = 0;
> > >
> > > +	if (chan->dw->chip->mf != EDMA_MF_HDMA_NATIVE &&
> > > +	    config->peripheral_config)
> > > +		return dw_edma_device_config_irq_mode(chan, config);
> > > +
> > >  	chan->non_ll = false;
> > >  	if (chan->dw->chip->mf == EDMA_MF_HDMA_NATIVE) {
> > >  		if (config->peripheral_config &&
> > > @@ -213,10 +241,6 @@ static int dw_edma_device_config(struct dma_chan *dchan,
> > >
> > >  		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;
> > >  	}
> > >
> > >  	memcpy(&chan->config, config, sizeof(*config));
> > > @@ -893,6 +917,17 @@ static void dw_edma_wait_termination(struct dma_chan *dchan)
> > >  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);
> > >  	cancel_work_sync(&chan->irq_work);
> > > @@ -903,12 +938,18 @@ 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;
> > > +		/* Avoid a redundant write racing with shared-IRQ readers. */
> > > +		if (chan->irq_mode != default_mode)
> > > +			chan->irq_mode = default_mode;
> > > +	}
> > >
> > >  	vchan_free_chan_resources(&chan->vc);
> > >  }
> > > diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
> > > index 3c8e2ef9dee0..54491c9e4b5e 100644
> > > --- a/include/linux/dma/edma.h
> > > +++ b/include/linux/dma/edma.h
> > > @@ -92,6 +92,12 @@ enum dw_edma_chip_flags {
> > >   * handed over to and driven by the remote side, and the recipe above is
> > >   * applied by the driving instance.
> > >   *
> > > + * On a local eDMA-compatible instance, clients may pass this enum through
> > > + * dma_slave_config.peripheral_config to switch an idle, unconfigured channel
> > > + * between LOCAL and REMOTE routing. Before synchronizing a REMOTE channel,
> > > + * the client must stop remote programming and own every channel affected by
> > > + * the hardware quiesce.
> > > + *
> > >   * 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.
> > > --
> > > 2.51.0
> > >