* [PATCH v8] dmaengine: dw-edma: Enable HDMA 64R/W Channels
@ 2026-07-13 6:48 Devendra K Verma
2026-07-13 7:07 ` sashiko-bot
0 siblings, 1 reply; 2+ messages in thread
From: Devendra K Verma @ 2026-07-13 6:48 UTC (permalink / raw)
To: bhelgaas, mani, vkoul, Frank.Li
Cc: dmaengine, linux-kernel, michal.simek, devendra.verma
From: Devendra K Verma <devendra.verma@amd.com>
As per 'Designware Cores PCI Express Controller Databook',
Section 7.1 - Overview, HDMA supports 64 Read and 64 Write
channels. Current controller driver supports up to 8 read and
write channels only. In order to utilize all the channels the
controller driver need to have the channel related structs
and variables as per the number of channels supported by IP.
Following changes are made to enable 64 Read / 64 Write
channel support:
o Defined HDMA specific macros to reflect the channel count.
o The count of ll_regions and dt_regions in dw_edma_chip and
dw_edma_pcie_data shall be in accordance to number of read
and write channels.
o In dw_edma_probe() configure the channels as per the channels
of the IP used.
o Changed mask types to u64 for higher channel counts.
Signed-off-by: Devendra K Verma <devendra.verma@amd.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
Changes in v7:
o Added Reviewed-by Tag
Changes in v6:
o In declaring bitmap variables wr/rd_mask, replaced constants
with the macros defined for max channel count.
Changes in v5:
o Changed the {wr,rd}_mask type to BITMAP type for eDMA/HDMA
as per the review comment.
o Changed the 'mask' var type to pointer to ul.
Changes in v4:
o Changed 'mask' variable to a bitmap type as per the
review comment.
Changes in v3:
o Reverted the FIX for AI reported GET_CH_32() issue, as
per the recommendation of reviewers, need to create
separate patch for it.
Changes in v2:
o Fixed the pre-existing bug related to GET_CH_32
interchanging the channel direction and id.
This bug was not caused by any version of this patch.
o Fixed the issue when using for_each_set_bit() for mask
of u64 type.
Changes in v1:
o On review recommendation of sashiko bot, in the function
dw_hdma_v0_core_off(), the loop iterates over registers
as per the number of channels enabled and not on total
number of channels supported.
o Changed mask types to u64 for higher channel counts.
---
drivers/dma/dw-edma/dw-edma-core.c | 19 +++++++++++++------
drivers/dma/dw-edma/dw-edma-core.h | 5 +++--
drivers/dma/dw-edma/dw-edma-pcie.c | 8 ++++----
drivers/dma/dw-edma/dw-edma-v0-core.c | 6 +++---
drivers/dma/dw-edma/dw-hdma-v0-core.c | 27 +++++++++++++++++++--------
drivers/dma/dw-edma/dw-hdma-v0-regs.h | 2 +-
include/linux/dma/edma.h | 10 ++++++----
7 files changed, 49 insertions(+), 28 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index c2feb3adc79f..0eb24e707c9c 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -925,9 +925,9 @@ static int dw_edma_channel_setup(struct dw_edma *dw, u32 wr_alloc, u32 rd_alloc)
irq = &dw->irq[pos];
if (chan->dir == EDMA_DIR_WRITE)
- irq->wr_mask |= BIT(chan->id);
+ bitmap_set(irq->wr_mask, chan->id, 1);
else
- irq->rd_mask |= BIT(chan->id);
+ bitmap_set(irq->rd_mask, chan->id, 1);
irq->dw = dw;
memcpy(&chan->msi, &irq->msi, sizeof(chan->msi));
@@ -1079,6 +1079,8 @@ int dw_edma_probe(struct dw_edma_chip *chip)
struct dw_edma *dw;
u32 wr_alloc = 0;
u32 rd_alloc = 0;
+ u16 max_wr_cnt;
+ u16 max_rd_cnt;
int i, err;
if (!chip)
@@ -1094,20 +1096,25 @@ int dw_edma_probe(struct dw_edma_chip *chip)
dw->chip = chip;
- if (dw->chip->mf == EDMA_MF_HDMA_NATIVE)
+ if (dw->chip->mf == EDMA_MF_HDMA_NATIVE) {
dw_hdma_v0_core_register(dw);
- else
+ max_wr_cnt = HDMA_MAX_WR_CH;
+ max_rd_cnt = HDMA_MAX_RD_CH;
+ } else {
dw_edma_v0_core_register(dw);
+ max_wr_cnt = EDMA_MAX_WR_CH;
+ max_rd_cnt = EDMA_MAX_RD_CH;
+ }
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);
+ dw->wr_ch_cnt = min_t(u16, dw->wr_ch_cnt, max_wr_cnt);
dw->rd_ch_cnt = min_t(u16, chip->ll_rd_cnt,
dw_edma_core_ch_count(dw, EDMA_DIR_READ));
- dw->rd_ch_cnt = min_t(u16, dw->rd_ch_cnt, EDMA_MAX_RD_CH);
+ dw->rd_ch_cnt = min_t(u16, dw->rd_ch_cnt, max_rd_cnt);
if (!dw->wr_ch_cnt && !dw->rd_ch_cnt)
return -EINVAL;
diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
index 902574b1ba86..25a6e8a958ad 100644
--- a/drivers/dma/dw-edma/dw-edma-core.h
+++ b/drivers/dma/dw-edma/dw-edma-core.h
@@ -91,9 +91,10 @@ struct dw_edma_chan {
struct dw_edma_irq {
struct msi_msg msi;
- u32 wr_mask;
- u32 rd_mask;
struct dw_edma *dw;
+
+ DECLARE_BITMAP(wr_mask, HDMA_MAX_WR_CH);
+ DECLARE_BITMAP(rd_mask, HDMA_MAX_RD_CH);
};
struct dw_edma {
diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
index 0b30ce138503..79f653da8e0f 100644
--- a/drivers/dma/dw-edma/dw-edma-pcie.c
+++ b/drivers/dma/dw-edma/dw-edma-pcie.c
@@ -61,11 +61,11 @@ struct dw_edma_pcie_data {
/* eDMA registers location */
struct dw_edma_block rg;
/* eDMA memory linked list location */
- struct dw_edma_block ll_wr[EDMA_MAX_WR_CH];
- struct dw_edma_block ll_rd[EDMA_MAX_RD_CH];
+ struct dw_edma_block ll_wr[HDMA_MAX_WR_CH];
+ struct dw_edma_block ll_rd[HDMA_MAX_RD_CH];
/* eDMA memory data location */
- struct dw_edma_block dt_wr[EDMA_MAX_WR_CH];
- struct dw_edma_block dt_rd[EDMA_MAX_RD_CH];
+ struct dw_edma_block dt_wr[HDMA_MAX_WR_CH];
+ struct dw_edma_block dt_rd[HDMA_MAX_RD_CH];
/* Other */
enum dw_edma_map_format mf;
u8 irqs;
diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
index 69e8279adec8..3f4e82516d92 100644
--- a/drivers/dma/dw-edma/dw-edma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
@@ -239,7 +239,7 @@ dw_edma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir,
irqreturn_t ret = IRQ_NONE;
struct dw_edma_chan *chan;
unsigned long off;
- u32 mask;
+ unsigned long *mask;
if (dir == EDMA_DIR_WRITE) {
total = dw->wr_ch_cnt;
@@ -252,7 +252,7 @@ dw_edma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir,
}
val = dw_edma_v0_core_status_done_int(dw, dir);
- val &= mask;
+ val &= *mask;
for_each_set_bit(pos, &val, total) {
chan = &dw->chan[pos + off];
@@ -263,7 +263,7 @@ dw_edma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir,
}
val = dw_edma_v0_core_status_abort_int(dw, dir);
- val &= mask;
+ val &= *mask;
for_each_set_bit(pos, &val, total) {
chan = &dw->chan[pos + off];
diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
index 632abb8b481c..0181bd276e22 100644
--- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
@@ -53,13 +53,24 @@ __dw_ch_regs(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch)
static void dw_hdma_v0_core_off(struct dw_edma *dw)
{
int id;
+ enum dw_edma_dir dir;
+
+ dir = EDMA_DIR_WRITE;
+ for (id = 0; id < dw->wr_ch_cnt; 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);
+ }
- for (id = 0; id < HDMA_V0_MAX_NR_CH; id++) {
- SET_BOTH_CH_32(dw, id, int_setup,
- HDMA_V0_STOP_INT_MASK | HDMA_V0_ABORT_INT_MASK);
- SET_BOTH_CH_32(dw, id, int_clear,
- HDMA_V0_STOP_INT_MASK | HDMA_V0_ABORT_INT_MASK);
- SET_BOTH_CH_32(dw, id, ch_en, 0);
+ dir = EDMA_DIR_READ;
+ for (id = 0; id < dw->rd_ch_cnt; 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);
}
}
@@ -118,7 +129,7 @@ dw_hdma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir,
unsigned long total, pos, val;
irqreturn_t ret = IRQ_NONE;
struct dw_edma_chan *chan;
- unsigned long off, mask;
+ unsigned long off, *mask;
if (dir == EDMA_DIR_WRITE) {
total = dw->wr_ch_cnt;
@@ -130,7 +141,7 @@ dw_hdma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir,
mask = dw_irq->rd_mask;
}
- for_each_set_bit(pos, &mask, total) {
+ for_each_set_bit(pos, mask, total) {
chan = &dw->chan[pos + off];
val = dw_hdma_v0_core_status_int(chan);
diff --git a/drivers/dma/dw-edma/dw-hdma-v0-regs.h b/drivers/dma/dw-edma/dw-hdma-v0-regs.h
index 7759ba9b4850..48e40efceb2e 100644
--- a/drivers/dma/dw-edma/dw-hdma-v0-regs.h
+++ b/drivers/dma/dw-edma/dw-hdma-v0-regs.h
@@ -11,7 +11,7 @@
#include <linux/dmaengine.h>
-#define HDMA_V0_MAX_NR_CH 8
+#define HDMA_V0_MAX_NR_CH 64
#define HDMA_V0_CH_EN BIT(0)
#define HDMA_V0_LOCAL_ABORT_INT_EN BIT(6)
#define HDMA_V0_REMOTE_ABORT_INT_EN BIT(5)
diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
index 1fafd5b0e315..da7a5cc93ad4 100644
--- a/include/linux/dma/edma.h
+++ b/include/linux/dma/edma.h
@@ -14,6 +14,8 @@
#define EDMA_MAX_WR_CH 8
#define EDMA_MAX_RD_CH 8
+#define HDMA_MAX_WR_CH 64
+#define HDMA_MAX_RD_CH 64
struct dw_edma;
@@ -89,12 +91,12 @@ struct dw_edma_chip {
u16 ll_wr_cnt;
u16 ll_rd_cnt;
/* link list address */
- struct dw_edma_region ll_region_wr[EDMA_MAX_WR_CH];
- struct dw_edma_region ll_region_rd[EDMA_MAX_RD_CH];
+ struct dw_edma_region ll_region_wr[HDMA_MAX_WR_CH];
+ struct dw_edma_region ll_region_rd[HDMA_MAX_RD_CH];
/* data region */
- struct dw_edma_region dt_region_wr[EDMA_MAX_WR_CH];
- struct dw_edma_region dt_region_rd[EDMA_MAX_RD_CH];
+ struct dw_edma_region dt_region_wr[HDMA_MAX_WR_CH];
+ struct dw_edma_region dt_region_rd[HDMA_MAX_RD_CH];
/* interrupt emulation */
int db_irq;
--
2.43.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v8] dmaengine: dw-edma: Enable HDMA 64R/W Channels
2026-07-13 6:48 [PATCH v8] dmaengine: dw-edma: Enable HDMA 64R/W Channels Devendra K Verma
@ 2026-07-13 7:07 ` sashiko-bot
0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-07-13 7:07 UTC (permalink / raw)
To: Devendra K Verma; +Cc: Frank.Li, dmaengine, vkoul
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [High] The DesignWare PCIe controller frontend fails initialization for platforms attempting to use 64 HDMA channels due to hardcoded legacy channel limits.
- [Medium] The dw-edma-pcie endpoint driver silently caps dynamically detected channel counts to 8.
Pre-existing issues:
- [High] A race condition during initialization causes a guaranteed NULL pointer dereference if a shared or spurious interrupt occurs early.
- [High] Incorrect argument order in dw_hdma_v0_core_ch_status leads to invalid register reads, breaking DMA status reporting.
--
--- Patch [1]: [PATCH v8] dmaengine: dw-edma: Enable HDMA 64R/W Channels ---
commit 639271379db4829feeac5972fb5074c44780d440
Author: Devendra K Verma <devendra.verma@amd.com>
dmaengine: dw-edma: Enable HDMA 64R/W Channels
This patch introduces support for up to 64 read and write channels in the
DesignWare HDMA controller. It defines new macros for HDMA maximum channel
counts, updates data structures, and transitions bitmasks to 64-bit bitmaps
to accommodate the expanded channel limits.
> Changes in v3:
> o Reverted the FIX for AI reported GET_CH_32() issue, as
> per the recommendation of reviewers, need to create
> separate patch for it.
[Severity: High]
This isn't a bug introduced by this patch, but does the incorrect argument
order in dw_hdma_v0_core_ch_status() still lead to invalid register reads?
When the DMA status is checked, dw_hdma_v0_core_ch_status() calls GET_CH_32()
with the channel ID and direction swapped:
tmp = FIELD_GET(HDMA_V0_CH_STATUS_MASK,
GET_CH_32(dw, chan->id, chan->dir, ch_stat));
The macro evaluates the numeric channel ID as the direction and uses the
direction as the channel index, which breaks DMA status reporting.
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
[ ... ]
> @@ -925,9 +925,9 @@ static int dw_edma_channel_setup(struct dw_edma *dw, u32 wr_alloc, u32 rd_alloc)
> irq = &dw->irq[pos];
>
> if (chan->dir == EDMA_DIR_WRITE)
> - irq->wr_mask |= BIT(chan->id);
> + bitmap_set(irq->wr_mask, chan->id, 1);
> else
> - irq->rd_mask |= BIT(chan->id);
> + bitmap_set(irq->rd_mask, chan->id, 1);
>
> irq->dw = dw;
[Severity: High]
This is a pre-existing issue, but does this initialization sequence leave a
race condition that causes a NULL pointer dereference if a shared interrupt
fires early?
In dw_edma_probe(), dw_edma_irq_request() calls request_irq() with IRQF_SHARED
while dw->irq[i].dw is still NULL.
If an interrupt fires immediately after registration,
dw_edma_interrupt_common() executes and dereferences dw_irq->dw to call
dw->core->handle_int. This will dereference the NULL pointer before
dw_edma_channel_setup() finishes initialization here.
> 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
> @@ -62,11 +62,11 @@ struct dw_edma_pcie_data {
> /* eDMA registers location */
> struct dw_edma_block rg;
> /* eDMA memory linked list location */
> - struct dw_edma_block ll_wr[EDMA_MAX_WR_CH];
> - struct dw_edma_block ll_rd[EDMA_MAX_RD_CH];
> + struct dw_edma_block ll_wr[HDMA_MAX_WR_CH];
> + struct dw_edma_block ll_rd[HDMA_MAX_RD_CH];
[Severity: Medium]
Does the dw-edma-pcie endpoint driver silently cap dynamically detected
channel counts to 8, defeating the purpose of the 64-channel support?
During probe, dw_edma_pcie_get_xilinx_dma_data() reads the channel count from
the hardware VSEC but caps it using the static template limit:
pdata->wr_ch_cnt = min(pdata->wr_ch_cnt,
FIELD_GET(DW_PCIE_XILINX_MDB_VSEC_DMA_WR_CH, val));
Since pdata is initialized from xilinx_mdb_data which statically defines a
maximum of 8 channels, the dynamically read channel count is bounded and
will never exceed 8.
> diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
> --- a/include/linux/dma/edma.h
> +++ b/include/linux/dma/edma.h
> @@ -14,6 +14,8 @@
>
> #define EDMA_MAX_WR_CH 8
> #define EDMA_MAX_RD_CH 8
> +#define HDMA_MAX_WR_CH 64
> +#define HDMA_MAX_RD_CH 64
[Severity: High]
Will the DesignWare PCIe controller frontend fail initialization with -EINVAL
for platforms attempting to use 64 HDMA channels?
When an HDMA platform is probed, dw_pcie_edma_find_channels() in
drivers/pci/controller/dwc/pcie-designware.c still uses the legacy
EDMA_MAX_WR_CH limit:
if (!pci->edma.ll_wr_cnt || pci->edma.ll_wr_cnt > EDMA_MAX_WR_CH ||
!pci->edma.ll_rd_cnt || pci->edma.ll_rd_cnt > EDMA_MAX_RD_CH)
return -EINVAL;
Additionally, dw_pcie_edma_irq_vector() rejects requested IRQ numbers
that are greater than the legacy maximums:
if (nr >= EDMA_MAX_WR_CH + EDMA_MAX_RD_CH)
return -EINVAL;
This artificial cap at 16 seems insufficient for platforms using the new
64-channel support.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260713064854.4065262-1-devverma@amd.com?part=1
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-07-13 7:07 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-13 6:48 [PATCH v8] dmaengine: dw-edma: Enable HDMA 64R/W Channels Devendra K Verma
2026-07-13 7:07 ` sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox