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: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?

-Sergey

>  	struct pci_dev *dma_dev;
>  	dma_cap_mask_t mask;
>  
> @@ -80,14 +78,14 @@ static int dw_spi_dma_init_mfld(struct device *dev, struct dw_spi *dws)
>  	dma_cap_set(DMA_SLAVE, mask);
>  
>  	/* 1. Init rx channel */
> -	slave.dma_dev = &dma_dev->dev;
> -	dws->rxchan = dma_request_channel(mask, dw_spi_dma_chan_filter, &slave);
> +	rx->dma_dev = &dma_dev->dev;
> +	dws->rxchan = dma_request_channel(mask, dw_spi_dma_chan_filter, rx);
>  	if (!dws->rxchan)
>  		goto err_exit;
>  
>  	/* 2. Init tx channel */
> -	slave.dst_id = 1;
> -	dws->txchan = dma_request_channel(mask, dw_spi_dma_chan_filter, &slave);
> +	tx->dma_dev = &dma_dev->dev;
> +	dws->txchan = dma_request_channel(mask, dw_spi_dma_chan_filter, tx);
>  	if (!dws->txchan)
>  		goto free_rxchan;
>  
> -- 
> 2.26.2
>