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

["Verma, Devendra" <devverma@amd.com>]

On 06-Aug-26 20:57, Frank Li wrote:
> On Thu, Aug 06, 2026 at 06:27:17PM +0530, Verma, Devendra wrote:
>> On 05-Aug-26 04:13, Frank Li wrote:
>>> 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.
>>>
>>
>> Here also we realized that using MDB would create the confusion later on
>> when more features related to CPM6 will come out.
> 
> Do it when you post new features. We will make decistion at that time.
> 
> Now only one DW_PCIE_XILINX_CPM6_VSEC_CH_SEP is NOT enough to support
> rename now.
>

Could you please tell how many changes are required to have the renaming
related patch to be pushed?

> Unnecessary rename will cause meanless conflict when back port some fix
> patches to stable tree in future.
>

This issue will occure irrespective of when patches are pushed.

Anyways, I will be changing the code as per recommendation.
Here we were trying to reduce the misunderstanding but it is not
recommended, for 2/3 of the same patch series, the variable causes
misunderstanding despite reasons/context.

I will push the changes, please review them. Thanks!

> Frank
> 
>> We are trying to contain the issue early, that is why putting up the
>> effort of segregating the macro names.
>>
>> As you have already suggested that the names can be changed but when the
>> need arises. We see the need and handling it early.
>> Request to consider the approach:
>> - All the Xilinx specific macros, providing common functionality,
>>    applicable to MDB and CPM6 are made name agnostic. The macros will
>>    go by the Xilinx name similar to Synopsys declarations.
>>
>> - Macros used specifically for MDB will have MDB to specify the
>>    association with the IP and similar approach for CPM6.
>>
>> Also, as of now, as we are aware of both MDB and CPM6, they look
>> related but there would be end users of these IPs who would not be knowing
>> the MDB if working on CPM6 and vice-versa. This distinction
>> helps in easy understanding rather than creating confusion due to
>> naming dependance on MDB.
>>
>> -Devendra
>>> 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
>>>>>>>>
>>>>>>
>>>>
>>