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

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

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)
}

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