Re: [PATCH v15 2/2] dmaengine: dw-edma: Add non-LL mode

[Koichiro Den]

On Wed, Jun 17, 2026 at 08:51:43AM -0500, Frank Li wrote:
> On Wed, Jun 17, 2026 at 05:13:32PM +0530, Verma, Devendra wrote:
> > [You don't often get email from devverma@xxxxxxx. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> >
> > Hi Koichiro
> >
> > My first reply was auto-formatted as per the column limit but got
> > expanded after I sent it.
> > Re-sending the reply with correct formatting.
> >
> > Please excuse for the spamming!
> > regards,
> > Devendra
> >
> > On 17-Jun-26 08:47, Koichiro Den wrote:
> > > On Wed, Mar 18, 2026 at 12:34:03PM +0530, Devendra K Verma wrote:
> > > > AMD MDB IP supports Linked List (LL) mode as well as non-LL mode.
> > > > The current code does not have the mechanisms to enable the
> > > > DMA transactions using the non-LL mode. The following two cases
> > > > are added with this patch:
> > > > - For the AMD (Xilinx) only, when a valid physical base address of
> > > >    the device side DDR is not configured, then the IP can still be
> > > >    used in non-LL mode. For all the channels DMA transactions will
> > > >    be using the non-LL mode only. This, the default non-LL mode,
> > > >    is not applicable for Synopsys IP with the current code addition.
> > > >
> > > > - If the default mode is LL-mode, for both AMD (Xilinx) and Synosys,
> > > >    and if user wants to use non-LL mode then user can do so via
> > > >    configuring the peripheral_config param of dma_slave_config.
> > > >
> > > > Signed-off-by: Devendra K Verma <devendra.verma@xxxxxxx>
> > > > Reviewed-by: Frank Li <Frank.Li@xxxxxxx>
> > > > ---
> > > > Changes in v15
> > > >     Rebased the branch
> > > >
> > > [snip]
> > > > +
> > > > +static void dw_hdma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
> > > > +{
> > > > +    struct dw_edma_chan *chan = chunk->chan;
> > > > +
> > > > +    if (chan->non_ll)
> > >
> > > Hi Devendra (cc: Frank),
> > >
> > > Sorry for dropping a comment now that this has already landed.
> > >
> > > I'm wondering about the lifetime of chan->non_ll. This patch lets a client
> > > select non-LL mode through dma_slave_config.peripheral_config for a transfer,
> > > but the state is stored on the channel.
> > >
> > > We use chan->non_ll in prep to choose bursts_max, then read it again later in
> > > dw_hdma_v0_core_start() to choose the LL vs non-LL start path. If the channel is
> > > reconfigured between prep and start, or before a later chunk is started from the
> > > interrupt path, couldn't we start a descriptor in a different mode from the one
> > > it was prepared for?
> > >
> >
> > The mode is implemented with the intention that after prep, the
> > submitted descriptor shall completed with the chosen mode. So, yes the
> > mode is decided in the prep call and all the subsequent descriptors are
> > completed with the chosen mode unless it is overridden by another prep
> > call.
> >
> > > (Note: Frank's not-yet-merged dma_prep_config v7 series [1] also looks at
> > > potential races around config+prep on the same channel from multiple process
> > > contexts, as I understand it. But this seems like a separate issue, since the
> > > state is read again at transfer start time.)
> > >
> > > Should non_ll be snapshotted into the descriptor/chunk, maybe as
> > > dw_edma_desc.non_ll, or is the rule that clients must not reconfigure the
> > > channel while anything is pending/running?
> > >
> >
> > I am not aware of any such rule which specifies that modes can not be
> > mixed but it would not be a good idea to mix both. Let me give an
> > example, in the non-LL mode the channels *can* utilize the LL-regions
> > for data transfers. If for such a non-LL data transfer where LL-region
> > is used and intended by the user then changing the mode after setting up
> > the mode to another one can cause data corruption.
> >
> > Eg:
> > Channel LL-region = ADDR
> > Mode set to non-LL -> DDR destination to ADDR to (ADDR + size)
> > First non-LL burst -> writes data to ADDR till size bytes.
> > Second burst configured for LL -> overwrites the data at ADDR with
> > descriptor information.
> 
> I don't think we need mix LL and no-LL, previously config suppose only do
> once before prepare and never change again.
> 
> But recently there are more user case to config for each prep. After [1]
> merge, we can consider more. There should be no risk now if DMA consumer
> don't change it during prepare.
> 
> Frank

Devendra, thanks for your quick response. That seems to confirm my concern.
Whether we keep that as an implicit assumption, or make the driver robust
against it, is probably a subsystem/maintainer call. I do not intend to push
this further for now.

Frank, thanks for the follow-up. Besides [1], I suspect that [2] may also affect
the eventual implementation if we decide to make it robust.

Anyway, thanks both for the quick clarification! Let's revisit this when [1]
and/or [2] settles.

[2] https://lore.kernel.org/dmaengine/20260109-edma_ll-v2-0-5c0b27b2c664@xxxxxxx/

Best regards,
Koichiro

> 
> >
> > This one causes the data corruption for the first burst
> >
> >
> > > Or was this already discussed, and there is some implicit restriction that
> > > clients must not mix LL and temporary non-LL requests from multiple contexts on
> > > the same channel?
> > >
> > > [1] https://lore.kernel.org/dmaengine/20260521-dma_prep_config-v7-0-1f73f4899883@xxxxxxx/
> > >
> > > Thanks,
> > > Koichiro
> > >
> > > > +            dw_hdma_v0_core_non_ll_start(chunk);
> > > > +    else
> > > > +            dw_hdma_v0_core_ll_start(chunk, first);
> > > > +}
> > > > +
> > > >   static void dw_hdma_v0_core_ch_config(struct dw_edma_chan *chan)
> > > >   {
> > > >      struct dw_edma *dw = chan->dw;
> > > > diff --git a/drivers/dma/dw-edma/dw-hdma-v0-regs.h b/drivers/dma/dw-edma/dw-hdma-v0-regs.h
> > > > index eab5fd7177e5..7759ba9b4850 100644
> > > > --- a/drivers/dma/dw-edma/dw-hdma-v0-regs.h
> > > > +++ b/drivers/dma/dw-edma/dw-hdma-v0-regs.h
> > > > @@ -12,6 +12,7 @@
> > > >   #include <linux/dmaengine.h>
> > > >
> > > >   #define HDMA_V0_MAX_NR_CH                  8
> > > > +#define HDMA_V0_CH_EN                               BIT(0)
> > > >   #define HDMA_V0_LOCAL_ABORT_INT_EN         BIT(6)
> > > >   #define HDMA_V0_REMOTE_ABORT_INT_EN                BIT(5)
> > > >   #define HDMA_V0_LOCAL_STOP_INT_EN          BIT(4)
> > > > diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
> > > > index 270b5458aecf..61d6064fcfed 100644
> > > > --- a/include/linux/dma/edma.h
> > > > +++ b/include/linux/dma/edma.h
> > > > @@ -97,6 +97,7 @@ struct dw_edma_chip {
> > > >      enum dw_edma_map_format mf;
> > > >
> > > >      struct dw_edma          *dw;
> > > > +    bool                    cfg_non_ll;
> > > >   };
> > > >
> > > >   /* Export to the platform drivers */
> > > > --
> > > > 2.43.0
> > > >
> >