linux-kernel - Re: [PATCH v3 03/16] spi: dw: Discard static DW DMA slave structures

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>] [day] [month] [year] [list]

Message-ID: <CAHp75VcPSBBZu+bhnpojry3SFK7-qF7Tuif2MnsPEfbMf72_JA@mail.gmail.com>
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