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

[Andy Shevchenko <andy.shevchenko@xxxxxxxxx>]

On Thu, May 21, 2020 at 3:12 PM Serge Semin
<Sergey.Semin@xxxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxx> 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