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

[Verma, Devendra]

On 05-Jun-26 23:45, Frank Li wrote:
> On Fri, Jun 05, 2026 at 11:48:05AM +0000, Verma, Devendra wrote:
> > Public
> >
> > > -----Original Message-----
> > > From: Frank Li <Frank.li@xxxxxxx>
> > > Sent: Friday, June 5, 2026 01:28
> > > To: Verma, Devendra <Devendra.Verma@xxxxxxx>
> > > Cc: bhelgaas@xxxxxxxxxx; mani@xxxxxxxxxx; vkoul@xxxxxxxxxx;
> > > Frank.Li@xxxxxxxxxx; dmaengine@xxxxxxxxxxxxxxx; linux-
> > > kernel@xxxxxxxxxxxxxxx; Simek, Michal <michal.simek@xxxxxxx>
> > > 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@xxxxxxx>
> > > > ---
> > > > 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

[Reformatted the comment to fit within the visible window]

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 to be
configured shall come from dw->wr_ch_cnt and dw->rd_ch_cnt. In the new
code 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.

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

Removed this fix. Will include it as part of separate series.
I will upload the next version with the above recommendation.

-Devendra