* [PATCH v2 0/3] dmaengine: dw-edma: Prepare channels for remote use
@ 2026-08-28 16:36 Koichiro Den
2026-08-28 16:36 ` [PATCH v2 1/3] dmaengine: Allow drivers to assign static channel IDs Koichiro Den
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Koichiro Den @ 2026-08-28 16:36 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Manivannan Sadhasivam; +Cc: dmaengine, linux-kernel
Hi,
This small series contains standalone refactoring and new infrastructure
for dmaengine and dw-edma.
For each patch, the commit message trailer describes its provenance,
keeping tags (incl. Reviewed-by) as-is.
It prepares PCIe EPC-local DMA channels for remote use. For context,
(b) is expected to be superseded by a reworked version of (a):
(a). vNTB-embedded v1
https://lore.kernel.org/r/20260312165005.1148676-1-den@valinux.co.jp/
(b). PCI DMA EPF v7
https://lore.kernel.org/all/20260813063757.3131865-1-den@valinux.co.jp/
I will post a reworked version of (a) shortly. It will depend on this
small series.
Best regards,
Koichiro
---
Changes in v2:
- Split and rework vNTB v1 patches 1, 4, and 5 into this prerequisite
series.
- Fold in the relevant PCI DMA EPF v7 review. See individual patch
trailers for details and links.
v1: https://lore.kernel.org/r/20260312165005.1148676-1-den@valinux.co.jp/
Koichiro Den (3):
dmaengine: Allow drivers to assign static channel IDs
dmaengine: dw-edma: Configure remote interrupt routing
dmaengine: dw-edma: Account for the MSI vector offset
drivers/dma/dmaengine.c | 13 +++--
drivers/dma/dw-edma/dw-edma-core.c | 76 ++++++++++++++++++++++++------
include/linux/dma/edma.h | 6 +++
include/linux/dmaengine.h | 20 ++++++++
4 files changed, 96 insertions(+), 19 deletions(-)
base-commit: 0d995da5fb97e8c312834575604d4423eb6225b7
--
2.51.0
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH v2 1/3] dmaengine: Allow drivers to assign static channel IDs 2026-08-28 16:36 [PATCH v2 0/3] dmaengine: dw-edma: Prepare channels for remote use Koichiro Den @ 2026-08-28 16:36 ` Koichiro Den 2026-08-28 16:55 ` sashiko-bot 2026-08-28 16:36 ` [PATCH v2 2/3] dmaengine: dw-edma: Configure remote interrupt routing Koichiro Den 2026-08-28 16:36 ` [PATCH v2 3/3] dmaengine: dw-edma: Account for the MSI vector offset Koichiro Den 2 siblings, 1 reply; 12+ messages in thread From: Koichiro Den @ 2026-08-28 16:36 UTC (permalink / raw) To: Vinod Koul, Frank Li, Manivannan Sadhasivam; +Cc: dmaengine, linux-kernel The dmaengine core assigns channel IDs in registration order. If a driver skips a hardware channel, chan_id can differ from the hardware numbering and a client cannot reliably correlate a requested channel with hardware resources. Let a driver request an exact channel ID before device registration. Reserve static IDs through the existing IDA so they remain unique, while retaining automatic IDA allocation as the default. For example, idma32 uses chan_id to select DMA_CTL_CH() and DMA_XBAR_SEL(), so it relies on ascending registration order to match chan_id with the hardware channel number. Use direction-flattened IDs for dw-edma channels. Unlike the direction-local hardware channel number, these IDs are unique within the DMA device. Suggested-by: Frank Li <Frank.Li@nxp.com> Reviewed-by: Frank Li <Frank.Li@nxp.com> Signed-off-by: Koichiro Den <den@valinux.co.jp> --- Changes in v2: - Carry over PCI DMA EPF v7 patch 1. https://lore.kernel.org/r/20260813063757.3131865-2-den@valinux.co.jp/ - Add the idma32 example requested during v7 review. (Frank) https://lore.kernel.org/r/an4TXwT37vWYEzCn@SMW015318/ drivers/dma/dmaengine.c | 13 ++++++++----- drivers/dma/dw-edma/dw-edma-core.c | 1 + include/linux/dmaengine.h | 20 ++++++++++++++++++++ 3 files changed, 29 insertions(+), 5 deletions(-) diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c index 6ffd8bd82154..cc64a4679e6f 100644 --- a/drivers/dma/dmaengine.c +++ b/drivers/dma/dmaengine.c @@ -1078,6 +1078,7 @@ static int __dma_async_device_channel_register(struct dma_device *device, struct dma_chan *chan, const char *name) { + unsigned int id; int rc; chan->local = alloc_percpu(typeof(*chan->local)); @@ -1089,11 +1090,13 @@ static int __dma_async_device_channel_register(struct dma_device *device, goto err_free_local; } - /* - * When the chan_id is a negative value, we are dynamically adding - * the channel. Otherwise we are static enumerating. - */ - chan->chan_id = ida_alloc(&device->chan_ida, GFP_KERNEL); + if (chan->chan_id & DMA_CHAN_ID_STATIC) { + id = chan->chan_id & ~DMA_CHAN_ID_STATIC; + chan->chan_id = ida_alloc_range(&device->chan_ida, id, id, + GFP_KERNEL); + } else { + chan->chan_id = ida_alloc(&device->chan_ida, GFP_KERNEL); + } if (chan->chan_id < 0) { pr_err("%s: unable to alloc ida for chan: %d\n", __func__, chan->chan_id); diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c index 03b2c2188351..a678c70a78fe 100644 --- a/drivers/dma/dw-edma/dw-edma-core.c +++ b/drivers/dma/dw-edma/dw-edma-core.c @@ -988,6 +988,7 @@ static int dw_edma_channel_setup(struct dw_edma *dw, u32 wr_alloc, u32 rd_alloc) &dw->chip->dt_region_rd[chan->id]; vchan_init(&chan->vc, dma); + dmaengine_set_static_chan_id(&chan->vc.chan, i); dw_edma_core_ch_config(chan); } diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h index fe33a20abc61..f669b79d7731 100644 --- a/include/linux/dmaengine.h +++ b/include/linux/dmaengine.h @@ -369,6 +369,26 @@ struct dma_chan { void *private; }; +#define DMA_CHAN_ID_STATIC BIT(30) + +/** + * dmaengine_set_static_chan_id - request an exact DMA engine channel ID + * @chan: DMA channel + * @id: channel ID, unique within the DMA device + * + * Drivers may call this after initializing @chan and before registering its + * DMA device. The dmaengine core reserves @id from the device IDA instead of + * assigning the next available ID. + */ +static inline void dmaengine_set_static_chan_id(struct dma_chan *chan, + unsigned int id) +{ + if (WARN_ON_ONCE(id >= DMA_CHAN_ID_STATIC)) + return; + + chan->chan_id = DMA_CHAN_ID_STATIC | id; +} + /** * struct dma_chan_dev - relate sysfs device node to backing channel device * @chan: driver channel device -- 2.51.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/3] dmaengine: Allow drivers to assign static channel IDs 2026-08-28 16:36 ` [PATCH v2 1/3] dmaengine: Allow drivers to assign static channel IDs Koichiro Den @ 2026-08-28 16:55 ` sashiko-bot 0 siblings, 0 replies; 12+ messages in thread From: sashiko-bot @ 2026-08-28 16:55 UTC (permalink / raw) To: Koichiro Den; +Cc: Frank.Li, vkoul, dmaengine Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: Pre-existing issues: - [High] Incorrect error handling for `device_register()` violates the driver model API, leading to a memory leak of device internals and potential Use-After-Free. -- commit a264234e1c749394dbf84d5c9e286ff9f6d8a114 Author: Koichiro Den <den@valinux.co.jp> dmaengine: Allow drivers to assign static channel IDs The dmaengine core assigns channel IDs in registration order. Let a driver request an exact channel ID before device registration. Reserve static IDs through the existing IDA so they remain unique, while retaining automatic IDA allocation as the default. Link: https://lore.kernel.org/r/20260813063757.3131865-2-den@valinux.co.jp/ Link: https://lore.kernel.org/r/an4TXwT37vWYEzCn@SMW015318/ > diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c > index 6ffd8bd82154a..cc64a4679e6fa 100644 > --- a/drivers/dma/dmaengine.c > +++ b/drivers/dma/dmaengine.c [ ... ] > @@ -1089,11 +1090,13 @@ static int __dma_async_device_channel_register(struct dma_device *device, > goto err_free_local; > } > > - /* > - * When the chan_id is a negative value, we are dynamically adding > - * the channel. Otherwise we are static enumerating. > - */ > - chan->chan_id = ida_alloc(&device->chan_ida, GFP_KERNEL); > + if (chan->chan_id & DMA_CHAN_ID_STATIC) { > + id = chan->chan_id & ~DMA_CHAN_ID_STATIC; > + chan->chan_id = ida_alloc_range(&device->chan_ida, id, id, > + GFP_KERNEL); > + } else { > + chan->chan_id = ida_alloc(&device->chan_ida, GFP_KERNEL); > + } > if (chan->chan_id < 0) { > pr_err("%s: unable to alloc ida for chan: %d\n", > __func__, chan->chan_id); [Severity: High] This isn't a bug introduced by this patch, but does this error handling leak device internals and risk a use-after-free? Looking at __dma_async_device_channel_register() further down in this function, if device_register(&chan->dev->device) fails: rc = device_register(&chan->dev->device); if (rc) goto err_out_ida; ... err_out_ida: ida_free(&device->chan_ida, chan->chan_id); err_free_dev: kfree(chan->dev); When device_register() returns an error, the initial reference taken by kobject_init() still needs to be dropped. Doesn't calling kfree(chan->dev) directly skip the internal dev->p cleanup triggered by put_device(), permanently leaking dev->p? Also, if the failure occurs late in device_add() after sysfs files have been exposed, userspace might have opened them. Could freeing the memory directly while userspace holds a reference to the kobject result in a use-after-free upon subsequent access? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260828163611.2691264-1-den@valinux.co.jp?part=1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 2/3] dmaengine: dw-edma: Configure remote interrupt routing 2026-08-28 16:36 [PATCH v2 0/3] dmaengine: dw-edma: Prepare channels for remote use Koichiro Den 2026-08-28 16:36 ` [PATCH v2 1/3] dmaengine: Allow drivers to assign static channel IDs Koichiro Den @ 2026-08-28 16:36 ` Koichiro Den 2026-08-28 16:49 ` sashiko-bot 2026-08-28 18:41 ` Frank Li 2026-08-28 16:36 ` [PATCH v2 3/3] dmaengine: dw-edma: Account for the MSI vector offset Koichiro Den 2 siblings, 2 replies; 12+ messages in thread From: Koichiro Den @ 2026-08-28 16:36 UTC (permalink / raw) To: Vinod Koul, Frank Li, Manivannan Sadhasivam; +Cc: dmaengine, linux-kernel 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; + 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/3] dmaengine: dw-edma: Configure remote interrupt routing 2026-08-28 16:36 ` [PATCH v2 2/3] dmaengine: dw-edma: Configure remote interrupt routing Koichiro Den @ 2026-08-28 16:49 ` sashiko-bot 2026-08-28 18:41 ` Frank Li 1 sibling, 0 replies; 12+ messages in thread From: sashiko-bot @ 2026-08-28 16:49 UTC (permalink / raw) To: Koichiro Den; +Cc: vkoul, Frank.Li, dmaengine Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: New issues: - [High] dw_edma_device_config silently ignores standard generic DMA configuration fields when peripheral_config is provided for non-HDMA devices. - [Medium] Lockless reads of `chan->irq_mode` in shared interrupt handlers will race with writes in `dw_edma_free_chan_resources` and `dw_edma_device_config_irq_mode`. Pre-existing issues: - [High] If a DMA channel fails to terminate within 5 seconds, dw_edma_wait_termination times out but leaves the hardware engine running, leading to potential corruption of future transfers. -- commit ea103c42c7be1020b46df769c1c7bb35f1349b52 Author: Koichiro Den <den@valinux.co.jp> Subject: dmaengine: dw-edma: Configure remote interrupt routing The patch uses dma_slave_config to set dw_edma_ch_irq_mode on idle channels of a local eDMA-compatible instance, allowing endpoint functions to route interrupts remotely. It also adds hardware quiesce during synchronization when restoring channel routing. > diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c > index a678c70a78fe6..a8c6bd508fcd4 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) [ ... ] > +static int dw_edma_device_config_irq_mode(struct dw_edma_chan *chan, > + struct dma_slave_config *config) > +{ [ ... ] > + chan->irq_mode = mode; [Severity: Medium] Could this plain C write to chan->irq_mode race with lockless reads in shared interrupt handlers? While dw_edma_device_config_irq_mode() (and dw_edma_free_chan_resources()) writes to chan->irq_mode, an interrupt for another channel on the same shared IRQ line could fire. If the handler iterates over all channels and locklessly reads chan->irq_mode via dw_edma_core_ch_ignore_irq(), this could trigger KCSAN warnings and theoretically risk torn reads. > + > + 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); [Severity: High] Does this early return silently discard standard generic DMA configuration fields? If a client driver calls dmaengine_slave_config() on a non-HDMA channel and provides both standard configuration fields (like src_addr) and a peripheral_config (to set IRQ mode), the early return bypasses copying the standard config fields. Since it also leaves chan->configured = false, subsequent dmaengine_prep_* calls could unexpectedly fail, even though this function returns success. [ ... ] > @@ -893,6 +917,17 @@ static void dw_edma_wait_termination(struct dma_chan *dchan) [Severity: High] This isn't a bug introduced by this patch, but I noticed a pre-existing issue where dw_edma_wait_termination() can leave the hardware running on timeout: drivers/dma/dw-edma/dw-edma-core.c:dw_edma_wait_termination() { ... while (time_before(jiffies, timeout)) { scoped_guard(spinlock_irqsave, &chan->vc.lock) stopping = chan->request == EDMA_REQ_STOP; if (!stopping) return; fsleep(1000); } dev_warn(chan->dw->chip->dev, "timeout waiting for channel termination\n"); } If a local DMA channel is requested to terminate via dmaengine_terminate_all() and chan->request = EDMA_REQ_STOP is set, but the hardware is not forcefully stopped via registers, could this lead to potential memory corruption or a use-after-free? If dw_edma_wait_termination() times out after 5 seconds and returns, software will free the DMA client buffers. If the hardware engine subsequently completes the transfer or unsticks, it may write to the freed memory. > static void dw_edma_device_synchronize(struct dma_chan *dchan) > { [ ... ] > @@ -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; [Severity: Medium] Similarly, will this plain C write to chan->irq_mode also race with a lockless read in a shared IRQ handler? > + } > > vchan_free_chan_resources(&chan->vc); > } -- Sashiko AI review · https://sashiko.dev/#/patchset/20260828163611.2691264-1-den@valinux.co.jp?part=2 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/3] dmaengine: dw-edma: Configure remote interrupt routing 2026-08-28 16:36 ` [PATCH v2 2/3] dmaengine: dw-edma: Configure remote interrupt routing Koichiro Den 2026-08-28 16:49 ` sashiko-bot @ 2026-08-28 18:41 ` Frank Li 2026-08-29 18:13 ` Koichiro Den 1 sibling, 1 reply; 12+ messages in thread From: Frank Li @ 2026-08-28 18:41 UTC (permalink / raw) To: Koichiro Den Cc: Vinod Koul, Frank Li, Manivannan Sadhasivam, dmaengine, linux-kernel 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? 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 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/3] dmaengine: dw-edma: Configure remote interrupt routing 2026-08-28 18:41 ` Frank Li @ 2026-08-29 18:13 ` Koichiro Den 2026-08-31 15:55 ` Frank Li 0 siblings, 1 reply; 12+ messages in thread From: Koichiro Den @ 2026-08-29 18:13 UTC (permalink / raw) To: Frank Li Cc: Vinod Koul, Frank Li, Manivannan Sadhasivam, dmaengine, linux-kernel 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? [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 > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/3] dmaengine: dw-edma: Configure remote interrupt routing 2026-08-29 18:13 ` Koichiro Den @ 2026-08-31 15:55 ` Frank Li 0 siblings, 0 replies; 12+ messages in thread From: Frank Li @ 2026-08-31 15:55 UTC (permalink / raw) To: Koichiro Den Cc: Vinod Koul, Frank Li, Manivannan Sadhasivam, dmaengine, linux-kernel 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 > > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 3/3] dmaengine: dw-edma: Account for the MSI vector offset 2026-08-28 16:36 [PATCH v2 0/3] dmaengine: dw-edma: Prepare channels for remote use Koichiro Den 2026-08-28 16:36 ` [PATCH v2 1/3] dmaengine: Allow drivers to assign static channel IDs Koichiro Den 2026-08-28 16:36 ` [PATCH v2 2/3] dmaengine: dw-edma: Configure remote interrupt routing Koichiro Den @ 2026-08-28 16:36 ` Koichiro Den 2026-08-28 18:46 ` Frank Li 2026-08-31 16:02 ` Frank Li 2 siblings, 2 replies; 12+ messages in thread From: Koichiro Den @ 2026-08-28 16:36 UTC (permalink / raw) To: Vinod Koul, Frank Li, Manivannan Sadhasivam; +Cc: dmaengine, linux-kernel get_cached_msi_msg() returns the base message shared by a multi-MSI descriptor. dw-edma currently derives per-channel data from its local IRQ index and does not adjust a common IRQ at all. Both assume eDMA starts at the descriptor's first vector. That is not true when eDMA receives a tail subset. Compose each message from the IRQ offset relative to the descriptor base in both paths. While at it, avoid reading PCI MSI attributes from descriptors owned by non-PCI devices. Signed-off-by: Koichiro Den <den@valinux.co.jp> --- Changes in v2: - Rework v1 patch 5 for an MSI allocation shared with vNTB. https://lore.kernel.org/r/20260312165005.1148676-6-den@valinux.co.jp/ drivers/dma/dw-edma/dw-edma-core.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c index a8c6bd508fcd..6af8a414e4a2 100644 --- a/drivers/dma/dw-edma/dw-edma-core.c +++ b/drivers/dma/dw-edma/dw-edma-core.c @@ -7,6 +7,7 @@ */ #include <linux/module.h> +#include <linux/pci.h> #include <linux/delay.h> #include <linux/device.h> #include <linux/kernel.h> @@ -1075,12 +1076,23 @@ static inline void dw_edma_dec_irq_alloc(int *nr_irqs, u32 *alloc, u16 cnt) } } +static void dw_edma_compose_msi(int irq, struct msi_msg *msi) +{ + struct msi_desc *desc = irq_get_msi_desc(irq); + + if (!desc) + return; + + get_cached_msi_msg(irq, msi); + if (dev_is_pci(desc->dev) && !desc->pci.msi_attrib.is_msix) + msi->data += irq - desc->irq; +} + static int dw_edma_irq_request(struct dw_edma *dw, u32 *wr_alloc, u32 *rd_alloc) { struct dw_edma_chip *chip = dw->chip; struct device *dev = dw->chip->dev; - struct msi_desc *msi_desc; int i, err = 0; u32 ch_cnt; int irq; @@ -1105,8 +1117,7 @@ static int dw_edma_irq_request(struct dw_edma *dw, return err; } - if (irq_get_msi_desc(irq)) - get_cached_msi_msg(irq, &dw->irq[0].msi); + dw_edma_compose_msi(irq, &dw->irq[0].msi); dw->nr_irqs = 1; } else { @@ -1129,12 +1140,7 @@ static int dw_edma_irq_request(struct dw_edma *dw, &dw->irq[i]); if (err) goto err_irq_free; - msi_desc = irq_get_msi_desc(irq); - if (msi_desc) { - get_cached_msi_msg(irq, &dw->irq[i].msi); - if (!msi_desc->pci.msi_attrib.is_msix) - dw->irq[i].msi.data = dw->irq[0].msi.data + i; - } + dw_edma_compose_msi(irq, &dw->irq[i].msi); } dw->nr_irqs = i; -- 2.51.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 3/3] dmaengine: dw-edma: Account for the MSI vector offset 2026-08-28 16:36 ` [PATCH v2 3/3] dmaengine: dw-edma: Account for the MSI vector offset Koichiro Den @ 2026-08-28 18:46 ` Frank Li 2026-08-29 17:43 ` Koichiro Den 2026-08-31 16:02 ` Frank Li 1 sibling, 1 reply; 12+ messages in thread From: Frank Li @ 2026-08-28 18:46 UTC (permalink / raw) To: Koichiro Den Cc: Vinod Koul, Frank Li, Manivannan Sadhasivam, dmaengine, linux-kernel On Sat, Aug 29, 2026 at 01:36:11AM +0900, Koichiro Den wrote: > get_cached_msi_msg() returns the base message shared by a multi-MSI > descriptor. dw-edma currently derives per-channel data from its local IRQ > index and does not adjust a common IRQ at all. Both assume eDMA starts at > the descriptor's first vector. > > That is not true when eDMA receives a tail subset. Compose each message > from the IRQ offset relative to the descriptor base in both paths. > > While at it, avoid reading PCI MSI attributes from descriptors owned by > non-PCI devices. > > Signed-off-by: Koichiro Den <den@valinux.co.jp> > --- > Changes in v2: > - Rework v1 patch 5 for an MSI allocation shared with vNTB. > https://lore.kernel.org/r/20260312165005.1148676-6-den@valinux.co.jp/ > > drivers/dma/dw-edma/dw-edma-core.c | 24 +++++++++++++++--------- > 1 file changed, 15 insertions(+), 9 deletions(-) > > diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c > index a8c6bd508fcd..6af8a414e4a2 100644 > --- a/drivers/dma/dw-edma/dw-edma-core.c > +++ b/drivers/dma/dw-edma/dw-edma-core.c > @@ -7,6 +7,7 @@ > */ > > #include <linux/module.h> > +#include <linux/pci.h> > #include <linux/delay.h> > #include <linux/device.h> > #include <linux/kernel.h> > @@ -1075,12 +1076,23 @@ static inline void dw_edma_dec_irq_alloc(int *nr_irqs, u32 *alloc, u16 cnt) > } > } > > +static void dw_edma_compose_msi(int irq, struct msi_msg *msi) > +{ > + struct msi_desc *desc = irq_get_msi_desc(irq); > + > + if (!desc) > + return; > + > + get_cached_msi_msg(irq, msi); > + if (dev_is_pci(desc->dev) && !desc->pci.msi_attrib.is_msix) > + msi->data += irq - desc->irq; > +} > + > static int dw_edma_irq_request(struct dw_edma *dw, > u32 *wr_alloc, u32 *rd_alloc) > { > struct dw_edma_chip *chip = dw->chip; > struct device *dev = dw->chip->dev; > - struct msi_desc *msi_desc; > int i, err = 0; > u32 ch_cnt; > int irq; > @@ -1105,8 +1117,7 @@ static int dw_edma_irq_request(struct dw_edma *dw, > return err; > } > > - if (irq_get_msi_desc(irq)) > - get_cached_msi_msg(irq, &dw->irq[0].msi); > + dw_edma_compose_msi(irq, &dw->irq[0].msi); where this function? I missed dependence? Frank > > dw->nr_irqs = 1; > } else { > @@ -1129,12 +1140,7 @@ static int dw_edma_irq_request(struct dw_edma *dw, > &dw->irq[i]); > if (err) > goto err_irq_free; > - msi_desc = irq_get_msi_desc(irq); > - if (msi_desc) { > - get_cached_msi_msg(irq, &dw->irq[i].msi); > - if (!msi_desc->pci.msi_attrib.is_msix) > - dw->irq[i].msi.data = dw->irq[0].msi.data + i; > - } > + dw_edma_compose_msi(irq, &dw->irq[i].msi); > } > > dw->nr_irqs = i; > -- > 2.51.0 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 3/3] dmaengine: dw-edma: Account for the MSI vector offset 2026-08-28 18:46 ` Frank Li @ 2026-08-29 17:43 ` Koichiro Den 0 siblings, 0 replies; 12+ messages in thread From: Koichiro Den @ 2026-08-29 17:43 UTC (permalink / raw) To: Frank Li Cc: Vinod Koul, Frank Li, Manivannan Sadhasivam, dmaengine, linux-kernel On Fri, Aug 28, 2026 at 01:46:17PM -0500, Frank Li wrote: > On Sat, Aug 29, 2026 at 01:36:11AM +0900, Koichiro Den wrote: > > get_cached_msi_msg() returns the base message shared by a multi-MSI > > descriptor. dw-edma currently derives per-channel data from its local IRQ > > index and does not adjust a common IRQ at all. Both assume eDMA starts at > > the descriptor's first vector. > > > > That is not true when eDMA receives a tail subset. Compose each message > > from the IRQ offset relative to the descriptor base in both paths. > > > > While at it, avoid reading PCI MSI attributes from descriptors owned by > > non-PCI devices. > > > > Signed-off-by: Koichiro Den <den@valinux.co.jp> > > --- > > Changes in v2: > > - Rework v1 patch 5 for an MSI allocation shared with vNTB. > > https://lore.kernel.org/r/20260312165005.1148676-6-den@valinux.co.jp/ > > > > drivers/dma/dw-edma/dw-edma-core.c | 24 +++++++++++++++--------- > > 1 file changed, 15 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c > > index a8c6bd508fcd..6af8a414e4a2 100644 > > --- a/drivers/dma/dw-edma/dw-edma-core.c > > +++ b/drivers/dma/dw-edma/dw-edma-core.c > > @@ -7,6 +7,7 @@ > > */ > > > > #include <linux/module.h> > > +#include <linux/pci.h> > > #include <linux/delay.h> > > #include <linux/device.h> > > #include <linux/kernel.h> > > @@ -1075,12 +1076,23 @@ static inline void dw_edma_dec_irq_alloc(int *nr_irqs, u32 *alloc, u16 cnt) > > } > > } > > > > +static void dw_edma_compose_msi(int irq, struct msi_msg *msi) ^(A) > > +{ > > + struct msi_desc *desc = irq_get_msi_desc(irq); > > + > > + if (!desc) > > + return; > > + > > + get_cached_msi_msg(irq, msi); > > + if (dev_is_pci(desc->dev) && !desc->pci.msi_attrib.is_msix) > > + msi->data += irq - desc->irq; > > +} > > + > > static int dw_edma_irq_request(struct dw_edma *dw, > > u32 *wr_alloc, u32 *rd_alloc) > > { > > struct dw_edma_chip *chip = dw->chip; > > struct device *dev = dw->chip->dev; > > - struct msi_desc *msi_desc; > > int i, err = 0; > > u32 ch_cnt; > > int irq; > > @@ -1105,8 +1117,7 @@ static int dw_edma_irq_request(struct dw_edma *dw, > > return err; > > } > > > > - if (irq_get_msi_desc(irq)) > > - get_cached_msi_msg(irq, &dw->irq[0].msi); > > + dw_edma_compose_msi(irq, &dw->irq[0].msi); > > where this function? I missed dependence? It's defined just above in this patch, as shown in (A). Best regards, Koichiro > > Frank > > > > dw->nr_irqs = 1; > > } else { > > @@ -1129,12 +1140,7 @@ static int dw_edma_irq_request(struct dw_edma *dw, > > &dw->irq[i]); > > if (err) > > goto err_irq_free; > > - msi_desc = irq_get_msi_desc(irq); > > - if (msi_desc) { > > - get_cached_msi_msg(irq, &dw->irq[i].msi); > > - if (!msi_desc->pci.msi_attrib.is_msix) > > - dw->irq[i].msi.data = dw->irq[0].msi.data + i; > > - } > > + dw_edma_compose_msi(irq, &dw->irq[i].msi); > > } > > > > dw->nr_irqs = i; > > -- > > 2.51.0 > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 3/3] dmaengine: dw-edma: Account for the MSI vector offset 2026-08-28 16:36 ` [PATCH v2 3/3] dmaengine: dw-edma: Account for the MSI vector offset Koichiro Den 2026-08-28 18:46 ` Frank Li @ 2026-08-31 16:02 ` Frank Li 1 sibling, 0 replies; 12+ messages in thread From: Frank Li @ 2026-08-31 16:02 UTC (permalink / raw) To: Koichiro Den Cc: Vinod Koul, Frank Li, Manivannan Sadhasivam, dmaengine, linux-kernel On Sat, Aug 29, 2026 at 01:36:11AM +0900, Koichiro Den wrote: > get_cached_msi_msg() returns the base message shared by a multi-MSI > descriptor. dw-edma currently derives per-channel data from its local IRQ > index and does not adjust a common IRQ at all. Both assume eDMA starts at > the descriptor's first vector. > > That is not true when eDMA receives a tail subset. Compose each message > from the IRQ offset relative to the descriptor base in both paths. > > While at it, avoid reading PCI MSI attributes from descriptors owned by > non-PCI devices. > > Signed-off-by: Koichiro Den <den@valinux.co.jp> > --- Reviewed-by: Frank Li <Frank.Li@nxp.com> > Changes in v2: > - Rework v1 patch 5 for an MSI allocation shared with vNTB. > https://lore.kernel.org/r/20260312165005.1148676-6-den@valinux.co.jp/ > > drivers/dma/dw-edma/dw-edma-core.c | 24 +++++++++++++++--------- > 1 file changed, 15 insertions(+), 9 deletions(-) > > diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c > index a8c6bd508fcd..6af8a414e4a2 100644 > --- a/drivers/dma/dw-edma/dw-edma-core.c > +++ b/drivers/dma/dw-edma/dw-edma-core.c > @@ -7,6 +7,7 @@ > */ > > #include <linux/module.h> > +#include <linux/pci.h> > #include <linux/delay.h> > #include <linux/device.h> > #include <linux/kernel.h> > @@ -1075,12 +1076,23 @@ static inline void dw_edma_dec_irq_alloc(int *nr_irqs, u32 *alloc, u16 cnt) > } > } > > +static void dw_edma_compose_msi(int irq, struct msi_msg *msi) > +{ > + struct msi_desc *desc = irq_get_msi_desc(irq); > + > + if (!desc) > + return; > + > + get_cached_msi_msg(irq, msi); > + if (dev_is_pci(desc->dev) && !desc->pci.msi_attrib.is_msix) > + msi->data += irq - desc->irq; > +} > + > static int dw_edma_irq_request(struct dw_edma *dw, > u32 *wr_alloc, u32 *rd_alloc) > { > struct dw_edma_chip *chip = dw->chip; > struct device *dev = dw->chip->dev; > - struct msi_desc *msi_desc; > int i, err = 0; > u32 ch_cnt; > int irq; > @@ -1105,8 +1117,7 @@ static int dw_edma_irq_request(struct dw_edma *dw, > return err; > } > > - if (irq_get_msi_desc(irq)) > - get_cached_msi_msg(irq, &dw->irq[0].msi); > + dw_edma_compose_msi(irq, &dw->irq[0].msi); > > dw->nr_irqs = 1; > } else { > @@ -1129,12 +1140,7 @@ static int dw_edma_irq_request(struct dw_edma *dw, > &dw->irq[i]); > if (err) > goto err_irq_free; > - msi_desc = irq_get_msi_desc(irq); > - if (msi_desc) { > - get_cached_msi_msg(irq, &dw->irq[i].msi); > - if (!msi_desc->pci.msi_attrib.is_msix) > - dw->irq[i].msi.data = dw->irq[0].msi.data + i; > - } > + dw_edma_compose_msi(irq, &dw->irq[i].msi); > } > > dw->nr_irqs = i; > -- > 2.51.0 > ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2026-08-31 16:02 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-08-28 16:36 [PATCH v2 0/3] dmaengine: dw-edma: Prepare channels for remote use Koichiro Den 2026-08-28 16:36 ` [PATCH v2 1/3] dmaengine: Allow drivers to assign static channel IDs Koichiro Den 2026-08-28 16:55 ` sashiko-bot 2026-08-28 16:36 ` [PATCH v2 2/3] dmaengine: dw-edma: Configure remote interrupt routing Koichiro Den 2026-08-28 16:49 ` sashiko-bot 2026-08-28 18:41 ` Frank Li 2026-08-29 18:13 ` Koichiro Den 2026-08-31 15:55 ` Frank Li 2026-08-28 16:36 ` [PATCH v2 3/3] dmaengine: dw-edma: Account for the MSI vector offset Koichiro Den 2026-08-28 18:46 ` Frank Li 2026-08-29 17:43 ` Koichiro Den 2026-08-31 16:02 ` Frank Li
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox