Re: [PATCH v2 06/19] spi: dw: Discard static DW DMA slave structures

[Andy Shevchenko]

On Sat, May 16, 2020 at 05:20:30PM +0300, Serge Semin wrote:
> On Fri, May 15, 2020 at 03:34:22PM +0300, Andy Shevchenko wrote:
> > On Fri, May 15, 2020 at 01:47:45PM +0300, Serge Semin wrote:
> > > Having them declared is redundant since each struct dw_dma_chan has
> > > the same structure embedded and the structure from the passed dma_chan
> > > private pointer will be copied there as a result of the next calls
> > > chain:
> > > dma_request_channel() -> find_candidate() -> dma_chan_get() ->
> > > device_alloc_chan_resources() = dwc_alloc_chan_resources() ->
> > > dw_dma_filter().
> > > So just remove the static dw_dma_chan structures and use a locally
> > > declared data instance with dst_id/src_id set to the same values as
> > > the static copies used to have.
> > 
> > ...
> > 
> > > -static struct dw_dma_slave mid_dma_tx = { .dst_id = 1 };
> > > -static struct dw_dma_slave mid_dma_rx = { .src_id = 0 };
> > 
> > > +	struct dw_dma_slave slave = {0};
> > 
> > I really would like to leave them separated and as in the original form, i.e.
> > 
> > 	struct dw_dma_slave tx = { .dst_id = 1 };
> > 	struct dw_dma_slave rx = { .src_id = 0 };
> > 
> > those src and dst IDs are put in that form on purpose...
> 
> As long as you don't tell us what purpose it is, my position won't change.

It's not the way when your changes makes this the older (upstreamed) stuff's
issue, it's an opposite. But I will help you here...

> These structures declared here just hold the static memory and nothing
> else. Allocating them on stack is better.

I'm not talking about stack, it's fine for me, what I'm talking about is *how*
they are being initialized. Read my message again carefully, please.

-- 
With Best Regards,
Andy Shevchenko