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

[Jonathan Cameron <jic23@kernel.org>]

On Fri, 06 Nov 2026 10:54:17 +0800
Liu Yufei <lyf98405@gmail.com> wrote:

> The BMI270's own trigger uses iio_trigger_poll_nested() from
> the threaded IRQ handler, bypassing the pollfunc top half.
> This leaves the buffer timestamp at zero or a stale value.
> 
> Capture the timestamp in the hard IRQ handler for the device's
> own trigger. Preserve the pollfunc timestamp for ordinary
> external triggers, and use the current IIO clock when a nested
> external trigger has no timestamp. Clear the pollfunc timestamp
> after processing.
> 
> Testing on Orange Pi Zero 2W reproduced 256 zero timestamps
> with the original own trigger and 256 repeated stale timestamps
> after switching from an ordinary to a nested external trigger.
> With v2, 1280 samples across the tested trigger and restart
> paths had no zero or non-increasing timestamps.
> 
> 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>
> ---
> Fix invalid BMI270 IIO buffer timestamps when the device's own
> trigger uses iio_trigger_poll_nested(). This bypasses the
> pollfunc top half, so its timestamp can remain zero or retain
> an old value.
> 
> Capture the timestamp in the hard IRQ handler for the device's
> own trigger. Keep pollfunc timestamps for ordinary external
> triggers, and use the current IIO clock when a nested external
> trigger has no timestamp. Clear the pollfunc timestamp after use.
> 
> Tested on Orange Pi Zero 2W (arm64, Linux 7.3.0), with a
> physical BMI270 connected over I2C, using INT1 at 100 Hz.
> The baseline produced 256/256 zero timestamps using its own
> trigger, and 256/256 stale timestamps after switching an
> external trigger from ordinary to nested polling.
> 
> With v2, 1,280 samples across own hardware triggering,
> ordinary and nested external triggering, and stop/restart
> tests had no zero, repeated, or decreasing timestamps.
> External triggers were generated by a temporary test module;
> sensor data was read from the physical BMI270.
> ---
> Changes in v2:
> - Capture the timestamp in the hard IRQ handler.
> - Use the IRQ timestamp for the device's own trigger.
> - Fall back to the current time when no pollfunc timestamp exists.
> - Clear the pollfunc timestamp after processing.
> - Link to v1: https://patch.msgid.link/20261028-bmi270-fix-timestamp-v1-1-99c0e5fe7d0c@gmail.com
> 
> To: Alex Lanzano <lanzano.alex@gmail.com>
> To: Jonathan Cameron <jic23@kernel.org>
> To: David Lechner <dlechner@baylibre.com>
> To: Nuno Sá <nuno.sa@analog.com>
> To: Andy Shevchenko <andy@kernel.org>
> To: Gustavo Silva <gustavograzs@gmail.com>
> Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: linux-iio@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  drivers/iio/imu/bmi270/bmi270_core.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/imu/bmi270/bmi270_core.c b/drivers/iio/imu/bmi270/bmi270_core.c
> index fbcf6bb87acc..6b11e91eabff 100644
> --- a/drivers/iio/imu/bmi270/bmi270_core.c
> +++ b/drivers/iio/imu/bmi270/bmi270_core.c
> @@ -157,6 +157,7 @@ struct bmi270_data {
>  	const struct bmi270_chip_info *chip_info;
>  	enum bmi270_irq_pin irq_pin;
>  	struct iio_trigger *trig;
> +	s64 irq_timestamp;
>  	 /* Protect device's private data from concurrent access */
>  	struct mutex mutex;
>  	bool steps_enabled;
> @@ -755,6 +756,16 @@ static int bmi270_get_odr(struct bmi270_data *data, int chan_type, int *odr,
>  	return -EINVAL;
>  }
>  
> +static irqreturn_t bmi270_irq_handler(int irq, void *private)
> +{
> +	struct iio_dev *indio_dev = private;
> +	struct bmi270_data *data = iio_priv(indio_dev);
> +
> +	data->irq_timestamp = iio_get_time_ns(indio_dev);
> +
> +	return IRQ_WAKE_THREAD;
> +}
> +
>  static irqreturn_t bmi270_irq_thread_handler(int irq, void *private)
>  {
>  	struct iio_dev *indio_dev = private;
> @@ -842,6 +853,13 @@ static irqreturn_t bmi270_trigger_handler(int irq, void *p)
>  	struct bmi270_data *data = iio_priv(indio_dev);
>  	int ret;
>  
> +	if (iio_trigger_using_own(indio_dev)) {
> +		pf->timestamp = data->irq_timestamp;
> +		data->irq_timestamp = 0;
> +	} else if (!pf->timestamp) {

Using 0 timestamp as a flag is not a good pattern. It may more
may not be valid, we simply don't know.

If it's not our trigger though then whatever timestamp was there
probably isn't a close to the data capture timestamp, so under
those circumstances it is fine to unconditionally set the
timestamp here.

> +		pf->timestamp = iio_get_time_ns(indio_dev);
> +	}
> +
>  	guard(mutex)(&data->mutex);
>  
>  	ret = regmap_bulk_read(data->regmap, BMI270_ACCEL_X_REG,
> @@ -854,6 +872,7 @@ static irqreturn_t bmi270_trigger_handler(int irq, void *p)
>  	iio_push_to_buffers_with_timestamp(indio_dev, &data->buffer,
>  					   pf->timestamp);
>  done:
> +	pf->timestamp = 0;
>  	iio_trigger_notify_done(indio_dev->trig);
>  	return IRQ_HANDLED;
>  }
> @@ -1430,7 +1449,7 @@ static int bmi270_trigger_probe(struct bmi270_data *data,
>  	data->trig->ops = &bmi270_trigger_ops;
>  	iio_trigger_set_drvdata(data->trig, data);
>  
> -	ret = devm_request_threaded_irq(data->dev, irq, NULL,
> +	ret = devm_request_threaded_irq(data->dev, irq, bmi270_irq_handler,
>  					bmi270_irq_thread_handler,
>  					IRQF_ONESHOT, "bmi270-int", indio_dev);
>  	if (ret)
> 
> ---
> base-commit: 09f742b49cb2728086d78be2b0b35e4d1d71fc85
> change-id: 20261028-bmi270-fix-timestamp-48a194b69db1
> 
> Best regards,
> --  
> Liu Yufei <lyf98405@gmail.com>
>