Re: [PATCH] iio: imu: st_lsm6dsx: fall back to 25 us/tick without a trim

[Lorenzo Bianconi <lorenzo@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 7041 bytes --]

> Hi all,
> 
> Patch attached. It is the fallback in st_lsm6dsx_init_hw_timer() as
> discussed.
> 
> On Toradex Linux 6.6.142, an ISM330DLC on SPI (whoami 0x6a) on a Verdin
> i.MX8M Plus published every FIFO sample with one timestamp until the scale
> was 25000 ns/tick again. After that the timestamp bytes advanced and pitch
> updated. I have not booted this patch on mainline. It applies cleanly on
> mainline 23c62304dde2.
> 
> This is my first kernel patch, please let me know if there is anything else
> I need to do.
> 
> Thanks,
> Phil
> 
> On 15/11/2026 8:11 pm, Francesco Lavra wrote:
> > On Sun, 2026-11-15 at 00:09 +0000, Jonathan Cameron wrote:
> > > On Tue, 10 Nov 2026 02:38:14 +1300
> > > Phil Williams <phil.williams@advisoft.co.nz> wrote:
> > > 
> > > > Hey Guys,
> > > > 
> > > > I ran into an issue where every FIFO sample from an ISM330DLC came out
> > > > with the same timestamp, so our attitude math froze the pitch. The
> > > > accelerometer and gyroscope values were fine. I've patched it locally,
> > > > so this is just an FYI.
> > > > 
> > > > Make what you want of it, but here is an AI generated description of
> > > > the
> > > > issue:
> > > > 
> > > > Commit 8abbf45fcda0 ("iio: st_lsm6dsx: Fixed calibrated timestamp
> > > > calculation") drops the timestamp scale for the older parts in this
> > > > driver. Stable backport 981f3da383ce is what we hit, in 6.6.142.
> > > > 
> > > > st_lsm6dsx_init_hw_timer() used to do:
> > > > 
> > > >       hw->ts_gain = ST_LSM6DSX_TS_SENSITIVITY; /* 25000 ns/tick */
> > > > 
> > > > That commit deletes the constant and reads the gain from the per-chip
> > > > table:
> > > > 
> > > >       hw->ts_gain = ts_settings->ts_sensitivity;
> > > > 
> > > > ts_sensitivity is only filled in on the entries that already had
> > > > freq_fine (LSM6DSO / LSM6DSR / LSM6DSV and that generation). The
> > > > untagged-FIFO entries have no freq_fine register, so they were left at
> > > > 0:
> > > > 
> > > >       LSM6DS3
> > > >       LSM6DS3H
> > > >       LSM6DSL, LSM6DSM, ISM330DLC, LSM6DS3TR-C
> > > > 
> > > > st_lsm6dsx_read_fifo() still does:
> > > > 
> > > >       ts = data[1] << 16 | data[0] << 8 | data[3];
> > > >       ts *= hw->ts_gain;
> > > >       iio_push_to_buffers_with_timestamp(..., sensor->ts_ref + ts);
> > > > 
> > > > With ts_gain == 0 every sample is published as ts_ref, the
> > > > CLOCK_REALTIME value taken when the buffer was enabled. The accel and
> > > > gyro values are fine. Only the timestamp is stuck. The raw count is
> > > > still checked against 0xff0000 before the multiply, so ts_ref is
> > > > refreshed when the counter gets there. At 25 µs/tick that is about 7
> > > > minutes. The driver comment says 1.638 s, which is 2^16 ticks, but the
> > > > test is 0xff0000.
> > > > 
> > > > How we found it: an ISM330DLC on SPI (whoami 0x6a), on a Toradex Verdin
> > > > i.MX8M Plus, after moving onto 6.6.142. Userspace integrates attitude
> > > > from the delta of the IIO buffer timestamp and treats a non-increasing
> > > > timestamp as dt = 0, so pitch, roll and yaw froze at the first sample.
> > > > A
> > > > strace of the process reading /dev/iio:deviceX showed the xyz int16s
> > > > changing and the 8 timestamp bytes identical across samples. The same
> > > > binary against the previous kernel, which still used the 25000
> > > > constant,
> > > > did not do this.
> > > > 
> > > > Setting the old scale on those three entries fixes it. freq_fine stays
> > > > unset, so the trim register is not read and the gain stays exactly
> > > > 25000, which is what those parts had before:
> > > > 
> > > >       .ts_sensitivity = 25000,
> > > >       .ts_trim_coeff = 37500,
> > > > 
> > > > After booting that kernel the timestamp bytes increment and pitch
> > > > tracks
> > > > the unit again. The entries that already had ts_sensitivity (25000, or
> > > > 21701 on the freq_fine 0x4f part) were left alone.
> > > > 
> > > > Fixes: 8abbf45fcda0 ("iio: st_lsm6dsx: Fixed calibrated timestamp
> > > > calculation")
> > This analysis looks correct to me too.
> > But I wouldn't set .ts_trim_coeff for the parts where .freq_fine is 0: for
> > those, there is no trimming to be done.
> > So either set .ts_sensitivity = 25000 for all parts that currently don't
> > have it set, or maybe better use 25000 in st_lsm6dsx_init_hw_timer() as a
> > fallback value when .ts_sensitivity is not set.

> From 141aa31454bd87ed20b185544456bf28692c4c2d Mon Sep 17 00:00:00 2001
> From: Phil Williams <phil.williams@advisoft.co.nz>
> Date: Sun, 15 Nov 2026 21:04:06 +1300
> Subject: [PATCH] iio: imu: st_lsm6dsx: fall back to 25 us/tick without a trim
> 
> Commit 8abbf45fcda0 ("iio: st_lsm6dsx: Fixed calibrated timestamp
> calculation") takes the FIFO timestamp scale from ts_sensitivity.
> That field is only filled in for parts that have freq_fine. The
> untagged-FIFO parts are left at zero:
> 
>   LSM6DS3
>   LSM6DS3H
>   LSM6DSL, LSM6DSM, ISM330DLC and LSM6DS3TR-C
> 
> st_lsm6dsx_read_fifo() multiplies the raw counter by hw->ts_gain, so
> every sample is published with the timestamp taken when the buffer
> was enabled. The accelerometer and gyroscope values still update.
> Userspace that integrates attitude from the timestamp delta treats a
> stuck timestamp as dt = 0.
> 
> These parts have no freq_fine register, so there is no trim to apply.
> Use the previous fixed scale of 25000 ns/tick when ts_sensitivity is
> unset. Entries that already set it are left alone.
> 
> Fixes: 8abbf45fcda0 ("iio: st_lsm6dsx: Fixed calibrated timestamp calculation")
> Cc: stable@vger.kernel.org
> Suggested-by: Jonathan Cameron <jic23@kernel.org>
> Signed-off-by: Phil Williams <phil.williams@advisoft.co.nz>

Hi Phil,

just a nit inline Fixing it:

Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>

> ---
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> index 823f8ac..98db90a 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> @@ -2525,8 +2525,14 @@ static int st_lsm6dsx_init_hw_timer(struct st_lsm6dsx_hw *hw)
>  			return err;
>  	}
>  
> -	/* calibrate timestamp sensitivity */
> +	/*
> +	 * Parts with no freq_fine register have no trim and leave
> +	 * ts_sensitivity unset. Those parts used a fixed 25 us/tick.
> +	 * A zero gain publishes every FIFO sample with one timestamp.
> +	 */
>  	hw->ts_gain = ts_settings->ts_sensitivity;
> +	if (!hw->ts_gain)
> +		hw->ts_gain = 25000;

What about re-adding the macro here?

#define ST_LSM6DSX_TS_SENSITIVITY	25000UL /* 25us */

Regards,
Lorenzo

>  	if (ts_settings->freq_fine) {
>  		err = regmap_read(hw->regmap, ts_settings->freq_fine, &val);
>  		if (err < 0)
> -- 
> 2.53.0
> 


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]