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

[Frank Li]

On Thu, Mar 05, 2026 at 12:15:41PM +0000, Verma, Devendra wrote:
> [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.

When read "a->non_ll", need good back check what type of a to know which
one.  if use difference name
	a->non_ll;
	b->cfg_no_ll;

It will not think more about what is a/b.

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

I don't want to take more time at this kind small stuff. I am fine if
vnod Or mani (who pick up these patches) think it is okay.

Frank

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