Re: [PATCH v3] dmaengine: dw-edma: Enable HDMA 64R/W Channels

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

On Fri, Jun 05, 2026 at 11:48:05AM +0000, Verma, Devendra wrote:
> Public
>
> > -----Original Message-----
> > From: Frank Li <Frank.li@nxp.com>
> > Sent: Friday, June 5, 2026 01:28
> > To: Verma, Devendra <Devendra.Verma@amd.com>
> > Cc: bhelgaas@google.com; mani@kernel.org; vkoul@kernel.org;
> > Frank.Li@kernel.org; dmaengine@vger.kernel.org; linux-
> > kernel@vger.kernel.org; Simek, Michal <michal.simek@amd.com>
> > Subject: Re: [PATCH v3] dmaengine: dw-edma: Enable HDMA 64R/W
> > Channels
> >
> > On Wed, Jun 03, 2026 at 08:11:47PM +0530, Devendra K Verma wrote:
> > > As per 'Designware Cores PCI Express Controller Databook', Section 7.1
> > > - Overview, HDMA supports 64 Read and 64 Write channels. Current
> > > controller driver supports up to 8 read and write channels only. In
> > > order to utilize all the channels the controller driver need to have
> > > the channel related structs and variables as per the number of
> > > channels supported by IP.
> > > Following changes are made to enable 64 Read / 64 Write channel
> > > support:
> > >
> > >  o Defined HDMA specific macros to reflect the channel count.
> > >  o The count of ll_regions and dt_regions in dw_edma_chip and
> > >    dw_edma_pcie_data shall be in accordance to number of read
> > >    and write channels.
> > >  o In dw_edma_probe() configure the channels as per the channels
> > >    of the IP used.
> > >  o Changed mask types to u64 for higher channel counts.
> > >
> > > Signed-off-by: Devendra K Verma <devendra.verma@amd.com>
> > > ---
> > > Changes in v2:
> > >   o Fixed the pre-existing bug related to GET_CH_32
> > >     interchanging the channel direction and id.
> > >     This bug was not caused by any version of this patch.
> > >   o Fixed the issue when using for_each_set_bit() for mask
> > >     of u64 type.
> > >
> > > Changes in v1:
> > >   o On review recommendation of sashiko bot, in the function
> > >     dw_hdma_v0_core_off(), the loop iterates over registers
> > >     as per the number of channels enabled and not on total
> > >     number of channels supported.
> > >   o Changed mask types to u64 for higher channel counts.
> > > ---
> > ...
> > > +++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> > > @@ -53,13 +53,24 @@ __dw_ch_regs(struct dw_edma *dw, enum
> > dw_edma_dir
> > > dir, u16 ch)  static void dw_hdma_v0_core_off(struct dw_edma *dw)  {
> > >     int id;
> > > +   enum dw_edma_dir dir;
> > > +
> > > +   dir = EDMA_DIR_WRITE;
> > > +   for (id = 0; id < dw->wr_ch_cnt; id++) {
> > > +           SET_CH_32(dw, dir, id, int_setup,
> > > +                     HDMA_V0_STOP_INT_MASK |
> > HDMA_V0_ABORT_INT_MASK);
> > > +           SET_CH_32(dw, dir, id, int_clear,
> > > +                     HDMA_V0_STOP_INT_MASK |
> > HDMA_V0_ABORT_INT_MASK);
> > > +           SET_CH_32(dw, dir, id, ch_en, 0);
> > > +   }
> > >
> > > -   for (id = 0; id < HDMA_V0_MAX_NR_CH; id++) {
> > > -           SET_BOTH_CH_32(dw, id, int_setup,
> > > -                          HDMA_V0_STOP_INT_MASK |
> > HDMA_V0_ABORT_INT_MASK);
> > > -           SET_BOTH_CH_32(dw, id, int_clear,
> > > -                          HDMA_V0_STOP_INT_MASK |
> > HDMA_V0_ABORT_INT_MASK);
> > > -           SET_BOTH_CH_32(dw, id, ch_en, 0);
> > > +   dir = EDMA_DIR_READ;
> > > +   for (id = 0; id < dw->rd_ch_cnt; id++) {
> > > +           SET_CH_32(dw, dir, id, int_setup,
> > > +                     HDMA_V0_STOP_INT_MASK |
> > HDMA_V0_ABORT_INT_MASK);
> > > +           SET_CH_32(dw, dir, id, int_clear,
> > > +                     HDMA_V0_STOP_INT_MASK |
> > HDMA_V0_ABORT_INT_MASK);
> > > +           SET_CH_32(dw, dir, id, ch_en, 0);
> >
> > why SET_BOTH_CH_32 not work for 64 channel?
> >
>
> SET_BOTH_CH_32 works, but this needs to be done on the channels enabled for the IP.
> HDMA supports maximum of 64 channels. So if some IP enables 8 or fewer read / write channels only then the number of channels come from dw->wr_ch_cnt and dw->rd_ch_cnt. Now the logic is derived by individual read & write enabled channel count. Earlier, it was assumed that user will enable max of 8 channels which would have worked well using SET_BOTH_CH_32() but as the channels grow, the assumption that equal number of read / write channels and that they are set to max count are enabled might not hold true.

Make sense, please wrap your reply, it is hard to read

>
> - Devendra
>
> > >     }
> > >  }
> > >
> > > @@ -79,7 +90,7 @@ static enum dma_status
> > dw_hdma_v0_core_ch_status(struct dw_edma_chan *chan)
> > >     u32 tmp;
> > >
> > >     tmp = FIELD_GET(HDMA_V0_CH_STATUS_MASK,
> > > -                   GET_CH_32(dw, chan->id, chan->dir, ch_stat));
> > > +                   GET_CH_32(dw, chan->dir, chan->id, ch_stat));
> >
> > why need swtich id and dir here ?
> >
> > Frank
>
> This is the correct order of arguments to the GET_CH_32. The second & third arguments shall be direction and channel_id respectively. It is a pre-existing issue reported by AI bot.

AI found existing problem, need seperate patch to fix it.

Frank