On 7/10/21 7:18 AM, Uwe Kleine-König wrote:
Hello,
Cc += Mark + linux-spi
On Fri, Jul 09, 2021 at 11:39:48AM -0500, David Lechner wrote:
On 7/9/21 5:11 AM, Uwe Kleine-König wrote:
The ADS7950 requires that CS is deasserted after each SPI word. Before
commit e2540da86ef8 ("iio: adc: ti-ads7950: use SPI_CS_WORD to reduce
CPU usage") the driver used a message with one spi transfer per channel
where each but the last one had .cs_change set to enforce a CS toggle.
This was wrongly translated into a message with a single transfer and
.cs_change set which results in a CS toggle after each word but the
last which corrupts the first adc conversion of all readouts after the
first readout.
Fixes: e2540da86ef8 ("iio: adc: ti-ads7950: use SPI_CS_WORD to reduce CPU usage")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
---
drivers/iio/adc/ti-ads7950.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/iio/adc/ti-ads7950.c b/drivers/iio/adc/ti-ads7950.c
index 2383eacada87..a2b83f0bd526 100644
--- a/drivers/iio/adc/ti-ads7950.c
+++ b/drivers/iio/adc/ti-ads7950.c
@@ -568,7 +568,6 @@ static int ti_ads7950_probe(struct spi_device *spi)
st->ring_xfer.tx_buf = &st->tx_buf[0];
st->ring_xfer.rx_buf = &st->rx_buf[0];
/* len will be set later */
- st->ring_xfer.cs_change = true;
spi_message_add_tail(&st->ring_xfer, &st->ring_msg);
Yes, it seems like the SPI_CS_WORD flag should have replaced this (it's
been too long, I can't remember if it was intentional). And removing it
doesn't seem to break anything for me.
If it's not broken for you without my patch, your spi bus driver doesn't
honor .cs_change in the last transfer. Out of interest: Which bus are
you using? I wonder if the driver should refuse the request if it cannot
honer .cs_change?! (spi-imx does honor it only if gpios are used as chip
select, the native chip selects cannot do that.)
I'm using spi-davinci. It uses the standard spi_transfer_one_message()
which handles cs_change. But I suspect when the SPI_CS_WORD flag is set,
and the message is big enough to use DMA, the hardware is probably
automatically toggling CS after the last transfer before the cs_change
logic asserts it again.
So unless there is a valid use case where we need both SPI_CS_WORD
and cs_change, I don't think we need to fix spi-davinci.
