linux-kernel - Re: [RESEND PATCH] dmaengine: dw-edma: Add Simple Mode Support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <2xlz5noopmv72h7yczpubmqrrbjwkn6wd6ehnu3rcqnpkuzw7s@i45jgrbuctpm>
Date: Thu, 31 Jul 2025 19:01:34 +0530
From: Manivannan Sadhasivam <mani@...nel.org>
To: "Verma, Devendra" <Devendra.Verma@....com>
Cc: "dmaengine@...r.kernel.org" <dmaengine@...r.kernel.org>, 
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "vkoul@...nel.org" <vkoul@...nel.org>
Subject: Re: [RESEND PATCH] dmaengine: dw-edma: Add Simple Mode Support

On Thu, Jul 31, 2025 at 09:32:12AM GMT, Verma, Devendra wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
> 
> Hi Manivannan
> 
> Please check my response inline.
> 
> Regards,
> Devendra
> 
> > -----Original Message-----
> > From: Manivannan Sadhasivam <mani@...nel.org>
> > Sent: Wednesday, July 23, 2025 19:26
> > To: Verma, Devendra <Devendra.Verma@....com>
> > Cc: dmaengine@...r.kernel.org; linux-kernel@...r.kernel.org; vkoul@...nel.org
> > Subject: Re: [RESEND PATCH] dmaengine: dw-edma: Add Simple Mode Support
> >
> > Caution: This message originated from an External Source. Use proper caution
> > when opening attachments, clicking links, or responding.
> >
> >
> > On Mon, Jun 23, 2025 at 11:47:33AM GMT, Devendra K Verma wrote:
> > > The HDMA IP supports the simple mode (non-linked list).
> > > In this mode the channel registers are configured to initiate a single
> > > DMA data transfer. The channel can be configured in simple mode via
> > > peripheral param of dma_slave_config param.
> > >
> > > Signed-off-by: Devendra K Verma <devverma@....com>
> > > ---
> > >  drivers/dma/dw-edma/dw-edma-core.c    | 10 +++++
> > >  drivers/dma/dw-edma/dw-edma-core.h    |  2 +
> > >  drivers/dma/dw-edma/dw-hdma-v0-core.c | 53
> > ++++++++++++++++++++++++++-
> > >  include/linux/dma/edma.h              |  8 ++++
> > >  4 files changed, 72 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/dma/dw-edma/dw-edma-core.c
> > > b/drivers/dma/dw-edma/dw-edma-core.c
> > > index c2b88cc99e5d..4dafd6554277 100644
> > > --- a/drivers/dma/dw-edma/dw-edma-core.c
> > > +++ b/drivers/dma/dw-edma/dw-edma-core.c
> > > @@ -235,9 +235,19 @@ static int dw_edma_device_config(struct dma_chan
> > *dchan,
> > >                                struct dma_slave_config *config)  {
> > >       struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
> > > +     struct dw_edma_peripheral_config *pconfig = config->peripheral_config;
> > > +     unsigned long flags;
> > > +
> > > +     if (WARN_ON(config->peripheral_config &&
> > > +                 config->peripheral_size != sizeof(*pconfig)))
> > > +             return -EINVAL;
> > >
> > > +     spin_lock_irqsave(&chan->vc.lock, flags);
> > >       memcpy(&chan->config, config, sizeof(*config));
> > > +
> > > +     chan->non_ll_en = pconfig ? pconfig->non_ll_en : false;
> >
> > Who is allocating 'dw_edma_peripheral_config' and setting 'non_ll_en' flag? We
> > cannot introduce a flag without anyone using it in upstream.
> >
> > - Mani
> >
> 
> The caller of the DMA engine client APIs will be responsible for allocating
> and setting the non_ll_en flag. This flag helps in setting the channel in to
> non-Linked-List mode. This functionality is supported by the DMA controller.
> The peripheral_config is just configuring the peripheral (DMA) in to the modes
> it supports based on user discretion. As this functionality is used by the
> clients to differentiate between the modes supported by the controller it
> may not require the equivalent code in upstream.
> 

As I stated above, we *must not* have an unused interface in upstream drivers. I
understand that fact that your downstream driver is setting this flag and you
want to keep your driver downstream. But the flag would be unused in upstream
and that is not acceptable, sorry.

- Mani


-- 
மணிவண்ணன் சதாசிவம்