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

["Verma, Devendra" <devverma@amd.com>]

On 01-Oct-26 01:55, Frank Li wrote:
> On Tue, Sep 15, 2026 at 12:53:33AM +0900, Koichiro Den wrote:
>> On Tue, Sep 15, 2026 at 12:49:01AM +0900, Koichiro Den wrote:
>>> On Mon, Sep 14, 2026 at 10:22:49AM -0500, Frank Li wrote:
>>>> On Sun, Sep 13, 2026 at 02:40:22AM +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 or HDMA instance. Releasing a remote-routed channel
>>>>> quiesces the hardware and drains its local IRQ before restoring 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 v4:
>>>>>    - Drop unnecessary READ_ONCE()/WRITE_ONCE() for irq_mode. (Frank)
>>>>>    - Allow repeated dmaengine_slave_config() calls on idle channels
>>>>>      when the IRQ mode is unchanged. (Sashiko)
>>>>>    - Simplify IRQ mode handling, assuming the channel has no pending
>>>>>      interrupt status when its mode changes. Treat racing reads by
>>>>>      shared IRQ handlers and same-value stores on release as harmless.
>>>>>    - Support native HDMA.
>>>>>
>>>>>   drivers/dma/dw-edma/dw-edma-core.c | 136 ++++++++++++++++++++++-------
>>>>>   include/linux/dma/edma.h           |  21 +++++
>>>>>   2 files changed, 125 insertions(+), 32 deletions(-)
>>>>>
>>>>> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
>>>>> index a678c70a78fe..c978da30bac5 100644
>>>>> --- a/drivers/dma/dw-edma/dw-edma-core.c
>>>>> +++ b/drivers/dma/dw-edma/dw-edma-core.c
>>>>> @@ -177,48 +177,76 @@ 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->status != EDMA_ST_IDLE || chan->request != EDMA_REQ_NONE)
>>>>> +		return -EBUSY;
>>>>> +
>>>>> +	/* IRQ routing cannot change after the initial configuration. */
>>>>> +	if (chan->irq_mode == mode)
>>>>> +		return 0;
>>>>> +
>>>>> +	if (chan->configured)
>>>>> +		return -EBUSY;
>>>>> +
>>>>> +	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.
>>>>> -		 *
>>>>> -		 * 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;
>>>>> +	/*
>>>>> +	 * 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 (cfg_non_ll && !non_ll) {
>>>>> -				dev_err(dchan->device->dev, "invalid configuration\n");
>>>>> -				return -EINVAL;
>>>>> -			}
>>>>> +	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;
>>>>>   		}
>>>>>
>>>>> -		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;
>>>>> +		non_ll = dw_config->non_ll;
>>>>> +	}
>>>>> +
>>>>> +	if (flags & DW_EDMA_CH_CONFIG_IRQ_MODE) {
>>>>> +		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 +918,53 @@ 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;
>>>>> +
>>>>> +	/*
>>>>> +	 * A shared handler may retain this channel's status across quiesce.
>>>>> +	 * With nr_irqs == 1, it scans both directions even if routing and
>>>>> +	 * delegation are direction-wide. Drain it before allowing a routing change.
>>>>> +	 */
>>>>> +	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;
>>>>> +
>>>>> +	/*
>>>>> +	 * irq_mode is fixed after initial configuration. The free path
>>>>> +	 * restores it only after synchronization.
>>>>> +	 */
>>>>> +	remote = chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL &&
>>>>> +		 chan->irq_mode == DW_EDMA_CH_IRQ_REMOTE;
>>>>> +
>>>>> +	/*
>>>>> +	 * Peer-driven transfers bypass local descriptor tracking, so quiesce
>>>>> +	 * the hardware explicitly.
>>>>> +	 */
>>>>> +	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);
>>>>> @@ -907,8 +977,10 @@ static void dw_edma_free_chan_resources(struct dma_chan *dchan)
>>>>>   	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;
>>>>> +		chan->irq_mode = dw_edma_get_default_irq_mode(chan);
>>>>> +	}
>>>>>
>>>>>   	vchan_free_chan_resources(&chan->vc);
>>>>>   }
>>>>> diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
>>>>> index 3c8e2ef9dee0..43831fa57357 100644
>>>>> --- a/include/linux/dma/edma.h
>>>>> +++ b/include/linux/dma/edma.h
>>>>> @@ -101,6 +101,27 @@ 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: the
>>>>> + * entire direction for eDMA-compatible layouts, or the individual channel for
>>>>> + * native HDMA.
>>>>> + */
>>>>> +struct dw_edma_chan_config {
>>>>> +	u32 flags;
>>>>> +	bool non_ll;
>>>>> +	enum dw_edma_ch_irq_mode irq_mode;
>>>>> +};
>>>>> +
>>>>
>>>> Do you have any user in kernel tree use non_ll?
>>>
>>> I don't think so.
>>>
>>> Devendra, I would appreciate your input here, if you have any thoughts on
>>> Frank's question, or the new dw_edma_chan_config. I haven't found any in-tree
>>> user of the non-LL peripheral_config interface introduced here:
>>> https://lore.kernel.org/r/20260318070403.1634706-3-devendra.verma@amd.com/
>>> so I guess any users would be out-of-tree at least as of now, unless I'm missing
>>> something.
>>
>> Ouch, I meant to put Devendra in To, not Cc. Sorry for the noise.
> 
> Consider not in-tree non-ll consumer. It should be fine to change API.
> 
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> 

Hi Koichiro, Frank

Thank you for your patience!
I was out for few weeks. To your question, there are no users in
the kernel tree for non_ll case via the dmaengine_slave_config().

- Devendra

>>
>> Best regards,
>> Koichiro
>>
>>>
>>> Best regards,
>>> Koichiro
>>>
>>>>
>>>> Frank
>>>>
>>>>>   /**
>>>>>    * struct dw_edma_chip - representation of DesignWare eDMA controller hardware
>>>>>    * @dev:		 struct device of the eDMA controller
>>>>> --
>>>>> 2.51.0
>>>>>