Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1526378 > unrolled thread

[PATCH v2 2/4] spi: spi-fsl-dspi: Fix continuous selection format

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v2 2/4] spi: spi-fsl-dspi: Fix continuous selection format Sanchayan Maity <maitysanchayan@gmail.com> - 2016-11-21 07:10 +0100
    Re: [PATCH v2 2/4] spi: spi-fsl-dspi: Fix continuous selection format Stefan Agner <stefan@agner.ch> - 2016-11-22 00:30 +0100
      Re: [PATCH v2 2/4] spi: spi-fsl-dspi: Fix continuous selection format maitysanchayan@gmail.com - 2016-11-22 07:30 +0100

#1526378 — [PATCH v2 2/4] spi: spi-fsl-dspi: Fix continuous selection format

Current DMA implementation was not handling the continuous selection
format viz. SPI chip select would be deasserted even between sequential
serial transfers. Use the cs_change variable and correctly set or
reset the CONT bit accordingly for case where peripherals require
the chip select to be asserted between sequential transfers.

Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
---
 drivers/spi/spi-fsl-dspi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index b1ee1f5..41422cd 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -261,6 +261,8 @@ static int dspi_next_xfer_dma_submit(struct fsl_dspi *dspi)
 	dspi->dma->tx_dma_buf[i] = SPI_PUSHR_TXDATA(val) |
 					SPI_PUSHR_PCS(dspi->cs) |
 					SPI_PUSHR_CTAS(0);
+	if (!dspi->cs_change)
+		dspi->dma->tx_dma_buf[i] |= SPI_PUSHR_CONT;
 	dspi->tx += tx_word + 1;
 
 	dma->tx_desc = dmaengine_prep_slave_single(dma->chan_tx,
-- 
2.10.2

[toc] | [next] | [standalone]

#1527128

On 2016-11-20 21:54, Sanchayan Maity wrote:
> Current DMA implementation was not handling the continuous selection
> format viz. SPI chip select would be deasserted even between sequential
> serial transfers. Use the cs_change variable and correctly set or
> reset the CONT bit accordingly for case where peripherals require
> the chip select to be asserted between sequential transfers.
> 
> Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
> ---
>  drivers/spi/spi-fsl-dspi.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
> index b1ee1f5..41422cd 100644
> --- a/drivers/spi/spi-fsl-dspi.c
> +++ b/drivers/spi/spi-fsl-dspi.c
> @@ -261,6 +261,8 @@ static int dspi_next_xfer_dma_submit(struct fsl_dspi *dspi)
>  	dspi->dma->tx_dma_buf[i] = SPI_PUSHR_TXDATA(val) |
>  					SPI_PUSHR_PCS(dspi->cs) |
>  					SPI_PUSHR_CTAS(0);
> +	if (!dspi->cs_change)
> +		dspi->dma->tx_dma_buf[i] |= SPI_PUSHR_CONT;
>  	dspi->tx += tx_word + 1;
>  
>  	dma->tx_desc = dmaengine_prep_slave_single(dma->chan_tx,

Other transfer mode use:

if ((dspi->cs_change) && (!dspi->len))                                  
                                                           
        dspi_pushr &= ~SPI_PUSHR_CONT;

which indicates that they only clear SPI_PUSHR_CONT at the very end of a
transfer... The DMA code currently deselects after every DMA transfer if
dspi->cs_change is set.

Maybe we should use the helper dspi_data_to_pushr to fill the DMA buffer
and _clear_ SPI_PUSHR_CONT if necessary like the other transfer modes
do... Then we can use the for loop to fill the complete buffer and get
rid of some code dupplication.

I see that dspi_data_to_pushr does move len too, which we did not in the
DMA case. dspi->len gets incremented only on successful DMA transfer in
dspi_dma_xfer. However, I wonder if that is not even a bug: We increment
dspi->tx always, but len only on success. This makes len go off sync
with regards to the tx pointer which does not help anybody. So lets get
rid of the update code in dspi_dma_xfer

--
Stefan

[toc] | [prev] | [next] | [standalone]

#1527251

On 16-11-21 15:15:41, Stefan Agner wrote:
> On 2016-11-20 21:54, Sanchayan Maity wrote:
> > Current DMA implementation was not handling the continuous selection
> > format viz. SPI chip select would be deasserted even between sequential
> > serial transfers. Use the cs_change variable and correctly set or
> > reset the CONT bit accordingly for case where peripherals require
> > the chip select to be asserted between sequential transfers.
> > 
> > Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
> > ---
> >  drivers/spi/spi-fsl-dspi.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
> > index b1ee1f5..41422cd 100644
> > --- a/drivers/spi/spi-fsl-dspi.c
> > +++ b/drivers/spi/spi-fsl-dspi.c
> > @@ -261,6 +261,8 @@ static int dspi_next_xfer_dma_submit(struct fsl_dspi *dspi)
> >  	dspi->dma->tx_dma_buf[i] = SPI_PUSHR_TXDATA(val) |
> >  					SPI_PUSHR_PCS(dspi->cs) |
> >  					SPI_PUSHR_CTAS(0);
> > +	if (!dspi->cs_change)
> > +		dspi->dma->tx_dma_buf[i] |= SPI_PUSHR_CONT;
> >  	dspi->tx += tx_word + 1;
> >  
> >  	dma->tx_desc = dmaengine_prep_slave_single(dma->chan_tx,
> 
> Other transfer mode use:
> 
> if ((dspi->cs_change) && (!dspi->len))                                  
>                                                            
>         dspi_pushr &= ~SPI_PUSHR_CONT;
> 
> which indicates that they only clear SPI_PUSHR_CONT at the very end of a
> transfer... The DMA code currently deselects after every DMA transfer if
> dspi->cs_change is set.
> 
> Maybe we should use the helper dspi_data_to_pushr to fill the DMA buffer
> and _clear_ SPI_PUSHR_CONT if necessary like the other transfer modes
> do... Then we can use the for loop to fill the complete buffer and get
> rid of some code dupplication.
> 
> I see that dspi_data_to_pushr does move len too, which we did not in the
> DMA case. dspi->len gets incremented only on successful DMA transfer in
> dspi_dma_xfer. However, I wonder if that is not even a bug: We increment
> dspi->tx always, but len only on success. This makes len go off sync
> with regards to the tx pointer which does not help anybody. So lets get
> rid of the update code in dspi_dma_xfer
> 

Thanks for the feedback. Using dspi_data_to_pushr really cleans up that
tx path very nicely. Why didn't I see it. Will send a follow up patch
soon after testing again.

- Sanchayan.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web