[PATCH v3 00/13] dmaengine: dw-edma: Prepare for PCI EP DMA (part 1/3)

[PATCH v3 00/13] dmaengine: dw-edma: Prepare for PCI EP DMA (part 1/3)

[Koichiro Den]

Hi,

This is v3, part 1 of three series for PCI endpoint DMA.

The three series are:

  * part 1: dmaengine: dw-edma: Prepare for PCI EP DMA
  * part 2: PCI: endpoint: Expose endpoint DMA resources
  * part 3: PCI: endpoint: Add PCI DMA endpoint function

Most v3 changes address review comments from Frank and Sashiko. v3 also
extends the DesignWare DMA groundwork for HDMA native.

This series is (re-)based on linux-next next-20260619, where the dmaengine
and PCI endpoint changes needed by the full three-part series are both
naturally available. Parts 2 and 3 depend on this series.


Scope
=====

This series prepares dw-edma and dw-edma-pcie for endpoint-local DMA
channels that are delegated to a PCI host. It does not add the endpoint
metadata format, DesignWare endpoint resource exposure, or the endpoint
function driver; those are added by parts 2 and 3.

This part is the DesignWare dmaengine backend work. The endpoint resource
and endpoint function pieces in parts 2 and 3 keep the generic PCI endpoint
interfaces separate from the DesignWare implementation.

In summary, this series:

  * adds per-channel interrupt routing so a channel can report completion
    either to the local endpoint side or to the remote host side,
  * adds quiesce operations for the resources represented by a dw-edma
    instance,
  * adds helpers to reserve exact DesignWare hardware channels and hand
    their interrupt ownership to the remote side,
  * adds partial channel ownership mode for dw-edma instances that share a
    controller with another OS instance, and
  * prepares dw-edma-pcie to describe device-specific DMA layouts through
    match data.

---
Changelog
=========

Changes in v3:
  - Replace the public dw-edma hardware-channel filter API with delegated
    channel request/release helpers, keeping the DMAengine filter private
    to dw-edma. (Frank)
  - Rework IRQ routing so local routing is the zero value, existing
    dw-edma-pcie instances stay remote-routed, and delegated endpoint-local
    channels are handed to the remote side explicitly. (Frank/Sashiko)
  - Add HDMA native interrupt routing and allow channel-granular partial
    ownership for HDMA native.
  - Add quiesce operations and use them for delegated-channel reclaim and
    partial-owned remove paths.
  - Reintroduce the IRQ data initialization fix because partial-owned probe
    skips the core_off() reset that previously made the early-IRQ window
    unlikely.
  - Adjust dw-edma-pcie match-data preparation for the CPM6 entry present
    in the new base, and reject dynamic PCI IDs without match data.

Changes in v2:
  - Move non-LL state and platform ops into match data. (Frank)
  - Use a named .driver_data initializer for the Xilinx MDB ID entry and
    fix the vsec_data rename patch title. (Frank)
  - Replace the dma_get_slave_channel() export with a dw-edma channel
    filter for dma_request_channel(). (Sashiko)
  - Rework the IRQ-routing config as dw_edma_irq_config, keep HDMA native
    int config separate, and reject remote IRQ mode on local instances.
    (Sashiko)
  - Report IRQ_HANDLED only for status that was actually serviced and drop
    the lockless free_chan_resources() reset. (Sashiko)
  - Tighten partial ownership: reject unsupported map formats early and
    require direction-wide ownership for supported shared-register
    layouts. (Sashiko)

v2: https://lore.kernel.org/dmaengine/20260525062420.3315904-1-den@valinux.co.jp/
v1: https://lore.kernel.org/dmaengine/20260521063115.2842238-1-den@valinux.co.jp/


Best regards,
Koichiro


Koichiro Den (13):
  dmaengine: dw-edma: Add per-channel interrupt routing control
  dmaengine: dw-edma: Add core quiesce operations
  dmaengine: dw-edma: Add delegated channel request helpers
  dmaengine: dw-edma: Initialize IRQ data before requesting IRQs
  dmaengine: dw-edma: Add partial channel ownership mode
  dmaengine: dw-edma-pcie: Track non-LL mode in DMA data
  dmaengine: dw-edma-pcie: Add capability match data
  dmaengine: dw-edma-pcie: Rename vsec_data to dma_data
  dmaengine: dw-edma-pcie: Add platform ops to match data
  dmaengine: dw-edma-pcie: Add register offset match flag
  dmaengine: dw-edma-pcie: Factor out descriptor block address lookup
  dmaengine: dw-edma-pcie: Handle optional data blocks
  dmaengine: dw-edma-pcie: Add chip flags to match data

 drivers/dma/dw-edma/dw-edma-core.c    | 150 +++++++++++++--
 drivers/dma/dw-edma/dw-edma-core.h    |  27 +++
 drivers/dma/dw-edma/dw-edma-pcie.c    | 253 +++++++++++++++++---------
 drivers/dma/dw-edma/dw-edma-v0-core.c |  46 ++++-
 drivers/dma/dw-edma/dw-hdma-v0-core.c |  68 +++++--
 include/linux/dma/edma.h              |  51 ++++++
 6 files changed, 479 insertions(+), 116 deletions(-)

-- 
2.51.0

[PATCH v3 01/13] dmaengine: dw-edma: Add per-channel interrupt routing control

[Koichiro Den]

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 v0 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, use the dedicated remote
stop/abort enables without local stop/abort enables.

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 that 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.

Suggested-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v3:
  - Remove DW_EDMA_CH_IRQ_DEFAULT; local routing is the zero value
    (Frank).
  - Set existing dw-edma-pcie host-side instances to remote interrupt
    routing in this patch, preserving the legacy IMWr completion model.
  - Remove an unreachable HDMA native check (Sashiko).
  - Clarify local/remote instance ownership after Frank's question.
  - Mark non-owner IRQ handling guard paths unlikely.
  - Add HDMA native interrupt routing while keeping the existing non-LL
    int config ABI.
  - Keep HDMA native linked-list local interrupt generation enabled for
    remote-routed channels while masking the local edma_int[] output.
  - Use remote-only stop/abort enables for HDMA native non-LL remote-routed
    channels.
  - Drop the peripheral_config IRQ-routing ABI; initial routing comes from
    chip setup and channel ownership handoff can override it.
  - Keep dma_slave_config from resetting channel ownership routing.

 drivers/dma/dw-edma/dw-edma-core.c    | 14 +++++++++
 drivers/dma/dw-edma/dw-edma-core.h    | 13 +++++++++
 drivers/dma/dw-edma/dw-edma-pcie.c    |  1 +
 drivers/dma/dw-edma/dw-edma-v0-core.c | 22 ++++++++++----
 drivers/dma/dw-edma/dw-hdma-v0-core.c | 41 ++++++++++++++++-----------
 include/linux/dma/edma.h              | 30 ++++++++++++++++++++
 6 files changed, 99 insertions(+), 22 deletions(-)

diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index 89a4c498a17b..7a24248b84e9 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -219,6 +219,17 @@ static void dw_edma_device_caps(struct dma_chan *dchan,
 	}
 }
 
+static enum dw_edma_ch_irq_mode dw_edma_get_irq_mode(struct dw_edma_chan *chan)
+{
+	struct dw_edma_chip *chip = chan->dw->chip;
+
+	if (chip->irq_mode == DW_EDMA_CH_IRQ_REMOTE &&
+	    !(chip->flags & DW_EDMA_CHIP_LOCAL))
+		return DW_EDMA_CH_IRQ_REMOTE;
+
+	return DW_EDMA_CH_IRQ_LOCAL;
+}
+
 static int dw_edma_device_config(struct dma_chan *dchan,
 				 struct dma_slave_config *config)
 {
@@ -853,6 +864,8 @@ static int dw_edma_alloc_chan_resources(struct dma_chan *dchan)
 	if (chan->status != EDMA_ST_IDLE)
 		return -EBUSY;
 
+	chan->irq_mode = dw_edma_get_irq_mode(chan);
+
 	return 0;
 }
 
@@ -904,6 +917,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_irq_mode(chan);
 
 		if (chan->dir == EDMA_DIR_WRITE)
 			chan->ll_max = (chip->ll_region_wr[chan->id].sz / EDMA_LL_SZ);
diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
index 6474cacf7195..42f2f25ef377 100644
--- a/drivers/dma/dw-edma/dw-edma-core.h
+++ b/drivers/dma/dw-edma/dw-edma-core.h
@@ -81,6 +81,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;
@@ -224,4 +226,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-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
index 791c46e8ae4c..70ea031147d1 100644
--- a/drivers/dma/dw-edma/dw-edma-pcie.c
+++ b/drivers/dma/dw-edma/dw-edma-pcie.c
@@ -419,6 +419,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
 	chip->dev = dev;
 
 	chip->mf = vsec_data->mf;
+	chip->irq_mode = DW_EDMA_CH_IRQ_REMOTE;
 	chip->nr_irqs = nr_irqs;
 	chip->ops = &dw_edma_pcie_plat_ops;
 	chip->cfg_non_ll = 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 cfdd6463252e..1781ba4f022e 100644
--- a/drivers/dma/dw-edma/dw-edma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
@@ -256,9 +256,11 @@ 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);
-
 		ret = IRQ_HANDLED;
 	}
 
@@ -267,9 +269,11 @@ 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);
-
 		ret = IRQ_HANDLED;
 	}
 
@@ -331,7 +335,8 @@ static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
 		j--;
 		if (!j) {
 			control |= DW_EDMA_V0_LIE;
-			if (!(chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL))
+			if (!(chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL) &&
+			    chan->irq_mode != DW_EDMA_CH_IRQ_LOCAL)
 				control |= DW_EDMA_V0_RIE;
 		}
 
@@ -408,12 +413,17 @@ static void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
 				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);
diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
index 2beec876b184..7ba6bdbffc17 100644
--- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
@@ -49,6 +49,26 @@ __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)
+{
+	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);
+
+	if (chan->irq_mode == DW_EDMA_CH_IRQ_REMOTE && chan->non_ll)
+		return val | HDMA_V0_REMOTE_ABORT_INT_EN |
+		       HDMA_V0_REMOTE_STOP_INT_EN;
+
+	if (chan->irq_mode == DW_EDMA_CH_IRQ_REMOTE)
+		return 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;
+
+	return val | HDMA_V0_LOCAL_ABORT_INT_EN | HDMA_V0_LOCAL_STOP_INT_EN;
+}
+
 /* HDMA management callbacks */
 static void dw_hdma_v0_core_off(struct dw_edma *dw)
 {
@@ -132,6 +152,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)) {
@@ -238,11 +260,7 @@ static void dw_hdma_v0_core_ll_start(struct dw_edma_chunk *chunk, bool first)
 		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);
@@ -293,17 +311,8 @@ static void dw_hdma_v0_core_non_ll_start(struct dw_edma_chunk *chunk)
 	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 */
diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
index 1fafd5b0e315..c0906221a7c7 100644
--- a/include/linux/dma/edma.h
+++ b/include/linux/dma/edma.h
@@ -60,6 +60,34 @@ 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).
+ *
+ * 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, and the remote CPU programming examples use remote enables without
+ * local 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
@@ -76,6 +104,7 @@ enum dw_edma_chip_flags {
  * @db_irq:		 Virtual IRQ dedicated to interrupt emulation
  * @db_offset:		 Offset from DMA register base
  * @mf:			 DMA register map format
+ * @irq_mode:		 default per-channel interrupt routing
  * @dw:			 struct dw_edma that is filled by dw_edma_probe()
  */
 struct dw_edma_chip {
@@ -101,6 +130,7 @@ struct dw_edma_chip {
 	resource_size_t		db_offset;
 
 	enum dw_edma_map_format	mf;
+	enum dw_edma_ch_irq_mode	irq_mode;
 
 	struct dw_edma		*dw;
 	bool			cfg_non_ll;
-- 
2.51.0

Re: [PATCH v3 01/13] dmaengine: dw-edma: Add per-channel interrupt routing control

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] V0 eDMA fails to set the Remote Interrupt Enable (RIE) bit for remote-routed channels on a local endpoint controller.
--

commit df5a2947e52cc02cfc126b63bc365277ac3752fa
Author: Koichiro Den <den@valinux.co.jp>
dmaengine: dw-edma: Add per-channel interrupt routing control

This commit adds per-channel interrupt routing control to the DesignWare
eDMA driver, allowing channels to route completion signals locally or
remotely. It updates the v0 eDMA and HDMA paths to configure interrupt masks
and generation based on the selected 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
[ ... ]
> @@ -331,7 +335,8 @@ static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk)
>  		j--;
>  		if (!j) {
>  			control |= DW_EDMA_V0_LIE;
> -			if (!(chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL))
> +			if (!(chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL) &&
> +			    chan->irq_mode != DW_EDMA_CH_IRQ_LOCAL)
>  				control |= DW_EDMA_V0_RIE;
>  		}

[Severity: High]
When a local endpoint controller delegates a channel to a remote host, the
irq_mode is set to DW_EDMA_CH_IRQ_REMOTE, but DW_EDMA_CHIP_LOCAL is also
present in the chip flags.

Because the check in dw_edma_v0_core_write_chunk() explicitly requires that
DW_EDMA_CHIP_LOCAL is not present, the first condition evaluates to false
and DW_EDMA_V0_RIE is never set.

According to the commit message, requesting a remote-only interrupt requires
setting both LIE and RIE while masking the local interrupt. Since
dw_edma_v0_core_start() correctly masks the local interrupt for these
channels, does this prevent the DMA completion event from being sent
entirely, potentially causing the remote host to wait forever?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260620170040.3756043-1-den@valinux.co.jp?part=1

[PATCH v3 02/13] dmaengine: dw-edma: Add core quiesce operations

[Koichiro Den]

Add core operations that quiesce only the resources represented by a
dw-edma instance, separate from the existing full controller off path.

For v0 eDMA and HDMA compatible register layouts, quiescing one channel
must quiesce the whole direction because the enable and interrupt
mask/clear registers are direction-wide. For HDMA native, the operation
can quiesce the represented per-channel registers 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 v3:
  - New patch. Add quiesce primitives before delegated-channel release
    and partial-owned remove start using them.
  - Note: Devendra's under-review "dmaengine: dw-edma: Enable HDMA 64R/W
    Channels" series may require a follow-up rebase if it lands first.

 drivers/dma/dw-edma/dw-edma-core.h    | 14 ++++++++++++++
 drivers/dma/dw-edma/dw-edma-v0-core.c | 24 ++++++++++++++++++++++++
 drivers/dma/dw-edma/dw-hdma-v0-core.c | 27 +++++++++++++++++++++++++++
 3 files changed, 65 insertions(+)

diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
index 42f2f25ef377..f9d4e0411f8f 100644
--- a/drivers/dma/dw-edma/dw-edma-core.h
+++ b/drivers/dma/dw-edma/dw-edma-core.h
@@ -122,6 +122,8 @@ typedef void (*dw_edma_handler_t)(struct dw_edma_chan *);
 
 struct dw_edma_core_ops {
 	void (*off)(struct dw_edma *dw);
+	void (*quiesce)(struct dw_edma *dw);
+	void (*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,
@@ -174,6 +176,18 @@ void dw_edma_core_off(struct dw_edma *dw)
 	dw->core->off(dw);
 }
 
+static inline
+void dw_edma_core_quiesce(struct dw_edma *dw)
+{
+	dw->core->quiesce(dw);
+}
+
+static inline
+void dw_edma_core_ch_quiesce(struct dw_edma_chan *chan)
+{
+	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 1781ba4f022e..316d8c94eff9 100644
--- a/drivers/dma/dw-edma/dw-edma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
@@ -160,6 +160,15 @@ 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_dir_off(struct dw_edma *dw, enum dw_edma_dir dir)
+{
+	SET_RW_32(dw, dir, int_mask,
+		  EDMA_V0_DONE_INT_MASK | EDMA_V0_ABORT_INT_MASK);
+	SET_RW_32(dw, dir, int_clear,
+		  EDMA_V0_DONE_INT_MASK | EDMA_V0_ABORT_INT_MASK);
+	SET_RW_32(dw, dir, engine_en, 0);
+}
+
 static void dw_edma_v0_core_off(struct dw_edma *dw)
 {
 	SET_BOTH_32(dw, int_mask,
@@ -169,6 +178,19 @@ static void dw_edma_v0_core_off(struct dw_edma *dw)
 	SET_BOTH_32(dw, engine_en, 0);
 }
 
+static void dw_edma_v0_core_quiesce(struct dw_edma *dw)
+{
+	if (dw->wr_ch_cnt)
+		dw_edma_v0_core_dir_off(dw, EDMA_DIR_WRITE);
+	if (dw->rd_ch_cnt)
+		dw_edma_v0_core_dir_off(dw, EDMA_DIR_READ);
+}
+
+static void dw_edma_v0_core_ch_quiesce(struct dw_edma_chan *chan)
+{
+	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;
@@ -546,6 +568,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 7ba6bdbffc17..63c30a6eb88c 100644
--- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
@@ -70,6 +70,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, int_clear,
+		  HDMA_V0_STOP_INT_MASK | HDMA_V0_ABORT_INT_MASK);
+	SET_CH_32(dw, dir, id, ch_en, 0);
+}
+
 static void dw_hdma_v0_core_off(struct dw_edma *dw)
 {
 	int id;
@@ -83,6 +93,21 @@ static void dw_hdma_v0_core_off(struct dw_edma *dw)
 	}
 }
 
+static void 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);
+}
+
+static void dw_hdma_v0_core_ch_quiesce(struct dw_edma_chan *chan)
+{
+	dw_hdma_v0_core_ch_off(chan->dw, chan->dir, chan->id);
+}
+
 static u16 dw_hdma_v0_core_ch_count(struct dw_edma *dw, enum dw_edma_dir dir)
 {
 	/*
@@ -362,6 +387,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

Re: [PATCH v3 02/13] dmaengine: dw-edma: Add core quiesce operations

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The v0 eDMA `ch_quiesce` operation globally disables the DMA engine and clobbers interrupt states for all channels in the direction.
- [High] `dw_edma_v0_core_dir_off` modifies the shared direction-wide `int_mask` register without holding `dw->lock`, causing a data race.
--

commit d7eb32d006bf71fa3372ef97f6fa793fa851c5c8
Author: Koichiro Den <den@valinux.co.jp>

dmaengine: dw-edma: Add core quiesce operations

This adds core operations to quiesce only the resources represented by a
dw-edma instance. It implements per-channel and per-direction quiesce
operations for v0 eDMA and HDMA compatible register layouts.

> diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> index 1781ba4f022ec..316d8c94eff9c 100644
> --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c

[ ... ]

> @@ -160,6 +160,15 @@ 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_dir_off(struct dw_edma *dw, enum dw_edma_dir dir)
> +{
> +	SET_RW_32(dw, dir, int_mask,
> +		  EDMA_V0_DONE_INT_MASK | EDMA_V0_ABORT_INT_MASK);

[Severity: High]
Does this modification of the shared int_mask register need to be
protected by dw->lock?

In dw_edma_v0_core_start(), modifications to int_mask are protected by
raw_spin_lock_irqsave(&dw->lock, flags). However,
dw_edma_v0_core_dir_off() directly overwrites the entire int_mask
register without acquiring dw->lock.

Could this cause a data race where a concurrent read-modify-write cycle
in dw_edma_v0_core_start() clobbers the mask state configured here,
leaving the interrupt configuration inconsistent?

> +	SET_RW_32(dw, dir, int_clear,
> +		  EDMA_V0_DONE_INT_MASK | EDMA_V0_ABORT_INT_MASK);
> +	SET_RW_32(dw, dir, engine_en, 0);
> +}

[ ... ]

> +static void dw_edma_v0_core_ch_quiesce(struct dw_edma_chan *chan)
> +{
> +	dw_edma_v0_core_dir_off(chan->dw, chan->dir);
> +}

[Severity: High]
Will this globally disable the DMA engine and clobber interrupt states
for all active channels in this direction?

The .ch_quiesce operation is intended as a per-channel action. However,
dw_edma_v0_core_ch_quiesce() wraps dw_edma_v0_core_dir_off(), which
writes 0 to engine_en and masks all interrupts for the entire direction.

If a device has multiple active channels in the same direction, would
releasing one delegated channel halt and silently drop pending interrupts
for all other active channels in that direction?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260620170040.3756043-1-den@valinux.co.jp?part=2

[PATCH v3 03/13] dmaengine: dw-edma: Add delegated channel request helpers

[Koichiro Den]

Endpoint functions that expose endpoint-local DesignWare eDMA channels
to a remote host need to reserve exact hardware channels and hand
interrupt ownership to the remote side before publishing the channels.

Add DW eDMA-specific helpers that request a write/read hardware channel
through DMAengine, keep the hardware-channel filter private to dw-edma,
and switch the selected endpoint-local channel to remote interrupt
routing after the channel has been successfully reserved. The matching
release helper can quiesce the channel while it is still remote-routed,
then restores the channel's default routing before releasing the
DMAengine reservation. This lets callers skip quiesce when unwinding a
reservation that was never exposed to host programming.

Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v3:
  - New patch. Replace the public hardware-channel filter API with
    delegated channel request helpers so the filter stays private to
    dw-edma and delegated IRQ handoff is handled by dw-edma.
  - Hide the hardware-channel filter inside dw-edma instead of exposing
    it through public headers (Frank); add delegated-channel helpers
    instead.
  - Set endpoint-local delegated channels to remote IRQ routing after
    dma_request_channel().
  - Allow delegated-channel release to skip quiesce for reservations
    that were never exposed to host programming.

 drivers/dma/dw-edma/dw-edma-core.c | 81 ++++++++++++++++++++++++++++++
 include/linux/dma/edma.h           | 14 ++++++
 2 files changed, 95 insertions(+)

diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index 7a24248b84e9..ca0504eac1fc 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -1192,6 +1192,87 @@ int dw_edma_remove(struct dw_edma_chip *chip)
 }
 EXPORT_SYMBOL_GPL(dw_edma_remove);
 
+struct dw_edma_delegated_chan_filter {
+	struct device *dma_dev;
+	bool write;
+	u16 id;
+};
+
+static bool dw_edma_delegated_chan_filter(struct dma_chan *dchan, void *param)
+{
+	struct dw_edma_delegated_chan_filter *filter = param;
+	struct dw_edma_chan *chan;
+
+	if (!filter || dchan->device->dev != filter->dma_dev)
+		return false;
+
+	chan = dchan2dw_edma_chan(dchan);
+
+	return chan->dir == (filter->write ? EDMA_DIR_WRITE : EDMA_DIR_READ) &&
+	       chan->id == filter->id;
+}
+
+static int dw_edma_delegate_chan(struct dma_chan *dchan)
+{
+	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
+
+	if (!(chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL))
+		return -EINVAL;
+	if (chan->configured || chan->status != EDMA_ST_IDLE ||
+	    chan->request != EDMA_REQ_NONE)
+		return -EBUSY;
+
+	chan->irq_mode = DW_EDMA_CH_IRQ_REMOTE;
+
+	return 0;
+}
+
+struct dma_chan *dw_edma_request_delegated_chan(struct device *dma_dev,
+						bool write, u16 id)
+{
+	struct dw_edma_delegated_chan_filter filter = {
+		.dma_dev = dma_dev,
+		.write = write,
+		.id = id,
+	};
+	struct dma_chan *dchan;
+	dma_cap_mask_t mask;
+
+	if (!dma_dev)
+		return NULL;
+
+	dma_cap_zero(mask);
+	dma_cap_set(DMA_SLAVE, mask);
+
+	dchan = dma_request_channel(mask, dw_edma_delegated_chan_filter,
+				    &filter);
+	if (!dchan)
+		return NULL;
+
+	if (dw_edma_delegate_chan(dchan)) {
+		dma_release_channel(dchan);
+		return NULL;
+	}
+
+	return dchan;
+}
+EXPORT_SYMBOL_GPL(dw_edma_request_delegated_chan);
+
+void dw_edma_release_delegated_chan(struct dma_chan *dchan, bool quiesce)
+{
+	struct dw_edma_chan *chan;
+
+	if (!dchan)
+		return;
+
+	chan = dchan2dw_edma_chan(dchan);
+	if (quiesce)
+		dw_edma_core_ch_quiesce(chan);
+	chan->irq_mode = dw_edma_get_irq_mode(chan);
+	dma_release_channel(dchan);
+}
+EXPORT_SYMBOL_GPL(dw_edma_release_delegated_chan);
+
 MODULE_LICENSE("GPL v2");
 MODULE_DESCRIPTION("Synopsys DesignWare eDMA controller core driver");
 MODULE_AUTHOR("Gustavo Pimentel <gustavo.pimentel@synopsys.com>");
diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
index c0906221a7c7..0ba8a1143fb2 100644
--- a/include/linux/dma/edma.h
+++ b/include/linux/dma/edma.h
@@ -140,6 +140,9 @@ struct dw_edma_chip {
 #if IS_REACHABLE(CONFIG_DW_EDMA)
 int dw_edma_probe(struct dw_edma_chip *chip);
 int dw_edma_remove(struct dw_edma_chip *chip);
+struct dma_chan *dw_edma_request_delegated_chan(struct device *dma_dev,
+						bool write, u16 id);
+void dw_edma_release_delegated_chan(struct dma_chan *chan, bool quiesce);
 #else
 static inline int dw_edma_probe(struct dw_edma_chip *chip)
 {
@@ -150,6 +153,17 @@ static inline int dw_edma_remove(struct dw_edma_chip *chip)
 {
 	return 0;
 }
+
+static inline struct dma_chan *
+dw_edma_request_delegated_chan(struct device *dma_dev, bool write, u16 id)
+{
+	return NULL;
+}
+
+static inline void dw_edma_release_delegated_chan(struct dma_chan *chan,
+						  bool quiesce)
+{
+}
 #endif /* CONFIG_DW_EDMA */
 
 #endif /* _DW_EDMA_H */
-- 
2.51.0

Re: [PATCH v3 03/13] dmaengine: dw-edma: Add delegated channel request helpers

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [Critical] Delegating a channel to a remote host fails to mask its hardware interrupts locally, resulting in an unrecoverable infinite IRQ storm and permanent IRQ disablement.
- [High] Calling `dw_edma_release_delegated_chan` with `quiesce=true` forcefully disables the entire DMA direction on eDMA v0, disrupting all other active channels.
- [Medium] Lockless modification of shared hardware registers `int_mask`, `int_clear`, and `engine_en` in `dw_edma_v0_core_dir_off()` races with channel start.

Pre-existing issues:
- [High] `dw_edma_device_terminate_all()` accesses and modifies channel state locklessly, causing a TOCTOU data race with the interrupt handler.
--

commit e6b7ee23ebe516f403f0d3278b28713356c19ade
Author: Koichiro Den <den@valinux.co.jp>

dmaengine: dw-edma: Add delegated channel request helpers

Endpoint functions that expose endpoint-local DesignWare eDMA channels
to a remote host need to reserve exact hardware channels and hand
interrupt ownership to the remote side before publishing the channels.
This commit adds helpers to request hardware channels through DMAengine
and switch the interrupt routing after reservation. It also adds a
matching release helper that can optionally quiesce the channel.

> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 7a24248b84e94..ca0504eac1fca 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c

[ ... ]

> @@ -1192,6 +1192,87 @@ int dw_edma_remove(struct dw_edma_chip *chip)
> +static int dw_edma_delegate_chan(struct dma_chan *dchan)
> +{
> +	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
> +
> +	if (!(chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL))
> +		return -EINVAL;
> +	if (chan->configured || chan->status != EDMA_ST_IDLE ||
> +	    chan->request != EDMA_REQ_NONE)
> +		return -EBUSY;
> +
> +	chan->irq_mode = DW_EDMA_CH_IRQ_REMOTE;

[Severity: Critical]
Does setting the software irq_mode to DW_EDMA_CH_IRQ_REMOTE without updating
the hardware interrupt mask leave the local hardware interrupts enabled?

If a channel was previously used locally, dw_edma_v0_core_start() clears its
interrupt mask bit in the hardware int_mask register.

When delegated to a remote host, the remote host programs and starts a
transfer, eventually asserting the DONE or ABORT interrupt line.

The local interrupt handler dw_edma_v0_core_handle_int() checks the channel:

dw_edma_v0_core_handle_int() {
        ...
        if (unlikely(dw_edma_core_ch_ignore_irq(chan)))
                continue;
        ...
}

Because dw_edma_core_ch_ignore_irq() returns true for a remote-routed
channel, the handler skips clearing the interrupt and returns IRQ_NONE.

Can this result in an unrecoverable infinite IRQ storm and permanent IRQ
disablement for all active local DMA channels?

> +
> +	return 0;
> +}

[ ... ]

> +void dw_edma_release_delegated_chan(struct dma_chan *dchan, bool quiesce)
> +{
> +	struct dw_edma_chan *chan;
> +
> +	if (!dchan)
> +		return;
> +
> +	chan = dchan2dw_edma_chan(dchan);
> +	if (quiesce)
> +		dw_edma_core_ch_quiesce(chan);

[Severity: High]
Does calling dw_edma_core_ch_quiesce() here destructively abort all other
active channels in the same direction on eDMA v0 hardware?

When dw_edma_core_ch_quiesce() is called on v0 hardware, it maps to
dw_edma_v0_core_dir_off():

dw_edma_v0_core_ch_quiesce() {
        ...
        dw_edma_v0_core_dir_off(chan->dw, chan->dir);
        ...
}

Which then forcefully zeros out the global engine_en register and masks
interrupts for the entire direction:

dw_edma_v0_core_dir_off() {
        ...
        SET_RW_32(dw, dir, engine_en, 0);
        ...
}

If other independent software clients are actively using different channels
in the same direction, will they be disrupted?

[Severity: Medium]
Is it safe to call dw_edma_core_ch_quiesce() without acquiring the dw->lock
spinlock?

dw_edma_v0_core_dir_off() performs lockless writes to shared direction-wide
registers like int_mask and engine_en:

dw_edma_v0_core_dir_off() {
        ...
        SET_RW_32(dw, dir, int_mask,
                  EDMA_V0_DONE_INT_MASK | EDMA_V0_ABORT_INT_MASK);
        ...
}

Concurrently, another CPU could be running dw_edma_v0_core_start() for a
different channel in the same direction, which performs a read-modify-write
on the exact same int_mask register while holding dw->lock:

dw_edma_v0_core_start() {
        ...
        raw_spin_lock_irqsave(&dw->lock, flags);
        tmp = GET_RW_32(dw, chan->dir, int_mask);
        ...
        SET_RW_32(dw, chan->dir, int_mask, tmp);
        ...
}

Could this data race result in lost updates to the interrupt mask?

> +	chan->irq_mode = dw_edma_get_irq_mode(chan);
> +	dma_release_channel(dchan);
> +}
> +EXPORT_SYMBOL_GPL(dw_edma_release_delegated_chan);

[ ... ]

[Severity: High]
This is a pre-existing issue, but does dw_edma_device_terminate_all()
evaluate and manipulate channel state locklessly?

dw_edma_device_terminate_all() {
        ...
        if (!chan->configured) {
                /* Do nothing */
        } else if (chan->status == EDMA_ST_PAUSE) {
        ...
}

Concurrently, dw_edma_done_interrupt() handles completed transfers and
manipulates the exact same fields while holding the chan->vc.lock:

dw_edma_done_interrupt() {
        ...
        spin_lock_irqsave(&chan->vc.lock, flags);
        vd = vchan_next_desc(&chan->vc);
        if (vd) {
                switch (chan->request) {
        ...
}

Can this lack of synchronization corrupt the DMA channel's state machine,
leading to leaked descriptors or failures to properly abort ongoing
transfers if terminate_all races with the interrupt handler?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260620170040.3756043-1-den@valinux.co.jp?part=3

[PATCH v3 04/13] dmaengine: dw-edma: Initialize IRQ data before requesting IRQs

[Koichiro Den]

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 v3:
  - Reintroduce 20260521142153.2957432-4-den@valinux.co.jp as a
    prerequisite for partial-owned probe, which skips the core_off()
    reset that previously made the early-IRQ window unlikely.

 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 ca0504eac1fc..c782eaa12021 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -943,7 +943,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",
@@ -1024,6 +1023,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) {
@@ -1046,6 +1046,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

Re: [PATCH v3 04/13] dmaengine: dw-edma: Initialize IRQ data before requesting IRQs

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The `dw_edma_device_terminate_all()` function fails to immediately halt the DMA hardware, leading to memory corruption or IOMMU faults when clients subsequently unmap and free DMA memory.
--

commit e17b2c11b8213371ac3e2d7a92dfd8d2c53290ea
Author: Koichiro Den <den@valinux.co.jp>

dmaengine: dw-edma: Initialize IRQ data before requesting IRQs

This patch fixes a potential NULL pointer dereference in the dw-edma driver
by setting the dw back pointer in struct dw_edma_irq before installing
the IRQ handlers. This ensures the pointer is valid if a shared interrupt
triggers immediately after request_irq().

> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index ca0504eac1fca..c782eaa12021e 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -943,7 +943,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;

[Severity: High]
This is a pre-existing issue, but does dw_edma_device_terminate_all() fail
to immediately halt the DMA hardware?

drivers/dma/dw-edma/dw-edma-core.c:dw_edma_device_terminate_all() {
    ...
	} else {
		chan->request = EDMA_REQ_STOP;
	}

	return err;
}

The dmaengine_terminate_sync() API mandates that a DMA channel is
synchronously stopped, or that the driver implements device_synchronize to
wait for completion.

However, dw_edma_device_terminate_all() sets chan->request = EDMA_REQ_STOP
and returns 0 without writing to the hardware to halt the transfer or
implementing device_synchronize.

If the DMA engine client assumes the channel is stopped, might it proceed
to unmap and free the DMA memory while the DMA hardware continues to read
or write to this unmapped physical memory?

Could this lead to use-after-free and memory corruption or IOMMU faults?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260620170040.3756043-1-den@valinux.co.jp?part=4

[PATCH v3 05/13] dmaengine: dw-edma: Add partial channel ownership mode

[Koichiro Den]

A DesignWare eDMA instance may represent only a subset of a controller
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 in
probe() and uses the limited quiesce path in 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.

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 shared
at channel granularity.

Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v3:
  - Allow partial ownership for HDMA native, which has per-channel
    registers.
  - Quiesce represented resources on remove; v2 only skipped core_off(),
    which could leave those channels or directions running.
  - Revise the commit message.

 drivers/dma/dw-edma/dw-edma-core.c | 52 ++++++++++++++++++++++++------
 include/linux/dma/edma.h           |  7 ++++
 2 files changed, 49 insertions(+), 10 deletions(-)

diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index c782eaa12021..d87791205837 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -750,6 +750,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.
@@ -1081,6 +1084,8 @@ 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;
@@ -1092,6 +1097,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;
@@ -1105,13 +1121,25 @@ 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);
+	hw_wr_ch_cnt = min_t(u16, dw_edma_core_ch_count(dw, EDMA_DIR_WRITE),
+			     EDMA_MAX_WR_CH);
+	hw_rd_ch_cnt = min_t(u16, dw_edma_core_ch_count(dw, EDMA_DIR_READ),
+			     EDMA_MAX_RD_CH);
 
-	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);
+	if ((chip->flags & DW_EDMA_CHIP_PARTIAL) &&
+	    (chip->mf == EDMA_MF_EDMA_UNROLL ||
+	     chip->mf == EDMA_MF_HDMA_COMPAT)) {
+		/*
+		 * 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;
+	}
+
+	dw->wr_ch_cnt = min_t(u16, chip->ll_wr_cnt, hw_wr_ch_cnt);
+	dw->rd_ch_cnt = min_t(u16, chip->ll_rd_cnt, hw_rd_ch_cnt);
 
 	if (!dw->wr_ch_cnt && !dw->rd_ch_cnt)
 		return -EINVAL;
@@ -1128,8 +1156,10 @@ 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)) {
+		/* 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);
@@ -1173,8 +1203,10 @@ int dw_edma_remove(struct dw_edma_chip *chip)
 	if (!dw)
 		return -ENODEV;
 
-	/* Disable eDMA */
-	dw_edma_core_off(dw);
+	if (chip->flags & DW_EDMA_CHIP_PARTIAL)
+		dw_edma_core_quiesce(dw);
+	else
+		dw_edma_core_off(dw);
 
 	/* Free irqs */
 	for (i = (dw->nr_irqs - 1); i >= 0; i--)
diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
index 0ba8a1143fb2..3c730c88f0ab 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

Re: [PATCH v3 05/13] dmaengine: dw-edma: Add partial channel ownership mode

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] In partial ownership mode, `dw_edma_probe()` skips the global core reset but fails to quiesce the DMA channels it owns, exposing the driver to fatal race conditions if the hardware is active from a prior state.

Pre-existing issues:
- [High] Teardown order in `dw_edma_remove()` disables hardware and frees IRQs before unregistering the DMA device.
--

commit b3810423aa75ab232c8bec54552ca8cb72acd420
Author: Koichiro Den <den@valinux.co.jp>

dmaengine: dw-edma: Add partial channel ownership mode

This commit adds a partial ownership flag to allow a dw-edma instance to
represent a subset of a controller shared with another OS instance. It skips
the initial core reset during probe and uses a limited quiesce path during
remove, while enforcing ownership granularity for shared registers.

> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index c782eaa12021e..d87791205837b 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
[ ... ]
> @@ -1128,8 +1156,10 @@ 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)) {
> +		/* Disable eDMA only when this instance owns the controller. */
> +		dw_edma_core_off(dw);
> +	}

[Severity: High]
If the instance is in partial ownership mode, could the hardware be
active from a prior state (e.g. kexec or an endpoint driver)?

By skipping the global core reset without calling dw_edma_core_quiesce()
in its place, the hardware remains active. The driver requests IRQs via
dw_edma_irq_request() just below this, before calling
dw_edma_channel_setup().

If a pending interrupt fires during this window, the handler will invoke
dw_edma_done_interrupt(), which locks and dereferences uninitialized
vchan structures:

dw_edma_interrupt_write()
  dw_edma_done_interrupt()
    spin_lock_irqsave(&chan->vc.lock, flags)
    vchan_next_desc()

Since vchan_init() has not been called yet during dw_edma_channel_setup(),
does this sequence lead to a kernel panic or NULL pointer dereference?

>  
>  	/* Request IRQs */
>  	err = dw_edma_irq_request(dw, &wr_alloc, &rd_alloc);
[ ... ]
> @@ -1173,8 +1203,10 @@ int dw_edma_remove(struct dw_edma_chip *chip)
>  	if (!dw)
>  		return -ENODEV;
>  
> -	/* Disable eDMA */
> -	dw_edma_core_off(dw);
> +	if (chip->flags & DW_EDMA_CHIP_PARTIAL)
> +		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 the teardown order in
dw_edma_remove() expose the driver to a race condition?

The hardware is disabled or quiesced and the IRQs are freed before
dma_async_device_unregister() is called. By leaving the dmaengine device
registered during this teardown, clients can still issue new transfers via
vchan_issue_pending().

This would invoke dw_edma_v0_core_start(), writing to registers of a
disabled DMA engine, potentially racing with dw_edma_core_quiesce().
Additionally, transfers submitted after free_irq() will hang indefinitely
since completion interrupts are no longer available.

Should dma_async_device_unregister() be called before tearing down the
underlying hardware and interrupts?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260620170040.3756043-1-den@valinux.co.jp?part=5

[PATCH v3 06/13] dmaengine: dw-edma-pcie: Track non-LL mode in DMA data

[Koichiro Den]

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 v3:
  - No changes since v2.

 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 70ea031147d1..0ea8d59782b4 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,
@@ -422,7 +422,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
 	chip->irq_mode = DW_EDMA_CH_IRQ_REMOTE;
 	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;
@@ -431,7 +431,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];
@@ -458,7 +458,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

Re: [PATCH v3 06/13] dmaengine: dw-edma-pcie: Track non-LL mode in DMA data

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Calling pci_free_irq_vectors() in remove() violates API guidelines for devices enabled with pcim_enable_device() and leads to a double free.
--

commit 4807e012ff4fafc258bc7806f34cdce5751bf7bf
Author: Koichiro Den <den@valinux.co.jp>
Subject: dmaengine: dw-edma-pcie: Track non-LL mode in DMA data

The dw-edma-pcie driver copies static template data into a mutable
dw_edma_pcie_data instance before applying capability-derived updates. This
commit keeps the derived non-LL mode in that copy as well.

> diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
> index 70ea031147d18..0ea8d59782b44 100644
> --- a/drivers/dma/dw-edma/dw-edma-pcie.c
> +++ b/drivers/dma/dw-edma/dw-edma-pcie.c

[ ... ]

> @@ -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;

This is a pre-existing issue, but I noticed a potential problem in how this
driver handles IRQ cleanup.

[Severity: High]
Does dw_edma_pcie_remove() result in a double-free of the IRQ vectors?

This driver calls pcim_enable_device() in dw_edma_pcie_probe(), which marks
the PCI device as managed. Later in the probe function,
pci_alloc_irq_vectors() is called, which automatically registers a devres
action to free the vectors on device detach.

However, dw_edma_pcie_remove() manually calls pci_free_irq_vectors():

drivers/dma/dw-edma/dw-edma-pcie.c:dw_edma_pcie_remove() {
	...
	/* Freeing IRQs */
	pci_free_irq_vectors(pdev);
}

Since the devres action remains registered, will it attempt to free the IRQs
again, causing a double-free on driver unload?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260620170040.3756043-1-den@valinux.co.jp?part=6

[PATCH v3 07/13] dmaengine: dw-edma-pcie: Add capability match data

[Koichiro Den]

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 v3:
  - Adjust context for the AMD (Xilinx) CPM6 match added in the new
    base; carry the same match-data conversion over that entry and
    update the commit message accordingly.
  - Reject dynamic PCI IDs without match data before dereferencing the
    match data.

 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 0ea8d59782b4..c08a77c0e508 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);
@@ -442,8 +478,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;
 
@@ -452,8 +488,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;
 	}
@@ -469,8 +505,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;
 
@@ -479,8 +515,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;
 	}
@@ -561,12 +597,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

Re: [PATCH v3 07/13] dmaengine: dw-edma-pcie: Add capability match data

[sashiko-bot]

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 dw_edma_pcie_probe()
--

commit abe1646ec71967f21d88dc1513ac61dd2092e6d7
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, splitting it from static template assignment. No functional change
is intended for the existing Synopsys and AMD matches.

> diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
> index 0ea8d59782b44..c08a77c0e5085 100644
> --- a/drivers/dma/dw-edma/dw-edma-pcie.c
> +++ b/drivers/dma/dw-edma/dw-edma-pcie.c
[ ... ]
> @@ -469,8 +505,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);

[Severity: High]
This is a pre-existing issue, but does dw_edma_pcie_probe() leak IRQ
vectors allocated by pci_alloc_irq_vectors() when it returns directly
on error paths?

Looking at dw_edma_pcie_probe(), after pci_alloc_irq_vectors() succeeds,
there are several error paths that return without calling
pci_free_irq_vectors(pdev).

For example, if mapped BAR addresses are NULL when checking chip->reg_base,
or when returning -ENOMEM inside the region mapping loop shown above:

    chip->reg_base = pcim_iomap_table(pdev)[vsec_data->rg.bar];
    if (!chip->reg_base)
        return -ENOMEM;

Since system IRQ capacity is limited, could repeated probe failures on
these paths exhaust system-wide IRQ vectors and prevent other devices
from functioning?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260620170040.3756043-1-den@valinux.co.jp?part=7

[PATCH v3 08/13] dmaengine: dw-edma-pcie: Rename vsec_data to dma_data

[Koichiro Den]

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 v3:
  - Keep dma_data allocation before pcim_enable_device() (Frank).

 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 c08a77c0e508..5249324ad6bf 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,24 +452,24 @@ 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->irq_mode = DW_EDMA_CH_IRQ_REMOTE;
 	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)
@@ -479,7 +477,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;
 
@@ -489,16 +487,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)
@@ -506,7 +504,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;
 
@@ -516,7 +514,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;
 	}
@@ -534,31 +532,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

Re: [PATCH v3 08/13] dmaengine: dw-edma-pcie: Rename vsec_data to dma_data

[sashiko-bot]

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 dw_edma_pcie_probe().
--

commit de019354672cdd91d71c2d84fb752c9367dce2c3
Author: Koichiro Den <den@valinux.co.jp>

dmaengine: dw-edma-pcie: Rename vsec_data to dma_data

This commit renames the local variable vsec_data to dma_data in
dw_edma_pcie_probe() to reflect that DMA layout data can now be obtained
from device-specific capabilities, not just VSEC.

> 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

[ ... ]

> @@ -454,24 +452,24 @@ 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->irq_mode = DW_EDMA_CH_IRQ_REMOTE;
>  	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;

[Severity: High]
This is a pre-existing issue, but does this error path leak the IRQ
vectors allocated earlier in the function?

The function dw_edma_pcie_probe() successfully allocates IRQ vectors using
pci_alloc_irq_vectors() before reaching this point, but returning directly
here leaves those resources stranded. 

Are there other error paths further down in this function, such as when
pci_dev_msi_enabled() fails or dw_edma_probe() returns an error, that
might also need a pci_free_irq_vectors() cleanup before returning?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260620170040.3756043-1-den@valinux.co.jp?part=8

[PATCH v3 09/13] dmaengine: dw-edma-pcie: Add platform ops to match data

[Koichiro Den]

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 v3:
  - Adjust context for the AMD (Xilinx) CPM6 match added in the new
    base; keep it on the same dw_edma_pcie_plat_ops as the existing
    matches.

 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 5249324ad6bf..96038aaca079 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);
@@ -455,7 +456,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
 	chip->mf = dma_data->mf;
 	chip->irq_mode = DW_EDMA_CH_IRQ_REMOTE;
 	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;
@@ -597,17 +598,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

Re: [PATCH v3 09/13] dmaengine: dw-edma-pcie: Add platform ops to match data

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Double free of PCI IRQ vectors on device removal.
--

commit ea9357c364d1768529dfa87f1f7bc66276215c93
Author: Koichiro Den <den@valinux.co.jp>

dmaengine: dw-edma-pcie: Add platform ops to match data

This commit moves the platform operations pointer into the match data
structure. It updates existing matches to use the standard platform ops
pointer without introducing any functional changes.

> diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
> index 5249324ad6bff..96038aaca0791 100644
> --- a/drivers/dma/dw-edma/dw-edma-pcie.c
> +++ b/drivers/dma/dw-edma/dw-edma-pcie.c
[ ... ]
> @@ -597,17 +598,20 @@ static void dw_edma_pcie_remove(struct pci_dev *pdev)

[Severity: High]
This isn't a bug introduced by this patch, but looking at
dw_edma_pcie_remove(), is there a double-free risk with the IRQ vectors?

In dw_edma_pcie_probe(), the device is enabled using pcim_enable_device(),
which means IRQ vectors allocated by pci_alloc_irq_vectors() are
automatically managed and freed by devres during device teardown.

However, dw_edma_pcie_remove() explicitly frees them again:

static void dw_edma_pcie_remove(struct pci_dev *pdev)
{
        struct dw_edma_chip *chip = pci_get_drvdata(pdev);
        ...
        /* Freeing IRQs */
        pci_free_irq_vectors(pdev);
}

Does this manual call to pci_free_irq_vectors() on a devres-managed device
lead to a double-free when pcim_msi_release() is later invoked?

>  
>  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,
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260620170040.3756043-1-den@valinux.co.jp?part=9

[PATCH v3 10/13] dmaengine: dw-edma-pcie: Add register offset match flag

[Koichiro Den]

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.

No functional change intended.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v3:
  - Update commit message to describe the AMD (Xilinx) CPM6 match
    present in the new base.

 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 96038aaca079..caf7c05b0631 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 */
@@ -465,6 +466,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

Re: [PATCH v3 10/13] dmaengine: dw-edma-pcie: Add register offset match flag

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The driver adds statically defined offsets to PCI BAR mapping pointers without validating them against the size of the mapped BAR, enabling out-of-bounds MMIO accesses.
--

commit ba4f6509ffb3ea527d21eedc70cede2c4bd0c13c
Author: Koichiro Den <den@valinux.co.jp>

dmaengine: dw-edma-pcie: Add register offset match flag

This commit adds a match-data flag for devices whose DMA register block
starts at an offset inside the mapped BAR. It applies this offset to the
register base during the PCIe probe.

> 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
[ ... ]
> @@ -465,6 +466,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;

[Severity: High]
This is a pre-existing issue, but since the new code adds another offset
here, could a malicious or malfunctioning PCIe device cause out-of-bounds
MMIO accesses by reporting an artificially small BAR size?

The driver maps the device BARs using pcim_iomap_regions(), which maps
exactly the size of the BAR as reported by the device during enumeration.
It then calculates pointers by adding statically defined offsets to the
mapped base address in dw_edma_pcie_probe() without bounds checks:

drivers/dma/dw-edma/dw-edma-pcie.c:dw_edma_pcie_probe() {
    ...
    ll_region->vaddr.io += ll_block->off;
    ...
    dt_region->vaddr.io += dt_block->off;
    ...
}

Does the driver need to verify that the mapped region is large enough to
contain the hardcoded offsets and sizes before adding them, to prevent
potential kernel panics or memory corruption when the DMA engine accesses
these pointers?

>  
>  	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];

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260620170040.3756043-1-den@valinux.co.jp?part=10

[PATCH v3 11/13] dmaengine: dw-edma-pcie: Factor out descriptor block address lookup

[Koichiro Den]

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 v3:
  - Update commit message and describe the AMD (Xilinx) CPM6 match
    present in the new base.

 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 caf7c05b0631..62740c8c3f93 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)
 {
@@ -480,9 +494,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];
@@ -490,9 +503,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;
 	}
 
@@ -507,9 +519,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];
@@ -517,9 +528,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

[PATCH v3 12/13] dmaengine: dw-edma-pcie: Handle optional data blocks

[Koichiro Den]

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 v3:
  - Update commit message to describe the AMD (Xilinx) CPM6 match
    present in the new base.

 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 62740c8c3f93..622ec974a521 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) {
@@ -498,6 +500,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;
@@ -523,6 +528,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;
@@ -556,10 +564,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++) {
@@ -568,10 +580,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

Re: [PATCH v3 12/13] dmaengine: dw-edma-pcie: Handle optional data blocks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing pci_free_irq_vectors() in error paths after successful pci_alloc_irq_vectors() causes IRQ resource leaks.
--

commit 4c520b14ae411615c26fba953ffdb3c7f374c19a
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 prepares the driver for future metadata
providers where a zero-sized data block indicates it is not present.

> diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
> index 62740c8c3f939..622ec974a521e 100644
> --- a/drivers/dma/dw-edma/dw-edma-pcie.c
> +++ b/drivers/dma/dw-edma/dw-edma-pcie.c
[ ... ]
> @@ -498,6 +500,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 isn't a bug introduced by this patch, but does this error path (and
several others in this function) leak IRQ vectors?

Earlier in dw_edma_pcie_probe(), pci_alloc_irq_vectors() is called to allocate
the IRQ vectors. However, multiple subsequent error paths, such as this return
when pcim_iomap_table(pdev) returns NULL, return an error directly without
cleaning up the allocated IRQ vectors.

There are also similar error paths later in the function that fail to call
pci_free_irq_vectors(), such as:

	if (!pci_dev_msi_enabled(pdev)) {
		pci_err(pdev, "enable interrupt failed\n");
		return -EPERM;
	}

and:

	err = dw_edma_probe(chip);
	if (err) {
		pci_err(pdev, "eDMA probe failed\n");
		return err;
	}

Since the driver manually frees these vectors in dw_edma_pcie_remove(), should
the probe function also perform its own cleanup on failure to prevent IRQ
resource exhaustion?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260620170040.3756043-1-den@valinux.co.jp?part=12

[PATCH v3 13/13] dmaengine: dw-edma-pcie: Add chip flags to match data

[Koichiro Den]

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 v3:
  - No changes since v2.

 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 622ec974a521..1e75fefae9b8 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->irq_mode = DW_EDMA_CH_IRQ_REMOTE;
 	chip->nr_irqs = nr_irqs;
 	chip->ops = match->plat_ops;
-- 
2.51.0

Re: [PATCH v3 13/13] dmaengine: dw-edma-pcie: Add chip flags to match data

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing pci_free_irq_vectors() on error paths after successful pci_alloc_irq_vectors() allocation.
--

commit 47790820d57ed923e44318e08b431eee242c2796
Author: Koichiro Den <den@valinux.co.jp>

dmaengine: dw-edma-pcie: Add chip flags to match data

This commit allows PCI ID match data to pass dw_edma_chip flags into
dw_edma_probe(). It keeps per-device policy in the match data instead of
open-coding it in the probe function.

> 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

[ ... ]

> @@ -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->irq_mode = DW_EDMA_CH_IRQ_REMOTE;
>  	chip->nr_irqs = nr_irqs;
>  	chip->ops = match->plat_ops;

[Severity: High]
This is a pre-existing issue, but does dw_edma_pcie_probe() leak IRQ
resources on error paths?

Earlier in dw_edma_pcie_probe(), IRQ vectors are successfully allocated:

    nr_irqs = pci_alloc_irq_vectors(pdev, 1, dma_data->irqs,
                                    PCI_IRQ_MSI | PCI_IRQ_MSIX);

However, multiple subsequent error paths return directly without calling
pci_free_irq_vectors(), for instance when mapping fails:

    chip->reg_base = pcim_iomap_table(pdev)[dma_data->rg.bar];
    if (!chip->reg_base)
            return -ENOMEM;

Another instance occurs further down if dw_edma_probe() fails, returning
the error without freeing the vectors. 

Should these error paths be updated to call pci_free_irq_vectors() before
returning to prevent IRQ resource exhaustion?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260620170040.3756043-1-den@valinux.co.jp?part=13