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

[Frank Li <Frank.li@oss.nxp.com>]

On Fri, Aug 14, 2026 at 10:33:27AM +0530, 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@amd.com>
> > > > > ---
> > > > > 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?

you origial variable is "xilinx_cpm6_dma_data", but comments is
"MDB register location"

You change MDB to CPM6, belong fix typo/comments in original code.
Should combine with other code style cleanup patch.

Frank

>
> > > >
> > > > >    	.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.
>
> -Devendra
>
> > > Frank
> >
> > >
> > > -Devendra
> > >
> > > > Frank
> > > > >
> > > > >    	if (!pdata)
> > > > >    		return -ENODEV;
> > > > > --
> > > > > 2.43.0
> > > > >
> > >
>