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  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [day] [month] [year] [list]
Date:   Thu, 21 May 2020 15:49:10 +0300
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Serge Semin <Sergey.Semin@...kalelectronics.ru>
Cc:     Serge Semin <fancer.lancer@...il.com>,
        Mark Brown <broonie@...nel.org>,
        Georgy Vlasov <Georgy.Vlasov@...kalelectronics.ru>,
        Ramil Zaripov <Ramil.Zaripov@...kalelectronics.ru>,
        Alexey Malahov <Alexey.Malahov@...kalelectronics.ru>,
        Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        Paul Burton <paulburton@...nel.org>,
        Ralf Baechle <ralf@...ux-mips.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Arnd Bergmann <arnd@...db.de>,
        Rob Herring <robh+dt@...nel.org>, linux-mips@...r.kernel.org,
        devicetree <devicetree@...r.kernel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@...el.com>,
        Jarkko Nikula <jarkko.nikula@...ux.intel.com>,
        Clement Leger <cleger@...ray.eu>,
        linux-spi <linux-spi@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 03/16] spi: dw: Discard static DW DMA slave structures

On Thu, May 21, 2020 at 3:12 PM Serge Semin
<Sergey.Semin@...kalelectronics.ru> wrote:
> On Thu, May 21, 2020 at 12:57:17PM +0300, Andy Shevchenko wrote:
> > On Thu, May 21, 2020 at 4:23 AM Serge Semin
> > <Sergey.Semin@...kalelectronics.ru> wrote:

...

> > Thanks for an update, but that's not what I asked for...
> >
> > > -static struct dw_dma_slave mid_dma_tx = { .dst_id = 1 };
> > > -static struct dw_dma_slave mid_dma_rx = { .src_id = 0 };
> >
> > >  static int mid_spi_dma_init_mfld(struct device *dev, struct dw_spi *dws)
> > >  {
> > > +       struct dw_dma_slave slave = {
> > > +               .src_id = 0,
> > > +               .dst_id = 0
> > > +       };

> > > -       struct dw_dma_slave *tx = dws->dma_tx;
> > > -       struct dw_dma_slave *rx = dws->dma_rx;
> >
> > May we simple do
> >
> > struct dw_dma_slave tx = { .dst_id = 1 };
> > struct dw_dma_slave rx = { .src_id = 0 };
> >
> > please?
>
> Well, for me both solutions are equal

I don't think so.

> except mine consumes less stack memory.

And brought confusion and less readability. :-(

> The only reason why your solution might be better is that if DW DMA driver or
> the DMA engine subsystem changed the dw_dma_slave structure instance passed to
> the dma_request_channel() method, which non of them do. So I'll leave this for
> Mark to decide. Mark, could you give us your final word about this?

I explained already why I prefer to see them in that form. Reader can
easily understand what request line is used for what channel.
In your code it's hidden somewhere and on top of that that _one_
structure on the stack adds more confusion.

-- 
With Best Regards,
Andy Shevchenko

Powered by blists - more mailing lists