* [PATCH v5 01/14] dmaengine: dw-edma: Factor out HDMA interrupt setup helper
2026-07-17 5:02 [PATCH v5 00/14] dmaengine: dw-edma: Prepare for PCI EP DMA (part 1/3) Koichiro Den
@ 2026-07-17 5:02 ` Koichiro Den
2026-07-17 5:02 ` [PATCH v5 02/14] dmaengine: dw-edma: Add per-channel interrupt routing control Koichiro Den
` (12 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: Koichiro Den @ 2026-07-17 5:02 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Manivannan Sadhasivam
Cc: Marek Vasut, Yoshihiro Shimoda, dmaengine, linux-kernel
The HDMA linked-list and non-linked-list start paths both program the
stop/abort interrupt setup register using the same local/remote enable
policy. Only the interrupt-mask handling differs by transfer mode.
Factor the common setup into dw_hdma_v0_core_int_setup() before adding
per-channel interrupt routing support. No functional change intended.
Suggested-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v5:
- No changes.
drivers/dma/dw-edma/dw-hdma-v0-core.c | 34 ++++++++++++++-------------
1 file changed, 18 insertions(+), 16 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
index 4bf5a441afbf..b455da3d9b0f 100644
--- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
@@ -49,6 +49,21 @@ __dw_ch_regs(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch)
writel(value, &(__dw_ch_regs(dw, EDMA_DIR_READ, ch)->name)); \
} while (0)
+static u32 dw_hdma_v0_core_int_setup(struct dw_edma_chan *chan, u32 val)
+{
+ if (chan->non_ll)
+ val |= HDMA_V0_STOP_INT_MASK | HDMA_V0_ABORT_INT_MASK;
+ else
+ val &= ~(HDMA_V0_STOP_INT_MASK | HDMA_V0_ABORT_INT_MASK);
+
+ val |= HDMA_V0_LOCAL_STOP_INT_EN | HDMA_V0_LOCAL_ABORT_INT_EN;
+ if (!(chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL))
+ val |= HDMA_V0_REMOTE_STOP_INT_EN |
+ HDMA_V0_REMOTE_ABORT_INT_EN;
+
+ return val;
+}
+
/* HDMA management callbacks */
static void dw_hdma_v0_core_off(struct dw_edma *dw)
{
@@ -203,11 +218,7 @@ static void dw_hdma_v0_core_ch_enable(struct dw_edma_chan *chan)
SET_CH_32(dw, chan->dir, chan->id, ch_en, BIT(0));
/* Interrupt unmask - stop, abort */
tmp = GET_CH_32(dw, chan->dir, chan->id, int_setup);
- tmp &= ~(HDMA_V0_STOP_INT_MASK | HDMA_V0_ABORT_INT_MASK);
- /* Interrupt enable - stop, abort */
- tmp |= HDMA_V0_LOCAL_STOP_INT_EN | HDMA_V0_LOCAL_ABORT_INT_EN;
- if (!(dw->chip->flags & DW_EDMA_CHIP_LOCAL))
- tmp |= HDMA_V0_REMOTE_STOP_INT_EN | HDMA_V0_REMOTE_ABORT_INT_EN;
+ tmp = dw_hdma_v0_core_int_setup(chan, tmp);
SET_CH_32(dw, chan->dir, chan->id, int_setup, tmp);
/* Channel control */
SET_CH_32(dw, chan->dir, chan->id, control1, HDMA_V0_LINKLIST_EN);
@@ -260,17 +271,8 @@ static void dw_hdma_v0_core_non_ll_start(struct dw_edma_chan *chan,
SET_CH_32(dw, chan->dir, chan->id, transfer_size, child->sz);
/* Interrupt setup */
- val = GET_CH_32(dw, chan->dir, chan->id, int_setup) |
- HDMA_V0_STOP_INT_MASK |
- HDMA_V0_ABORT_INT_MASK |
- HDMA_V0_LOCAL_STOP_INT_EN |
- HDMA_V0_LOCAL_ABORT_INT_EN;
-
- if (!(dw->chip->flags & DW_EDMA_CHIP_LOCAL)) {
- val |= HDMA_V0_REMOTE_STOP_INT_EN |
- HDMA_V0_REMOTE_ABORT_INT_EN;
- }
-
+ val = GET_CH_32(dw, chan->dir, chan->id, int_setup);
+ val = dw_hdma_v0_core_int_setup(chan, val);
SET_CH_32(dw, chan->dir, chan->id, int_setup, val);
/* Channel control setup */
--
2.51.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v5 02/14] dmaengine: dw-edma: Add per-channel interrupt routing control
2026-07-17 5:02 [PATCH v5 00/14] dmaengine: dw-edma: Prepare for PCI EP DMA (part 1/3) Koichiro Den
2026-07-17 5:02 ` [PATCH v5 01/14] dmaengine: dw-edma: Factor out HDMA interrupt setup helper Koichiro Den
@ 2026-07-17 5:02 ` Koichiro Den
2026-07-17 5:21 ` sashiko-bot
2026-07-17 5:02 ` [PATCH v5 03/14] dmaengine: dw-edma: Add core quiesce operations Koichiro Den
` (11 subsequent siblings)
13 siblings, 1 reply; 22+ messages in thread
From: Koichiro Den @ 2026-07-17 5:02 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Manivannan Sadhasivam
Cc: Marek Vasut, Yoshihiro Shimoda, dmaengine, linux-kernel
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 channel interrupt routing state and initialize it from the
controller instance configuration. Update the eDMA and HDMA native paths
so linked-list interrupt generation, HDMA non-linked-list interrupt
enables, and DONE/ABORT masking follow the selected mode. For HDMA
native non-linked-list channels, keep the local stop/abort enables set
so status is latched. In remote mode, also enable remote signaling and
mask the local interrupt pins.
Keep the existing dw-edma-pcie host-side instances in remote interrupt
routing mode so their IMWr/MSI completion model remains unchanged after
local routing becomes the zero value.
Note:
- The routing mode describes where a channel should report completion.
It does not by itself say whether this dw-edma instance owns the
interrupt status. A local instance must ignore remote-only channels,
and a remote instance must ignore local-only channels, even if such
interrupts are unexpectedly delivered. Otherwise the non-owner side
could steal the interrupt from the owner by clearing shared DONE/ABORT
status.
Cc: Devendra K Verma <devendra.verma@amd.com>
Suggested-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v5:
- Keep LSIE/LAIE set for STOP/ABORT status latching and mask the local
interrupt pins in remote mode. Fix the v4 regression observed on
HDMA 6.30a.
drivers/dma/dw-edma/dw-edma-core.c | 10 +++++++++
drivers/dma/dw-edma/dw-edma-core.h | 13 +++++++++++
drivers/dma/dw-edma/dw-edma-v0-core.c | 30 +++++++++++++++++++++----
drivers/dma/dw-edma/dw-hdma-v0-core.c | 25 ++++++++++++++-------
include/linux/dma/edma.h | 32 +++++++++++++++++++++++++++
5 files changed, 98 insertions(+), 12 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index 30eeb7bffad8..8a1ec0fd057b 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -133,6 +133,15 @@ static void dw_edma_device_caps(struct dma_chan *dchan,
}
}
+static enum dw_edma_ch_irq_mode
+dw_edma_get_default_irq_mode(struct dw_edma_chan *chan)
+{
+ struct dw_edma_chip *chip = chan->dw->chip;
+
+ return chip->flags & DW_EDMA_CHIP_LOCAL ? DW_EDMA_CH_IRQ_LOCAL :
+ DW_EDMA_CH_IRQ_REMOTE;
+}
+
static int dw_edma_device_config(struct dma_chan *dchan,
struct dma_slave_config *config)
{
@@ -818,6 +827,7 @@ static int dw_edma_channel_setup(struct dw_edma *dw, u32 wr_alloc, u32 rd_alloc)
chan->configured = false;
chan->request = EDMA_REQ_NONE;
chan->status = EDMA_ST_IDLE;
+ chan->irq_mode = dw_edma_get_default_irq_mode(chan);
if (chan->dir == EDMA_DIR_WRITE)
chan->ll_region = chip->ll_region_wr[chan->id];
diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
index 3c958ca05144..90ca88f5443a 100644
--- a/drivers/dma/dw-edma/dw-edma-core.h
+++ b/drivers/dma/dw-edma/dw-edma-core.h
@@ -74,6 +74,8 @@ struct dw_edma_chan {
struct msi_msg msi;
+ enum dw_edma_ch_irq_mode irq_mode;
+
enum dw_edma_request request;
enum dw_edma_status status;
u8 configured;
@@ -247,4 +249,15 @@ dw_edma_core_db_offset(struct dw_edma *dw)
return dw->core->db_offset(dw);
}
+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)
+ 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 7b4933c66f9f..9f4f8a93ed0e 100644
--- a/drivers/dma/dw-edma/dw-edma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
@@ -256,6 +256,9 @@ 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 (unlikely(dw_edma_core_ch_ignore_irq(chan)))
+ continue;
+
dw_edma_v0_core_clear_done_int(chan);
done(chan);
@@ -267,6 +270,9 @@ 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 (unlikely(dw_edma_core_ch_ignore_irq(chan)))
+ continue;
+
dw_edma_v0_core_clear_abort_int(chan);
abort(chan);
@@ -354,12 +360,17 @@ static void dw_edma_v0_core_ch_enable(struct dw_edma_chan *chan)
break;
}
}
- /* Interrupt unmask - done, abort */
+ /* Interrupt mask/unmask - done, abort */
raw_spin_lock_irqsave(&dw->lock, flags);
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);
@@ -474,7 +485,18 @@ dw_edma_v0_core_ll_data(struct dw_edma_chan *chan, struct dw_edma_burst *burst,
if (irq) {
control |= DW_EDMA_V0_LIE;
- if (!(chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL))
+ /*
+ * A local instance never issues transfers on a remote-routed
+ * channel: on CHIP_LOCAL instances, REMOTE routing denotes a
+ * channel handed over to the remote side, which programs the
+ * linked list through its own instance. The remote-only
+ * recipe (LIE|RIE with the local interrupt masked) is thus
+ * applied by the instance that owns the transfer, and the
+ * LIE-only write below never executes for a remote-routed
+ * channel.
+ */
+ if (!(chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL) &&
+ chan->irq_mode == DW_EDMA_CH_IRQ_REMOTE)
control |= DW_EDMA_V0_RIE;
}
diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
index b455da3d9b0f..52f54e14544d 100644
--- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
@@ -51,15 +51,22 @@ __dw_ch_regs(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch)
static u32 dw_hdma_v0_core_int_setup(struct dw_edma_chan *chan, u32 val)
{
- if (chan->non_ll)
- val |= HDMA_V0_STOP_INT_MASK | HDMA_V0_ABORT_INT_MASK;
- else
- val &= ~(HDMA_V0_STOP_INT_MASK | HDMA_V0_ABORT_INT_MASK);
+ val &= ~(HDMA_V0_LOCAL_ABORT_INT_EN | HDMA_V0_REMOTE_ABORT_INT_EN |
+ HDMA_V0_LOCAL_STOP_INT_EN | HDMA_V0_REMOTE_STOP_INT_EN |
+ HDMA_V0_ABORT_INT_MASK | HDMA_V0_STOP_INT_MASK);
- val |= HDMA_V0_LOCAL_STOP_INT_EN | HDMA_V0_LOCAL_ABORT_INT_EN;
- if (!(chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL))
- val |= HDMA_V0_REMOTE_STOP_INT_EN |
- HDMA_V0_REMOTE_ABORT_INT_EN;
+ /*
+ * HDMA_INT_STATUS.STOP and .ABORT are latched only when LSIE and
+ * LAIE are enabled. A remote handler needs those status bits to
+ * identify the source of the IMWr, so keep local generation enabled
+ * and mask the local interrupt pins instead.
+ */
+ val |= HDMA_V0_LOCAL_ABORT_INT_EN | HDMA_V0_LOCAL_STOP_INT_EN;
+
+ if (chan->irq_mode == DW_EDMA_CH_IRQ_REMOTE)
+ val |= HDMA_V0_REMOTE_ABORT_INT_EN |
+ HDMA_V0_REMOTE_STOP_INT_EN |
+ HDMA_V0_ABORT_INT_MASK | HDMA_V0_STOP_INT_MASK;
return val;
}
@@ -147,6 +154,8 @@ dw_hdma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir,
for_each_set_bit(pos, &mask, total) {
chan = &dw->chan[pos + off];
+ if (unlikely(dw_edma_core_ch_ignore_irq(chan)))
+ continue;
val = dw_hdma_v0_core_status_int(chan);
if (FIELD_GET(HDMA_V0_STOP_INT_MASK, val)) {
diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
index 1fafd5b0e315..d29a8df76f8c 100644
--- a/include/linux/dma/edma.h
+++ b/include/linux/dma/edma.h
@@ -60,6 +60,38 @@ 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_LOCAL: local interrupt only (edma_int[])
+ * @DW_EDMA_CH_IRQ_REMOTE: remote interrupt only (IMWr/MSI), without
+ * delivering local edma_int[].
+ *
+ * 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 linked-list 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. See the
+ * DesignWare endpoint databook 6.30a, Linked List Mode interrupt handling
+ * ("Software Programming of an Endpoint's LIE and RIE Bits for Linked List
+ * Transfers", Attention).
+ *
+ * A local (DW_EDMA_CHIP_LOCAL) instance never issues transfers on a
+ * remote-routed channel: REMOTE routing on such an instance denotes a channel
+ * handed over to and driven by the remote side, and the recipe above is
+ * applied by the driving instance.
+ *
+ * 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.
+ */
+enum dw_edma_ch_irq_mode {
+ DW_EDMA_CH_IRQ_LOCAL = 0,
+ DW_EDMA_CH_IRQ_REMOTE,
+};
+
/**
* struct dw_edma_chip - representation of DesignWare eDMA controller hardware
* @dev: struct device of the eDMA controller
--
2.51.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v5 02/14] dmaengine: dw-edma: Add per-channel interrupt routing control
2026-07-17 5:02 ` [PATCH v5 02/14] dmaengine: dw-edma: Add per-channel interrupt routing control Koichiro Den
@ 2026-07-17 5:21 ` sashiko-bot
0 siblings, 0 replies; 22+ messages in thread
From: sashiko-bot @ 2026-07-17 5:21 UTC (permalink / raw)
To: Koichiro Den; +Cc: dmaengine, vkoul, Frank.Li
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Cross-system read-modify-write (RMW) race on the shared `int_mask` register causes lost unmasks and DMA hangs. (Pre-existing)
--
--- Patch [2]: [PATCH v5 02/14] dmaengine: dw-edma: Add per-channel interrupt routing control ---
commit ce3c3f8c0117acd509b753117d45f3c2cf75e27d
Author: Koichiro Den <den@valinux.co.jp>
dmaengine: dw-edma: Add per-channel interrupt routing control
This patch adds per-channel interrupt routing control for the DesignWare
eDMA controller. It introduces a new routing mode to determine where a
channel should report completion and updates the interrupt masking and
generation logic accordingly.
> diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
[ ... ]
> @@ -354,12 +360,17 @@ static void dw_edma_v0_core_ch_enable(struct dw_edma_chan *chan)
> break;
> }
> }
> - /* Interrupt unmask - done, abort */
> + /* Interrupt mask/unmask - done, abort */
> raw_spin_lock_irqsave(&dw->lock, flags);
>
> 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);
[Severity: High]
This is a pre-existing issue, but does this read-modify-write sequence on the
shared int_mask register risk a race condition across the PCIe bus?
Since the PCIe Host and local endpoint drivers can independently execute
dw_edma_v0_core_ch_enable() concurrently, the local dw->lock does not
synchronize operations across the bus.
If the Host performs a read-modify-write that sets its mask bit to 1, could
this overwrite a concurrent read-modify-write by the endpoint clearing its
mask bit to 0? If the endpoint's unmask operation is reverted by the Host,
the endpoint's channel would remain masked, which appears to lead to a
lost interrupt and a DMA hang.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260717050308.2144108-1-den@valinux.co.jp?part=2
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v5 03/14] dmaengine: dw-edma: Add core quiesce operations
2026-07-17 5:02 [PATCH v5 00/14] dmaengine: dw-edma: Prepare for PCI EP DMA (part 1/3) Koichiro Den
2026-07-17 5:02 ` [PATCH v5 01/14] dmaengine: dw-edma: Factor out HDMA interrupt setup helper Koichiro Den
2026-07-17 5:02 ` [PATCH v5 02/14] dmaengine: dw-edma: Add per-channel interrupt routing control Koichiro Den
@ 2026-07-17 5:02 ` Koichiro Den
2026-07-17 5:17 ` sashiko-bot
2026-07-17 5:02 ` [PATCH v5 04/14] dmaengine: dw-edma: Initialize IRQ data before requesting IRQs Koichiro Den
` (10 subsequent siblings)
13 siblings, 1 reply; 22+ messages in thread
From: Koichiro Den @ 2026-07-17 5:02 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Manivannan Sadhasivam
Cc: Marek Vasut, Yoshihiro Shimoda, dmaengine, linux-kernel
Add core operations that quiesce only the resources represented by a
dw-edma instance, separate from the existing full controller off path.
For unrolled eDMA, quiesce masks the direction, disables ENGINE_EN,
waits for it to read back clear, then clears pending interrupt status.
If ENGINE_EN does not clear, return the timeout to the caller.
HDMA compatibility mode does not implement ENGINE_EN, so quiesce clears
CHi_PWR_EN for each represented channel instead. Both register maps
share interrupt control per direction, so quiescing one channel
quiesces the whole direction. Callers must own that direction and stop
the peer from programming it first.
HDMA native has per-channel registers and can quiesce the represented
channel directly.
No caller is added yet, so this is a no-functional-change preparation
for delegated channel reclaim and partial-owned remove paths.
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v5:
- Propagate detected quiesce failures to callers.
- Wait for ENGINE_EN to clear when quiescing unrolled eDMA.
- Use CHi_PWR_EN because HDMA compatibility mode does not implement
ENGINE_EN, as documented in the 6.10a-lca06 databook.
- Clarify the full-direction ownership contract and register-map names.
- Use scoped_guard() for new code.
- Drop Frank's Reviewed-by tag due to these changes.
drivers/dma/dw-edma/dw-edma-core.h | 14 +++
drivers/dma/dw-edma/dw-edma-v0-core.c | 141 +++++++++++++++++++++-----
drivers/dma/dw-edma/dw-hdma-v0-core.c | 31 ++++++
3 files changed, 158 insertions(+), 28 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
index 90ca88f5443a..1a9f1b58d2fd 100644
--- a/drivers/dma/dw-edma/dw-edma-core.h
+++ b/drivers/dma/dw-edma/dw-edma-core.h
@@ -115,6 +115,8 @@ typedef void (*dw_edma_handler_t)(struct dw_edma_chan *);
struct dw_edma_core_ops {
void (*off)(struct dw_edma *dw);
+ int (*quiesce)(struct dw_edma *dw);
+ int (*ch_quiesce)(struct dw_edma_chan *chan);
u16 (*ch_count)(struct dw_edma *dw, enum dw_edma_dir dir);
enum dma_status (*ch_status)(struct dw_edma_chan *chan);
irqreturn_t (*handle_int)(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir,
@@ -180,6 +182,18 @@ void dw_edma_core_off(struct dw_edma *dw)
dw->core->off(dw);
}
+static inline
+int dw_edma_core_quiesce(struct dw_edma *dw)
+{
+ return dw->core->quiesce(dw);
+}
+
+static inline
+int dw_edma_core_ch_quiesce(struct dw_edma_chan *chan)
+{
+ return chan->dw->core->ch_quiesce(chan);
+}
+
static inline
u16 dw_edma_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
{
diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
index 9f4f8a93ed0e..3e9512a7b201 100644
--- a/drivers/dma/dw-edma/dw-edma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
@@ -7,6 +7,7 @@
*/
#include <linux/bitfield.h>
+#include <linux/iopoll.h>
#include <linux/irqreturn.h>
#include <linux/io-64-nonatomic-lo-hi.h>
@@ -160,6 +161,87 @@ static inline u32 readl_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
readl_ch(dw, dir, ch, &(__dw_ch_regs(dw, dir, ch)->name))
/* eDMA management callbacks */
+static void dw_edma_v0_core_ch_power(struct dw_edma *dw,
+ enum dw_edma_dir dir, u16 id, bool enable)
+{
+ u32 value = enable ? BIT(0) : 0;
+
+ if (WARN_ON_ONCE(id >= EDMA_V0_MAX_NR_CH))
+ return;
+
+ switch (id) {
+ case 0:
+ SET_RW_COMPAT(dw, dir, ch0_pwr_en, value);
+ break;
+ case 1:
+ SET_RW_COMPAT(dw, dir, ch1_pwr_en, value);
+ break;
+ case 2:
+ SET_RW_COMPAT(dw, dir, ch2_pwr_en, value);
+ break;
+ case 3:
+ SET_RW_COMPAT(dw, dir, ch3_pwr_en, value);
+ break;
+ case 4:
+ SET_RW_COMPAT(dw, dir, ch4_pwr_en, value);
+ break;
+ case 5:
+ SET_RW_COMPAT(dw, dir, ch5_pwr_en, value);
+ break;
+ case 6:
+ SET_RW_COMPAT(dw, dir, ch6_pwr_en, value);
+ break;
+ case 7:
+ SET_RW_COMPAT(dw, dir, ch7_pwr_en, value);
+ break;
+ }
+}
+
+static int dw_edma_v0_core_engine_disable(struct dw_edma *dw,
+ enum dw_edma_dir dir)
+{
+ u32 value;
+ int ret;
+
+ SET_RW_32(dw, dir, engine_en, 0);
+ ret = read_poll_timeout(GET_RW_32, value, !(value & BIT(0)), 100,
+ 200000, false, dw, dir, engine_en);
+ if (ret)
+ dev_warn(dw->chip->dev, "%s engine did not stop within 200ms\n",
+ dir == EDMA_DIR_WRITE ? "write" : "read");
+
+ return ret;
+}
+
+static int dw_edma_v0_core_dir_off(struct dw_edma *dw, enum dw_edma_dir dir)
+{
+ u16 count, id;
+ int ret = 0;
+
+ scoped_guard(raw_spinlock_irqsave, &dw->lock)
+ SET_RW_32(dw, dir, int_mask,
+ EDMA_V0_DONE_INT_MASK | EDMA_V0_ABORT_INT_MASK);
+
+ if (dw->chip->mf == EDMA_MF_HDMA_COMPAT) {
+ /*
+ * DWC PCIe Controller Databook 6.10a-lca06, "Legacy DMA
+ * and HDMA Software Compatibility": HDMA compatibility mode
+ * does not implement ENGINE_EN, but retains CHi_PWR_EN for
+ * per-channel enable and disable.
+ */
+ count = dir == EDMA_DIR_WRITE ? dw->wr_ch_cnt : dw->rd_ch_cnt;
+ for (id = 0; id < count; id++)
+ dw_edma_v0_core_ch_power(dw, dir, id, false);
+ } else {
+ ret = dw_edma_v0_core_engine_disable(dw, dir);
+ }
+
+ SET_RW_32(dw, dir, int_clear,
+ EDMA_V0_DONE_INT_MASK | EDMA_V0_ABORT_INT_MASK);
+
+ return ret;
+}
+
static void dw_edma_v0_core_off(struct dw_edma *dw)
{
SET_BOTH_32(dw, int_mask,
@@ -169,6 +251,33 @@ static void dw_edma_v0_core_off(struct dw_edma *dw)
SET_BOTH_32(dw, engine_en, 0);
}
+static int dw_edma_v0_core_quiesce(struct dw_edma *dw)
+{
+ int ret = 0;
+ int err;
+
+ if (dw->wr_ch_cnt)
+ ret = dw_edma_v0_core_dir_off(dw, EDMA_DIR_WRITE);
+ if (dw->rd_ch_cnt) {
+ err = dw_edma_v0_core_dir_off(dw, EDMA_DIR_READ);
+ if (!ret)
+ ret = err;
+ }
+
+ return ret;
+}
+
+/*
+ * The unrolled eDMA and HDMA compatibility register maps share interrupt
+ * control per direction, so the whole direction is quiesced. Callers must
+ * own the direction entirely and prevent the peer from programming it after
+ * this point. Partial ownership mode validates direction granularity.
+ */
+static int dw_edma_v0_core_ch_quiesce(struct dw_edma_chan *chan)
+{
+ return dw_edma_v0_core_dir_off(chan->dw, chan->dir);
+}
+
static u16 dw_edma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
{
u32 num_ch;
@@ -332,34 +441,8 @@ static void dw_edma_v0_core_ch_enable(struct dw_edma_chan *chan)
/* Enable engine */
SET_RW_32(dw, chan->dir, engine_en, BIT(0));
- if (dw->chip->mf == EDMA_MF_HDMA_COMPAT) {
- switch (chan->id) {
- case 0:
- SET_RW_COMPAT(dw, chan->dir, ch0_pwr_en, BIT(0));
- break;
- case 1:
- SET_RW_COMPAT(dw, chan->dir, ch1_pwr_en, BIT(0));
- break;
- case 2:
- SET_RW_COMPAT(dw, chan->dir, ch2_pwr_en, BIT(0));
- break;
- case 3:
- SET_RW_COMPAT(dw, chan->dir, ch3_pwr_en, BIT(0));
- break;
- case 4:
- SET_RW_COMPAT(dw, chan->dir, ch4_pwr_en, BIT(0));
- break;
- case 5:
- SET_RW_COMPAT(dw, chan->dir, ch5_pwr_en, BIT(0));
- break;
- case 6:
- SET_RW_COMPAT(dw, chan->dir, ch6_pwr_en, BIT(0));
- break;
- case 7:
- SET_RW_COMPAT(dw, chan->dir, ch7_pwr_en, BIT(0));
- break;
- }
- }
+ if (dw->chip->mf == EDMA_MF_HDMA_COMPAT)
+ dw_edma_v0_core_ch_power(dw, chan->dir, chan->id, true);
/* Interrupt mask/unmask - done, abort */
raw_spin_lock_irqsave(&dw->lock, flags);
@@ -553,6 +636,8 @@ static resource_size_t dw_edma_v0_core_db_offset(struct dw_edma *dw)
static const struct dw_edma_core_ops dw_edma_v0_core = {
.off = dw_edma_v0_core_off,
+ .quiesce = dw_edma_v0_core_quiesce,
+ .ch_quiesce = dw_edma_v0_core_ch_quiesce,
.ch_count = dw_edma_v0_core_ch_count,
.ch_status = dw_edma_v0_core_ch_status,
.handle_int = dw_edma_v0_core_handle_int,
diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
index 52f54e14544d..77260c076cd3 100644
--- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
@@ -72,6 +72,16 @@ static u32 dw_hdma_v0_core_int_setup(struct dw_edma_chan *chan, u32 val)
}
/* HDMA management callbacks */
+static void dw_hdma_v0_core_ch_off(struct dw_edma *dw, enum dw_edma_dir dir,
+ u16 id)
+{
+ SET_CH_32(dw, dir, id, int_setup,
+ HDMA_V0_STOP_INT_MASK | HDMA_V0_ABORT_INT_MASK);
+ SET_CH_32(dw, dir, id, ch_en, 0);
+ SET_CH_32(dw, dir, id, int_clear,
+ HDMA_V0_STOP_INT_MASK | HDMA_V0_ABORT_INT_MASK);
+}
+
static void dw_hdma_v0_core_off(struct dw_edma *dw)
{
int id;
@@ -85,6 +95,25 @@ static void dw_hdma_v0_core_off(struct dw_edma *dw)
}
}
+static int dw_hdma_v0_core_quiesce(struct dw_edma *dw)
+{
+ int id;
+
+ for (id = 0; id < dw->wr_ch_cnt; id++)
+ dw_hdma_v0_core_ch_off(dw, EDMA_DIR_WRITE, id);
+ for (id = 0; id < dw->rd_ch_cnt; id++)
+ dw_hdma_v0_core_ch_off(dw, EDMA_DIR_READ, id);
+
+ return 0;
+}
+
+static int dw_hdma_v0_core_ch_quiesce(struct dw_edma_chan *chan)
+{
+ dw_hdma_v0_core_ch_off(chan->dw, chan->dir, chan->id);
+
+ return 0;
+}
+
static u16 dw_hdma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
{
/*
@@ -355,6 +384,8 @@ static resource_size_t dw_hdma_v0_core_db_offset(struct dw_edma *dw)
static const struct dw_edma_core_ops dw_hdma_v0_core = {
.off = dw_hdma_v0_core_off,
+ .quiesce = dw_hdma_v0_core_quiesce,
+ .ch_quiesce = dw_hdma_v0_core_ch_quiesce,
.ch_count = dw_hdma_v0_core_ch_count,
.ch_status = dw_hdma_v0_core_ch_status,
.handle_int = dw_hdma_v0_core_handle_int,
--
2.51.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v5 03/14] dmaengine: dw-edma: Add core quiesce operations
2026-07-17 5:02 ` [PATCH v5 03/14] dmaengine: dw-edma: Add core quiesce operations Koichiro Den
@ 2026-07-17 5:17 ` sashiko-bot
0 siblings, 0 replies; 22+ messages in thread
From: sashiko-bot @ 2026-07-17 5:17 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] dw_edma_v0_core_off() fails to actually stop the DMA engines in HDMA compatibility mode because it relies on the non-existent ENGINE_EN register.
--
--- Patch [3]: [PATCH v5 03/14] dmaengine: dw-edma: Add core quiesce operations ---
commit b74cbf35e0d2a9867f8a66e3ed4776b896fc6152
Author: Koichiro Den <den@valinux.co.jp>
dmaengine: dw-edma: Add core quiesce operations
This commit adds core operations to quiesce only the resources represented
by a dw-edma instance. It implements specific handling for unrolled eDMA
and HDMA compatibility mode.
> diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
[ ... ]
> static void dw_edma_v0_core_off(struct dw_edma *dw)
> {
> SET_BOTH_32(dw, int_mask,
> EDMA_V0_DONE_INT_MASK | EDMA_V0_ABORT_INT_MASK);
> SET_BOTH_32(dw, int_clear,
> EDMA_V0_DONE_INT_MASK | EDMA_V0_ABORT_INT_MASK);
> SET_BOTH_32(dw, engine_en, 0);
> }
[Severity: High]
This isn't a bug introduced by this patch, but looking at
dw_edma_v0_core_off() in drivers/dma/dw-edma/dw-edma-v0-core.c, does it
fail to actually stop the DMA engines in HDMA compatibility mode?
Since HDMA compatibility mode does not implement the ENGINE_EN register (as
noted in the commit message and addressed in the new quiesce path), writing
0 to engine_en here doesn't seem to stop the channels.
If a user unbinds the driver or unloads the module, could the DMA controller
continue executing transfers into memory that is subsequently unmapped and
freed, since chX_pwr_en is never cleared during the remove path?
>
> +static int dw_edma_v0_core_quiesce(struct dw_edma *dw)
> +{
> + int ret = 0;
> + int err;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260717050308.2144108-1-den@valinux.co.jp?part=3
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v5 04/14] dmaengine: dw-edma: Initialize IRQ data before requesting IRQs
2026-07-17 5:02 [PATCH v5 00/14] dmaengine: dw-edma: Prepare for PCI EP DMA (part 1/3) Koichiro Den
` (2 preceding siblings ...)
2026-07-17 5:02 ` [PATCH v5 03/14] dmaengine: dw-edma: Add core quiesce operations Koichiro Den
@ 2026-07-17 5:02 ` Koichiro Den
2026-07-17 5:16 ` sashiko-bot
2026-07-17 5:02 ` [PATCH v5 05/14] dmaengine: dw-edma: Add partial channel ownership mode Koichiro Den
` (9 subsequent siblings)
13 siblings, 1 reply; 22+ messages in thread
From: Koichiro Den @ 2026-07-17 5:02 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Manivannan Sadhasivam
Cc: Marek Vasut, Yoshihiro Shimoda, dmaengine, linux-kernel
dw_edma_irq_request() passes struct dw_edma_irq to request_irq() before
dw_edma_channel_setup() fills the back pointer. A shared interrupt can
therefore enter the handler with dw_irq->dw still NULL, leading to a
NULL pointer dereference.
Set the back pointer before installing each handler.
Fixes: e63d79d1ffcd ("dmaengine: Add Synopsys eDMA IP core driver")
Cc: stable@vger.kernel.org
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v5:
- No changes.
drivers/dma/dw-edma/dw-edma-core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index 8a1ec0fd057b..392981f3b7bb 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -854,7 +854,6 @@ static int dw_edma_channel_setup(struct dw_edma *dw, u32 wr_alloc, u32 rd_alloc)
else
irq->rd_mask |= BIT(chan->id);
- irq->dw = dw;
memcpy(&chan->msi, &irq->msi, sizeof(chan->msi));
dev_vdbg(dev, "MSI:\t\tChannel %s[%u] addr=0x%.8x%.8x, data=0x%.8x\n",
@@ -935,6 +934,7 @@ static int dw_edma_irq_request(struct dw_edma *dw,
if (chip->nr_irqs == 1) {
/* Common IRQ shared among all channels */
irq = chip->ops->irq_vector(dev, 0);
+ dw->irq[0].dw = dw;
err = request_irq(irq, dw_edma_interrupt_common,
IRQF_SHARED, dw->name, &dw->irq[0]);
if (err) {
@@ -957,6 +957,7 @@ static int dw_edma_irq_request(struct dw_edma *dw,
for (i = 0; i < (*wr_alloc + *rd_alloc); i++) {
irq = chip->ops->irq_vector(dev, i);
+ dw->irq[i].dw = dw;
err = request_irq(irq,
i < *wr_alloc ?
dw_edma_interrupt_write :
--
2.51.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v5 04/14] dmaengine: dw-edma: Initialize IRQ data before requesting IRQs
2026-07-17 5:02 ` [PATCH v5 04/14] dmaengine: dw-edma: Initialize IRQ data before requesting IRQs Koichiro Den
@ 2026-07-17 5:16 ` sashiko-bot
0 siblings, 0 replies; 22+ messages in thread
From: sashiko-bot @ 2026-07-17 5:16 UTC (permalink / raw)
To: Koichiro Den; +Cc: dmaengine, Frank.Li, vkoul
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] Teardown race condition in dw_edma_remove where the hardware is disabled and IRQs are freed before the DMA device is unregistered.
- [High] State modification and linked list accesses are performed locklessly in the pause, resume, and terminate_all routines, leading to data races and list corruption.
- [High] dw_edma_device_terminate_all() fails to stop the hardware synchronously and neglects to clear pending virt-dma descriptors, leading to DMA use-after-free and memory leaks.
--
--- Patch [4]: [PATCH v5 04/14] dmaengine: dw-edma: Initialize IRQ data before requesting IRQs ---
commit 65ff3b1fccd7b06e00ca72a89bc12e41c6366a28
Author: Koichiro Den <den@valinux.co.jp>
dmaengine: dw-edma: Initialize IRQ data before requesting IRQs
This commit prevents a potential NULL pointer dereference in the shared
interrupt handler. It fixes the initialization order by setting the back
pointer before requesting the IRQ.
Cc: stable@vger.kernel.org
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 8a1ec0fd057bd..392981f3b7bb5 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
[ ... ]
> static int dw_edma_device_resume(struct dma_chan *dchan)
> {
> struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
> int err = 0;
[ ... ]
> } else {
> chan->status = EDMA_ST_BUSY;
> dw_edma_start_transfer(chan);
> }
>
> return err;
> }
[Severity: High]
This is a pre-existing issue, but does dw_edma_start_transfer() need to be
called with the vc.lock held here?
Looking at dw_edma_start_transfer(), it appears to locklessly call
vchan_next_desc(), which reads the desc_issued linked list. Could a
concurrent list modification from dmaengine_issue_pending() or the interrupt
handler corrupt the list pointers?
[ ... ]
> static int dw_edma_device_terminate_all(struct dma_chan *dchan)
> {
> struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
> int err = 0;
[ ... ]
> } else {
> chan->request = EDMA_REQ_STOP;
> }
>
> return err;
> }
[Severity: High]
This is a pre-existing issue, but is setting chan->request sufficient to
safely terminate the transfer?
By not synchronously stopping the hardware via dw_edma_core_ch_quiesce(),
the hardware might continue running and writing to memory. If the client
subsequently frees the DMA buffer, could this result in a use-after-free or
IOMMU fault?
Additionally, this locklessly modifies chan->request, bypassing
synchronization with the interrupt handler. Does this routine also need to
acquire the lock and call vchan_get_all_descriptors() to purge pending
queues and prevent descriptor memory leaks?
[ ... ]
> int dw_edma_remove(struct dw_edma_chip *chip)
> {
[ ... ]
> /* Disable eDMA */
> dw_edma_core_off(dw);
>
> /* Free irqs */
> for (i = (dw->nr_irqs - 1); i >= 0; i--)
> free_irq(chip->ops->irq_vector(dev, i), &dw->irq[i]);
> dw_edma_emul_irq_free(dw);
>
> /* Deregister eDMA device */
> dma_async_device_unregister(&dw->dma);
[Severity: High]
This is a pre-existing issue, but is it safe to disable the hardware and
free the IRQs before unregistering the dmaengine device?
If a concurrent client calls dma_async_issue_pending() during driver unbind,
dw_edma_start_transfer() could re-enable the eDMA hardware. Because IRQs are
already freed, the software will never be notified when the transfer
completes.
Could this leave the hardware running while the DMA driver is fully removed,
potentially causing memory corruption?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260717050308.2144108-1-den@valinux.co.jp?part=4
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v5 05/14] dmaengine: dw-edma: Add partial channel ownership mode
2026-07-17 5:02 [PATCH v5 00/14] dmaengine: dw-edma: Prepare for PCI EP DMA (part 1/3) Koichiro Den
` (3 preceding siblings ...)
2026-07-17 5:02 ` [PATCH v5 04/14] dmaengine: dw-edma: Initialize IRQ data before requesting IRQs Koichiro Den
@ 2026-07-17 5:02 ` Koichiro Den
2026-07-17 5:20 ` sashiko-bot
2026-07-17 5:03 ` [PATCH v5 06/14] dmaengine: dw-edma-pcie: Track non-LL mode in DMA data Koichiro Den
` (8 subsequent siblings)
13 siblings, 1 reply; 22+ messages in thread
From: Koichiro Den @ 2026-07-17 5:02 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Manivannan Sadhasivam
Cc: Marek Vasut, Yoshihiro Shimoda, dmaengine, linux-kernel
A DesignWare eDMA instance may represent only a subset of channels that
is also initialized by another OS instance, such as an endpoint-side OS.
Add a partial ownership flag for instances that must preserve
controller-wide state owned by that peer.
In partial ownership mode, dw-edma skips the initial core reset and uses
the limited quiesce path in probe() and remove() instead of the full
core-off path. The flag also makes the driver validate the ownership
granularity required by each register layout before registering
channels.
Probe fails if the limited quiesce cannot stop its resources; remove
reports the error after completing the remaining driver teardown.
Partial instances also skip interrupt-emulation doorbell allocation: the
emulated doorbell is a controller-level resource, and a partial owner
must not claim it on behalf of the whole block.
For EDMA_MF_EDMA_UNROLL and EDMA_MF_HDMA_COMPAT, the driver programs
per-direction registers, such as DMA_{WRITE,READ}_INT_MASK_OFF and
DMA_{WRITE,READ}_INT_CLEAR_OFF. These register layouts have at most
EDMA_MAX_{WR,RD}_CH channels per direction, so the capped hardware
channel count still represents the whole direction. A partial instance
can therefore expose write or read channels only if it owns every
channel in that direction; otherwise two OS instances could update the
same direction-wide registers without a shared locking protocol.
In contrast, HDMA native uses per-channel registers, so it can be owned
at channel granularity.
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v5:
- Use min() for channel count limits. (Frank)
- Fail probe and report remove errors when partial quiesce fails.
- Drop Frank's Reviewed-by tag after the quiesce changes.
drivers/dma/dw-edma/dw-edma-core.c | 81 +++++++++++++++++++++++++-----
include/linux/dma/edma.h | 7 +++
2 files changed, 76 insertions(+), 12 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index 392981f3b7bb..cef75da3f2f6 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -662,6 +662,9 @@ static int dw_edma_emul_irq_alloc(struct dw_edma *dw)
chip->db_irq = 0;
chip->db_offset = ~0;
+ if (chip->flags & DW_EDMA_CHIP_PARTIAL)
+ return 0;
+
/*
* Only meaningful when the core provides the deassert sequence
* for interrupt emulation.
@@ -988,10 +991,33 @@ static int dw_edma_irq_request(struct dw_edma *dw,
return err;
}
+static int dw_edma_check_partial(struct dw_edma_chip *chip,
+ u16 hw_wr_ch_cnt, u16 hw_rd_ch_cnt)
+{
+ if (!(chip->flags & DW_EDMA_CHIP_PARTIAL))
+ return 0;
+
+ if (chip->mf != EDMA_MF_EDMA_UNROLL &&
+ chip->mf != EDMA_MF_HDMA_COMPAT)
+ return 0;
+
+ /*
+ * Direction-wide registers are shared by all channels in that
+ * direction, so a direction must have a single owner.
+ */
+ if ((chip->ll_wr_cnt && chip->ll_wr_cnt != hw_wr_ch_cnt) ||
+ (chip->ll_rd_cnt && chip->ll_rd_cnt != hw_rd_ch_cnt))
+ return -EOPNOTSUPP;
+
+ return 0;
+}
+
int dw_edma_probe(struct dw_edma_chip *chip)
{
struct device *dev;
struct dw_edma *dw;
+ u16 hw_wr_ch_cnt;
+ u16 hw_rd_ch_cnt;
u32 wr_alloc = 0;
u32 rd_alloc = 0;
int i, err;
@@ -1003,6 +1029,17 @@ int dw_edma_probe(struct dw_edma_chip *chip)
if (!dev || !chip->ops)
return -EINVAL;
+ if (chip->flags & DW_EDMA_CHIP_PARTIAL) {
+ switch (chip->mf) {
+ case EDMA_MF_EDMA_UNROLL:
+ case EDMA_MF_HDMA_COMPAT:
+ case EDMA_MF_HDMA_NATIVE:
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }
+ }
+
dw = devm_kzalloc(dev, sizeof(*dw), GFP_KERNEL);
if (!dw)
return -ENOMEM;
@@ -1016,13 +1053,21 @@ int dw_edma_probe(struct dw_edma_chip *chip)
raw_spin_lock_init(&dw->lock);
- dw->wr_ch_cnt = min_t(u16, chip->ll_wr_cnt,
- dw_edma_core_ch_count(dw, EDMA_DIR_WRITE));
- dw->wr_ch_cnt = min_t(u16, dw->wr_ch_cnt, EDMA_MAX_WR_CH);
+ /*
+ * chip->ll_*_cnt describes the channels exposed by this instance. Keep
+ * the usable hardware counts separate for partial ownership checks.
+ */
+ hw_wr_ch_cnt = min(dw_edma_core_ch_count(dw, EDMA_DIR_WRITE),
+ EDMA_MAX_WR_CH);
+ hw_rd_ch_cnt = min(dw_edma_core_ch_count(dw, EDMA_DIR_READ),
+ EDMA_MAX_RD_CH);
+
+ err = dw_edma_check_partial(chip, hw_wr_ch_cnt, hw_rd_ch_cnt);
+ if (err)
+ return err;
- dw->rd_ch_cnt = min_t(u16, chip->ll_rd_cnt,
- dw_edma_core_ch_count(dw, EDMA_DIR_READ));
- dw->rd_ch_cnt = min_t(u16, dw->rd_ch_cnt, EDMA_MAX_RD_CH);
+ dw->wr_ch_cnt = min(chip->ll_wr_cnt, hw_wr_ch_cnt);
+ dw->rd_ch_cnt = min(chip->ll_rd_cnt, hw_rd_ch_cnt);
if (!dw->wr_ch_cnt && !dw->rd_ch_cnt)
return -EINVAL;
@@ -1039,8 +1084,18 @@ int dw_edma_probe(struct dw_edma_chip *chip)
snprintf(dw->name, sizeof(dw->name), "dw-edma-core:%s",
dev_name(chip->dev));
- /* Disable eDMA, only to establish the ideal initial conditions */
- dw_edma_core_off(dw);
+ if (chip->flags & DW_EDMA_CHIP_PARTIAL) {
+ /*
+ * Do not reset the shared controller, but drain stale state
+ * from resources represented by this instance.
+ */
+ err = dw_edma_core_quiesce(dw);
+ if (err)
+ return err;
+ } else {
+ /* Disable eDMA only when this instance owns the controller. */
+ dw_edma_core_off(dw);
+ }
/* Request IRQs */
err = dw_edma_irq_request(dw, &wr_alloc, &rd_alloc);
@@ -1078,14 +1133,16 @@ int dw_edma_remove(struct dw_edma_chip *chip)
struct dw_edma_chan *chan, *_chan;
struct device *dev = chip->dev;
struct dw_edma *dw = chip->dw;
- int i;
+ int i, err = 0;
/* Skip removal if no private data found */
if (!dw)
return -ENODEV;
- /* Disable eDMA */
- dw_edma_core_off(dw);
+ if (chip->flags & DW_EDMA_CHIP_PARTIAL)
+ err = dw_edma_core_quiesce(dw);
+ else
+ dw_edma_core_off(dw);
/* Free irqs */
for (i = (dw->nr_irqs - 1); i >= 0; i--)
@@ -1100,7 +1157,7 @@ int dw_edma_remove(struct dw_edma_chip *chip)
list_del(&chan->vc.chan.device_node);
}
- return 0;
+ return err;
}
EXPORT_SYMBOL_GPL(dw_edma_remove);
diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
index d29a8df76f8c..f827f2dd5299 100644
--- a/include/linux/dma/edma.h
+++ b/include/linux/dma/edma.h
@@ -55,9 +55,16 @@ enum dw_edma_map_format {
/**
* enum dw_edma_chip_flags - Flags specific to an eDMA chip
* @DW_EDMA_CHIP_LOCAL: eDMA is used locally by an endpoint
+ * @DW_EDMA_CHIP_PARTIAL: Only channels described by this instance are
+ * owned by this driver. Controller-wide state
+ * must be preserved, and layouts with shared
+ * direction-wide registers must only be shared at
+ * direction granularity. Layouts with per-channel
+ * registers may be shared at channel granularity.
*/
enum dw_edma_chip_flags {
DW_EDMA_CHIP_LOCAL = BIT(0),
+ DW_EDMA_CHIP_PARTIAL = BIT(1),
};
/**
--
2.51.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v5 05/14] dmaengine: dw-edma: Add partial channel ownership mode
2026-07-17 5:02 ` [PATCH v5 05/14] dmaengine: dw-edma: Add partial channel ownership mode Koichiro Den
@ 2026-07-17 5:20 ` sashiko-bot
0 siblings, 0 replies; 22+ messages in thread
From: sashiko-bot @ 2026-07-17 5:20 UTC (permalink / raw)
To: Koichiro Den; +Cc: vkoul, Frank.Li, dmaengine
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Incorrect teardown order in dw_edma_remove() causes a race condition by freeing IRQs and disabling hardware before unregistering the DMA device.
- [High] The driver requests shared IRQs before initializing its channel data structures, creating a race window where an early interrupt could access uninitialized memory.
--
--- Patch [5]: [PATCH v5 05/14] dmaengine: dw-edma: Add partial channel ownership mode ---
commit 430bec8da60538800159c6651e2aa6c04e375a54
Author: Koichiro Den <den@valinux.co.jp>
dmaengine: dw-edma: Add partial channel ownership mode
This commit introduces a partial ownership flag for DesignWare eDMA instances
to preserve controller-wide state owned by another OS instance. It skips the
initial core reset, using a limited quiesce path instead of the full core-off
path during probe and remove.
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 392981f3b7bb5..cef75da3f2f6b 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
[ ... ]
> @@ -1039,8 +1084,18 @@ int dw_edma_probe(struct dw_edma_chip *chip)
> snprintf(dw->name, sizeof(dw->name), "dw-edma-core:%s",
> dev_name(chip->dev));
>
> - /* Disable eDMA, only to establish the ideal initial conditions */
> - dw_edma_core_off(dw);
> + if (chip->flags & DW_EDMA_CHIP_PARTIAL) {
> + /*
> + * Do not reset the shared controller, but drain stale state
> + * from resources represented by this instance.
> + */
> + err = dw_edma_core_quiesce(dw);
> + if (err)
> + return err;
> + } else {
> + /* Disable eDMA only when this instance owns the controller. */
> + dw_edma_core_off(dw);
> + }
>
> /* Request IRQs */
> err = dw_edma_irq_request(dw, &wr_alloc, &rd_alloc);
[Severity: High]
This is a pre-existing issue, but does requesting shared IRQs before
initializing channel data structures open a race window?
In dw_edma_probe(), dw_edma_irq_request() installs an IRQF_SHARED handler.
Following this, dw_edma_channel_setup() begins initializing channels and
exposes them to the interrupt handler by setting the mask bit:
irq->wr_mask |= BIT(chan->id);
If a peer device on the shared IRQ line triggers an interrupt before
vchan_init() has initialized the channel's spinlocks and list heads, could
the handler execute dw_edma_done_interrupt() and attempt to acquire
uninitialized locks or dereference uninitialized lists?
[ ... ]
> @@ -1078,14 +1133,16 @@ int dw_edma_remove(struct dw_edma_chip *chip)
> struct dw_edma_chan *chan, *_chan;
> struct device *dev = chip->dev;
> struct dw_edma *dw = chip->dw;
> - int i;
> + int i, err = 0;
>
> /* Skip removal if no private data found */
> if (!dw)
> return -ENODEV;
>
> - /* Disable eDMA */
> - dw_edma_core_off(dw);
> + if (chip->flags & DW_EDMA_CHIP_PARTIAL)
> + err = dw_edma_core_quiesce(dw);
> + else
> + dw_edma_core_off(dw);
>
> /* Free irqs */
> for (i = (dw->nr_irqs - 1); i >= 0; i--)
> free_irq(chip->ops->irq_vector(dev, i), &dw->irq[i]);
> dw_edma_emul_irq_free(dw);
>
> /* Deregister eDMA device */
> dma_async_device_unregister(&dw->dma);
[Severity: High]
This is a pre-existing issue, but does disabling the hardware and freeing
IRQs before unregistering the DMA device create a race condition?
If a client DMA driver holds a reference to the eDMA channel and submits a
new DMA request after dw_edma_remove() powers down the hardware, but before
dma_async_device_unregister() removes the device from the subsystem, could
the client access powered-down hardware registers and cause a bus fault?
Would it be safer to reverse this teardown order so the public interface is
unregistered before freeing the underlying resources?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260717050308.2144108-1-den@valinux.co.jp?part=5
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v5 06/14] dmaengine: dw-edma-pcie: Track non-LL mode in DMA data
2026-07-17 5:02 [PATCH v5 00/14] dmaengine: dw-edma: Prepare for PCI EP DMA (part 1/3) Koichiro Den
` (4 preceding siblings ...)
2026-07-17 5:02 ` [PATCH v5 05/14] dmaengine: dw-edma: Add partial channel ownership mode Koichiro Den
@ 2026-07-17 5:03 ` Koichiro Den
2026-07-17 5:03 ` [PATCH v5 07/14] dmaengine: dw-edma-pcie: Add capability match data Koichiro Den
` (7 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: Koichiro Den @ 2026-07-17 5:03 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Manivannan Sadhasivam
Cc: Marek Vasut, Yoshihiro Shimoda, dmaengine, linux-kernel
The dw-edma-pcie driver copies static template data into a mutable
dw_edma_pcie_data instance before applying capability-derived updates.
Keep the derived non-LL mode in that copy as well, instead of only
tracking it in a local variable in dw_edma_pcie_probe().
This prepares for keeping capability parsing behind match data without a
separate non-LL output parameter.
No functional change intended.
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 v5:
- No changes.
drivers/dma/dw-edma/dw-edma-pcie.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
index 791c46e8ae4c..16bb938c6e40 100644
--- a/drivers/dma/dw-edma/dw-edma-pcie.c
+++ b/drivers/dma/dw-edma/dw-edma-pcie.c
@@ -73,6 +73,7 @@ struct dw_edma_pcie_data {
u16 wr_ch_cnt;
u16 rd_ch_cnt;
u64 devmem_phys_off;
+ bool cfg_non_ll;
};
static const struct dw_edma_pcie_data snps_edda_data = {
@@ -326,7 +327,6 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
struct dw_edma_chip *chip;
int err, nr_irqs;
int i, mask;
- bool non_ll = false;
if (!pdata)
return -ENODEV;
@@ -361,14 +361,14 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
* the HDMA IP.
*/
if (vsec_data->devmem_phys_off == DW_PCIE_XILINX_MDB_INVALID_ADDR)
- non_ll = true;
+ vsec_data->cfg_non_ll = true;
/*
* Configure the channel LL and data blocks if number of
* channels enabled in VSEC capability are more than the
* channels configured in xilinx_mdb_data.
*/
- if (!non_ll)
+ if (!vsec_data->cfg_non_ll)
dw_edma_set_chan_region_offset(vsec_data, BAR_2, 0,
DW_PCIE_XILINX_MDB_LL_OFF_GAP,
DW_PCIE_XILINX_MDB_LL_SIZE,
@@ -421,7 +421,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
chip->mf = vsec_data->mf;
chip->nr_irqs = nr_irqs;
chip->ops = &dw_edma_pcie_plat_ops;
- chip->cfg_non_ll = non_ll;
+ chip->cfg_non_ll = vsec_data->cfg_non_ll;
chip->ll_wr_cnt = vsec_data->wr_ch_cnt;
chip->ll_rd_cnt = vsec_data->rd_ch_cnt;
@@ -430,7 +430,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
if (!chip->reg_base)
return -ENOMEM;
- for (i = 0; i < chip->ll_wr_cnt && !non_ll; i++) {
+ for (i = 0; i < chip->ll_wr_cnt && !vsec_data->cfg_non_ll; i++) {
struct dw_edma_region *ll_region = &chip->ll_region_wr[i];
struct dw_edma_region *dt_region = &chip->dt_region_wr[i];
struct dw_edma_block *ll_block = &vsec_data->ll_wr[i];
@@ -457,7 +457,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
dt_region->sz = dt_block->sz;
}
- for (i = 0; i < chip->ll_rd_cnt && !non_ll; i++) {
+ for (i = 0; i < chip->ll_rd_cnt && !vsec_data->cfg_non_ll; i++) {
struct dw_edma_region *ll_region = &chip->ll_region_rd[i];
struct dw_edma_region *dt_region = &chip->dt_region_rd[i];
struct dw_edma_block *ll_block = &vsec_data->ll_rd[i];
--
2.51.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v5 07/14] dmaengine: dw-edma-pcie: Add capability match data
2026-07-17 5:02 [PATCH v5 00/14] dmaengine: dw-edma: Prepare for PCI EP DMA (part 1/3) Koichiro Den
` (5 preceding siblings ...)
2026-07-17 5:03 ` [PATCH v5 06/14] dmaengine: dw-edma-pcie: Track non-LL mode in DMA data Koichiro Den
@ 2026-07-17 5:03 ` Koichiro Den
2026-07-17 5:12 ` sashiko-bot
2026-07-17 5:03 ` [PATCH v5 08/14] dmaengine: dw-edma-pcie: Rename vsec_data to dma_data Koichiro Den
` (6 subsequent siblings)
13 siblings, 1 reply; 22+ messages in thread
From: Koichiro Den @ 2026-07-17 5:03 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Manivannan Sadhasivam
Cc: Marek Vasut, Yoshihiro Shimoda, dmaengine, linux-kernel
Move device-specific capability parsing behind per-device match data.
The existing probe path mixes two decisions: which static template a PCI
ID uses, and which device-specific capability parser adjusts that
template. Split those decisions so device-specific discovery can be
added through match data instead of adding more vendor checks to
dw_edma_pcie_probe().
No functional change is intended for the existing Synopsys EDDA and
AMD (Xilinx) MDB/CPM6 matches. They still copy the same static template
data and run the same capability parsing logic before BAR mapping. The
AMD (Xilinx) MDB/CPM6 entries also keep using endpoint memory physical
addresses for descriptor windows through a new match-data flag.
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 v5:
- No changes.
drivers/dma/dw-edma/dw-edma-pcie.c | 139 ++++++++++++++++++++---------
1 file changed, 96 insertions(+), 43 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
index 16bb938c6e40..23f09ee89639 100644
--- a/drivers/dma/dw-edma/dw-edma-pcie.c
+++ b/drivers/dma/dw-edma/dw-edma-pcie.c
@@ -76,6 +76,19 @@ struct dw_edma_pcie_data {
bool cfg_non_ll;
};
+struct dw_edma_pcie_match_data {
+ const struct dw_edma_pcie_data *data;
+ /*
+ * Mandatory callback. It may leave @pdata unchanged when the static
+ * template already describes the device.
+ */
+ int (*parse_caps)(struct pci_dev *pdev,
+ struct dw_edma_pcie_data *pdata);
+ unsigned long flags;
+};
+
+#define DW_EDMA_PCIE_F_DEVMEM_PHYS_OFF BIT(0)
+
static const struct dw_edma_pcie_data snps_edda_data = {
/* eDMA registers location */
.rg.bar = BAR_0,
@@ -310,24 +323,70 @@ static void dw_edma_pcie_get_xilinx_dma_data(struct pci_dev *pdev,
pdata->devmem_phys_off = off;
}
+static int
+dw_edma_pcie_parse_synopsys_caps(struct pci_dev *pdev,
+ struct dw_edma_pcie_data *pdata)
+{
+ dw_edma_pcie_get_synopsys_dma_data(pdev, pdata);
+
+ return 0;
+}
+
+static int
+dw_edma_pcie_parse_xilinx_caps(struct pci_dev *pdev,
+ struct dw_edma_pcie_data *pdata)
+{
+ dw_edma_pcie_get_xilinx_dma_data(pdev, pdata);
+
+ /*
+ * There is no valid address found for the LL memory space on the
+ * device side. In the absence of LL base address use the non-LL mode or
+ * simple mode supported by the HDMA IP.
+ */
+ if (pdata->devmem_phys_off == DW_PCIE_XILINX_MDB_INVALID_ADDR) {
+ pdata->cfg_non_ll = true;
+ return 0;
+ }
+
+ /*
+ * Configure the channel LL and data blocks if number of channels
+ * enabled in VSEC capability are more than the channels configured in
+ * xilinx_mdb_data.
+ */
+ dw_edma_set_chan_region_offset(pdata, BAR_2, 0,
+ DW_PCIE_XILINX_MDB_LL_OFF_GAP,
+ DW_PCIE_XILINX_MDB_LL_SIZE,
+ DW_PCIE_XILINX_MDB_DT_OFF_GAP,
+ DW_PCIE_XILINX_MDB_DT_SIZE);
+
+ return 0;
+}
+
static u64 dw_edma_get_phys_addr(struct pci_dev *pdev,
+ const struct dw_edma_pcie_match_data *match,
struct dw_edma_pcie_data *pdata,
enum pci_barno bar)
{
- if (pdev->vendor == PCI_VENDOR_ID_XILINX)
+ if (match->flags & DW_EDMA_PCIE_F_DEVMEM_PHYS_OFF)
return pdata->devmem_phys_off;
+
return pci_bus_address(pdev, bar);
}
static int dw_edma_pcie_probe(struct pci_dev *pdev,
const struct pci_device_id *pid)
{
- struct dw_edma_pcie_data *pdata = (void *)pid->driver_data;
+ const struct dw_edma_pcie_match_data *match = (void *)pid->driver_data;
+ const struct dw_edma_pcie_data *pdata;
struct device *dev = &pdev->dev;
struct dw_edma_chip *chip;
int err, nr_irqs;
int i, mask;
+ if (!match)
+ return -ENODEV;
+ pdata = match->data;
+
if (!pdata)
return -ENODEV;
@@ -345,36 +404,13 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
memcpy(vsec_data, pdata, sizeof(struct dw_edma_pcie_data));
- /*
- * Tries to find if exists a PCIe Vendor-Specific Extended Capability
- * for the DMA, if one exists, then reconfigures it.
- */
- dw_edma_pcie_get_synopsys_dma_data(pdev, vsec_data);
-
- if (pdev->vendor == PCI_VENDOR_ID_XILINX) {
- dw_edma_pcie_get_xilinx_dma_data(pdev, vsec_data);
-
- /*
- * There is no valid address found for the LL memory
- * space on the device side. In the absence of LL base
- * address use the non-LL mode or simple mode supported by
- * the HDMA IP.
- */
- if (vsec_data->devmem_phys_off == DW_PCIE_XILINX_MDB_INVALID_ADDR)
- vsec_data->cfg_non_ll = true;
-
- /*
- * Configure the channel LL and data blocks if number of
- * channels enabled in VSEC capability are more than the
- * channels configured in xilinx_mdb_data.
- */
- if (!vsec_data->cfg_non_ll)
- dw_edma_set_chan_region_offset(vsec_data, BAR_2, 0,
- DW_PCIE_XILINX_MDB_LL_OFF_GAP,
- DW_PCIE_XILINX_MDB_LL_SIZE,
- DW_PCIE_XILINX_MDB_DT_OFF_GAP,
- DW_PCIE_XILINX_MDB_DT_SIZE);
- }
+ /* Let device-specific discovery override the static template data. */
+ if (!match->parse_caps)
+ return -EINVAL;
+
+ err = match->parse_caps(pdev, vsec_data);
+ if (err)
+ return err;
/* Mapping PCI BAR regions */
mask = BIT(vsec_data->rg.bar);
@@ -441,8 +477,8 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
return -ENOMEM;
ll_region->vaddr.io += ll_block->off;
- ll_region->paddr = dw_edma_get_phys_addr(pdev, vsec_data,
- ll_block->bar);
+ ll_region->paddr = dw_edma_get_phys_addr(pdev, match,
+ vsec_data, ll_block->bar);
ll_region->paddr += ll_block->off;
ll_region->sz = ll_block->sz;
@@ -451,8 +487,8 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
return -ENOMEM;
dt_region->vaddr.io += dt_block->off;
- dt_region->paddr = dw_edma_get_phys_addr(pdev, vsec_data,
- dt_block->bar);
+ dt_region->paddr = dw_edma_get_phys_addr(pdev, match,
+ vsec_data, dt_block->bar);
dt_region->paddr += dt_block->off;
dt_region->sz = dt_block->sz;
}
@@ -468,8 +504,8 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
return -ENOMEM;
ll_region->vaddr.io += ll_block->off;
- ll_region->paddr = dw_edma_get_phys_addr(pdev, vsec_data,
- ll_block->bar);
+ ll_region->paddr = dw_edma_get_phys_addr(pdev, match,
+ vsec_data, ll_block->bar);
ll_region->paddr += ll_block->off;
ll_region->sz = ll_block->sz;
@@ -478,8 +514,8 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
return -ENOMEM;
dt_region->vaddr.io += dt_block->off;
- dt_region->paddr = dw_edma_get_phys_addr(pdev, vsec_data,
- dt_block->bar);
+ dt_region->paddr = dw_edma_get_phys_addr(pdev, match,
+ vsec_data, dt_block->bar);
dt_region->paddr += dt_block->off;
dt_region->sz = dt_block->sz;
}
@@ -560,12 +596,29 @@ static void dw_edma_pcie_remove(struct pci_dev *pdev)
pci_free_irq_vectors(pdev);
}
+static const struct dw_edma_pcie_match_data snps_edda_match_data = {
+ .data = &snps_edda_data,
+ .parse_caps = dw_edma_pcie_parse_synopsys_caps,
+};
+
+static const struct dw_edma_pcie_match_data xilinx_mdb_match_data = {
+ .data = &xilinx_mdb_data,
+ .parse_caps = dw_edma_pcie_parse_xilinx_caps,
+ .flags = DW_EDMA_PCIE_F_DEVMEM_PHYS_OFF,
+};
+
+static const struct dw_edma_pcie_match_data xilinx_cpm6_dma_match_data = {
+ .data = &xilinx_cpm6_dma_data,
+ .parse_caps = dw_edma_pcie_parse_xilinx_caps,
+ .flags = DW_EDMA_PCIE_F_DEVMEM_PHYS_OFF,
+};
+
static const struct pci_device_id dw_edma_pcie_id_table[] = {
- { PCI_DEVICE_DATA(SYNOPSYS, EDDA, &snps_edda_data) },
+ { PCI_DEVICE_DATA(SYNOPSYS, EDDA, &snps_edda_match_data) },
{ PCI_VDEVICE(XILINX, PCI_DEVICE_ID_XILINX_B054),
- (kernel_ulong_t)&xilinx_mdb_data },
+ .driver_data = (kernel_ulong_t)&xilinx_mdb_match_data },
{ PCI_VDEVICE(XILINX, PCI_DEVICE_ID_XILINX_B00F),
- .driver_data = (kernel_ulong_t)&xilinx_cpm6_dma_data },
+ .driver_data = (kernel_ulong_t)&xilinx_cpm6_dma_match_data },
{ }
};
MODULE_DEVICE_TABLE(pci, dw_edma_pcie_id_table);
--
2.51.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v5 07/14] dmaengine: dw-edma-pcie: Add capability match data
2026-07-17 5:03 ` [PATCH v5 07/14] dmaengine: dw-edma-pcie: Add capability match data Koichiro Den
@ 2026-07-17 5:12 ` sashiko-bot
0 siblings, 0 replies; 22+ messages in thread
From: sashiko-bot @ 2026-07-17 5:12 UTC (permalink / raw)
To: Koichiro Den; +Cc: dmaengine, Frank.Li, vkoul
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing pci_free_irq_vectors() cleanup on error paths in the probe function.
--
--- Patch [7]: [PATCH v5 07/14] dmaengine: dw-edma-pcie: Add capability match data ---
commit 352070c0889b8cd6fe579fca832e28f5f50ba2e5
Author: Koichiro Den <den@valinux.co.jp>
dmaengine: dw-edma-pcie: Add capability match data
This commit moves device-specific capability parsing behind per-device
match data to avoid mixing static templates with vendor checks. It splits
these decisions so device-specific discovery can be added through match
data instead of adding more vendor checks.
> diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
> --- a/drivers/dma/dw-edma/dw-edma-pcie.c
> +++ b/drivers/dma/dw-edma/dw-edma-pcie.c
[ ... ]
> @@ -441,8 +477,8 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
> return -ENOMEM;
[Severity: High]
This is a pre-existing issue, but does this error path leak the IRQ vectors
allocated earlier in dw_edma_pcie_probe()?
If pcim_iomap_table() fails, we return -ENOMEM directly without calling
pci_free_irq_vectors(). Failing to free them can permanently reduce the
system's available IRQ capacity. Should these error paths be updated to
properly clean up the allocated IRQ vectors?
>
> ll_region->vaddr.io += ll_block->off;
> - ll_region->paddr = dw_edma_get_phys_addr(pdev, vsec_data,
> - ll_block->bar);
> + ll_region->paddr = dw_edma_get_phys_addr(pdev, match,
> + vsec_data, ll_block->bar);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260717050308.2144108-1-den@valinux.co.jp?part=7
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v5 08/14] dmaengine: dw-edma-pcie: Rename vsec_data to dma_data
2026-07-17 5:02 [PATCH v5 00/14] dmaengine: dw-edma: Prepare for PCI EP DMA (part 1/3) Koichiro Den
` (6 preceding siblings ...)
2026-07-17 5:03 ` [PATCH v5 07/14] dmaengine: dw-edma-pcie: Add capability match data Koichiro Den
@ 2026-07-17 5:03 ` Koichiro Den
2026-07-17 5:03 ` [PATCH v5 09/14] dmaengine: dw-edma-pcie: Add platform ops to match data Koichiro Den
` (5 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: Koichiro Den @ 2026-07-17 5:03 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Manivannan Sadhasivam
Cc: Marek Vasut, Yoshihiro Shimoda, dmaengine, linux-kernel
dw_edma_pcie_probe() now obtains DMA layout data through device-specific
capability callbacks, not only from PCIe Vendor-Specific Extended
Capabilities. Rename the local data copy from vsec_data to dma_data
before adding endpoint DMA BAR metadata discovery, which does not rely
on VSEC.
No functional change intended.
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v5:
- No changes.
drivers/dma/dw-edma/dw-edma-pcie.c | 74 +++++++++++++++---------------
1 file changed, 36 insertions(+), 38 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
index 23f09ee89639..46f16d7b6d51 100644
--- a/drivers/dma/dw-edma/dw-edma-pcie.c
+++ b/drivers/dma/dw-edma/dw-edma-pcie.c
@@ -390,9 +390,9 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
if (!pdata)
return -ENODEV;
- struct dw_edma_pcie_data *vsec_data __free(kfree) =
- kmalloc_obj(*vsec_data);
- if (!vsec_data)
+ struct dw_edma_pcie_data *dma_data __free(kfree) =
+ kmemdup(pdata, sizeof(*dma_data), GFP_KERNEL);
+ if (!dma_data)
return -ENOMEM;
/* Enable PCI device */
@@ -402,25 +402,23 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
return err;
}
- memcpy(vsec_data, pdata, sizeof(struct dw_edma_pcie_data));
-
/* Let device-specific discovery override the static template data. */
if (!match->parse_caps)
return -EINVAL;
- err = match->parse_caps(pdev, vsec_data);
+ err = match->parse_caps(pdev, dma_data);
if (err)
return err;
/* Mapping PCI BAR regions */
- mask = BIT(vsec_data->rg.bar);
- for (i = 0; i < vsec_data->wr_ch_cnt; i++) {
- mask |= BIT(vsec_data->ll_wr[i].bar);
- mask |= BIT(vsec_data->dt_wr[i].bar);
+ mask = BIT(dma_data->rg.bar);
+ for (i = 0; i < dma_data->wr_ch_cnt; i++) {
+ mask |= BIT(dma_data->ll_wr[i].bar);
+ mask |= BIT(dma_data->dt_wr[i].bar);
}
- for (i = 0; i < vsec_data->rd_ch_cnt; i++) {
- mask |= BIT(vsec_data->ll_rd[i].bar);
- mask |= BIT(vsec_data->dt_rd[i].bar);
+ for (i = 0; i < dma_data->rd_ch_cnt; i++) {
+ mask |= BIT(dma_data->ll_rd[i].bar);
+ mask |= BIT(dma_data->dt_rd[i].bar);
}
err = pcim_iomap_regions(pdev, mask, pci_name(pdev));
if (err) {
@@ -443,7 +441,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
return -ENOMEM;
/* IRQs allocation */
- nr_irqs = pci_alloc_irq_vectors(pdev, 1, vsec_data->irqs,
+ nr_irqs = pci_alloc_irq_vectors(pdev, 1, dma_data->irqs,
PCI_IRQ_MSI | PCI_IRQ_MSIX);
if (nr_irqs < 1) {
pci_err(pdev, "fail to alloc IRQ vector (number of IRQs=%u)\n",
@@ -454,23 +452,23 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
/* Data structure initialization */
chip->dev = dev;
- chip->mf = vsec_data->mf;
+ chip->mf = dma_data->mf;
chip->nr_irqs = nr_irqs;
chip->ops = &dw_edma_pcie_plat_ops;
- chip->cfg_non_ll = vsec_data->cfg_non_ll;
+ chip->cfg_non_ll = dma_data->cfg_non_ll;
- chip->ll_wr_cnt = vsec_data->wr_ch_cnt;
- chip->ll_rd_cnt = vsec_data->rd_ch_cnt;
+ chip->ll_wr_cnt = dma_data->wr_ch_cnt;
+ chip->ll_rd_cnt = dma_data->rd_ch_cnt;
- chip->reg_base = pcim_iomap_table(pdev)[vsec_data->rg.bar];
+ chip->reg_base = pcim_iomap_table(pdev)[dma_data->rg.bar];
if (!chip->reg_base)
return -ENOMEM;
- for (i = 0; i < chip->ll_wr_cnt && !vsec_data->cfg_non_ll; i++) {
+ for (i = 0; i < chip->ll_wr_cnt && !dma_data->cfg_non_ll; i++) {
struct dw_edma_region *ll_region = &chip->ll_region_wr[i];
struct dw_edma_region *dt_region = &chip->dt_region_wr[i];
- struct dw_edma_block *ll_block = &vsec_data->ll_wr[i];
- struct dw_edma_block *dt_block = &vsec_data->dt_wr[i];
+ struct dw_edma_block *ll_block = &dma_data->ll_wr[i];
+ struct dw_edma_block *dt_block = &dma_data->dt_wr[i];
ll_region->vaddr.io = pcim_iomap_table(pdev)[ll_block->bar];
if (!ll_region->vaddr.io)
@@ -478,7 +476,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
ll_region->vaddr.io += ll_block->off;
ll_region->paddr = dw_edma_get_phys_addr(pdev, match,
- vsec_data, ll_block->bar);
+ dma_data, ll_block->bar);
ll_region->paddr += ll_block->off;
ll_region->sz = ll_block->sz;
@@ -488,16 +486,16 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
dt_region->vaddr.io += dt_block->off;
dt_region->paddr = dw_edma_get_phys_addr(pdev, match,
- vsec_data, dt_block->bar);
+ dma_data, dt_block->bar);
dt_region->paddr += dt_block->off;
dt_region->sz = dt_block->sz;
}
- for (i = 0; i < chip->ll_rd_cnt && !vsec_data->cfg_non_ll; i++) {
+ for (i = 0; i < chip->ll_rd_cnt && !dma_data->cfg_non_ll; i++) {
struct dw_edma_region *ll_region = &chip->ll_region_rd[i];
struct dw_edma_region *dt_region = &chip->dt_region_rd[i];
- struct dw_edma_block *ll_block = &vsec_data->ll_rd[i];
- struct dw_edma_block *dt_block = &vsec_data->dt_rd[i];
+ struct dw_edma_block *ll_block = &dma_data->ll_rd[i];
+ struct dw_edma_block *dt_block = &dma_data->dt_rd[i];
ll_region->vaddr.io = pcim_iomap_table(pdev)[ll_block->bar];
if (!ll_region->vaddr.io)
@@ -505,7 +503,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
ll_region->vaddr.io += ll_block->off;
ll_region->paddr = dw_edma_get_phys_addr(pdev, match,
- vsec_data, ll_block->bar);
+ dma_data, ll_block->bar);
ll_region->paddr += ll_block->off;
ll_region->sz = ll_block->sz;
@@ -515,7 +513,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
dt_region->vaddr.io += dt_block->off;
dt_region->paddr = dw_edma_get_phys_addr(pdev, match,
- vsec_data, dt_block->bar);
+ dma_data, dt_block->bar);
dt_region->paddr += dt_block->off;
dt_region->sz = dt_block->sz;
}
@@ -533,31 +531,31 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
pci_dbg(pdev, "Version:\tUnknown (0x%x)\n", chip->mf);
pci_dbg(pdev, "Registers:\tBAR=%u, off=0x%.8lx, sz=0x%zx bytes, addr(v=%p)\n",
- vsec_data->rg.bar, vsec_data->rg.off, vsec_data->rg.sz,
+ dma_data->rg.bar, dma_data->rg.off, dma_data->rg.sz,
chip->reg_base);
for (i = 0; i < chip->ll_wr_cnt; i++) {
pci_dbg(pdev, "L. List:\tWRITE CH%.2u, BAR=%u, off=0x%.8lx, sz=0x%zx bytes, addr(v=%p, p=%pa)\n",
- i, vsec_data->ll_wr[i].bar,
- vsec_data->ll_wr[i].off, chip->ll_region_wr[i].sz,
+ i, dma_data->ll_wr[i].bar,
+ dma_data->ll_wr[i].off, chip->ll_region_wr[i].sz,
chip->ll_region_wr[i].vaddr.io, &chip->ll_region_wr[i].paddr);
pci_dbg(pdev, "Data:\tWRITE CH%.2u, BAR=%u, off=0x%.8lx, sz=0x%zx bytes, addr(v=%p, p=%pa)\n",
- i, vsec_data->dt_wr[i].bar,
- vsec_data->dt_wr[i].off, chip->dt_region_wr[i].sz,
+ i, dma_data->dt_wr[i].bar,
+ dma_data->dt_wr[i].off, chip->dt_region_wr[i].sz,
chip->dt_region_wr[i].vaddr.io, &chip->dt_region_wr[i].paddr);
}
for (i = 0; i < chip->ll_rd_cnt; i++) {
pci_dbg(pdev, "L. List:\tREAD CH%.2u, BAR=%u, off=0x%.8lx, sz=0x%zx bytes, addr(v=%p, p=%pa)\n",
- i, vsec_data->ll_rd[i].bar,
- vsec_data->ll_rd[i].off, chip->ll_region_rd[i].sz,
+ i, dma_data->ll_rd[i].bar,
+ dma_data->ll_rd[i].off, chip->ll_region_rd[i].sz,
chip->ll_region_rd[i].vaddr.io, &chip->ll_region_rd[i].paddr);
pci_dbg(pdev, "Data:\tREAD CH%.2u, BAR=%u, off=0x%.8lx, sz=0x%zx bytes, addr(v=%p, p=%pa)\n",
- i, vsec_data->dt_rd[i].bar,
- vsec_data->dt_rd[i].off, chip->dt_region_rd[i].sz,
+ i, dma_data->dt_rd[i].bar,
+ dma_data->dt_rd[i].off, chip->dt_region_rd[i].sz,
chip->dt_region_rd[i].vaddr.io, &chip->dt_region_rd[i].paddr);
}
--
2.51.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v5 09/14] dmaengine: dw-edma-pcie: Add platform ops to match data
2026-07-17 5:02 [PATCH v5 00/14] dmaengine: dw-edma: Prepare for PCI EP DMA (part 1/3) Koichiro Den
` (7 preceding siblings ...)
2026-07-17 5:03 ` [PATCH v5 08/14] dmaengine: dw-edma-pcie: Rename vsec_data to dma_data Koichiro Den
@ 2026-07-17 5:03 ` Koichiro Den
2026-07-17 5:03 ` [PATCH v5 10/14] dmaengine: dw-edma-pcie: Add register offset match flag Koichiro Den
` (4 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: Koichiro Den @ 2026-07-17 5:03 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Manivannan Sadhasivam
Cc: Marek Vasut, Yoshihiro Shimoda, dmaengine, linux-kernel
Move the platform ops pointer into match data. Existing EDDA/MDB/CPM6
matches keep using dw_edma_pcie_plat_ops.
No functional changes intended.
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 v5:
- No changes.
drivers/dma/dw-edma/dw-edma-pcie.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
index 46f16d7b6d51..531d1227a012 100644
--- a/drivers/dma/dw-edma/dw-edma-pcie.c
+++ b/drivers/dma/dw-edma/dw-edma-pcie.c
@@ -78,6 +78,7 @@ struct dw_edma_pcie_data {
struct dw_edma_pcie_match_data {
const struct dw_edma_pcie_data *data;
+ const struct dw_edma_plat_ops *plat_ops;
/*
* Mandatory callback. It may leave @pdata unchanged when the static
* template already describes the device.
@@ -403,7 +404,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
}
/* Let device-specific discovery override the static template data. */
- if (!match->parse_caps)
+ if (!match->parse_caps || !match->plat_ops)
return -EINVAL;
err = match->parse_caps(pdev, dma_data);
@@ -454,7 +455,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
chip->mf = dma_data->mf;
chip->nr_irqs = nr_irqs;
- chip->ops = &dw_edma_pcie_plat_ops;
+ chip->ops = match->plat_ops;
chip->cfg_non_ll = dma_data->cfg_non_ll;
chip->ll_wr_cnt = dma_data->wr_ch_cnt;
@@ -596,17 +597,20 @@ static void dw_edma_pcie_remove(struct pci_dev *pdev)
static const struct dw_edma_pcie_match_data snps_edda_match_data = {
.data = &snps_edda_data,
+ .plat_ops = &dw_edma_pcie_plat_ops,
.parse_caps = dw_edma_pcie_parse_synopsys_caps,
};
static const struct dw_edma_pcie_match_data xilinx_mdb_match_data = {
.data = &xilinx_mdb_data,
+ .plat_ops = &dw_edma_pcie_plat_ops,
.parse_caps = dw_edma_pcie_parse_xilinx_caps,
.flags = DW_EDMA_PCIE_F_DEVMEM_PHYS_OFF,
};
static const struct dw_edma_pcie_match_data xilinx_cpm6_dma_match_data = {
.data = &xilinx_cpm6_dma_data,
+ .plat_ops = &dw_edma_pcie_plat_ops,
.parse_caps = dw_edma_pcie_parse_xilinx_caps,
.flags = DW_EDMA_PCIE_F_DEVMEM_PHYS_OFF,
};
--
2.51.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v5 10/14] dmaengine: dw-edma-pcie: Add register offset match flag
2026-07-17 5:02 [PATCH v5 00/14] dmaengine: dw-edma: Prepare for PCI EP DMA (part 1/3) Koichiro Den
` (8 preceding siblings ...)
2026-07-17 5:03 ` [PATCH v5 09/14] dmaengine: dw-edma-pcie: Add platform ops to match data Koichiro Den
@ 2026-07-17 5:03 ` Koichiro Den
2026-07-17 5:03 ` [PATCH v5 11/14] dmaengine: dw-edma-pcie: Factor out descriptor block address lookup Koichiro Den
` (3 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: Koichiro Den @ 2026-07-17 5:03 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Manivannan Sadhasivam
Cc: Marek Vasut, Yoshihiro Shimoda, dmaengine, linux-kernel
Add a match-data flag for devices whose DMA register block starts at an
offset inside the mapped BAR. Existing Synopsys EDDA and AMD (Xilinx)
MDB/CPM6 matches keep using the BAR mapping base directly.
The offsets handled here come from static, trusted match data. Later
metadata-derived offsets are validated when the endpoint DMA metadata
path parses them at that trust boundary.
No functional change intended.
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v5:
- Fix broken commit message.
drivers/dma/dw-edma/dw-edma-pcie.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
index 531d1227a012..327b59828bfa 100644
--- a/drivers/dma/dw-edma/dw-edma-pcie.c
+++ b/drivers/dma/dw-edma/dw-edma-pcie.c
@@ -89,6 +89,7 @@ struct dw_edma_pcie_match_data {
};
#define DW_EDMA_PCIE_F_DEVMEM_PHYS_OFF BIT(0)
+#define DW_EDMA_PCIE_F_REG_OFFSET BIT(1)
static const struct dw_edma_pcie_data snps_edda_data = {
/* eDMA registers location */
@@ -464,6 +465,8 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
chip->reg_base = pcim_iomap_table(pdev)[dma_data->rg.bar];
if (!chip->reg_base)
return -ENOMEM;
+ if (match->flags & DW_EDMA_PCIE_F_REG_OFFSET)
+ chip->reg_base += dma_data->rg.off;
for (i = 0; i < chip->ll_wr_cnt && !dma_data->cfg_non_ll; i++) {
struct dw_edma_region *ll_region = &chip->ll_region_wr[i];
--
2.51.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v5 11/14] dmaengine: dw-edma-pcie: Factor out descriptor block address lookup
2026-07-17 5:02 [PATCH v5 00/14] dmaengine: dw-edma: Prepare for PCI EP DMA (part 1/3) Koichiro Den
` (9 preceding siblings ...)
2026-07-17 5:03 ` [PATCH v5 10/14] dmaengine: dw-edma-pcie: Add register offset match flag Koichiro Den
@ 2026-07-17 5:03 ` Koichiro Den
2026-07-17 5:03 ` [PATCH v5 12/14] dmaengine: dw-edma-pcie: Handle optional data blocks Koichiro Den
` (2 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: Koichiro Den @ 2026-07-17 5:03 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Manivannan Sadhasivam
Cc: Marek Vasut, Yoshihiro Shimoda, dmaengine, linux-kernel
Add an optional physical address override to struct dw_edma_block and
use a helper to compute descriptor block addresses.
No functional change intended. Existing Synopsys EDDA and AMD (Xilinx)
MDB/CPM6 block descriptors leave the override unset, so the helper still
returns the same values as before.
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v5:
- No changes.
drivers/dma/dw-edma/dw-edma-pcie.c | 34 +++++++++++++++++++-----------
1 file changed, 22 insertions(+), 12 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
index 327b59828bfa..a5d65cc9706c 100644
--- a/drivers/dma/dw-edma/dw-edma-pcie.c
+++ b/drivers/dma/dw-edma/dw-edma-pcie.c
@@ -55,6 +55,8 @@
struct dw_edma_block {
enum pci_barno bar;
off_t off;
+ u64 paddr;
+ bool paddr_valid;
size_t sz;
};
@@ -375,6 +377,18 @@ static u64 dw_edma_get_phys_addr(struct pci_dev *pdev,
return pci_bus_address(pdev, bar);
}
+static u64 dw_edma_get_block_addr(struct pci_dev *pdev,
+ const struct dw_edma_pcie_match_data *match,
+ struct dw_edma_pcie_data *pdata,
+ const struct dw_edma_block *block)
+{
+ if (block->paddr_valid)
+ return block->paddr;
+
+ return dw_edma_get_phys_addr(pdev, match, pdata, block->bar) +
+ block->off;
+}
+
static int dw_edma_pcie_probe(struct pci_dev *pdev,
const struct pci_device_id *pid)
{
@@ -479,9 +493,8 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
return -ENOMEM;
ll_region->vaddr.io += ll_block->off;
- ll_region->paddr = dw_edma_get_phys_addr(pdev, match,
- dma_data, ll_block->bar);
- ll_region->paddr += ll_block->off;
+ ll_region->paddr = dw_edma_get_block_addr(pdev, match, dma_data,
+ ll_block);
ll_region->sz = ll_block->sz;
dt_region->vaddr.io = pcim_iomap_table(pdev)[dt_block->bar];
@@ -489,9 +502,8 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
return -ENOMEM;
dt_region->vaddr.io += dt_block->off;
- dt_region->paddr = dw_edma_get_phys_addr(pdev, match,
- dma_data, dt_block->bar);
- dt_region->paddr += dt_block->off;
+ dt_region->paddr = dw_edma_get_block_addr(pdev, match, dma_data,
+ dt_block);
dt_region->sz = dt_block->sz;
}
@@ -506,9 +518,8 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
return -ENOMEM;
ll_region->vaddr.io += ll_block->off;
- ll_region->paddr = dw_edma_get_phys_addr(pdev, match,
- dma_data, ll_block->bar);
- ll_region->paddr += ll_block->off;
+ ll_region->paddr = dw_edma_get_block_addr(pdev, match, dma_data,
+ ll_block);
ll_region->sz = ll_block->sz;
dt_region->vaddr.io = pcim_iomap_table(pdev)[dt_block->bar];
@@ -516,9 +527,8 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
return -ENOMEM;
dt_region->vaddr.io += dt_block->off;
- dt_region->paddr = dw_edma_get_phys_addr(pdev, match,
- dma_data, dt_block->bar);
- dt_region->paddr += dt_block->off;
+ dt_region->paddr = dw_edma_get_block_addr(pdev, match, dma_data,
+ dt_block);
dt_region->sz = dt_block->sz;
}
--
2.51.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v5 12/14] dmaengine: dw-edma-pcie: Handle optional data blocks
2026-07-17 5:02 [PATCH v5 00/14] dmaengine: dw-edma: Prepare for PCI EP DMA (part 1/3) Koichiro Den
` (10 preceding siblings ...)
2026-07-17 5:03 ` [PATCH v5 11/14] dmaengine: dw-edma-pcie: Factor out descriptor block address lookup Koichiro Den
@ 2026-07-17 5:03 ` Koichiro Den
2026-07-17 5:18 ` sashiko-bot
2026-07-17 5:03 ` [PATCH v5 13/14] dmaengine: dw-edma-pcie: Add chip flags to match data Koichiro Den
2026-07-17 5:03 ` [PATCH v5 14/14] dmaengine: dw-edma: Program endpoint function numbers Koichiro Den
13 siblings, 1 reply; 22+ messages in thread
From: Koichiro Den @ 2026-07-17 5:03 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Manivannan Sadhasivam
Cc: Marek Vasut, Yoshihiro Shimoda, dmaengine, linux-kernel
Skip data block BAR mapping and debug output when a channel has no data
block size. This lets future providers describe channels that only need
descriptor memory exposed.
No functional change intended for existing Synopsys EDDA and
AMD (Xilinx) MDB/CPM6 devices. Their static channel descriptions still
provide data block sizes where data block windows are used. A zero-sized
data block now means "not present" for future metadata providers.
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v5:
- No changes.
drivers/dma/dw-edma/dw-edma-pcie.c | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
index a5d65cc9706c..8c4d1359a545 100644
--- a/drivers/dma/dw-edma/dw-edma-pcie.c
+++ b/drivers/dma/dw-edma/dw-edma-pcie.c
@@ -430,11 +430,13 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
mask = BIT(dma_data->rg.bar);
for (i = 0; i < dma_data->wr_ch_cnt; i++) {
mask |= BIT(dma_data->ll_wr[i].bar);
- mask |= BIT(dma_data->dt_wr[i].bar);
+ if (dma_data->dt_wr[i].sz)
+ mask |= BIT(dma_data->dt_wr[i].bar);
}
for (i = 0; i < dma_data->rd_ch_cnt; i++) {
mask |= BIT(dma_data->ll_rd[i].bar);
- mask |= BIT(dma_data->dt_rd[i].bar);
+ if (dma_data->dt_rd[i].sz)
+ mask |= BIT(dma_data->dt_rd[i].bar);
}
err = pcim_iomap_regions(pdev, mask, pci_name(pdev));
if (err) {
@@ -497,6 +499,9 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
ll_block);
ll_region->sz = ll_block->sz;
+ if (!dt_block->sz)
+ continue;
+
dt_region->vaddr.io = pcim_iomap_table(pdev)[dt_block->bar];
if (!dt_region->vaddr.io)
return -ENOMEM;
@@ -522,6 +527,9 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
ll_block);
ll_region->sz = ll_block->sz;
+ if (!dt_block->sz)
+ continue;
+
dt_region->vaddr.io = pcim_iomap_table(pdev)[dt_block->bar];
if (!dt_region->vaddr.io)
return -ENOMEM;
@@ -555,10 +563,14 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
dma_data->ll_wr[i].off, chip->ll_region_wr[i].sz,
chip->ll_region_wr[i].vaddr.io, &chip->ll_region_wr[i].paddr);
+ if (!dma_data->dt_wr[i].sz)
+ continue;
+
pci_dbg(pdev, "Data:\tWRITE CH%.2u, BAR=%u, off=0x%.8lx, sz=0x%zx bytes, addr(v=%p, p=%pa)\n",
i, dma_data->dt_wr[i].bar,
dma_data->dt_wr[i].off, chip->dt_region_wr[i].sz,
- chip->dt_region_wr[i].vaddr.io, &chip->dt_region_wr[i].paddr);
+ chip->dt_region_wr[i].vaddr.io,
+ &chip->dt_region_wr[i].paddr);
}
for (i = 0; i < chip->ll_rd_cnt; i++) {
@@ -567,10 +579,14 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
dma_data->ll_rd[i].off, chip->ll_region_rd[i].sz,
chip->ll_region_rd[i].vaddr.io, &chip->ll_region_rd[i].paddr);
+ if (!dma_data->dt_rd[i].sz)
+ continue;
+
pci_dbg(pdev, "Data:\tREAD CH%.2u, BAR=%u, off=0x%.8lx, sz=0x%zx bytes, addr(v=%p, p=%pa)\n",
i, dma_data->dt_rd[i].bar,
dma_data->dt_rd[i].off, chip->dt_region_rd[i].sz,
- chip->dt_region_rd[i].vaddr.io, &chip->dt_region_rd[i].paddr);
+ chip->dt_region_rd[i].vaddr.io,
+ &chip->dt_region_rd[i].paddr);
}
pci_dbg(pdev, "Nr. IRQs:\t%u\n", chip->nr_irqs);
--
2.51.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v5 12/14] dmaengine: dw-edma-pcie: Handle optional data blocks
2026-07-17 5:03 ` [PATCH v5 12/14] dmaengine: dw-edma-pcie: Handle optional data blocks Koichiro Den
@ 2026-07-17 5:18 ` sashiko-bot
0 siblings, 0 replies; 22+ messages in thread
From: sashiko-bot @ 2026-07-17 5:18 UTC (permalink / raw)
To: Koichiro Den; +Cc: vkoul, Frank.Li, dmaengine
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] IRQ vectors allocated by pci_alloc_irq_vectors() are leaked in multiple error paths within dw_edma_pcie_probe().
--
--- Patch [12]: [PATCH v5 12/14] dmaengine: dw-edma-pcie: Handle optional data blocks ---
commit 668bf3e5a48eceeac33c0a1fc5620f41d4cdbd7d
Author: Koichiro Den <den@valinux.co.jp>
dmaengine: dw-edma-pcie: Handle optional data blocks
This commit skips data block BAR mapping and debug output when a channel has
no data block size. This allows future providers to describe channels that
only need descriptor memory exposed, with no functional change intended for
existing devices.
> diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
> index a5d65cc9706c2..8c4d1359a5456 100644
> --- a/drivers/dma/dw-edma/dw-edma-pcie.c
> +++ b/drivers/dma/dw-edma/dw-edma-pcie.c
[ ... ]
> @@ -497,6 +499,9 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
> ll_block);
> ll_region->sz = ll_block->sz;
>
> + if (!dt_block->sz)
> + continue;
> +
> dt_region->vaddr.io = pcim_iomap_table(pdev)[dt_block->bar];
> if (!dt_region->vaddr.io)
> return -ENOMEM;
[Severity: High]
This is a pre-existing issue, but does this error path in
dw_edma_pcie_probe() leak IRQ resources?
The probe function successfully allocates vectors via pci_alloc_irq_vectors()
earlier in the execution, but if this dt_region mapping fails, the function
returns -ENOMEM directly without calling pci_free_irq_vectors().
Failing to free IRQ resources can exhaust system IRQ capacity and prevent
future allocations. The same pattern appears to exist in the read channel
mapping loop and the final dw_edma_probe() error paths. Should there be a
common error handling label to properly clean up the allocated IRQ vectors?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260717050308.2144108-1-den@valinux.co.jp?part=12
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v5 13/14] dmaengine: dw-edma-pcie: Add chip flags to match data
2026-07-17 5:02 [PATCH v5 00/14] dmaengine: dw-edma: Prepare for PCI EP DMA (part 1/3) Koichiro Den
` (11 preceding siblings ...)
2026-07-17 5:03 ` [PATCH v5 12/14] dmaengine: dw-edma-pcie: Handle optional data blocks Koichiro Den
@ 2026-07-17 5:03 ` Koichiro Den
2026-07-17 5:03 ` [PATCH v5 14/14] dmaengine: dw-edma: Program endpoint function numbers Koichiro Den
13 siblings, 0 replies; 22+ messages in thread
From: Koichiro Den @ 2026-07-17 5:03 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Manivannan Sadhasivam
Cc: Marek Vasut, Yoshihiro Shimoda, dmaengine, linux-kernel
Allow PCI ID match data to pass dw_edma_chip flags into dw_edma_probe().
This keeps per-device policy in the match data instead of open-coding it
in probe().
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v5:
- No changes.
drivers/dma/dw-edma/dw-edma-pcie.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
index 8c4d1359a545..7f96e65ee274 100644
--- a/drivers/dma/dw-edma/dw-edma-pcie.c
+++ b/drivers/dma/dw-edma/dw-edma-pcie.c
@@ -88,6 +88,7 @@ struct dw_edma_pcie_match_data {
int (*parse_caps)(struct pci_dev *pdev,
struct dw_edma_pcie_data *pdata);
unsigned long flags;
+ u32 chip_flags;
};
#define DW_EDMA_PCIE_F_DEVMEM_PHYS_OFF BIT(0)
@@ -471,6 +472,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
chip->dev = dev;
chip->mf = dma_data->mf;
+ chip->flags = match->chip_flags;
chip->nr_irqs = nr_irqs;
chip->ops = match->plat_ops;
chip->cfg_non_ll = dma_data->cfg_non_ll;
--
2.51.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v5 14/14] dmaengine: dw-edma: Program endpoint function numbers
2026-07-17 5:02 [PATCH v5 00/14] dmaengine: dw-edma: Prepare for PCI EP DMA (part 1/3) Koichiro Den
` (12 preceding siblings ...)
2026-07-17 5:03 ` [PATCH v5 13/14] dmaengine: dw-edma-pcie: Add chip flags to match data Koichiro Den
@ 2026-07-17 5:03 ` Koichiro Den
2026-07-17 5:29 ` sashiko-bot
13 siblings, 1 reply; 22+ messages in thread
From: Koichiro Den @ 2026-07-17 5:03 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Manivannan Sadhasivam
Cc: Marek Vasut, Yoshihiro Shimoda, dmaengine, linux-kernel
The eDMA/HDMA transfers the driver issues carry a requester function
number in their TLPs, but nothing ever programs it: eDMA v0 leaves the
FUNC_NUM field of the channel control word zero and HDMA leaves the
per-channel func_num register at its reset value, so every transfer is
attributed to function 0. That is invisible in single-function setups,
but once the DMA block serves a non-zero endpoint function, its
requests must carry that function's number for the host to attribute
and translate them correctly.
Record the function number in the chip data (PCI_FUNC() of the probing
device for dw-edma-pcie) and program it per channel.
Endpoint-local chip instances keep func_no at 0, so transfers issued by
the endpoint-side driver remain PF0-attributed. Delegated channels are
programmed by the host-side dw-edma-pcie instance when it takes over the
channel, using that instance's PCI_FUNC().
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v5:
- Add Frank's Reviewed-by tag.
drivers/dma/dw-edma/dw-edma-core.c | 1 +
drivers/dma/dw-edma/dw-edma-core.h | 1 +
drivers/dma/dw-edma/dw-edma-pcie.c | 1 +
drivers/dma/dw-edma/dw-edma-v0-core.c | 10 +++++++++-
drivers/dma/dw-edma/dw-hdma-v0-core.c | 3 +++
drivers/dma/dw-edma/dw-hdma-v0-regs.h | 1 +
include/linux/dma/edma.h | 2 ++
7 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index cef75da3f2f6..43f8280b5811 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -818,6 +818,7 @@ static int dw_edma_channel_setup(struct dw_edma *dw, u32 wr_alloc, u32 rd_alloc)
chan = &dw->chan[i];
chan->dw = dw;
+ chan->func_no = chip->func_no;
if (i < dw->wr_ch_cnt) {
chan->id = i;
diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
index 1a9f1b58d2fd..3832e015aee9 100644
--- a/drivers/dma/dw-edma/dw-edma-core.h
+++ b/drivers/dma/dw-edma/dw-edma-core.h
@@ -68,6 +68,7 @@ struct dw_edma_chan {
struct dw_edma *dw;
int id;
enum dw_edma_dir dir;
+ u8 func_no;
u32 ll_max;
struct dw_edma_region ll_region; /* Linked list */
diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
index 7f96e65ee274..07ea237577d5 100644
--- a/drivers/dma/dw-edma/dw-edma-pcie.c
+++ b/drivers/dma/dw-edma/dw-edma-pcie.c
@@ -473,6 +473,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
chip->mf = dma_data->mf;
chip->flags = match->chip_flags;
+ chip->func_no = PCI_FUNC(pdev->devfn);
chip->nr_irqs = nr_irqs;
chip->ops = match->plat_ops;
chip->cfg_non_ll = dma_data->cfg_non_ll;
diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
index 3e9512a7b201..fa123fa167f9 100644
--- a/drivers/dma/dw-edma/dw-edma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
@@ -26,6 +26,8 @@ enum dw_edma_control {
DW_EDMA_V0_LLE = BIT(9),
};
+#define EDMA_V0_FUNC_NUM_MASK GENMASK(16, 12)
+
static inline struct dw_edma_v0_regs __iomem *__dw_regs(struct dw_edma *dw)
{
return dw->chip->reg_base;
@@ -160,6 +162,11 @@ static inline u32 readl_ch(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch,
#define GET_CH_32(dw, dir, ch, name) \
readl_ch(dw, dir, ch, &(__dw_ch_regs(dw, dir, ch)->name))
+static u32 dw_edma_v0_func_num(struct dw_edma_chan *chan)
+{
+ return FIELD_PREP(EDMA_V0_FUNC_NUM_MASK, chan->func_no);
+}
+
/* eDMA management callbacks */
static void dw_edma_v0_core_ch_power(struct dw_edma *dw,
enum dw_edma_dir dir, u16 id, bool enable)
@@ -464,7 +471,8 @@ static void dw_edma_v0_core_ch_enable(struct dw_edma_chan *chan)
/* Channel control */
SET_CH_32(dw, chan->dir, chan->id, ch_control1,
- (DW_EDMA_V0_CCS | DW_EDMA_V0_LLE));
+ DW_EDMA_V0_CCS | DW_EDMA_V0_LLE |
+ dw_edma_v0_func_num(chan));
/* Linked list */
/* llp is not aligned on 64bit -> keep 32bit accesses */
SET_CH_32(dw, chan->dir, chan->id, llp.lsb,
diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
index 77260c076cd3..fdb2c6fb08e4 100644
--- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
@@ -334,6 +334,9 @@ static void dw_hdma_v0_core_ch_config(struct dw_edma_chan *chan)
SET_CH_32(dw, chan->dir, chan->id, msi_abort.msb, chan->msi.address_hi);
/* config MSI data */
SET_CH_32(dw, chan->dir, chan->id, msi_msgdata, chan->msi.data);
+ /* Configure the requester function number used by outbound TLPs. */
+ SET_CH_32(dw, chan->dir, chan->id, func_num,
+ FIELD_PREP(HDMA_V0_FUNC_NUM_PF_MASK, chan->func_no));
}
static void
diff --git a/drivers/dma/dw-edma/dw-hdma-v0-regs.h b/drivers/dma/dw-edma/dw-hdma-v0-regs.h
index 7759ba9b4850..2bbcc7fabb0a 100644
--- a/drivers/dma/dw-edma/dw-hdma-v0-regs.h
+++ b/drivers/dma/dw-edma/dw-hdma-v0-regs.h
@@ -24,6 +24,7 @@
#define HDMA_V0_CONSUMER_CYCLE_BIT BIT(0)
#define HDMA_V0_DOORBELL_START BIT(0)
#define HDMA_V0_CH_STATUS_MASK GENMASK(1, 0)
+#define HDMA_V0_FUNC_NUM_PF_MASK GENMASK(7, 0)
struct dw_hdma_v0_ch_regs {
u32 ch_en; /* 0x0000 */
diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
index f827f2dd5299..45f66fc523d7 100644
--- a/include/linux/dma/edma.h
+++ b/include/linux/dma/edma.h
@@ -115,6 +115,7 @@ enum dw_edma_ch_irq_mode {
* @db_irq: Virtual IRQ dedicated to interrupt emulation
* @db_offset: Offset from DMA register base
* @mf: DMA register map format
+ * @func_no: PCI endpoint function number used by DMA TLPs
* @dw: struct dw_edma that is filled by dw_edma_probe()
*/
struct dw_edma_chip {
@@ -140,6 +141,7 @@ struct dw_edma_chip {
resource_size_t db_offset;
enum dw_edma_map_format mf;
+ u8 func_no;
struct dw_edma *dw;
bool cfg_non_ll;
--
2.51.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v5 14/14] dmaengine: dw-edma: Program endpoint function numbers
2026-07-17 5:03 ` [PATCH v5 14/14] dmaengine: dw-edma: Program endpoint function numbers Koichiro Den
@ 2026-07-17 5:29 ` sashiko-bot
0 siblings, 0 replies; 22+ messages in thread
From: sashiko-bot @ 2026-07-17 5:29 UTC (permalink / raw)
To: Koichiro Den; +Cc: Frank.Li, dmaengine, vkoul
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Using `PCI_FUNC(pdev->devfn)` incorrectly truncates the 8-bit PCI `devfn` to 3 bits, leading to corrupted Requester IDs in outgoing DMA TLPs for Alternative Routing-ID Interpretation (ARI) devices and non-zero device number RCiEPs.
--
--- Patch [14]: [PATCH v5 14/14] dmaengine: dw-edma: Program endpoint function numbers ---
commit 69615a1713850d90b9bf46ff89bf560c8d19478f
Author: Koichiro Den <den@valinux.co.jp>
dmaengine: dw-edma: Program endpoint function numbers
This commit programs the requester function number for eDMA/HDMA transfers
so that the host can attribute and translate them correctly. It records the
function number using PCI_FUNC() during probe and programs it per channel.
> diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
> index 7f96e65ee2746..07ea237577d5e 100644
> --- a/drivers/dma/dw-edma/dw-edma-pcie.c
> +++ b/drivers/dma/dw-edma/dw-edma-pcie.c
> @@ -473,6 +473,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
>
> chip->mf = dma_data->mf;
> chip->flags = match->chip_flags;
> + chip->func_no = PCI_FUNC(pdev->devfn);
[Severity: High]
Does PCI_FUNC(pdev->devfn) incorrectly truncate the 8-bit PCI devfn to 3 bits
for Alternative Routing-ID Interpretation (ARI) devices and non-zero device
number RCiEPs?
Because the PCI_FUNC() macro masks the value with 0x07, it discards the upper
5 bits. This might cause DMA transfers from an endpoint function with an ARI
function number >= 8 to carry an aliased Requester ID.
> chip->nr_irqs = nr_irqs;
> chip->ops = match->plat_ops;
[ ... ]
> diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> index 77260c076cd3a..fdb2c6fb08e41 100644
> --- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> @@ -334,6 +334,9 @@ static void dw_hdma_v0_core_ch_config(struct dw_edma_chan *chan)
> SET_CH_32(dw, chan->dir, chan->id, msi_abort.msb, chan->msi.address_hi);
> /* config MSI data */
> SET_CH_32(dw, chan->dir, chan->id, msi_msgdata, chan->msi.data);
> + /* Configure the requester function number used by outbound TLPs. */
> + SET_CH_32(dw, chan->dir, chan->id, func_num,
> + FIELD_PREP(HDMA_V0_FUNC_NUM_PF_MASK, chan->func_no));
[Severity: High]
Since HDMA_V0_FUNC_NUM_PF_MASK is defined as GENMASK(7, 0) and is intended to
hold the full Routing ID, will this field end up receiving the truncated
3-bit value set during probe? If so, could this lead to IOMMU faults by
blocking TLPs with mismatched Requester IDs?
> }
>
> static void
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260717050308.2144108-1-den@valinux.co.jp?part=14
^ permalink raw reply [flat|nested] 22+ messages in thread