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

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 <[email protected]>
>> ---
>> 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/

>> +/* 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.

> 
>>   	.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.

-Devendra

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