Re: [PATCH v3 1/2] dmaengine: dw-edma: Enable Chan Separation via VSEC

From: Verma, Devendra

Date: Tue Aug 18 2026 - 07:13:24 EST


Hi Frank

Could you check and provide suggestion on the comments
inline?

regards,
Devendra

On 14-Aug-26 10:33, Verma, Devendra wrote:

On 13-Aug-26 22:01, Frank Li wrote:
On Thu, Aug 13, 2026 at 05:26:51PM +0530, Verma, Devendra wrote:
On 10-Aug-26 22:44, Frank Li wrote:
On Mon, Aug 10, 2026 at 03:40:01PM +0530, Devendra K Verma wrote:
As per, 'Designware Cores PCI Express DM Controller - Reference
Manual', section 3.2.34.3, VSEC for DEVICE INFORMATION supports
the channel separation mechanisms. Basically, the HDMA IP allows
the user to configure the separation between DMA channel
registers and retrieve it via the VSEC capability mentioned
above.

HDMA IP supports the channel register space separation from
256B to 32KB. Default supported size is 256B.

Signed-off-by: Devendra K Verma <devverma@xxxxxxx>
---
Changes in v2:
     o Replaced 'ch_sep_sz' with 'ch_space_sz' wherever
       applicable upon reviewer recommendation.
     o Dropped 1/3 patch of this series, after review,
       now the 2/3 is 1/2 of the current patch series.

Changes in v1:
     o Modified dw_edma_get_ch_sep_sz() as per review comment.
       The function now supports ch_sep_sz up to 32KB.
     o Updated to description to reflect the supported channel
       separation sizes.
---
   drivers/dma/dw-edma/dw-edma-pcie.c | 20 +++++++++++++++++---
   1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw- edma/dw-edma-pcie.c
index 791c46e8ae4c..5d7507a64056 100644
--- a/drivers/dma/dw-edma/dw-edma-pcie.c
+++ b/drivers/dma/dw-edma/dw-edma-pcie.c
@@ -31,8 +31,10 @@

   #define DW_PCIE_XILINX_MDB_VSEC_DMA_ID        0x6
   #define DW_PCIE_XILINX_MDB_VSEC_ID        0x20
-#define DW_PCIE_XILINX_MDB_VSEC_DMA_BAR        GENMASK(10, 8)
   #define DW_PCIE_XILINX_MDB_VSEC_DMA_MAP        GENMASK(2, 0)
+#define DW_PCIE_XILINX_MDB_VSEC_DMA_BAR        GENMASK(10, 8)

Don't mix code cleanup or reorg into this improvement patches.


It was recommended by you in the v1 of the same patch series that is
why pushed it.

Ref: https://lore.kernel.org/all/amDbGHRbObX-lt5F@SMW015318/

You can fix the order by using prep patch, which just fix order, typo
or some cleanup.

But not mixed into new feature patches.


+/* AMD CPM6 (Xilinx) supported cap */
+#define DW_PCIE_XILINX_CPM6_VSEC_CH_SEP        GENMASK(18, 16)
   #define DW_PCIE_XILINX_MDB_VSEC_DMA_WR_CH    GENMASK(9, 0)
   #define DW_PCIE_XILINX_MDB_VSEC_DMA_RD_CH    GENMASK(25, 16)

@@ -73,6 +75,7 @@ struct dw_edma_pcie_data {
       u16                wr_ch_cnt;
       u16                rd_ch_cnt;
       u64                devmem_phys_off;
+    u32                ch_space_sz;
   };

   static const struct dw_edma_pcie_data snps_edda_data = {
@@ -127,7 +130,7 @@ static const struct dw_edma_pcie_data xilinx_mdb_data = {
   };

   static const struct dw_edma_pcie_data xilinx_cpm6_dma_data = {
-    /* MDB registers location */
+    /* CPM6 registers location */

This typo fix have not mixed this patch.

This is subtle change. Since the focus of this patch series is CPM6, the
namings and comments should align with the appropriate name.


Can we take up this change along with this patch series as it is related
to CPM6 only?


       .rg.bar                = BAR_0,
       .rg.off                = SZ_4K,    /*  4 Kbytes */
       .rg.sz                = SZ_8K,    /*  8 Kbytes */
@@ -189,6 +192,13 @@ static int dw_edma_pcie_irq_vector(struct device *dev, unsigned int nr)
       return pci_irq_vector(to_pci_dev(dev), nr);
   }

+static u32 dw_edma_get_ch_space_sz(u32 val)
+{
+    if (val > 0 && val <= 7)
+        return 256 << val;
+    return 256;
+}
+
   static u64 dw_edma_pcie_address(struct device *dev, phys_addr_t cpu_addr)
   {
       struct pci_dev *pdev = to_pci_dev(dev);
@@ -279,6 +289,10 @@ static void dw_edma_pcie_get_xilinx_dma_data(struct pci_dev *pdev,
       pdata->mf = map;
       pdata->rg.bar = FIELD_GET(DW_PCIE_XILINX_MDB_VSEC_DMA_BAR, val);

+    if (pdev->device == PCI_DEVICE_ID_XILINX_B00F)
+        pdata->ch_space_sz = dw_edma_get_ch_space_sz
+                    (FIELD_GET(DW_PCIE_XILINX_CPM6_VSEC_CH_SEP, val));
+
       pci_read_config_dword(pdev, vsec + 0xc, &val);
       pdata->wr_ch_cnt = min(pdata->wr_ch_cnt,
                      FIELD_GET(DW_PCIE_XILINX_MDB_VSEC_DMA_WR_CH, val));
@@ -324,9 +338,9 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
       struct dw_edma_pcie_data *pdata = (void *)pid->driver_data;
       struct device *dev = &pdev->dev;
       struct dw_edma_chip *chip;
+    bool non_ll = false;
       int err, nr_irqs;
       int i, mask;
-    bool non_ll = false;

unnecssary change here


Yeah, this change was part of some patch series I pushed.
This change was breaking the inverted X-mas format that's why
made this change.

But not related this patch. Use a small sperate patch to fix these small
problem.

Use a small patch to do clean up, for example

reorder register bit defination
fix commends about CPM6
fix inverted X-mas format.

The 1st and 3rd can be taken in a separate patch series but the 2nd
is related to CPM6 only so having it is beneficial with this patch
series, removes the misleading comment.

I will push the changes after your response.


Any update on the above suggestion?

-Devendra

> > Frank


-Devendra

Frank

       if (!pdata)
           return -ENODEV;
--
2.43.0