RE: [PATCH v7 2/2] dmaengine: dw-edma: Add non-LL mode

[Verma, Devendra]

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Bjorn Helgaas <helgaas@xxxxxxxxxx>
> Sent: Friday, December 12, 2025 11:52 PM
> To: Verma, Devendra <Devendra.Verma@xxxxxxx>
> Cc: bhelgaas@xxxxxxxxxx; mani@xxxxxxxxxx; vkoul@xxxxxxxxxx;
> dmaengine@xxxxxxxxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; Simek, Michal <michal.simek@xxxxxxx>
> Subject: Re: [PATCH v7 2/2] dmaengine: dw-edma: Add non-LL mode
>
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
>
>
> On Fri, Dec 12, 2025 at 05:50:56PM +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.
> > ...
>
> > +++ b/drivers/dma/dw-edma/dw-edma-core.c
> > @@ -223,8 +223,31 @@ static int dw_edma_device_config(struct
> dma_chan *dchan,
> >                                struct dma_slave_config *config)  {
> >       struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
> > +     int non_ll = 0;
>
> Other "non_ll" uses below are bool, so a little bit of an int/bool mix.
>
> The name also leads to a lot of double negative use ("!non_ll", etc), which is
> hard to read.  I suppose "non-LL" corresponds to some spec language, but it
> would be nice if we could avoid some of the negation by testing for "ll_mode"
> or calling the other mode "single_burst" or similar.
>

Yes, non-LL is being referred in the Synosys databook extensively to differentiate
between LL and non-LL mode.
I agree with the concern raised here but, at the moment, this is the only suitable
term that can handle the following cases:
1) Choice of variable of the DMA client to use non-LL mode,
2) Establish flow for the non-LL use-case in the driver.

Before, using the term with negation (non_ll), the possibility was explored
to use a term which does not result in double negation, eg, ll or ll_mode.
But this again breaks the above either one or both use-cases.
If using ll_mode as a variable, then with this, DMA client shall
either provide ll_mode=false or non_ll=true.

When ll_mode=false. This option would be as good as
passing a valid reference to peripheral_config pointer as
the value of ll_mode would never be used for ll_mode=true
due to default mode being LL.
On the basis of ll_mode=true, the DMA client given option, no code
is impacted with these patches.

When DMA client gives non_ll=true; this causes confusion,
DMA client does right but internally ll_mode as a variable is set
to establish the flow for non-LL mode. The different variable is
used for establishing the non-LL mode inside the driver code.
Also, it uses the combination of double negation variable.

Though, the use of non_ll, raises concern due to double
negation but its use fits the use-case from both DMA client
and in the driver to establish the non-LL flow. Additionally,
The use of non_ll also aligns with the documentation of the
vendor making it easier to follow.
Please let me know your thoughts on this.

> > +      * When there is no valid LLP base address available then the default
> > +      * DMA ops will use the non-LL mode.
> > +      * Cases where LL mode is enabled and client wants to use the non-LL
> > +      * mode then also client can do so via providing the peripheral_config
> > +      * param.
>
> Add blank line between paragraphs.

Sure, will take care of this.

Regards,
Devendra