Re: [PATCH v1 3/3] dmaengine: dw-edma: Add changes to support Channel Separation

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

On 23-Jul-26 20:58, Frank Li wrote:
> On Thu, Jul 23, 2026 at 03:56:33PM +0530, Verma, Devendra wrote:
>>
>>
>> On 22-Jul-26 20:52, Frank Li wrote:
>>> On Wed, Jul 22, 2026 at 04:46:40PM +0530, Devendra K Verma wrote:
>>>
>>> Add Koichiro Den, who did many work with this edma driver recently
>>>
>>
>> Sure, thanks!
>>
>>>> Following changes are made:
>>>> o In 'struct dw_hdma_v0_ch_regs', it is assumed the channel
>>>>     separation as 256-bytes which is default, but if an IP is
>>>
>>> channel register space size?
>>>
>>>>     configured for a different size of channel separation then
>>>>     user can retrieve it using the supported VSEC (0x6).
>>>>     Modified the struct to work with the channel separation
>>>>     supported by IP.
>>>>     CPM6 supports channel separation, default is 512.
>>>
>>>
>>>> o Changes in macros / functions where address of the channel
>>>>     is accessed based on channel index and direction. It enables
>>>>     the calculation of the channel reg base address based on
>>>>     the selected channel separation.
>>>> o Struct removed - dw_hdma_v0_ch, dw_hdma_v0_regs
>>>> o Functions removed - __dw_regs()
>>>
>>> Can you summary change and purpose? such as
>>>
>>> Support difference channel register space size for difference IP. Previous
>>> use fix default value 256, CMP6 support difference register space size,
>>> which default value is 512.
>>>
>>> Update channel register base address caluator method by use ch_space_sze
>>> and remove unused dw_hdma_v0_ch, dw_hdma_v0_regs.
>>>
>>> You can rephrase.
>>>
>>
>> Sure, I will rephrase this. Thank you for the suggestion!
>>
>>>>
>>>> Signed-off-by: Devendra K Verma <devverma@amd.com>
>>>> ---
>>>>    drivers/dma/dw-edma/dw-edma-pcie.c       |  4 ++++
>>>>    drivers/dma/dw-edma/dw-hdma-v0-core.c    | 23 ++++++++++++++---------
>>>>    drivers/dma/dw-edma/dw-hdma-v0-debugfs.c | 17 ++++++-----------
>>>>    drivers/dma/dw-edma/dw-hdma-v0-regs.h    | 10 ----------
>>>>    include/linux/dma/edma.h                 |  1 +
>>>>    5 files changed, 25 insertions(+), 30 deletions(-)
>>>>
>>>> diff --git a/drivers/dma/dw-edma/dw-edma-pcie.c b/drivers/dma/dw-edma/dw-edma-pcie.c
>>>> index 6295d01ba2f7..ca2a4be2f3a5 100644
>>>> --- a/drivers/dma/dw-edma/dw-edma-pcie.c
>>>> +++ b/drivers/dma/dw-edma/dw-edma-pcie.c
>>>> @@ -113,6 +113,7 @@ static const struct dw_edma_pcie_data snps_edda_data = {
>>>>    	.irqs				= 1,
>>>>    	.wr_ch_cnt			= 2,
>>>>    	.rd_ch_cnt			= 2,
>>>> +	.ch_sep_sz			= 256,
>>>>    };
>>>>
>>>>    static const struct dw_edma_pcie_data xilinx_mdb_data = {
>>>> @@ -126,6 +127,7 @@ static const struct dw_edma_pcie_data xilinx_mdb_data = {
>>>>    	.irqs				= 1,
>>>>    	.wr_ch_cnt			= 8,
>>>>    	.rd_ch_cnt			= 8,
>>>> +	.ch_sep_sz			= 256,
>>>>    };
>>>>
>>>>    static const struct dw_edma_pcie_data xilinx_cpm6_dma_data = {
>>>> @@ -139,6 +141,7 @@ static const struct dw_edma_pcie_data xilinx_cpm6_dma_data = {
>>>>    	.irqs				= 1,
>>>>    	.wr_ch_cnt			= 8,
>>>>    	.rd_ch_cnt			= 8,
>>>> +	.ch_sep_sz			= 512,
>>>>    };
>>>>
>>>>    static void dw_edma_set_chan_region_offset(struct dw_edma_pcie_data *pdata,
>>>> @@ -443,6 +446,7 @@ static int dw_edma_pcie_probe(struct pci_dev *pdev,
>>>>    	chip->nr_irqs = nr_irqs;
>>>>    	chip->ops = &dw_edma_pcie_plat_ops;
>>>>    	chip->cfg_non_ll = non_ll;
>>>> +	chip->ch_sep_sz = vsec_data->ch_sep_sz;
>>>
>>>
>>> This blocks PCI endpoint DMA, this driver is not only used by this.
>>> EPC controller will not set this value, so ch_sep_sz will be 0 for EPC
>>> controller.
>>>
>>> Frank
>>>
>>
>> The EPC driver is using the chip->reg_base + reg_offset for computing
>> the final addresse and not using the register mappings. If the
>> reg_offset are correct then this ch_sep_sz change will not affect the
>> EPC driver.
> 
> show me, how work, both use below method to get channel register address
> 
> __dw_ch_regs () {
> return dw->chip->reg_base + (ch_base * dw->chip->ch_sep_sz)
> }
> 

Yes, the dw_edma_probe() gets called in the EPC driver, I had checked
based on the dw_pcie->edma.reg_base. The ch_sep_sz is required.
I have added the ch_sep_sz as part of dw_pcie_edma_init_data().
Thanks for the suggestion.

-Devendra

> Frank
> 
>>
>>>>
>>>>    	chip->ll_wr_cnt = vsec_data->wr_ch_cnt;
>>>>    	chip->ll_rd_cnt = vsec_data->rd_ch_cnt;
>>>> diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
>>>> index 632abb8b481c..3f27a93c4d8c 100644
>>>> --- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
>>>> +++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
>>>> @@ -23,18 +23,23 @@ enum dw_hdma_control {
>>>>    	DW_HDMA_V0_LLE					= BIT(9),
>>>>    };
>>>>
>>>> -static inline struct dw_hdma_v0_regs __iomem *__dw_regs(struct dw_edma *dw)
>>>> -{
>>>> -	return dw->chip->reg_base;
>>>> -}
>>>> -
>>>>    static inline struct dw_hdma_v0_ch_regs __iomem *
>>>>    __dw_ch_regs(struct dw_edma *dw, enum dw_edma_dir dir, u16 ch)
>>>>    {
>>>> -	if (dir == EDMA_DIR_WRITE)
>>>> -		return &(__dw_regs(dw)->ch[ch].wr);
>>>> -	else
>>>> -		return &(__dw_regs(dw)->ch[ch].rd);
>>>> +	u32 ch_base;
>>>> +
>>>> +	/*
>>>> +	 * For Write, the channel register index starts at
>>>> +	 * wr_base(ch_idx) = (2 * ch_idx) * ch_sep_sz
>>>> +	 *
>>>> +	 * For Read channel,
>>>> +	 * rd_base(ch_idx) = (2 * ch_idx + 1) * ch_sep_sz
>>>> +	 */
>>>> +	ch_base = 2 * ch;
>>>> +	if (dir == EDMA_DIR_READ)
>>>> +		ch_base += 1;
>>>> +
>>>> +	return dw->chip->reg_base + (ch_base * dw->chip->ch_sep_sz);
>>>>    }
>>>>
>>>>    #define SET_CH_32(dw, dir, ch, name, value) \
>>>> diff --git a/drivers/dma/dw-edma/dw-hdma-v0-debugfs.c b/drivers/dma/dw-edma/dw-hdma-v0-debugfs.c
>>>> index dcdc57fe976c..33128685bbd9 100644
>>>> --- a/drivers/dma/dw-edma/dw-hdma-v0-debugfs.c
>>>> +++ b/drivers/dma/dw-edma/dw-hdma-v0-debugfs.c
>>>> @@ -13,22 +13,17 @@
>>>>    #include "dw-hdma-v0-regs.h"
>>>>    #include "dw-edma-core.h"
>>>>
>>>> -#define REGS_ADDR(dw, name)						       \
>>>> -	({								       \
>>>> -		struct dw_hdma_v0_regs __iomem *__regs = (dw)->chip->reg_base; \
>>>> -									       \
>>>> -		(void __iomem *)&__regs->name;				       \
>>>> -	})
>>>> -
>>>>    #define REGS_CH_ADDR(dw, name, _dir, _ch)				       \
>>>>    	({								       \
>>>> -		struct dw_hdma_v0_ch_regs __iomem *__ch_regs;		       \
>>>> +		struct dw_hdma_v0_ch_regs __iomem *__ch_regs;                  \
>>>> +		off_t __off = (dw)->chip->ch_sep_sz;			       \
>>>>    									       \
>>>> -		if (_dir == EDMA_DIR_READ)				       \
>>>> -			__ch_regs = REGS_ADDR(dw, ch[_ch].rd);		       \
>>>> +		if ((_dir) == EDMA_DIR_READ)				       \
>>>> +			__off *= (2 * (_ch) + 1);			       \
>>>>    		else							       \
>>>> -			__ch_regs = REGS_ADDR(dw, ch[_ch].wr);		       \
>>>> +			__off *= (2 * (_ch));				       \
>>>>    									       \
>>>> +		__ch_regs = ((dw)->chip->reg_base + __off);		       \
>>>>    		(void __iomem *)&__ch_regs->name;			       \
>>>>    	})
>>>>
>>>> diff --git a/drivers/dma/dw-edma/dw-hdma-v0-regs.h b/drivers/dma/dw-edma/dw-hdma-v0-regs.h
>>>> index 7759ba9b4850..2475b8d96133 100644
>>>> --- a/drivers/dma/dw-edma/dw-hdma-v0-regs.h
>>>> +++ b/drivers/dma/dw-edma/dw-hdma-v0-regs.h
>>>> @@ -84,16 +84,6 @@ struct dw_hdma_v0_ch_regs {
>>>>    		};
>>>>    	} msi_abort;
>>>>    	u32 msi_msgdata;			/* 0x00a8 */
>>>> -	u32 padding_2[21];			/* 0x00ac..0x00fc */
>>>> -} __packed;
>>>> -
>>>> -struct dw_hdma_v0_ch {
>>>> -	struct dw_hdma_v0_ch_regs wr;		/* 0x0000 */
>>>> -	struct dw_hdma_v0_ch_regs rd;		/* 0x0100 */
>>>> -} __packed;
>>>> -
>>>> -struct dw_hdma_v0_regs {
>>>> -	struct dw_hdma_v0_ch ch[HDMA_V0_MAX_NR_CH];	/* 0x0000..0x0fa8 */
>>>>    } __packed;
>>>>
>>>>    struct dw_hdma_v0_lli {
>>>> diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
>>>> index 1fafd5b0e315..a6c5728f03d3 100644
>>>> --- a/include/linux/dma/edma.h
>>>> +++ b/include/linux/dma/edma.h
>>>> @@ -104,6 +104,7 @@ struct dw_edma_chip {
>>>>
>>>>    	struct dw_edma		*dw;
>>>>    	bool			cfg_non_ll;
>>>> +	u32			ch_sep_sz;
>>>>    };
>>>>
>>>>    /* Export to the platform drivers */
>>>> --
>>>> 2.43.0
>>>>
>>