Re: [PATCH v2 1/3] dmaengine: dw-edma: Make Xilinx Macros Dev Name Agnostic

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

On Mon, Aug 03, 2026 at 05:45:14PM +0530, Verma, Devendra wrote:
>
> On 01-Aug-26 00:50, Frank Li wrote:
> > On Fri, Jul 31, 2026 at 09:54:01PM +0530, Verma, Devendra wrote:
> > >
> > >
> > > On 28-Jul-26 21:01, Frank Li wrote:
> > > > On Tue, Jul 28, 2026 at 02:47:42PM +0530, Devendra K Verma wrote:
> > > > > Xilinx specific macros for MDB device can be reused for the
> > > > > Xilinx supported other similar IP such as CPM6.
> > > > > Renamed the Xilinx specific macros in a way that can be
> > > > > reused for Xilinx supported upcoming IP, CPM6.
> > > > > Naming is in accordance with the naming done for Synopsys macros.
> > > > >
> > > > > Signed-off-by: Devendra K Verma <devverma@amd.com>
> > > > > ---
> > > > > Changes in v1:
> > > > >      o No change
> > > > > ---
> > > > >    drivers/dma/dw-edma/dw-edma-pcie.c | 58 +++++++++++++++---------------
> > > > >    1 file changed, 29 insertions(+), 29 deletions(-)
> > > > >
> > > > > diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
> > > > > index 791c46e8ae4c..ec5e057a0f11 100644
> > > > > --- a/drivers/dma/dw-edma/dw-edma-pcie.c
> > > > > +++ b/drivers/dma/dw-edma/dw-edma-pcie.c
> > > > > @@ -29,21 +29,21 @@
> > > > >    #define PCI_DEVICE_ID_XILINX_B054		0xb054
> > > > >    #define PCI_DEVICE_ID_XILINX_B00F		0xb00f
> > > > >
> > > > > -#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_WR_CH	GENMASK(9, 0)
> > > > > -#define DW_PCIE_XILINX_MDB_VSEC_DMA_RD_CH	GENMASK(25, 16)
> > > > > -
> > > > > -#define DW_PCIE_XILINX_MDB_DEVMEM_OFF_REG_HIGH	0xc
> > > > > -#define DW_PCIE_XILINX_MDB_DEVMEM_OFF_REG_LOW	0x8
> > > > > -#define DW_PCIE_XILINX_MDB_INVALID_ADDR		(~0ULL)
> > > > > -
> > > > > -#define DW_PCIE_XILINX_MDB_LL_OFF_GAP		0x200000
> > > > > -#define DW_PCIE_XILINX_MDB_LL_SIZE		0x800
> > > > > -#define DW_PCIE_XILINX_MDB_DT_OFF_GAP		0x100000
> > > > > -#define DW_PCIE_XILINX_MDB_DT_SIZE		0x800
> > > > > +#define DW_PCIE_XILINX_VSEC_DMA_ID		0x6
> > > > > +#define DW_PCIE_XILINX_VSEC_ID			0x20
> > > > > +#define DW_PCIE_XILINX_VSEC_DMA_BAR		GENMASK(10, 8)
> > > > > +#define DW_PCIE_XILINX_VSEC_DMA_MAP		GENMASK(2, 0)
> > > > > +#define DW_PCIE_XILINX_VSEC_DMA_WR_CH		GENMASK(9, 0)
> > > > > +#define DW_PCIE_XILINX_VSEC_DMA_RD_CH		GENMASK(25, 16)
> > > >
> > > > v1, you agree keep use old MDB ?
> > > >
> > > > Frank
> > > >
> > >
> > > I followed the suggestion you provided at the given link at the end.
> > > Talking of Xilinx based devices, the channel separation is fixed for MDB
> > > but not for CPM6. So, I have have renamed the specific functionality
> > > specific to CPM6 in 2/3 of the same patch series.
> > >
> > > This one cleans up the MDB references and makes the functionality generic
> > > for all the Xilinx devices.
> > >
> > > Suggestion: https://lore.kernel.org/all/amI09ZcfhX4SEPIH@SMW015318/
> >
> > Maybe I have not said cleanly
> >
> > Actually you only need add one macro to for CPM6, other use MDB macro can
> > work well.
> >
> > #define DW_PCIE_XILINX_CPM6_VSEC_CH_SEP                GENMASK(18, 16)
> >
> > These rename are totally unnecessary.  It is quite popular that new chip
> > reuse old chip's register name.
> >
> > We rename it utils we really can't handle these without rename.
>
> You have pointed out the issue in the last line. For the upcoming
> support we anticipated that it would be messy to use the MDB related
> stuff for CPM6 and that's why wanted to clean it before introducing
> functionality for CPM6.

Do it only when have to do that. Now only add one macro
DW_PCIE_XILINX_CPM6_VSEC_CH_SEP to support CPM6.

The prefix "DW_PCIE_XILINX_MDB" and "DW_PCIE_XILINX_VSEC" have not bring
much information,

The most important part following part after it.

Frank

>
> The MDB works, no denying, but mixing the two creates confusion and
> requires the understanding of MDB first and CPM6. For that reason
> the clean approach taken is:
> - Add the generic functionality supported by MDB and CPM6 under the
>   name Xilinx
> - Add the MDB and CPM6 specific functionality with their own
>   associated names, keeping it clean.
>
> - Devendra>
> > Frank
> >
> > >
> > > CPM6 specific declaration:
> > > https://lore.kernel.org/all/20260728091744.1086942-3-devverma@amd.com/
> > >
> > > - Devendra
> > >
> > > >
> > > > > +
> > > > > +#define DW_PCIE_XILINX_DEVMEM_OFF_REG_HIGH	0xc
> > > > > +#define DW_PCIE_XILINX_DEVMEM_OFF_REG_LOW	0x8
> > > > > +#define DW_PCIE_XILINX_INVALID_ADDR		(~0ULL)
> > > > > +
> > > > > +#define DW_PCIE_XILINX_LL_OFF_GAP		0x200000
> > > > > +#define DW_PCIE_XILINX_LL_SIZE			0x800
> > > > > +#define DW_PCIE_XILINX_DT_OFF_GAP		0x100000
> > > > > +#define DW_PCIE_XILINX_DT_SIZE			0x800
> > > > >
> > > > >    #define DW_BLOCK(a, b, c) \
> > > > >    	{ \
> > > > > @@ -258,10 +258,10 @@ static void dw_edma_pcie_get_xilinx_dma_data(struct pci_dev *pdev,
> > > > >    	u16 vsec;
> > > > >    	u64 off;
> > > > >
> > > > > -	pdata->devmem_phys_off = DW_PCIE_XILINX_MDB_INVALID_ADDR;
> > > > > +	pdata->devmem_phys_off = DW_PCIE_XILINX_INVALID_ADDR;
> > > > >
> > > > >    	vsec = pci_find_vsec_capability(pdev, PCI_VENDOR_ID_XILINX,
> > > > > -					DW_PCIE_XILINX_MDB_VSEC_DMA_ID);
> > > > > +					DW_PCIE_XILINX_VSEC_DMA_ID);
> > > > >    	if (!vsec)
> > > > >    		return;
> > > > >
> > > > > @@ -272,18 +272,18 @@ static void dw_edma_pcie_get_xilinx_dma_data(struct pci_dev *pdev,
> > > > >
> > > > >    	pci_dbg(pdev, "Detected Xilinx PCIe Vendor-Specific Extended Capability DMA\n");
> > > > >    	pci_read_config_dword(pdev, vsec + 0x8, &val);
> > > > > -	map = FIELD_GET(DW_PCIE_XILINX_MDB_VSEC_DMA_MAP, val);
> > > > > +	map = FIELD_GET(DW_PCIE_XILINX_VSEC_DMA_MAP, val);
> > > > >    	if (map != EDMA_MF_HDMA_NATIVE)
> > > > >    		return;
> > > > >
> > > > >    	pdata->mf = map;
> > > > > -	pdata->rg.bar = FIELD_GET(DW_PCIE_XILINX_MDB_VSEC_DMA_BAR, val);
> > > > > +	pdata->rg.bar = FIELD_GET(DW_PCIE_XILINX_VSEC_DMA_BAR, 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));
> > > > > +			       FIELD_GET(DW_PCIE_XILINX_VSEC_DMA_WR_CH, val));
> > > > >    	pdata->rd_ch_cnt = min(pdata->rd_ch_cnt,
> > > > > -			       FIELD_GET(DW_PCIE_XILINX_MDB_VSEC_DMA_RD_CH, val));
> > > > > +			       FIELD_GET(DW_PCIE_XILINX_VSEC_DMA_RD_CH, val));
> > > > >
> > > > >    	pci_read_config_dword(pdev, vsec + 0x14, &val);
> > > > >    	off = val;
> > > > > @@ -293,16 +293,16 @@ static void dw_edma_pcie_get_xilinx_dma_data(struct pci_dev *pdev,
> > > > >    	pdata->rg.off = off;
> > > > >
> > > > >    	vsec = pci_find_vsec_capability(pdev, PCI_VENDOR_ID_XILINX,
> > > > > -					DW_PCIE_XILINX_MDB_VSEC_ID);
> > > > > +					DW_PCIE_XILINX_VSEC_ID);
> > > > >    	if (!vsec)
> > > > >    		return;
> > > > >
> > > > >    	pci_read_config_dword(pdev,
> > > > > -			      vsec + DW_PCIE_XILINX_MDB_DEVMEM_OFF_REG_HIGH,
> > > > > +			      vsec + DW_PCIE_XILINX_DEVMEM_OFF_REG_HIGH,
> > > > >    			      &val);
> > > > >    	off = val;
> > > > >    	pci_read_config_dword(pdev,
> > > > > -			      vsec + DW_PCIE_XILINX_MDB_DEVMEM_OFF_REG_LOW,
> > > > > +			      vsec + DW_PCIE_XILINX_DEVMEM_OFF_REG_LOW,
> > > > >    			      &val);
> > > > >    	off <<= 32;
> > > > >    	off |= val;
> > > > > @@ -360,7 +360,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
> > > > >    		 * address use the non-LL mode or simple mode supported by
> > > > >    		 * the HDMA IP.
> > > > >    		 */
> > > > > -		if (vsec_data->devmem_phys_off == DW_PCIE_XILINX_MDB_INVALID_ADDR)
> > > > > +		if (vsec_data->devmem_phys_off == DW_PCIE_XILINX_INVALID_ADDR)
> > > > >    			non_ll = true;
> > > > >
> > > > >    		/*
> > > > > @@ -370,10 +370,10 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
> > > > >    		 */
> > > > >    		if (!non_ll)
> > > > >    			dw_edma_set_chan_region_offset(vsec_data, BAR_2, 0,
> > > > > -						       DW_PCIE_XILINX_MDB_LL_OFF_GAP,
> > > > > -						       DW_PCIE_XILINX_MDB_LL_SIZE,
> > > > > -						       DW_PCIE_XILINX_MDB_DT_OFF_GAP,
> > > > > -						       DW_PCIE_XILINX_MDB_DT_SIZE);
> > > > > +						       DW_PCIE_XILINX_LL_OFF_GAP,
> > > > > +						       DW_PCIE_XILINX_LL_SIZE,
> > > > > +						       DW_PCIE_XILINX_DT_OFF_GAP,
> > > > > +						       DW_PCIE_XILINX_DT_SIZE);
> > > > >    	}
> > > > >
> > > > >    	/* Mapping PCI BAR regions */
> > > > > --
> > > > > 2.43.0
> > > > >
> > >
>