Message-ID: <aUZUwL2fHQ6Q4zOI@sunspire.home.arpa>
Date: Sat, 20 Dec 2025 09:48:16 +0200
From: Petre Rodan <petre.rodan@...dimension.ro>
To: Marcelo Schmitt <marcelo.schmitt1@...il.com>
Cc: Jonathan Cameron <jic23@...nel.org>,
	David Lechner <dlechner@...libre.com>,
	Nuno Sá <nuno.sa@...log.com>,
	Andy Shevchenko <andy@...nel.org>,
	Andreas Klinger <ak@...klinger.de>, linux-iio@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Jonathan Cameron <Jonathan.Cameron@...wei.com>
Subject: Re: [PATCH 09/14] iio: pressure: mprls0025pa: mitigate SPI CS delay
 violation


Hello Marcelo,

thank you for the pointers.

On Sat, Dec 20, 2025 at 01:51:42AM -0300, Marcelo Schmitt wrote:
> On 12/18, Petre Rodan wrote:
> > Based on the sensor datasheet in chapter 7.6 SPI timing, Table 20,
> > during the SPI transfer there is a minimum time interval between the
> > CS being asserted and the first clock edge (tHDSS).
> > This minimum interval of 2.5us is being violated if two consecutive SPI
> > transfers are queued up, at least on my SPI controller (omap2_mcspi) [1]
> > As you can see in the first package that only contains a NOP the interval
> > is 0.75us (half a 800kHz clock cycle).
> > 
> > This patch mitigates the problem by implementing a different measurement
> > technique that does not involve checking for the EOC indicator before
> > reading the conversion, thus making sure SPI transfers are not queued up.
> The correct way of fixing that is with protocol specific delay. Generic delays
> like fsleep won't guarantee any delay between CS drop and fist SCLK edge.

sorry for the lack of detail in the commit log, I forgot to add it.

this patch changes the measurement sequence and as a consequence it also mitigates a problem where protocol-specific delays are not implemented in the SPI controller (for the corner case when multiple SPI transfers are queued up).

as per the datasheet pages 36 and 41, the measurement sequence can follow one of these two scenarios, for both bus implementations:

a) send measurement command and wait for the status byte to lose the busy flag.
this means that we would need to keep sending NOP commands and read the first (status) byte in a loop until the busy flag goes away. this is how the driver was designed before this series of patches.

b) send measurement command and wait for 5ms for the data conversion to occur.
same as a, but instead of polling the sensor for the status byte there's just a 5ms+ delay after which only one NOP command is sent.
the read that follows will get the conversion data plus the status byte.
depending on the status byte flags, either error out or pass the conversion along.

my patch replaces the measurement sequence defined by scenario a with scenario b and it only affects users that did not define the EoC interrupt pin in the device tree overlay.
the datasheet contains sample code (pages 38 and 43) and they also happen to use scenario b, so no polling.
in my testing the sensor always performed as expected (the busy flag was never set after the 5ms fsleep()).

> For SPI, we unfortunately we don't have any interface to set a pre-SCLK delay
> so I suggest to make an spi_message with a dummy transfer to cause the initial
> 2.5 µs delay. E.g.

for normal (non-queued) SPI messages the delay that exist between the CS assert and the first clock is ~4us, well above the 2.5us minimum.
the problem only appears when the SPI controller gets multiple xfers without any delays between them - which happens in scenario a when two transfers get queued up (NOP that reads the status byte and the NOP that reads the status + conversions).

this is just a hunch but I think that the CS timing is directly controlled by hardware when the issue happens.
see chapter 24.3.2.8 page 4903 in the AM335x technical reference manual [1].
I'm measuring exactly 0.5 clock cycles CS lead times which is the default for the TCS bit of the register MCSPI_CH(i)CONF.

[1] https://www.ti.com/lit/ug/spruh73q/spruh73q.pdf?ts=1766215152875

> /* dummy transfer with no data, just cause the delay */
> xfers[0].delay.value = 2500 * NSEC_PER_SEC;
> xfers[0].delay.unit = SPI_DELAY_UNIT_NSECS;
> 
> /* normal data transfer  */
> xfer[1].tx_buf = data->tx_buf;
> xfer[1].rx_buf = data->rx_buf;
> ...
> 
> Also, I don't see how the proposed change is 'implementing a different
> measurement technique'. Consider updating the commit description or providing
> better explanation.

I feel that using scenario b defined above we cure the problem before it happens.
my patch provides a less complex implementation, it's recommended by the manufacturer in their sample code and provides less code duplication (status byte parsing) with no downsides afaict.

> > see Option 2 in Table 19 SPI output measurement command.
> > Note that Honeywell's example code also follows this technique for both i2c
> > and SPI.
> > 
> > This change only affects users that do not define the EOC interrupt in the
> > device tree.
> > 
> > Remove defines that are no longer used.
> > 
> > [1] https://pasteboard.co/66WN38MRI1wc.png
> > 
> > Datasheet: https://prod-edam.honeywell.com/content/dam/honeywell-edam/sps/siot/en-us/products/sensors/pressure-sensors/board-mount-pressure-sensors/micropressure-mpr-series/documents/sps-siot-mpr-series-datasheet-32332628-ciid-172626.pdf?download=false
> > Signed-off-by: Petre Rodan <petre.rodan@...dimension.ro>

best regards,
peter

-- 
petre rodan

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)

Powered by blists - more mailing lists