From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefan Agner Subject: Re: [PATCH v2 2/4] spi: spi-fsl-dspi: Fix continuous selection format Date: Mon, 21 Nov 2016 15:15:41 -0800 Message-ID: <59ac10adfe92916770aa30146e958887@agner.ch> References: <1317eff8a40789c47311c219d9cfb4105863bd9f.1479706671.git.maitysanchayan@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: broonie@kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-spi@vger.kernel.org To: Sanchayan Maity Return-path: In-Reply-To: <1317eff8a40789c47311c219d9cfb4105863bd9f.1479706671.git.maitysanchayan@gmail.com> Sender: linux-kernel-owner@vger.kernel.org List-Id: linux-spi.vger.kernel.org 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 > --- > 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