DMA Engine development
 help / color / mirror / Atom feed
From: Frank Li <Frank.li@oss.nxp.com>
To: Koichiro Den <den@valinux.co.jp>
Cc: Vinod Koul <vkoul@kernel.org>, Frank Li <Frank.Li@kernel.org>,
	Manivannan Sadhasivam <mani@kernel.org>,
	Devendra K Verma <devverma@amd.com>,
	dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 2/3] dmaengine: dw-edma: Configure remote interrupt routing
Date: Fri, 4 Sep 2026 10:41:47 -0500	[thread overview]
Message-ID: <aprmu6McROxJrDLW@SMW015318> (raw)
In-Reply-To: <20260903064533.2269557-3-den@valinux.co.jp>

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
>

  parent reply	other threads:[~2026-09-04 15:41 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-03  6:45 [PATCH v3 0/3] dmaengine: dw-edma: Prepare channels for remote use Koichiro Den
2026-09-03  6:45 ` [PATCH v3 1/3] dmaengine: Allow drivers to assign static channel IDs Koichiro Den
2026-09-03  6:45 ` [PATCH v3 2/3] dmaengine: dw-edma: Configure remote interrupt routing Koichiro Den
2026-09-03  6:57   ` sashiko-bot
2026-09-04 15:41   ` Frank Li [this message]
2026-09-04 17:15     ` Koichiro Den
2026-09-04 17:46       ` Koichiro Den
2026-09-03  6:45 ` [PATCH v3 3/3] dmaengine: dw-edma: Account for the MSI vector offset Koichiro Den

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aprmu6McROxJrDLW@SMW015318 \
    --to=frank.li@oss.nxp.com \
    --cc=Frank.Li@kernel.org \
    --cc=den@valinux.co.jp \
    --cc=devverma@amd.com \
    --cc=dmaengine@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mani@kernel.org \
    --cc=vkoul@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox