dmaengine: dw-edma: Enable HDMA 64R/W Channels

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

New issues:
- [Medium] The 64-channel HDMA feature is unreachable due to hardcoded legacy limits in the driver bindings.

Pre-existing issues:
- [High] Potential NULL pointer dereference if a shared IRQ fires before dw_irq->dw is initialized. --

commit ae4f94060e0e6e5d4419cd4f95926541afa41dd0
Author: Devendra K Verma <[email protected]>

dmaengine: dw-edma: Enable HDMA 64R/W Channels

This patch expands the designware eDMA driver to support up to 64 read and 64 write channels for the HDMA controller, aligning with the hardware databook. It introduces new macros for the HDMA channel counts, expands the layout of the channel-related structures, updates channel masks to 64-bit, and updates the probe sequence to configure the core based on the controller type.

Quoted 14 lines

drivers/dma/dw-edma/dw-edma-core.c +2 -2

index c2feb3adc79fa..adf1b3939f96c 100644
@@ -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);
+			irq->wr_mask |= BIT_ULL(chan->id);
  		else
-			irq->rd_mask |= BIT(chan->id);
+			irq->rd_mask |= BIT_ULL(chan->id);
 

drivers/dma/dw-edma/dw-edma-core.c static int dw_edma_channel_setup(struct dw_edma *dw, u32 wr_alloc, u32 rd_alloc) · line 932

Quoted 1 line

drivers/dma/dw-edma/dw-edma-core.c +0 -0

  		irq->dw = dw;

[Severity: High]
This is a pre-existing issue, but could a shared interrupt fire before irq->dw is initialized here?

In dw_edma_probe(), dw_edma_irq_request() is called before dw_edma_channel_setup(). The IRQs are requested with IRQF_SHARED:

dw_edma_probe() {
     ...
     err = dw_edma_irq_request(dw, &wr_alloc, &rd_alloc);
     ...
     err = dw_edma_channel_setup(dw, wr_alloc, rd_alloc);
}

If the shared IRQ fires immediately after registration, the interrupt handler might dereference a NULL dw_irq->dw:

dw_edma_interrupt_write()

    dw_edma_core_handle_int()
        return dw_irq->dw->core->handle_int(dw_irq, dir, done, abort);

Should irq->dw be initialized prior to calling request_irq()?

[ ... ]

include/linux/dma/edma.h line 18

Quoted 11 lines

include/linux/dma/edma.h +2 -0

index 1fafd5b0e3153..da7a5cc93ad43 100644
@@ -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: Medium]
Will these expanded 64-channel limits actually be reachable with the current driver bindings?

Looking at drivers/pci/controller/dwc/pcie-designware.c, dw_pcie_edma_find_channels() still explicitly enforces the legacy 8-channel limit and returns -EINVAL if the configuration exceeds it:

     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;

Similarly, in drivers/dma/dw-edma/dw-edma-pcie.c, the Xilinx MDB HDMA data statically declares 8 channels and dw_edma_pcie_get_xilinx_dma_data() caps the write channels:

     pdata->wr_ch_cnt = min(pdata->wr_ch_cnt,
                    FIELD_GET(DW_PCIE_XILINX_MDB_VSEC_DMA_WR_CH, val));

Do the hardcoded legacy limits in the driver bindings prevent the new 64-channel feature from being fully utilized?

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 <[email protected]>

---

drivers/dma/dw-edma/dw-hdma-v0-core.c +2 -1

@@ -118,7 +129,8 @@ 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;
+	u64 mask;
 
 	if (dir == EDMA_DIR_WRITE) {
 		total = dw->wr_ch_cnt;

Quoted 19 lines · Devendra K 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 <[email protected]>

---

...

Quoted 12 lines

drivers/dma/dw-edma/dw-hdma-v0-core.c +2 -1

@@ -118,7 +129,8 @@ 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;
+	u64 mask;
 
  	if (dir == EDMA_DIR_WRITE) {
  		total = dw->wr_ch_cnt;

drivers/dma/dw-edma/dw-hdma-v0-core.c dw_hdma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir, · line 145

Quoted 5 lines

drivers/dma/dw-edma/dw-hdma-v0-core.c +1 -1

@@ -130,7 +142,11 @@ 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) {
+	while (mask) {

can you use DECLARE_BITMAP(status_mask, 64); and keep original for_each_set_bit() ref:

https://lore.kernel.org/dmaengine/aj1JrufD1vIZH06s@lizhi-Precision-Tower-5810/T/#u

Frank