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

[Verma, Devendra]

[Public]

> -----Original Message-----
> From: Frank Li <Frank.li@xxxxxxx>
> Sent: Wednesday, March 4, 2026 10:26 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 RESEND v10 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 Wed, Feb 25, 2026 at 12:06:12PM +0000, Verma, Devendra wrote:
> > [AMD Official Use Only - AMD Internal Distribution Only]
> >
> > > -----Original Message-----
> > > From: Frank Li <Frank.li@xxxxxxx>
> > > Sent: Wednesday, February 25, 2026 3:58 AM
> > > 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 RESEND v10 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 Mon, Feb 23, 2026 at 04:40:07PM +0000, Verma, Devendra wrote:
> > > > [AMD Official Use Only - AMD Internal Distribution Only]
> > > >
> > > > > -----Original Message-----
> > > > > From: Frank Li <Frank.li@xxxxxxx>
> > > > > Sent: Friday, February 20, 2026 9:33 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 RESEND v10 2/2] dmaengine: dw-edma: Add
> > > > > non-LL mode
> > > > >
> > > ...
> > > > > > But if it about writing a new function to check the LL mode
> > > > > > support then I think the current variable is good enough which
> > > > > > provides good readability and do not create any ambiguity
> > > > > > compared to the ll region size
> > > > > comparison.
> > > > >
> > > > > It is not big deal,  use 'bool cap_non_ll: 1' in dw_edma_chip.
> > > > > So we add more cap flags in future.
> > > > >
> > > > > Frank
> > > > >
> > > >
> > > > Hi Frank, could you elaborate what you mean by adding the cap flag?
> > > > How it is going To help identify the overall chip state?
> > > > I do not understand what is being implied here.
> > >
> > > non_ll in chan means current status, which indicate one channel work
> > > at non_ll mode or ll mode.
> > >
> > > here dw_edma_chip means hardware's captiblity, indicate if hardware
> > > support ll mode.
> > >
> > > Distingiush hardware limition or current working mode.
> > >
> > > Frank
> >
> > Thanks for the explanation!
> > Hardware supports the LL mode / non-LL mode, just that there is no
> > piece of code available which can perform the non-LL mode as only one
> > mode was supported initially by the respective developers.
> > So, providing it as capability does not look justified as in any
> > scenario hardware is capable of non-LL mode. Theoretically, non-LL
> > mode should have been the default mode.
> >
> > The non-LL mode is not a hardware limitation either. LL mode needs
> > extra configurations and in the absence of that, interpretation could
> > be, enable the supported other mode which is non-LL mode.
>
> Yes, that's reason why I don't want to add non-ll in dw_edma_chip, which
> should provide hardware's information.  non-ll actually miss ll_region
> information.
>

I think, non_ll can be interpreted without using the ll-region related information
as well. My view regarding the dw_edma_chip struct is slightly different,
it does not provide the hardware capability rather stores a snapshot of
configuration based on information provided by different means,
please take a look at my comment below related to this.

> >
> > With the current non_ll inside the dw_edma_chip, when non_ll = false,
> > indicates It supports both the modes LL and non-LL, but requires user
> inputs to enable it.
> > With non_ll = true, the dw_edma_chip or the hardware has no choice but
> > to work in non-LL mode only. This is the interpretation for the flag in non_ll.
>
> we need distingiush current state and HW/SW captiblity. in dma_chan, non_ll
> means current working state.
>
> but the same words 'non_ll' in dw_edma_chip is HW/SW capablity.
>
> dma_chan: non_ll       means current channel use LL OR non LL.
> dma_edma_chips: non_ll means only support non LL mode OR both.
>
> The same words "non_ll" means difference. We should try to avoid this case.
>
> if you want to add field in dw_edma_chip, avoid use the same words because
> their means is difference.
>
> Frank

Can we please simplify this interpretation, the non_ll in all the scenarios should mean non-LL mode
only if set to true.
dw_edma_chip : non_ll = true, it shall mean that all the channel, at chip level, can work in non-LL mode ONLY.
dw_edma_chan: non_ll = true, it shall mean that individual channel is configured for a transaction in non-LL mode.

Above all, a nice comment related to the flag shall be good enough to make the understanding clear, at the
places where declared.
Since the beginning my emphasis is that 'non_ll' flag should be treated for what it implies, i.e non-LL mode.
It was included in two different sets of structs to show the hierarchy how it could affect the overall functionality
depending upon where 'non_ll' is set to true.
Coming to the dw_edma_chip struct, I do not understand why the dw_edma_chip struct is about
hardware capability, it is more about the configuration of the chip which is filled anyway at the time
of probe() function calling. This struct does not provide any capability information at the time of probe() calling
rather it is filled based on the params configured by user either as static info (eg: snps_edda_data) or by reading
the capability registers (eg: VSEC and channels enabled by reading config space).
I hope this clears the doubt. Please let me know if any further information required related to the non_ll flag
Interpretation.

Regards,
Devendra

> >
> > With the capability, would it not make the statement, that if non_ll =
> > true, it supports non-LL mode but that does not mean to be mutually
> > exclusive and not support LL mode at the same time?
> > If there is a requirement regarding the capability then it can be
> > taken as a separate update but I am not sure what purpose it can serve wrt
> non-LL functionality.
> > Please let me know your thoughts on this and lets conclude.
>
> >
> > Thanks!
> >
> > > >
> > > > - Regards,
> > > > Devendra
> > > >
> > > > > >
> > > > > > > Frank
> > > > > > > >
> > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > > > Frank
> > > > > > > > > > > > > >  };
> > > > > > > > > > > > > >
> > > > > > > > > > > > > >  /* Export to the platform drivers */
> > > > > > > > > > > > > > --
> > > > > > > > > > > > > > 2.43.0
> > > > > > > > > > > > > >