dmaengine: dw-edma: Enable HDMA 64R/W Channels
Subsystems (2)
PATCH v3 1/1 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:
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.
---
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 | 4 +-- drivers/dma/dw-edma/dw-edma-pcie.c | 8 +++--- drivers/dma/dw-edma/dw-hdma-v0-core.c | 35 ++++++++++++++++++++------- drivers/dma/dw-edma/dw-hdma-v0-regs.h | 2 +- include/linux/dma/edma.h | 10 +++++--- 6 files changed, 52 insertions(+), 26 deletions(-)
drivers/dma/dw-edma/dw-edma-core.c +13 -6
@@ -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); 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 ;
drivers/dma/dw-edma/dw-edma-core.h +2 -2
@@ -91,8 +91,8 @@ struct dw_edma_chan {struct dw_edma_irq {struct msi_msg msi; -u32 wr_mask; -u32 rd_mask; +u64 wr_mask; +u64 rd_mask;struct dw_edma *dw; };
drivers/dma/dw-edma/dw-edma-pcie.c +4 -4
@@ -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;
drivers/dma/dw-edma/dw-hdma-v0-core.c +26 -9
@@ -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 ); } }
@@ -79,7 +90,7 @@ static enum dma_status dw_hdma_v0_core_ch_status(struct dw_edma_chan *chan)u32 tmp; tmp =FIELD_GET (HDMA_V0_CH_STATUS_MASK , -GET_CH_32 (dw, chan->id, chan->dir, ch_stat)); +GET_CH_32 (dw, chan->dir, chan->id, ch_stat));if (tmp ==1 )return DMA_IN_PROGRESS ;
@@ -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;
@@ -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) { + pos =__ffs64 (mask); +if (pos >= total) +break ; + chan = &dw->chan[pos + off]; val =dw_hdma_v0_core_status_int (chan);
@@ -147,6 +163,7 @@ dw_hdma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir, ret =IRQ_HANDLED ; } + mask &= mask -1 ; }return ret;
drivers/dma/dw-edma/dw-hdma-v0-regs.h +1 -1
@@ -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)
include/linux/dma/edma.h +6 -4
@@ -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 64struct 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;
Signature · 1 line
2.43.0
[email protected]
AI reviewThank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] The 64-channel HDMA feature is unreachable because platform glue drivers still enforce legacy 8-channel hardcoded limits.
Pre-existing issues:
- [High] The eDMA subsystem is deregistered AFTER the hardware core is disabled and IRQs are freed, violating tear-down symmetry.
- [High] The virt-dma descriptors are never freed when a channel is released, leading to a permanent memory leak. --
commit 20ba395bede7014c18b81804a5f6a941f3a95ebd
Author: Devendra K Verma <[email protected]>
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.
Quoted 4 lines
drivers/dma/dw-edma/dw-edma-core.c +0
-0
[ ... ]
Quoted 29 lines
drivers/dma/dw-edma/dw-edma-core.c +9
-2@@ -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 ));
drivers/dma/dw-edma/dw-edma-core.c int dw_edma_probe(struct dw_edma_chip *chip) · line 1120
Quoted 10 lines
drivers/dma/dw-edma/dw-edma-core.c +2
-2- 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 ;
[Severity: High]
This patch expands the eDMA core structures to support up to 64 HDMA channels. However, does it miss updating the PCIe instantiation wrappers, rendering the feature unusable?
In drivers/pci/controller/dwc/pcie-designware.c, dw_pcie_edma_find_channels() performs a sanity check that explicitly rejects any configuration exceeding EDMA_MAX_WR_CH (8), returning -EINVAL:
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, dw_pcie_edma_irq_vector() limits IRQ indices to 16 (EDMA_MAX_WR_CH + EDMA_MAX_RD_CH).
Furthermore, in drivers/dma/dw-edma/dw-edma-pcie.c, the functions dw_edma_pcie_get_synopsys_dma_data() and dw_edma_pcie_get_xilinx_dma_data() use min_t to bound the hardware-reported channel count against pdata->wr_ch_cnt (which is statically initialized to 2 or 8):
pdata->wr_ch_cnt = min_t(u16, pdata->wr_ch_cnt, FIELD_GET(DW_PCIE_SYNOPSYS_VSEC_DMA_WR_CH, val));
Will any attempt to configure an HDMA device with >8 channels be silently truncated to 8 or fail to probe completely?
[Severity: High]
This is a pre-existing issue, but looking at dw_edma_remove(), the eDMA subsystem is deregistered after the hardware core is disabled and IRQs are freed. Does this violate tear-down symmetry?
In dw_edma_remove(), the driver first calls dw_edma_core_off(dw) to disable the hardware, and then frees all IRQs using free_irq(). Only after that does it call dma_async_device_unregister(&dw->dma) and kill the virt-dma tasklets:
/* Disable eDMA */ dw_edma_core_off(dw); /* Free irqs */ for (i = (dw->nr_irqs - 1); i >= 0; i--) free_irq(chip->ops->irq_vector(dev, i), &dw->irq[i]); dw_edma_emul_irq_free(dw); /* Deregister eDMA device */ dma_async_device_unregister(&dw->dma);
If a client still holds a reference and has an active DMA transfer or submits a new one while the device is being removed, the transfer cannot complete because the hardware is disabled and interrupts are freed. Can this lead to a hung task or a DMA timeout?
[Severity: High]
This is a pre-existing issue, but does the driver leak virt-dma descriptors permanently when a channel is released?
The virt-dma framework requires drivers to call vchan_free_chan_resources() from their .device_free_chan_resources callback to free all pending, submitted, and allocated descriptors. The dw_edma_free_chan_resources() function waits for the channel to stop but fails to call vchan_free_chan_resources(&chan->vc) or manually free the descriptors:
static void dw_edma_free_chan_resources(struct dma_chan *dchan)
{
unsigned long timeout = jiffies + msecs_to_jiffies(5000);
int ret;
while (time_before(jiffies, timeout)) {
ret = dw_edma_device_terminate_all(dchan);
...
}
}As a result, every time a client allocates descriptors and then releases the channel, are all those descriptors permanently leaked?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
AMD General
Quoted 8 lines
-----Original Message-----
From: [email protected] <[email protected]>
Sent: Wednesday, June 3, 2026 20:28
To: Verma, Devendra <[email protected]>
Cc: [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH v3] dmaengine: dw-edma: Enable HDMA 64R/W Channels
Quoted 7 lines
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] The 64-channel HDMA feature is unreachable because platform glue drivers still enforce legacy 8-channel hardcoded limits.
This can be taken up in a separate patch but there is no glue driver at the moment that uses 64 channels.
Quoted 5 lines
Pre-existing issues:
- [High] The eDMA subsystem is deregistered AFTER the hardware core is disabled and IRQs are freed, violating tear-down symmetry.
- [High] The virt-dma descriptors are never freed when a channel is released, leading to a permanent memory leak.
These issues also can be taken up in a separate patch series.
Quoted 149 lines
--
commit
20ba395bede7014c18b81804a5f6a941f3a95ebd
Author: Devendra K Verma <[email protected]>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.
Quoted 5 lines
drivers/dma/dw-edma/dw-edma-core.c +0
-0[ ... ]
drivers/dma/dw-edma/dw-edma-core.c int dw_edma_probe(struct dw_edma_chip
Quoted 10 lines
drivers/dma/dw-edma/dw-edma-core.c +2
-0@@ -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)
Quoted 9 lines
drivers/dma/dw-edma/dw-edma-core.c +3
-2dw->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 ;drivers/dma/dw-edma/dw-edma-core.c int dw_edma_probe(struct dw_edma_chip *chip)
Quoted 10 lines
drivers/dma/dw-edma/dw-edma-core.c +4
-0+ }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));drivers/dma/dw-edma/dw-edma-core.c int dw_edma_probe(struct dw_edma_chip *chip) · line 1120
Quoted 10 lines
drivers/dma/dw-edma/dw-edma-core.c +2
-2- 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 ;[Severity: High]
This patch expands the eDMA core structures to support up to 64 HDMA channels.
However, does it miss updating the PCIe instantiation wrappers, rendering the feature unusable?In
drivers/pci/controller/dwc/pcie-designware.c,dw_pcie_edma_find_channels()performs a sanity check that explicitly rejects any configuration exceedingEDMA_MAX_WR_CH(8), returning-EINVAL: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,
dw_pcie_edma_irq_vector()limits IRQ indices to 16 (EDMA_MAX_WR_CH+EDMA_MAX_RD_CH).Furthermore, in
drivers/dma/dw-edma/dw-edma-pcie.c, the functionsdw_edma_pcie_get_synopsys_dma_data()anddw_edma_pcie_get_xilinx_dma_data()
usemin_tto bound the hardware-reported channel count againstpdata->wr_ch_cnt(which is statically initialized to 2 or 8):pdata->wr_ch_cnt = min_t(u16, pdata->wr_ch_cnt,
FIELD_GET(DW_PCIE_SYNOPSYS_VSEC_DMA_WR_CH, val));Will any attempt to configure an HDMA device with >8 channels be silently truncated to 8 or fail to probe completely?
[Severity: High]
This is a pre-existing issue, but looking atdw_edma_remove(), the eDMA subsystem is deregistered after the hardware core is disabled and IRQs are freed. Does this violate tear-down symmetry?In
dw_edma_remove(), the driver first callsdw_edma_core_off(dw)to disable the hardware, and then frees all IRQs usingfree_irq(). Only after that does it calldma_async_device_unregister(&dw->dma)and kill the virt-dma tasklets:/* Disable eDMA */ dw_edma_core_off(dw); /* Free irqs */ for (i = (dw->nr_irqs - 1); i >= 0; i--) free_irq(chip->ops->irq_vector(dev, i), &dw->irq[i]); dw_edma_emul_irq_free(dw); /* Deregister eDMA device */ dma_async_device_unregister(&dw->dma);If a client still holds a reference and has an active DMA transfer or submits a new one while the device is being removed, the transfer cannot complete because the hardware is disabled and interrupts are freed. Can this lead to a hung task or a DMA timeout?
[Severity: High]
This is a pre-existing issue, but does the driver leak virt-dma descriptors permanently when a channel is released?The virt-dma framework requires drivers to call
vchan_free_chan_resources()from their .device_free_chan_resources callback to free all pending, submitted, and allocated descriptors. Thedw_edma_free_chan_resources()function waits for the channel to stop but fails to callvchan_free_chan_resources(&chan->vc)
or manually free the descriptors:static void dw_edma_free_chan_resources(struct dma_chan *dchan) { unsigned long timeout = jiffies + msecs_to_jiffies(5000); int ret; while (time_before(jiffies, timeout)) { ret = dw_edma_device_terminate_all(dchan); ... } }As a result, every time a client allocates descriptors and then releases the channel, are all those descriptors permanently leaked?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260603144147.3249691-
[email protected]?part=1
Quoted 33 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 ofll_regionsanddt_regionsindw_edma_chipanddw_edma_pcie_datashall be in accordance to number of read and write channels.
o Indw_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]>---
Changes in v2:
o Fixed the pre-existing bug related toGET_CH_32interchanging the channel direction and id. This bug was not caused by any version of this patch.
o Fixed the issue when usingfor_each_set_bit()for mask of u64 type.Changes in v1:
o On review recommendation of sashiko bot, in the functiondw_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.
---
...
Quoted 19 lines
+++
b/drivers/dma/dw-edma/dw-hdma-v0-core.cdrivers/dma/dw-edma/dw-hdma-v0-core.c +10
-3@@ -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 );
drivers/dma/dw-edma/dw-hdma-v0-core.c __dw_ch_regs(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch) · line 73
Quoted 10 lines
drivers/dma/dw-edma/dw-hdma-v0-core.c +7
-3-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 );
why SET_BOTH_CH_32 not work for 64 channel?
drivers/dma/dw-edma/dw-hdma-v0-core.c static enum dma_status dw_hdma_v0_core_ch_status(struct dw_edma_chan *chan) · line 93
Quoted 9 lines
drivers/dma/dw-edma/dw-hdma-v0-core.c +1
-1} }@@ -79,7 +90,7 @@ static enum dma_status dw_hdma_v0_core_ch_status(struct dw_edma_chan *chan)u32 tmp; tmp =FIELD_GET (HDMA_V0_CH_STATUS_MASK , -GET_CH_32 (dw, chan->id, chan->dir, ch_stat)); +GET_CH_32 (dw, chan->dir, chan->id, ch_stat));
why need swtich id and dir here ?
Frank
Quoted 80 lines
drivers/dma/dw-edma/dw-hdma-v0-core.c +8
-2if (tmp ==1 )return DMA_IN_PROGRESS ;@@ -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;@@ -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) { + pos =__ffs64 (mask); +if (pos >= total) +break ; + chan = &dw->chan[pos + off]; val =dw_hdma_v0_core_status_int (chan);@@ -147,6 +163,7 @@ dw_hdma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir, ret =IRQ_HANDLED ; } + mask &= mask -1 ; }return ret;drivers/dma/dw-edma/dw-hdma-v0-regs.h +1
-1@@ -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)include/linux/dma/edma.h +6
-5@@ -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 64struct 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
Public
Quoted 75 lines
-----Original Message-----
From: Frank Li <[email protected]>
Sent: Friday, June 5, 2026 01:28
To: Verma, Devendra <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; linux- [email protected]; Simek, Michal <[email protected]>
Subject: Re: [PATCH v3] dmaengine: dw-edma: Enable HDMA 64R/W ChannelsQuoted 33 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 ofll_regionsanddt_regionsindw_edma_chipanddw_edma_pcie_datashall be in accordance to number of read and write channels.
o Indw_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]>---
Changes in v2:
o Fixed the pre-existing bug related toGET_CH_32interchanging the channel direction and id. This bug was not caused by any version of this patch.
o Fixed the issue when usingfor_each_set_bit()for mask of u64 type.Changes in v1:
o On review recommendation of sashiko bot, in the functiondw_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.
---...
Review __dw_ch_regs(struct dw_edma *dw, enum
Quoted 2 lines
+++
b/drivers/dma/dw-edma/dw-hdma-v0-core.c(quoted diff) +0
-0@@ -53,13 +53,24 @@ __dw_ch_regs(struct dw_edma *dw, enum
dw_edma_dirdrivers/dma/dw-edma/dw-hdma-v0-core.c __dw_ch_regs(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch)
Quoted 8 lines
drivers/dma/dw-edma/dw-hdma-v0-core.c +6
-0dir, 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);Review __dw_ch_regs(struct dw_edma *dw, enum · line 62
Quoted 2 lines
(quoted diff) +2
-0+ SET_CH_32(dw, dir, id, int_clear, + HDMA_V0_STOP_INT_MASK |
HDMA_V0_ABORT_INT_MASK);drivers/dma/dw-edma/dw-hdma-v0-core.c __dw_ch_regs(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch)
Quoted 6 lines
drivers/dma/dw-edma/dw-hdma-v0-core.c +2
-3+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);Review __dw_ch_regs(struct dw_edma *dw, enum · line 60
Quoted 2 lines
(quoted diff) +0
-2- SET_BOTH_CH_32(dw, id, int_clear, - HDMA_V0_STOP_INT_MASK |
HDMA_V0_ABORT_INT_MASK);drivers/dma/dw-edma/dw-hdma-v0-core.c __dw_ch_regs(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch)
Quoted 5 lines
drivers/dma/dw-edma/dw-hdma-v0-core.c +4
-1-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 |
Quoted 8 lines
HDMA_V0_ABORT_INT_MASK);Review __dw_ch_regs(struct dw_edma *dw, enum · line 71
Quoted 2 lines
(quoted diff) +2
-0+ SET_CH_32(dw, dir, id, int_clear, + HDMA_V0_STOP_INT_MASK |
HDMA_V0_ABORT_INT_MASK);Review __dw_ch_regs(struct dw_edma *dw, enum · line 72
Quoted 1 line
(quoted diff) +1
-0+ SET_CH_32(dw, dir, id, ch_en, 0);why
SET_BOTH_CH_32not work for 64 channel?
SET_BOTH_CH_32 works, but this needs to be done on the channels enabled for the IP.
HDMA supports maximum of 64 channels. So if some IP enables 8 or fewer read / write channels only then the number of channels come from dw->wr_ch_cnt and dw->rd_ch_cnt. Now the logic is derived by individual read & write enabled channel count. Earlier, it was assumed that user will enable max of 8 channels which would have worked well using SET_BOTH_CH_32() but as the channels grow, the assumption that equal number of read / write channels and that they are set to max count are enabled might not hold true.
- Devendra
Quoted 7 lines
Review static enum dma_status
Quoted 4 lines
(quoted diff) +0
-0} }@@ -79,7 +90,7 @@ static enum dma_status
dw_hdma_v0_core_ch_status(struct dw_edma_chan *chan)Quoted 2 lines
drivers/dma/dw-edma/dw-hdma-v0-core.c +0
-0u32 tmp;
Quoted 7 lines
drivers/dma/dw-edma/dw-hdma-v0-core.c static enum dma_status dw_hdma_v0_core_ch_status(struct dw_edma_chan *chan) · line 93
Quoted 3 lines
drivers/dma/dw-edma/dw-hdma-v0-core.c +1
-1tmp =FIELD_GET (HDMA_V0_CH_STATUS_MASK , -GET_CH_32 (dw, chan->id, chan->dir, ch_stat)); +GET_CH_32 (dw, chan->dir, chan->id, ch_stat));why need swtich id and dir here ?
Frank
This is the correct order of arguments to the GET_CH_32. The second & third arguments shall be direction and channel_id respectively. It is a pre-existing issue reported by AI bot.
Quoted 85 lines
drivers/dma/dw-edma/dw-hdma-v0-core.c dw_hdma_v0_core_handle_int(struct dw_edma_irq
Quoted 4 lines
drivers/dma/dw-edma/dw-hdma-v0-core.c +0
-0if (tmp ==1 )return DMA_IN_PROGRESS ;@@ -118,7 +129,8 @@ dw_hdma_v0_core_handle_int(struct dw_edma_irq
*dw_irq, enumdw_edma_dirdir,drivers/dma/dw-edma/dw-hdma-v0-core.c dw_hdma_v0_core_handle_int(struct
Quoted 10 lines
drivers/dma/dw-edma/dw-hdma-v0-core.c +2
-1unsigned 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;@@ -130,7 +142,11 @@ dw_hdma_v0_core_handle_int(struct
dw_edma_irq*dw_irq, enumdw_edma_dirdir,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,
Quoted 12 lines
drivers/dma/dw-edma/dw-hdma-v0-core.c +5
-1mask = dw_irq->rd_mask; } -for_each_set_bit (pos, &mask, total) { +while (mask) { + pos =__ffs64 (mask); +if (pos >= total) +break ; + chan = &dw->chan[pos + off]; val = dw_hdma_v0_core_status_int(chan); @@ -147,6 +163,7@@
Quoted 56 lines
dw_hdma_v0_core_handle_int(struct
dw_edma_irq*dw_irq, enumdw_edma_dirdir,ret = IRQ_HANDLED; }drivers/dma/dw-edma/dw-hdma-v0-core.c +1
-0+ mask &= mask -1 ; }return ret;drivers/dma/dw-edma/dw-hdma-v0-regs.h +1
-1@@ -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)include/linux/dma/edma.h index renamed from include/linux/dma/edma.h +0
-0
1fafd5b0e315..da7a5cc93ad4100644include/linux/dma/edma.h +6
-5@@ -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 64struct 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
Quoted 80 lines · Devendra
Public
Quoted 78 lines
-----Original Message-----
From: Frank Li <[email protected]>
Sent: Friday, June 5, 2026 01:28
To: Verma, Devendra <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; linux- [email protected]; Simek, Michal <[email protected]>
Subject: Re: [PATCH v3] dmaengine: dw-edma: Enable HDMA 64R/W ChannelsQuoted 33 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 ofll_regionsanddt_regionsindw_edma_chipanddw_edma_pcie_datashall be in accordance to number of read and write channels.
o Indw_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]>---
Changes in v2:
o Fixed the pre-existing bug related toGET_CH_32interchanging the channel direction and id. This bug was not caused by any version of this patch.
o Fixed the issue when usingfor_each_set_bit()for mask of u64 type.Changes in v1:
o On review recommendation of sashiko bot, in the functiondw_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.
---...
Review __dw_ch_regs(struct dw_edma *dw, enum
Quoted 2 lines
+++
b/drivers/dma/dw-edma/dw-hdma-v0-core.c(quoted diff) +0
-0@@ -53,13 +53,24 @@ __dw_ch_regs(struct dw_edma *dw, enum
dw_edma_dirdrivers/dma/dw-edma/dw-hdma-v0-core.c __dw_ch_regs(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch)
Quoted 8 lines
drivers/dma/dw-edma/dw-hdma-v0-core.c +6
-0dir, 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);Review __dw_ch_regs(struct dw_edma *dw, enum · line 62
Quoted 2 lines
(quoted diff) +2
-0+ SET_CH_32(dw, dir, id, int_clear, + HDMA_V0_STOP_INT_MASK |
HDMA_V0_ABORT_INT_MASK);drivers/dma/dw-edma/dw-hdma-v0-core.c __dw_ch_regs(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch)
Quoted 6 lines
drivers/dma/dw-edma/dw-hdma-v0-core.c +2
-3+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);Review __dw_ch_regs(struct dw_edma *dw, enum · line 60
Quoted 2 lines
(quoted diff) +0
-2- SET_BOTH_CH_32(dw, id, int_clear, - HDMA_V0_STOP_INT_MASK |
HDMA_V0_ABORT_INT_MASK);drivers/dma/dw-edma/dw-hdma-v0-core.c __dw_ch_regs(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch)
Quoted 5 lines
drivers/dma/dw-edma/dw-hdma-v0-core.c +4
-1-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);Quoted 2 lines
(quoted diff) +2
-0+ SET_CH_32(dw, dir, id, int_clear, + HDMA_V0_STOP_INT_MASK |
Quoted 8 lines
Quoted 5 lines
HDMA_V0_ABORT_INT_MASK);Review __dw_ch_regs(struct dw_edma *dw, enum · line 72
Quoted 1 line
(quoted diff) +1
-0+ SET_CH_32(dw, dir, id, ch_en, 0);why
SET_BOTH_CH_32not work for 64 channel?
SET_BOTH_CH_32works, but this needs to be done on the channels enabled for the IP.
HDMA supports maximum of 64 channels. So if some IP enables 8 or fewer read / write channels only then the number of channels come fromdw->wr_ch_cntanddw->rd_ch_cnt. Now the logic is derived by individual read & write enabled channel count. Earlier, it was assumed that user will enable max of 8 channels which would have worked well usingSET_BOTH_CH_32()but as the channels grow, the assumption that equal number of read / write channels and that they are set to max count are enabled might not hold true.
Make sense, please wrap your reply, it is hard to read
Quoted 14 lines
- Devendra
Quoted 11 lines
Review static enum dma_status
Quoted 4 lines
(quoted diff) +0
-0} }@@ -79,7 +90,7 @@ static enum dma_status
dw_hdma_v0_core_ch_status(struct dw_edma_chan *chan)Quoted 5 lines
drivers/dma/dw-edma/dw-hdma-v0-core.c +1
-1u32 tmp; tmp =FIELD_GET (HDMA_V0_CH_STATUS_MASK , -GET_CH_32 (dw, chan->id, chan->dir, ch_stat)); +GET_CH_32 (dw, chan->dir, chan->id, ch_stat));
Quoted 5 lines
Quoted 3 lines
why need swtich id and dir here ?
Frank
This is the correct order of arguments to the
GET_CH_32. The second & third arguments shall be direction andchannel_idrespectively. It is a pre-existing issue reported by AI bot.
AI found existing problem, need seperate patch to fix it.
Frank
Quoted 84 lines · Frank Li
Quoted 83 lines · Devendra
Public
Quoted 81 lines
-----Original Message-----
From: Frank Li <[email protected]>
Sent: Friday, June 5, 2026 01:28
To: Verma, Devendra <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; linux- [email protected]; Simek, Michal <[email protected]>
Subject: Re: [PATCH v3] dmaengine: dw-edma: Enable HDMA 64R/W ChannelsQuoted 33 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 ofll_regionsanddt_regionsindw_edma_chipanddw_edma_pcie_datashall be in accordance to number of read and write channels.
o Indw_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]>---
Changes in v2:
o Fixed the pre-existing bug related toGET_CH_32interchanging the channel direction and id. This bug was not caused by any version of this patch.
o Fixed the issue when usingfor_each_set_bit()for mask of u64 type.Changes in v1:
o On review recommendation of sashiko bot, in the functiondw_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.
---...
Review __dw_ch_regs(struct dw_edma *dw, enum
Quoted 2 lines
+++
b/drivers/dma/dw-edma/dw-hdma-v0-core.c(quoted diff) +0
-0@@ -53,13 +53,24 @@ __dw_ch_regs(struct dw_edma *dw, enum
dw_edma_dirdrivers/dma/dw-edma/dw-hdma-v0-core.c __dw_ch_regs(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch)
Quoted 8 lines
drivers/dma/dw-edma/dw-hdma-v0-core.c +6
-0dir, 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);Review __dw_ch_regs(struct dw_edma *dw, enum · line 62
Quoted 2 lines
(quoted diff) +2
-0+ SET_CH_32(dw, dir, id, int_clear, + HDMA_V0_STOP_INT_MASK |
HDMA_V0_ABORT_INT_MASK);drivers/dma/dw-edma/dw-hdma-v0-core.c __dw_ch_regs(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch)
Quoted 6 lines
drivers/dma/dw-edma/dw-hdma-v0-core.c +2
-3+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);Review __dw_ch_regs(struct dw_edma *dw, enum · line 60
Quoted 2 lines
(quoted diff) +0
-2- SET_BOTH_CH_32(dw, id, int_clear, - HDMA_V0_STOP_INT_MASK |
HDMA_V0_ABORT_INT_MASK);drivers/dma/dw-edma/dw-hdma-v0-core.c __dw_ch_regs(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch)
Quoted 5 lines
drivers/dma/dw-edma/dw-hdma-v0-core.c +4
-1-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);Review __dw_ch_regs(struct dw_edma *dw, enum · line 71
Quoted 2 lines
(quoted diff) +2
-0+ SET_CH_32(dw, dir, id, int_clear, + HDMA_V0_STOP_INT_MASK |
HDMA_V0_ABORT_INT_MASK);Quoted 1 line
(quoted diff) +1
-0+ SET_CH_32(dw, dir, id, ch_en, 0);
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
why
SET_BOTH_CH_32not work for 64 channel?
SET_BOTH_CH_32works, but this needs to be done on the channels enabled for the IP.
HDMA supports maximum of 64 channels. So if some IP enables 8 or fewer read / write channels only then the number of channels come fromdw->wr_ch_cntanddw->rd_ch_cnt. Now the logic is derived by individual read & write enabled channel count. Earlier, it was assumed that user will enable max of 8 channels which would have worked well usingSET_BOTH_CH_32()but as the channels grow, the assumption that equal number of read / write channels and that they are set to max count are enabled might not hold true.Make sense, please wrap your reply, it is hard to read
[Reformatted the comment to fit within the visible window]
SET_BOTH_CH_32 works, but this needs to be done on the channels enabled for the IP. HDMA supports maximum of 64 channels. So, if some IP enables 8 or fewer read / write channels only, then the number of channels to be configured shall come from dw->wr_ch_cnt and dw->rd_ch_cnt. In the new code the logic is derived by individual read & write enabled channel count. Earlier, it was assumed that user will enable max of 8 channels which would have worked well using SET_BOTH_CH_32() but as the channels grow, the assumption that equal number of read / write channels and that they are set to max count are enabled might not hold true.
Quoted 16 lines
Quoted 16 lines
- Devendra
Quoted 13 lines
Review static enum dma_status
Quoted 4 lines
(quoted diff) +0
-0} }@@ -79,7 +90,7 @@ static enum dma_status
dw_hdma_v0_core_ch_status(struct dw_edma_chan *chan)drivers/dma/dw-edma/dw-hdma-v0-core.c static enum dma_status dw_hdma_v0_core_ch_status(struct dw_edma_chan *chan) · line 93
Quoted 5 lines
drivers/dma/dw-edma/dw-hdma-v0-core.c +1
-1u32 tmp; tmp =FIELD_GET (HDMA_V0_CH_STATUS_MASK , -GET_CH_32 (dw, chan->id, chan->dir, ch_stat)); +GET_CH_32 (dw, chan->dir, chan->id, ch_stat));why need swtich id and dir here ?
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
Frank
This is the correct order of arguments to the
GET_CH_32. The second & third arguments shall be direction andchannel_idrespectively. It is a pre-existing issue reported by AI bot.AI found existing problem, need seperate patch to fix it.
Frank
Removed this fix. Will include it as part of separate series. I will upload the next version with the above recommendation.
-Devendra