[PATCH] iio: imu: bmi270: fix zero timestamp in triggered buffer

[PATCH] iio: imu: bmi270: fix zero timestamp in triggered buffer

[Liu Yufei]

When the device's own data ready trigger is used, the trigger is
polled with iio_trigger_poll_nested() from the threaded IRQ handler.
In that path the pollfunc top half, iio_pollfunc_store_time(), is
never run, so pf->timestamp is left at zero and every sample pushed
to the buffer carries a zero timestamp.

Take the timestamp in the trigger handler itself and drop the top
half, so that a valid timestamp is provided for all triggers.

Fixes: a1854d55f54b ("iio: imu: bmi270: add support for data ready interrupt trigger")
Cc: stable@vger.kernel.org
Signed-off-by: Liu Yufei <lyf98405@gmail.com>
---
 drivers/iio/imu/bmi270/bmi270_core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/imu/bmi270/bmi270_core.c b/drivers/iio/imu/bmi270/bmi270_core.c
index fbcf6bb87acc..40ed1ee5ac58 100644
--- a/drivers/iio/imu/bmi270/bmi270_core.c
+++ b/drivers/iio/imu/bmi270/bmi270_core.c
@@ -840,6 +840,7 @@ static irqreturn_t bmi270_trigger_handler(int irq, void *p)
 	struct iio_poll_func *pf = p;
 	struct iio_dev *indio_dev = pf->indio_dev;
 	struct bmi270_data *data = iio_priv(indio_dev);
+	s64 timestamp = iio_get_time_ns(indio_dev);
 	int ret;
 
 	guard(mutex)(&data->mutex);
@@ -852,7 +853,7 @@ static irqreturn_t bmi270_trigger_handler(int irq, void *p)
 		goto done;
 
 	iio_push_to_buffers_with_timestamp(indio_dev, &data->buffer,
-					   pf->timestamp);
+					   timestamp);
 done:
 	iio_trigger_notify_done(indio_dev->trig);
 	return IRQ_HANDLED;
@@ -1630,8 +1631,7 @@ int bmi270_core_probe(struct device *dev, struct regmap *regmap,
 	if (ret)
 		return ret;
 
-	ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
-					      iio_pollfunc_store_time,
+	ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL,
 					      bmi270_trigger_handler, NULL);
 	if (ret)
 		return ret;

---
base-commit: 09f742b49cb2728086d78be2b0b35e4d1d71fc85
change-id: 20261028-bmi270-fix-timestamp-48a194b69db1

Best regards,
--  
Liu Yufei <lyf98405@gmail.com>

Re: [PATCH] iio: imu: bmi270: fix zero timestamp in triggered buffer

[Andy Shevchenko]

On Wed, Oct 28, 2026 at 10:59:26AM +0800, Liu Yufei wrote:
> When the device's own data ready trigger is used, the trigger is
> polled with iio_trigger_poll_nested() from the threaded IRQ handler.
> In that path the pollfunc top half, iio_pollfunc_store_time(), is
> never run, so pf->timestamp is left at zero and every sample pushed
> to the buffer carries a zero timestamp.
> 
> Take the timestamp in the trigger handler itself and drop the top
> half, so that a valid timestamp is provided for all triggers.

...

> +	s64 timestamp = iio_get_time_ns(indio_dev);

Do we need this variable? Why not call it directly?

>  	iio_push_to_buffers_with_timestamp(indio_dev, &data->buffer,
> -					   pf->timestamp);
> +					   timestamp);

					   iio_get_time_ns(indio_dev);

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] iio: imu: bmi270: fix zero timestamp in triggered buffer

[Liu Yufei]

On Wed, Oct 28, 2026 at 10:58:25AM +0200, Andy Shevchenko wrote:
> On Wed, Oct 28, 2026 at 10:59:26AM +0800, Liu Yufei wrote:

...

> > +	s64 timestamp = iio_get_time_ns(indio_dev);
>
> Do we need this variable? Why not call it directly?

The intent was to capture the timestamp before the bus read, so it is
as close as possible to the data ready event rather than after the
regmap_bulk_read() over I2C/SPI.

If that difference is not considered significant here, I'm happy to
call it directly in v2.

Thanks,
Liu Yufei

Re: [PATCH] iio: imu: bmi270: fix zero timestamp in triggered buffer

[Andy Shevchenko]

On Wed, Oct 28, 2026 at 10:24:11PM +0800, Liu Yufei wrote:
> On Wed, Oct 28, 2026 at 10:58:25AM +0200, Andy Shevchenko wrote:
> > On Wed, Oct 28, 2026 at 10:59:26AM +0800, Liu Yufei wrote:

...

> > > +	s64 timestamp = iio_get_time_ns(indio_dev);
> >
> > Do we need this variable? Why not call it directly?
> 
> The intent was to capture the timestamp before the bus read, so it is
> as close as possible to the data ready event rather than after the
> regmap_bulk_read() over I2C/SPI.
> 
> If that difference is not considered significant here, I'm happy to
> call it directly in v2.

But the time of the event in the kernel should be ideally with data we get,
not in the past, right?

Jonathan, what our expectations / requirements on the timestamp capture?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] iio: imu: bmi270: fix zero timestamp in triggered buffer

[Jonathan Cameron]

On Thu, 29 Oct 2026 15:19:05 +0200
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Wed, Oct 28, 2026 at 10:24:11PM +0800, Liu Yufei wrote:
> > On Wed, Oct 28, 2026 at 10:58:25AM +0200, Andy Shevchenko wrote:  
> > > On Wed, Oct 28, 2026 at 10:59:26AM +0800, Liu Yufei wrote:  
> 
> ...
> 
> > > > +	s64 timestamp = iio_get_time_ns(indio_dev);  
> > >
> > > Do we need this variable? Why not call it directly?  
> > 
> > The intent was to capture the timestamp before the bus read, so it is
> > as close as possible to the data ready event rather than after the
> > regmap_bulk_read() over I2C/SPI.
> > 
> > If that difference is not considered significant here, I'm happy to
> > call it directly in v2.  
> 
> But the time of the event in the kernel should be ideally with data we get,
> not in the past, right?
> 
> Jonathan, what our expectations / requirements on the timestamp capture?
> 
As close to data capture as possible.  For drivers with a dataready
we grab early in the interrupt handling. Where it is being driven by
something else (such as polling elsewhere that didn't grab a timestamp)
we grab at time of read as that's the best we can do.

Jonathan

Re: [PATCH] iio: imu: bmi270: fix zero timestamp in triggered buffer

[Jonathan Cameron]

On Wed, 28 Oct 2026 10:59:26 +0800
Liu Yufei <lyf98405@gmail.com> wrote:

> When the device's own data ready trigger is used, the trigger is
> polled with iio_trigger_poll_nested() from the threaded IRQ handler.
> In that path the pollfunc top half, iio_pollfunc_store_time(), is
> never run, so pf->timestamp is left at zero and every sample pushed
> to the buffer carries a zero timestamp.
> 
> Take the timestamp in the trigger handler itself and drop the top
> half, so that a valid timestamp is provided for all triggers.
> 
> Fixes: a1854d55f54b ("iio: imu: bmi270: add support for data ready interrupt trigger")
> Cc: stable@vger.kernel.org
> Signed-off-by: Liu Yufei <lyf98405@gmail.com>

Firstly. Do you have a setup where you actually care about this, or is this
found by reading code / LLM etc? If nothing else that changes
the urgency.

This is an old problem. I really need to find some time to write
a more consistent solution for timestamp acquistion that would
indicate if we have a better timestamp or not and stop this
being a driver problem.  We used to avoid this by having most
triggers go through the irq_work dance but that was considered
too much of an overhead.

As Andy suggested we do want to grab the timestamp if appropriate
in the top half if it is our own trigger (the one registered
by the driver) because we then know it is the best possible
for a dataready trigger.

The way some drivers handle this is to check on whether the
trigger set is our own or not. Now ideally we'd do that check
in some helper code, but writing that is less than trivial
(which is why I've not done it already)

https://elixir.bootlin.com/linux/v7.3/source/drivers/iio/light/rpr0521.c
is an example I found with a quick grep, but perhaps not
coded in the cleanest way!  So take the idea, not necessarily
the implementation from there. 

Jonathan


> ---
>  drivers/iio/imu/bmi270/bmi270_core.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/imu/bmi270/bmi270_core.c b/drivers/iio/imu/bmi270/bmi270_core.c
> index fbcf6bb87acc..40ed1ee5ac58 100644
> --- a/drivers/iio/imu/bmi270/bmi270_core.c
> +++ b/drivers/iio/imu/bmi270/bmi270_core.c
> @@ -840,6 +840,7 @@ static irqreturn_t bmi270_trigger_handler(int irq, void *p)
>  	struct iio_poll_func *pf = p;
>  	struct iio_dev *indio_dev = pf->indio_dev;
>  	struct bmi270_data *data = iio_priv(indio_dev);
> +	s64 timestamp = iio_get_time_ns(indio_dev);
>  	int ret;
>  
>  	guard(mutex)(&data->mutex);
> @@ -852,7 +853,7 @@ static irqreturn_t bmi270_trigger_handler(int irq, void *p)
>  		goto done;
>  
>  	iio_push_to_buffers_with_timestamp(indio_dev, &data->buffer,
> -					   pf->timestamp);
> +					   timestamp);
>  done:
>  	iio_trigger_notify_done(indio_dev->trig);
>  	return IRQ_HANDLED;
> @@ -1630,8 +1631,7 @@ int bmi270_core_probe(struct device *dev, struct regmap *regmap,
>  	if (ret)
>  		return ret;
>  
> -	ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
> -					      iio_pollfunc_store_time,
> +	ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL,
>  					      bmi270_trigger_handler, NULL);
>  	if (ret)
>  		return ret;
> 
> ---
> base-commit: 09f742b49cb2728086d78be2b0b35e4d1d71fc85
> change-id: 20261028-bmi270-fix-timestamp-48a194b69db1
> 
> Best regards,
> --  
> Liu Yufei <lyf98405@gmail.com>
>