* [PATCH v7] dmaengine: dw-edma: Enable HDMA 64R/W Channels
@ 2026-07-08 13:43 Devendra K Verma
2026-07-08 13:59 ` sashiko-bot
2026-07-08 15:50 ` Frank Li
0 siblings, 2 replies; 4+ messages in thread
From: Devendra K Verma @ 2026-07-08 13:43 UTC (permalink / raw)
To: bhelgaas, mani, vkoul, Frank.Li
Cc: dmaengine, linux-kernel, michal.simek, devendra.verma
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>
---
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] 4+ messages in thread
* Re: [PATCH v7] dmaengine: dw-edma: Enable HDMA 64R/W Channels
2026-07-08 13:43 [PATCH v7] dmaengine: dw-edma: Enable HDMA 64R/W Channels Devendra K Verma
@ 2026-07-08 13:59 ` sashiko-bot
2026-07-13 6:46 ` Verma, Devendra
2026-07-08 15:50 ` Frank Li
1 sibling, 1 reply; 4+ messages in thread
From: sashiko-bot @ 2026-07-08 13:59 UTC (permalink / raw)
To: Devendra K Verma; +Cc: vkoul, dmaengine, Frank.Li
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [High] Designware PCIe core bounds checks reject HDMA configurations with more than 8 channels.
- [High] Standalone PCIe eDMA driver statically limits hardware to 8 channels.
Pre-existing issues:
- [Critical] Missing register offset when setting the eDMA/HDMA base register pointer.
- [High] Incorrect arguments passed to GET_CH_32 macro causing wrong channel registers to be accessed.
--
--- Patch [1]: [PATCH v7] dmaengine: dw-edma: Enable HDMA 64R/W Channels ---
commit 4834a312ef82b7f9b290aedb3a1f566db2548955
Author: Devendra K Verma <devendra.verma@amd.com>
dmaengine: dw-edma: Enable HDMA 64R/W Channels
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:
> diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
> index 1fafd5b0e3153..da7a5cc93ad43 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
[Severity: High]
This patch introduces 64-channel support for HDMA, but does this break
when used with the Designware PCIe host driver?
Looking at drivers/pci/controller/dwc/pcie-designware.c:dw_pcie_edma_find_channels():
/* Sanity check the channels count if the mapping was incorrect */
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;
And in dw_pcie_edma_irq_vector():
if (nr >= EDMA_MAX_WR_CH + EDMA_MAX_RD_CH)
return -EINVAL;
Will these sanity checks reject HDMA configurations with more than 8 channels
because they strictly enforce the EDMA_MAX_WR_CH (8) limit?
[Severity: High]
Does the standalone PCIe eDMA driver statically limit the hardware to
8 channels?
Looking at drivers/dma/dw-edma/dw-edma-pcie.c:
static const struct dw_edma_pcie_data xilinx_mdb_data = {
...
.wr_ch_cnt = 8,
.rd_ch_cnt = 8,
};
And later in dw_edma_pcie_get_xilinx_dma_data():
pdata->wr_ch_cnt = min(pdata->wr_ch_cnt,
FIELD_GET(DW_PCIE_XILINX_MDB_VSEC_DMA_WR_CH, val));
Doesn't the min() operation cap the dynamically detected hardware channel
count to the static limit of 8, negating the new 64-channel HDMA support
on Xilinx MDB controllers?
[Severity: Critical]
This is a pre-existing issue, but is there a missing register offset when
setting the eDMA/HDMA base register pointer?
In drivers/dma/dw-edma/dw-edma-pcie.c:dw_edma_pcie_probe():
chip->reg_base = pcim_iomap_table(pdev)[vsec_data->rg.bar];
if (!chip->reg_base)
return -ENOMEM;
Should this map the whole PCIe BAR but also add vsec_data->rg.off to
chip->reg_base? Because all register accesses via __dw_regs(dw) directly
dereference chip->reg_base, will all reads and writes hit the beginning
of the BAR instead of the actual hardware registers on hardware where
rg.off is non-zero?
[Severity: High]
This is a pre-existing issue, but are the arguments to GET_CH_32
swapped here?
In drivers/dma/dw-edma/dw-hdma-v0-core.c:dw_hdma_v0_core_ch_status():
static enum dma_status dw_hdma_v0_core_ch_status(struct dw_edma_chan *chan)
{
struct dw_edma *dw = chan->dw;
u32 tmp;
tmp = FIELD_GET(HDMA_V0_CH_STATUS_MASK,
GET_CH_32(dw, chan->id, chan->dir, ch_stat));
The GET_CH_32(dw, dir, ch, name) macro expects the direction as the second
argument and the channel ID as the third. Here it is invoked with
chan->id as the direction and chan->dir as the channel ID.
Since chan->id is evaluated as the direction, won't any channel ID > 0 be
treated as a READ direction? And won't chan->dir restrict the accessed
channel index to 0 or 1, meaning it will always read the status of channel
0 or 1 instead of the intended channel?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260708134343.3806759-1-devendra.verma@amd.com?part=1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v7] dmaengine: dw-edma: Enable HDMA 64R/W Channels
2026-07-08 13:43 [PATCH v7] dmaengine: dw-edma: Enable HDMA 64R/W Channels Devendra K Verma
2026-07-08 13:59 ` sashiko-bot
@ 2026-07-08 15:50 ` Frank Li
1 sibling, 0 replies; 4+ messages in thread
From: Frank Li @ 2026-07-08 15:50 UTC (permalink / raw)
To: Devendra K Verma
Cc: bhelgaas, mani, vkoul, Frank.Li, dmaengine, linux-kernel,
michal.simek
On Wed, Jul 08, 2026 at 07:13:43PM +0530, Devendra K Verma wrote:
> 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 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 [flat|nested] 4+ messages in thread
* Re: [PATCH v7] dmaengine: dw-edma: Enable HDMA 64R/W Channels
2026-07-08 13:59 ` sashiko-bot
@ 2026-07-13 6:46 ` Verma, Devendra
0 siblings, 0 replies; 4+ messages in thread
From: Verma, Devendra @ 2026-07-13 6:46 UTC (permalink / raw)
To: sashiko-reviews, Devendra K Verma; +Cc: vkoul, dmaengine, Frank.Li
On 08-Jul-26 19:29, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
>
> New issues:
> - [High] Designware PCIe core bounds checks reject HDMA configurations with more than 8 channels.
> - [High] Standalone PCIe eDMA driver statically limits hardware to 8 channels.
>
> Pre-existing issues:
> - [Critical] Missing register offset when setting the eDMA/HDMA base register pointer.
> - [High] Incorrect arguments passed to GET_CH_32 macro causing wrong channel registers to be accessed.
> --
>
> --- Patch [1]: [PATCH v7] dmaengine: dw-edma: Enable HDMA 64R/W Channels ---
> commit 4834a312ef82b7f9b290aedb3a1f566db2548955
> Author: Devendra K Verma <devendra.verma@amd.com>
>
> dmaengine: dw-edma: Enable HDMA 64R/W Channels
>
> 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:
>
>> diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
>> index 1fafd5b0e3153..da7a5cc93ad43 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
>
> [Severity: High]
> This patch introduces 64-channel support for HDMA, but does this break
> when used with the Designware PCIe host driver?
>
> Looking at drivers/pci/controller/dwc/pcie-designware.c:dw_pcie_edma_find_channels():
>
> /* Sanity check the channels count if the mapping was incorrect */
> 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;
>
This specific function targets the channel detection for non-HDMA IPs.
This piece of code is unaffected with the changes done to support 64 R/W
channels for HDMA IP.
> And in dw_pcie_edma_irq_vector():
>
> if (nr >= EDMA_MAX_WR_CH + EDMA_MAX_RD_CH)
> return -EINVAL;
>
> Will these sanity checks reject HDMA configurations with more than 8 channels
> because they strictly enforce the EDMA_MAX_WR_CH (8) limit?
>
This can be enhanced and shall be taken in a separate patch series.
The existing limits, functionality will not be affected and IP
shall work fine.
> [Severity: High]
> Does the standalone PCIe eDMA driver statically limit the hardware to
> 8 channels?
>
> Looking at drivers/dma/dw-edma/dw-edma-pcie.c:
>
> static const struct dw_edma_pcie_data xilinx_mdb_data = {
> ...
> .wr_ch_cnt = 8,
> .rd_ch_cnt = 8,
> };
>
> And later in dw_edma_pcie_get_xilinx_dma_data():
>
> pdata->wr_ch_cnt = min(pdata->wr_ch_cnt,
> FIELD_GET(DW_PCIE_XILINX_MDB_VSEC_DMA_WR_CH, val));
>
> Doesn't the min() operation cap the dynamically detected hardware channel
> count to the static limit of 8, negating the new 64-channel HDMA support
> on Xilinx MDB controllers?
For the Xilinx DMA, the limit is 8 Read / 8 Write channels only.
Enhancing the structs / arrays for 64 channels will not affect this
piece of code.
>
> [Severity: Critical]
> This is a pre-existing issue, but is there a missing register offset when
> setting the eDMA/HDMA base register pointer?
>
> In drivers/dma/dw-edma/dw-edma-pcie.c:dw_edma_pcie_probe():
>
> chip->reg_base = pcim_iomap_table(pdev)[vsec_data->rg.bar];
> if (!chip->reg_base)
> return -ENOMEM;
>
> Should this map the whole PCIe BAR but also add vsec_data->rg.off to
> chip->reg_base? Because all register accesses via __dw_regs(dw) directly
> dereference chip->reg_base, will all reads and writes hit the beginning
> of the BAR instead of the actual hardware registers on hardware where
> rg.off is non-zero?
This issue shall be taken in a separate patch series.
>
> [Severity: High]
> This is a pre-existing issue, but are the arguments to GET_CH_32
> swapped here?
>
> In drivers/dma/dw-edma/dw-hdma-v0-core.c:dw_hdma_v0_core_ch_status():
>
> static enum dma_status dw_hdma_v0_core_ch_status(struct dw_edma_chan *chan)
> {
> struct dw_edma *dw = chan->dw;
> u32 tmp;
>
> tmp = FIELD_GET(HDMA_V0_CH_STATUS_MASK,
> GET_CH_32(dw, chan->id, chan->dir, ch_stat));
>
> The GET_CH_32(dw, dir, ch, name) macro expects the direction as the second
> argument and the channel ID as the third. Here it is invoked with
> chan->id as the direction and chan->dir as the channel ID.
>
> Since chan->id is evaluated as the direction, won't any channel ID > 0 be
> treated as a READ direction? And won't chan->dir restrict the accessed
> channel index to 0 or 1, meaning it will always read the status of channel
> 0 or 1 instead of the intended channel?
>
This issue is being fixed as part of this patch series
https://lore.kernel.org/all/20260710080903.2392888-2-den@valinux.co.jp/
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-07-13 6:47 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-08 13:43 [PATCH v7] dmaengine: dw-edma: Enable HDMA 64R/W Channels Devendra K Verma
2026-07-08 13:59 ` sashiko-bot
2026-07-13 6:46 ` Verma, Devendra
2026-07-08 15:50 ` Frank Li
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox