Re: [PATCH v2 1/2] spi: dw: Make DMA request line assignments explicit for Intel Medfield

[Serge Semin <fancer.lancer@xxxxxxxxx>]

On Fri, May 29, 2020 at 09:49:55PM +0300, Andy Shevchenko wrote:
> On Fri, May 29, 2020 at 09:40:50PM +0300, Serge Semin wrote:
> > On Fri, May 29, 2020 at 09:31:49PM +0300, Andy Shevchenko wrote:
> > > The 2afccbd283ae ("spi: dw: Discard static DW DMA slave structures")
> > > did a clean up of global variables, which is fine, but messed up with
> > > the carefully provided information in the custom DMA slave structures.
> > > There reader can find an assignment of the DMA request lines in use.
> > > 
> > > Partially revert the above mentioned commit to restore readability
> > > and maintainability of the code.
> > > 
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> > > ---
> > > v2: rebased against latest spi/for-next
> > >  drivers/spi/spi-dw-dma.c | 14 ++++++--------
> > >  1 file changed, 6 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/drivers/spi/spi-dw-dma.c b/drivers/spi/spi-dw-dma.c
> > > index 1b96cec6d8cd..53d5257662e8 100644
> > > --- a/drivers/spi/spi-dw-dma.c
> > > +++ b/drivers/spi/spi-dw-dma.c
> > > @@ -61,10 +61,8 @@ static void dw_spi_dma_maxburst_init(struct dw_spi *dws)
> > >  
> > >  static int dw_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 dma_tx = { .dst_id = 1 }, *tx = &dma_tx;
> > > +	struct dw_dma_slave dma_rx = { .src_id = 0 }, *rx = &dma_rx;
> > 


> > You know my attitude to these changes.) But anyway what's the point in having
> > the *tx and *rx pointers here? Without any harm to the readability you can use
> > the structures names directly, don't you?
> 
> I will wait for Mark to decide.

So no response to a review comment? Shall I do the same when get a review from
you?.)

I am not asking about the whole patch purpose. You know what I think about it.
My question was about why *tx and *rx pointers are required? Just wondering, I
may misunderstand something... As I see it you could use dma_tx and dma_rx here
directly with the same level of readability.

-Sergey
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
>