Linux kernel -stable discussions
 help / color / mirror / Atom feed
* [PATCH] iio: imu: st_lsm6dsx: fall back to 25 us/tick without a trim
       [not found]   ` <2cc30a3e725639667085f3f82d453a7dfc9f853c.camel@baylibre.com>
@ 2026-11-15  8:17     ` Phil Williams
  2026-11-15  8:52       ` Lorenzo Bianconi
  0 siblings, 1 reply; 2+ messages in thread
From: Phil Williams @ 2026-11-15  8:17 UTC (permalink / raw)
  To: Francesco Lavra, Jonathan Cameron, linux-iio, Lorenzo Bianconi
  Cc: mario.tesi, linux-iio, stable, dlechner, nuno.sa, andy

[-- Attachment #1: Type: text/plain, Size: 4068 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.

[-- Attachment #2: 0001-iio-imu-st_lsm6dsx-fall-back-to-25-us-tick-without-a-trim.patch --]
[-- Type: text/plain, Size: 2159 bytes --]

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


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] iio: imu: st_lsm6dsx: fall back to 25 us/tick without a trim
  2026-11-15  8:17     ` [PATCH] iio: imu: st_lsm6dsx: fall back to 25 us/tick without a trim Phil Williams
@ 2026-11-15  8:52       ` Lorenzo Bianconi
  0 siblings, 0 replies; 2+ messages in thread
From: Lorenzo Bianconi @ 2026-11-15  8:52 UTC (permalink / raw)
  To: Phil Williams
  Cc: Francesco Lavra, Jonathan Cameron, linux-iio, mario.tesi, stable,
	dlechner, nuno.sa, andy

[-- 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 --]

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-11-15  8:52 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <a3a00c75-c830-4d1a-9a89-dd55641a422f@advisoft.co.nz>
     [not found] ` <20261115000940.24215eb0@jic23-hlaptop>
     [not found]   ` <2cc30a3e725639667085f3f82d453a7dfc9f853c.camel@baylibre.com>
2026-11-15  8:17     ` [PATCH] iio: imu: st_lsm6dsx: fall back to 25 us/tick without a trim Phil Williams
2026-11-15  8:52       ` Lorenzo Bianconi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox