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

[Verma, Devendra]

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Frank Li <Frank.li@xxxxxxx>
> Sent: Thursday, March 5, 2026 9:34 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 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.
>

This suggestion is taken. I will push in next version of patches. Thanks!

> >
> > 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, could you please approve the patches if changes look ok to you?

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