Re: [PATCH 10/12] iio: imu: inv_icm42600: add accurate timestamping

From: Jonathan Cameron
Date: Fri May 08 2020 - 10:43:16 EST

On Thu, 7 May 2020 16:42:20 +0200
Jean-Baptiste Maneyrol <jmaneyrol@xxxxxxxxxxxxxx> wrote:

> Add a timestamp channel with processed value that returns full
> precision 20 bits timestamp.
>
> Add a timestamping mechanism for buffer that provides accurate
> event timestamps when using watermark. This mechanism estimates
> device internal clock by comparing FIFO interrupts delta time and
> corresponding device elapsed time computed by parsing FIFO data.

Yikes. That is complex. Hmm. I always get lost in the maths in these
timestamp patches so my review may be a little superficial.

However a bit more in the way of docs would be good here.

The sysfs read of timestamp is unusual and I'm not really sure what it is for.
The delays in a sysfs read of that value are likely to be enough that it's
that useful for anything.

Also, do we make use of the timestamps within the fifo records?

J


>
> Signed-off-by: Jean-Baptiste Maneyrol <jmaneyrol@xxxxxxxxxxxxxx>
> ---
> drivers/iio/imu/inv_icm42600/Makefile | 1 +
> drivers/iio/imu/inv_icm42600/inv_icm42600.h | 10 +-
> .../iio/imu/inv_icm42600/inv_icm42600_accel.c | 32 ++-
> .../imu/inv_icm42600/inv_icm42600_buffer.c | 28 +-
> .../iio/imu/inv_icm42600/inv_icm42600_core.c | 6 +
> .../iio/imu/inv_icm42600/inv_icm42600_gyro.c | 32 ++-
> .../imu/inv_icm42600/inv_icm42600_timestamp.c | 246 ++++++++++++++++++
> .../imu/inv_icm42600/inv_icm42600_timestamp.h | 82 ++++++
> 8 files changed, 421 insertions(+), 16 deletions(-)
> create mode 100644 drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.c
> create mode 100644 drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.h
>
> diff --git a/drivers/iio/imu/inv_icm42600/Makefile b/drivers/iio/imu/inv_icm42600/Makefile
> index d6732118010c..1197b545a682 100644
> --- a/drivers/iio/imu/inv_icm42600/Makefile
> +++ b/drivers/iio/imu/inv_icm42600/Makefile
> @@ -7,6 +7,7 @@ inv-icm42600-y += inv_icm42600_accel.o
> inv-icm42600-y += inv_icm42600_temp.o
> inv-icm42600-y += inv_icm42600_trigger.o
> inv-icm42600-y += inv_icm42600_buffer.o
> +inv-icm42600-y += inv_icm42600_timestamp.o
>
> obj-$(CONFIG_INV_ICM42600_I2C) += inv-icm42600-i2c.o
> inv-icm42600-i2c-y += inv_icm42600_i2c.o
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600.h b/drivers/iio/imu/inv_icm42600/inv_icm42600.h
> index 947ca4dd245b..e15eddafe009 100644
> --- a/drivers/iio/imu/inv_icm42600/inv_icm42600.h
> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600.h
> @@ -16,6 +16,7 @@
> #include <linux/iio/trigger.h>
>
> #include "inv_icm42600_buffer.h"
> +#include "inv_icm42600_timestamp.h"
>
> enum inv_icm42600_chip {
> INV_CHIP_ICM42600,
> @@ -127,6 +128,7 @@ struct inv_icm42600_suspended {
> * @indio_accel: accelerometer IIO device.
> * @trigger: device internal interrupt trigger
> * @fifo: FIFO management structure.
> + * @timestamp: timestamp management structure.
> */
> struct inv_icm42600_state {
> struct mutex lock;
> @@ -142,6 +144,10 @@ struct inv_icm42600_state {
> struct iio_dev *indio_accel;
> struct iio_trigger *trigger;
> struct inv_icm42600_fifo fifo;
> + struct {
> + struct inv_icm42600_timestamp gyro;
> + struct inv_icm42600_timestamp accel;
> + } timestamp;
> };
>
> /* Virtual register addresses: @bank on MSB (4 upper bits), @address on LSB */
> @@ -382,11 +388,11 @@ int inv_icm42600_core_probe(struct regmap *regmap, int chip, int irq,
>
> int inv_icm42600_gyro_init(struct inv_icm42600_state *st);
>
> -int inv_icm42600_gyro_parse_fifo(struct iio_dev *indio_dev, int64_t ts);
> +int inv_icm42600_gyro_parse_fifo(struct iio_dev *indio_dev);
>
> int inv_icm42600_accel_init(struct inv_icm42600_state *st);
>
> -int inv_icm42600_accel_parse_fifo(struct iio_dev *indio_dev, int64_t ts);
> +int inv_icm42600_accel_parse_fifo(struct iio_dev *indio_dev);
>
> int inv_icm42600_trigger_init(struct inv_icm42600_state *st, int irq,
> int irq_type);
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
> index 4206be54d057..ac140c824c03 100644
> --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
> @@ -17,6 +17,7 @@
> #include "inv_icm42600.h"
> #include "inv_icm42600_temp.h"
> #include "inv_icm42600_buffer.h"
> +#include "inv_icm42600_timestamp.h"
>
> #define INV_ICM42600_ACCEL_CHAN(_modifier, _index, _ext_info) \
> { \
> @@ -66,7 +67,7 @@ static const struct iio_chan_spec inv_icm42600_accel_channels[] = {
> INV_ICM42600_ACCEL_CHAN(IIO_MOD_Z, INV_ICM42600_ACCEL_SCAN_Z,
> inv_icm42600_accel_ext_infos),
> INV_ICM42600_TEMP_CHAN(INV_ICM42600_ACCEL_SCAN_TEMP),
> - IIO_CHAN_SOFT_TIMESTAMP(INV_ICM42600_ACCEL_SCAN_TIMESTAMP),
> + INV_ICM42600_TIMESTAMP_CHAN(INV_ICM42600_ACCEL_SCAN_TIMESTAMP),
> };
>
> /* IIO buffer data */
> @@ -94,14 +95,20 @@ static irqreturn_t inv_icm42600_accel_handler(int irq, void *_data)
> struct iio_poll_func *pf = _data;
> struct iio_dev *indio_dev = pf->indio_dev;
> struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
> + struct inv_icm42600_timestamp *ts = &st->timestamp.accel;
> const size_t fifo_nb = st->fifo.nb.total;
> + const size_t accel_nb = st->fifo.nb.accel;
> + const uint32_t fifo_period = st->fifo.period;
> int ret;
>
> /* exit if no sample */
> if (fifo_nb == 0)
> goto out;
>
> - ret = inv_icm42600_accel_parse_fifo(indio_dev, pf->timestamp);
> + inv_icm42600_timestamp_interrupt(ts, fifo_period, fifo_nb, accel_nb,
> + pf->timestamp);
> +
> + ret = inv_icm42600_accel_parse_fifo(indio_dev);
> if (ret)
> dev_err(regmap_get_device(st->map), "accel fifo error %d\n",
> ret);
> @@ -143,6 +150,7 @@ static int inv_icm42600_accel_update_scan_mode(struct iio_dev *indio_dev,
> }
>
> /* update data FIFO write and FIFO watermark */
> + inv_icm42600_timestamp_apply_odr(&st->timestamp.accel, 0, 0, 0);
> ret = inv_icm42600_buffer_set_fifo_en(st, fifo_en | st->fifo.en);
> if (ret)
> goto out_unlock;
> @@ -347,6 +355,7 @@ static int inv_icm42600_accel_write_odr(struct inv_icm42600_state *st,
> mutex_lock(&st->lock);
> conf.odr = inv_icm42600_accel_odr_conv[idx / 2];
> ret = inv_icm42600_set_accel_conf(st, &conf, NULL);
> + inv_icm42600_timestamp_update_odr(&st->timestamp.accel, conf.odr);
> inv_icm42600_buffer_update_fifo_period(st);
> inv_icm42600_buffer_update_watermark(st);
> mutex_unlock(&st->lock);
> @@ -502,6 +511,9 @@ static int inv_icm42600_accel_read_raw(struct iio_dev *indio_dev,
> case IIO_TEMP:
> return inv_icm42600_temp_read_raw(indio_dev, chan, val, val2,
> mask);
> + case IIO_TIMESTAMP:
> + return inv_icm42600_timestamp_read_raw(indio_dev, chan, val,
> + val2, mask);
> default:
> return -EINVAL;
> }
> @@ -694,13 +706,18 @@ int inv_icm42600_accel_init(struct inv_icm42600_state *st)
> return devm_iio_device_register(dev, st->indio_accel);
> }
>
> -int inv_icm42600_accel_parse_fifo(struct iio_dev *indio_dev, int64_t ts)
> +int inv_icm42600_accel_parse_fifo(struct iio_dev *indio_dev)
> {
> struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
> + struct inv_icm42600_timestamp *ts = &st->timestamp.accel;
> + const size_t fifo_nb = st->fifo.nb.total;
> const size_t accel_nb = st->fifo.nb.accel;
> + const uint32_t fifo_period = st->fifo.period;
> ssize_t i, size;
> + unsigned int no;
> const void *accel, *gyro, *temp, *timestamp;
> unsigned int odr;
> + int64_t ts_val;
> struct inv_icm42600_accel_buffer buffer;
>
> /* exit if no accel sample */
> @@ -708,7 +725,7 @@ int inv_icm42600_accel_parse_fifo(struct iio_dev *indio_dev, int64_t ts)
> return 0;
>
> /* parse all fifo packets */
> - for (i = 0; i < st->fifo.count; i += size) {
> + for (i = 0, no = 0; i < st->fifo.count; i += size, ++no) {
> size = inv_icm42600_fifo_decode_packet(&st->fifo.data[i],
> &accel, &gyro, &temp, &timestamp, &odr);
> dev_dbg(regmap_get_device(st->map), "accel packet size = %zd\n",
> @@ -721,9 +738,14 @@ int inv_icm42600_accel_parse_fifo(struct iio_dev *indio_dev, int64_t ts)
> dev_dbg(regmap_get_device(st->map), "skip accel data\n");
> continue;
> }
> + /* update odr */
> + if (odr & INV_ICM42600_SENSOR_ACCEL)
> + inv_icm42600_timestamp_apply_odr(ts, fifo_period,
> + fifo_nb, no);
> memcpy(&buffer.accel, accel, sizeof(buffer.accel));
> memcpy(&buffer.temp, temp, sizeof(buffer.temp));
> - iio_push_to_buffers_with_timestamp(indio_dev, &buffer, ts);
> + ts_val = inv_icm42600_timestamp_get(ts);
> + iio_push_to_buffers_with_timestamp(indio_dev, &buffer, ts_val);
> }
>
> return 0;
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
> index b428abdc92ee..bea4c9810da7 100644
> --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
> @@ -15,6 +15,7 @@
> #include <linux/iio/trigger_consumer.h>
>
> #include "inv_icm42600.h"
> +#include "inv_icm42600_timestamp.h"
> #include "inv_icm42600_buffer.h"
>
> void inv_icm42600_buffer_update_fifo_period(struct inv_icm42600_state *st)
> @@ -194,14 +195,17 @@ static int inv_icm42600_buffer_postdisable(struct iio_dev *indio_dev)
> unsigned int *watermark;
> struct inv_icm42600_sensor_conf conf = INV_ICM42600_SENSOR_CONF_INIT;
> unsigned int sleep = 0;
> + struct inv_icm42600_timestamp *ts;
> int ret;
>
> if (indio_dev == st->indio_gyro) {
> sensor = INV_ICM42600_SENSOR_GYRO;
> watermark = &st->fifo.watermark.gyro;
> + ts = &st->timestamp.gyro;
> } else if (indio_dev == st->indio_accel) {
> sensor = INV_ICM42600_SENSOR_ACCEL;
> watermark = &st->fifo.watermark.accel;
> + ts = &st->timestamp.accel;
> } else {
> return -EINVAL;
> }
> @@ -223,6 +227,8 @@ static int inv_icm42600_buffer_postdisable(struct iio_dev *indio_dev)
> else
> ret = inv_icm42600_set_accel_conf(st, &conf, &sleep);
>
> + inv_icm42600_timestamp_reset(ts);
> +
> out_unlock:
> mutex_unlock(&st->lock);
> if (sleep)
> @@ -316,11 +322,25 @@ int inv_icm42600_buffer_hwfifo_flush(struct inv_icm42600_state *st,
> if (st->fifo.nb.total == 0)
> return 0;
>
> - ret = inv_icm42600_gyro_parse_fifo(st->indio_gyro, ts_gyro);
> - if (ret)
> - return ret;
> + if (st->fifo.nb.gyro > 0) {
> + inv_icm42600_timestamp_interrupt(&st->timestamp.gyro,
> + st->fifo.period, st->fifo.nb.total,
> + st->fifo.nb.gyro, ts_gyro);
> + ret = inv_icm42600_gyro_parse_fifo(st->indio_gyro);
> + if (ret)
> + return ret;
> + }
>
> - return inv_icm42600_accel_parse_fifo(st->indio_accel, ts_accel);
> + if (st->fifo.nb.accel > 0) {
> + inv_icm42600_timestamp_interrupt(&st->timestamp.accel,
> + st->fifo.period, st->fifo.nb.total,
> + st->fifo.nb.accel, ts_accel);
> + ret = inv_icm42600_accel_parse_fifo(st->indio_accel);
> + if (ret)
> + return ret;
> + }
> +
> + return ret;
> }
>
> int inv_icm42600_buffer_init(struct inv_icm42600_state *st)
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
> index 689089065ff9..563c4348de01 100644
> --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
> @@ -15,6 +15,7 @@
>
> #include "inv_icm42600.h"
> #include "inv_icm42600_buffer.h"
> +#include "inv_icm42600_timestamp.h"
>
> static const struct regmap_range_cfg inv_icm42600_regmap_ranges[] = {
> {
> @@ -516,6 +517,11 @@ int inv_icm42600_core_probe(struct regmap *regmap, int chip, int irq,
> if (ret)
> return ret;
>
> + /* initialize timestamping */
> + ret = inv_icm42600_timestamp_init(st);
> + if (ret)
> + return ret;
> +
> /* setup FIFO buffer */
> ret = inv_icm42600_buffer_init(st);
> if (ret)
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c
> index dafb104abc77..1245588170bd 100644
> --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c
> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c
> @@ -17,6 +17,7 @@
> #include "inv_icm42600.h"
> #include "inv_icm42600_temp.h"
> #include "inv_icm42600_buffer.h"
> +#include "inv_icm42600_timestamp.h"
>
> #define INV_ICM42600_GYRO_CHAN(_modifier, _index, _ext_info) \
> { \
> @@ -66,7 +67,7 @@ static const struct iio_chan_spec inv_icm42600_gyro_channels[] = {
> INV_ICM42600_GYRO_CHAN(IIO_MOD_Z, INV_ICM42600_GYRO_SCAN_Z,
> inv_icm42600_gyro_ext_infos),
> INV_ICM42600_TEMP_CHAN(INV_ICM42600_GYRO_SCAN_TEMP),
> - IIO_CHAN_SOFT_TIMESTAMP(INV_ICM42600_GYRO_SCAN_TIMESTAMP),

Interrupt timestamp was pretty much garbage, so I'd just not register that in the
first place. Introduce the timestamp for the first time in this patch.

> + INV_ICM42600_TIMESTAMP_CHAN(INV_ICM42600_GYRO_SCAN_TIMESTAMP),
> };
>
> /* IIO buffer data */
> @@ -94,14 +95,20 @@ static irqreturn_t inv_icm42600_gyro_handler(int irq, void *_data)
> struct iio_poll_func *pf = _data;
> struct iio_dev *indio_dev = pf->indio_dev;
> struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
> + struct inv_icm42600_timestamp *ts = &st->timestamp.gyro;
> const size_t fifo_nb = st->fifo.nb.total;
> + const size_t gyro_nb = st->fifo.nb.gyro;
> + const uint32_t fifo_period = st->fifo.period;
> int ret;
>
> /* exit if no sample */
> if (fifo_nb == 0)
> goto out;
>
> - ret = inv_icm42600_gyro_parse_fifo(indio_dev, pf->timestamp);
> + inv_icm42600_timestamp_interrupt(ts, fifo_period, fifo_nb, gyro_nb,
> + pf->timestamp);
> +
> + ret = inv_icm42600_gyro_parse_fifo(indio_dev);
> if (ret)
> dev_err(regmap_get_device(st->map), "gyro fifo error %d\n",
> ret);
> @@ -143,6 +150,7 @@ static int inv_icm42600_gyro_update_scan_mode(struct iio_dev *indio_dev,
> }
>
> /* update data FIFO write and FIFO watermark */
> + inv_icm42600_timestamp_apply_odr(&st->timestamp.gyro, 0, 0, 0);
> ret = inv_icm42600_buffer_set_fifo_en(st, fifo_en | st->fifo.en);
> if (ret)
> goto out_unlock;
> @@ -359,6 +367,7 @@ static int inv_icm42600_gyro_write_odr(struct inv_icm42600_state *st,
> mutex_lock(&st->lock);
> conf.odr = inv_icm42600_gyro_odr_conv[idx / 2];
> ret = inv_icm42600_set_gyro_conf(st, &conf, NULL);
> + inv_icm42600_timestamp_update_odr(&st->timestamp.gyro, conf.odr);
> inv_icm42600_buffer_update_fifo_period(st);
> inv_icm42600_buffer_update_watermark(st);
> mutex_unlock(&st->lock);
> @@ -514,6 +523,9 @@ static int inv_icm42600_gyro_read_raw(struct iio_dev *indio_dev,
> case IIO_TEMP:
> return inv_icm42600_temp_read_raw(indio_dev, chan, val, val2,
> mask);
> + case IIO_TIMESTAMP:
> + return inv_icm42600_timestamp_read_raw(indio_dev, chan, val,
> + val2, mask);
> default:
> return -EINVAL;
> }
> @@ -706,13 +718,18 @@ int inv_icm42600_gyro_init(struct inv_icm42600_state *st)
> return devm_iio_device_register(dev, st->indio_gyro);
> }
>
> -int inv_icm42600_gyro_parse_fifo(struct iio_dev *indio_dev, int64_t ts)
> +int inv_icm42600_gyro_parse_fifo(struct iio_dev *indio_dev)
> {
> struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
> + struct inv_icm42600_timestamp *ts = &st->timestamp.gyro;
> + const size_t fifo_nb = st->fifo.nb.total;
> const size_t gyro_nb = st->fifo.nb.gyro;
> + const uint32_t fifo_period = st->fifo.period;
> ssize_t i, size;
> + unsigned int no;
> const void *accel, *gyro, *temp, *timestamp;
> unsigned int odr;
> + int64_t ts_val;
> struct inv_icm42600_gyro_buffer buffer;
>
> /* exit if no gyro sample */
> @@ -720,7 +737,7 @@ int inv_icm42600_gyro_parse_fifo(struct iio_dev *indio_dev, int64_t ts)
> return 0;
>
> /* parse all fifo packets */
> - for (i = 0; i < st->fifo.count; i += size) {
> + for (i = 0, no = 0; i < st->fifo.count; i += size, ++no) {
> size = inv_icm42600_fifo_decode_packet(&st->fifo.data[i],
> &accel, &gyro, &temp, &timestamp, &odr);
> dev_dbg(regmap_get_device(st->map), "gyro packet size = %zd\n",
> @@ -733,9 +750,14 @@ int inv_icm42600_gyro_parse_fifo(struct iio_dev *indio_dev, int64_t ts)
> dev_dbg(regmap_get_device(st->map), "skip gyro data\n");
> continue;
> }
> + /* update odr */
> + if (odr & INV_ICM42600_SENSOR_GYRO)
> + inv_icm42600_timestamp_apply_odr(ts, fifo_period,
> + fifo_nb, no);
> memcpy(&buffer.gyro, gyro, sizeof(buffer.gyro));
> memcpy(&buffer.temp, temp, sizeof(buffer.temp));
> - iio_push_to_buffers_with_timestamp(indio_dev, &buffer, ts);
> + ts_val = inv_icm42600_timestamp_get(ts);
> + iio_push_to_buffers_with_timestamp(indio_dev, &buffer, ts_val);
> }
>
> return 0;
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.c
> new file mode 100644
> index 000000000000..79cf777e2e84
> --- /dev/null
> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.c
> @@ -0,0 +1,246 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2020 Invensense, Inc.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/mutex.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/math64.h>
> +#include <linux/iio/iio.h>
> +
> +#include "inv_icm42600.h"
> +#include "inv_icm42600_timestamp.h"
> +
> +/* internal chip period is 32kHz, 31250ns */
> +#define INV_ICM42600_TIMESTAMP_PERIOD 31250
> +/* allow a jitter of +/- 2% */
> +#define INV_ICM42600_TIMESTAMP_JITTER 2
> +/* compute min and max periods accepted */
> +#define INV_ICM42600_TIMESTAMP_MIN_PERIOD(_p) \
> + (((_p) * (100 - INV_ICM42600_TIMESTAMP_JITTER)) / 100)
> +#define INV_ICM42600_TIMESTAMP_MAX_PERIOD(_p) \
> + (((_p) * (100 + INV_ICM42600_TIMESTAMP_JITTER)) / 100)
> +
> +static void inv_update_acc(struct inv_icm42600_timestamp_acc *acc, uint32_t val)
> +{
> + uint64_t sum = 0;
> + size_t i;
> +
> + acc->values[acc->idx++] = val;
> + if (acc->idx >= ARRAY_SIZE(acc->values))
> + acc->idx = 0;
> +
> + for (i = 0; i < ARRAY_SIZE(acc->values); ++i) {
> + if (acc->values[i] == 0)
> + break;
> + sum += acc->values[i];
> + }
> +
> + acc->val = div_u64(sum, i);
> +}
> +
> +static int inv_icm42600_timestamp_read(struct inv_icm42600_state *st,
> + uint32_t *ts)
> +{
> + struct device *dev = regmap_get_device(st->map);
> + __le32 raw;
> + int ret;
> +
> + pm_runtime_get_sync(dev);
> + mutex_lock(&st->lock);
> +
> + ret = regmap_write(st->map, INV_ICM42600_REG_SIGNAL_PATH_RESET,
> + INV_ICM42600_SIGNAL_PATH_RESET_TMST_STROBE);
> + if (ret)
> + goto exit;
> +
> + /* Read timestamp value 3 registers little-endian */
> + raw = 0;
> + ret = regmap_bulk_read(st->map, INV_ICM42600_REG_TMSTVAL, &raw, 3);
> + if (ret)
> + goto exit;
> +
> + *ts = (uint32_t)le32_to_cpu(raw);
> +exit:
> + mutex_unlock(&st->lock);
> + pm_runtime_mark_last_busy(dev);
> + pm_runtime_put_autosuspend(dev);
> + return ret;
> +}
> +
> +int inv_icm42600_timestamp_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
> + uint32_t ts;
> + int ret;
> +
> + if (chan->type != IIO_TIMESTAMP)
> + return -EINVAL;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_PROCESSED:
> + ret = iio_device_claim_direct_mode(indio_dev);
> + if (ret)
> + return ret;
> + ret = inv_icm42600_timestamp_read(st, &ts);
> + iio_device_release_direct_mode(indio_dev);

Unusual to expose it as a readable channel. Why would you want to do this?

> + if (ret)
> + return ret;
> + *val = ts * 1000;
> + return IIO_VAL_INT;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static void inv_icm42600_init_ts(struct inv_icm42600_timestamp *ts,
> + struct device *dev, uint32_t period)
> +{
> + /* initial odr for sensor is 1kHz */
> + const uint32_t default_period = 1000000;
> +
> + ts->dev = dev;
> + ts->mult = default_period / INV_ICM42600_TIMESTAMP_PERIOD;
> + ts->new_mult = period / INV_ICM42600_TIMESTAMP_PERIOD;
> + ts->period = default_period;
> +
> + /* use theoretical value for chip period */
> + inv_update_acc(&ts->chip_period, INV_ICM42600_TIMESTAMP_PERIOD);
> +}
> +
> +int inv_icm42600_timestamp_init(struct inv_icm42600_state *st)
> +{
> + unsigned int val;
> +
> + inv_icm42600_init_ts(&st->timestamp.gyro, regmap_get_device(st->map),
> + inv_icm42600_odr_to_period(st->conf.gyro.odr));
> + inv_icm42600_init_ts(&st->timestamp.accel, regmap_get_device(st->map),
> + inv_icm42600_odr_to_period(st->conf.accel.odr));
> +
> + /* enable timestamp register */
> + val = INV_ICM42600_TMST_CONFIG_TMST_TO_REGS_EN |
> + INV_ICM42600_TMST_CONFIG_TMST_EN;
> + return regmap_update_bits(st->map, INV_ICM42600_REG_TMST_CONFIG,
> + INV_ICM42600_TMST_CONFIG_MASK, val);
> +}
> +
> +void inv_icm42600_timestamp_update_odr(struct inv_icm42600_timestamp *ts,
> + int odr)
> +{
> + uint32_t period;
> +
> + period = inv_icm42600_odr_to_period(odr);
> + ts->new_mult = period / INV_ICM42600_TIMESTAMP_PERIOD;
> + dev_dbg(ts->dev, "ts new mult: %u\n", ts->new_mult);
> +}
> +
> +static bool inv_validate_period(uint32_t period, uint32_t mult)
> +{
> + const uint32_t chip_period = INV_ICM42600_TIMESTAMP_PERIOD;
> + uint32_t period_min, period_max;
> +
> + /* check that time interval between interrupts is acceptable */
> + period_min = INV_ICM42600_TIMESTAMP_MIN_PERIOD(chip_period) * mult;
> + period_max = INV_ICM42600_TIMESTAMP_MAX_PERIOD(chip_period) * mult;
> + if (period > period_min && period < period_max)
> + return true;
> + else
> + return false;
> +}
> +
> +static bool inv_compute_chip_period(struct inv_icm42600_timestamp *ts,
> + uint32_t mult, uint32_t period)
> +{
> + uint32_t new_chip_period;
> +
> + if (!inv_validate_period(period, mult))
> + return false;
> +
> + /* update chip internal period estimation */
> + new_chip_period = period / mult;
> + inv_update_acc(&ts->chip_period, new_chip_period);
> +
> + dev_dbg(ts->dev, "ts chip period: %u - val %u\n", new_chip_period,
> + ts->chip_period.val);
> +
> + return true;
> +}
> +
> +void inv_icm42600_timestamp_interrupt(struct inv_icm42600_timestamp *ts,
> + uint32_t fifo_period, size_t fifo_nb,
> + size_t sensor_nb, int64_t timestamp)
> +{
> + struct inv_icm42600_timestamp_interval *it;
> + int64_t delta, interval;
> + const uint32_t fifo_mult = fifo_period / INV_ICM42600_TIMESTAMP_PERIOD;
> + uint32_t period = ts->period;
> + int32_t m;
> + bool valid = false;
> +
> + if (fifo_nb == 0)
> + return;
> +
> + /* update interrupt timestamp and compute chip and sensor periods */
> + it = &ts->it;
> + it->lo = it->up;
> + it->up = timestamp;
> + delta = it->up - it->lo;
> + dev_dbg(ts->dev, "ts it: %lld - %lld - %lld\n", it->lo, it->up, delta);
> + if (it->lo != 0) {
> + period = div_s64(delta, fifo_nb);
> + valid = inv_compute_chip_period(ts, fifo_mult, period);
> + if (valid)
> + ts->period = ts->mult * ts->chip_period.val;
> + }
> +
> + /* no previous data, compute theoritical value from interrupt */
> + if (ts->timestamp == 0) {
> + interval = (int64_t)ts->period * (int64_t)sensor_nb;
> + ts->timestamp = it->up - interval;
> + dev_dbg(ts->dev, "ts start: %lld\n", ts->timestamp);
> + return;
> + }
> +
> + /* if interrupt interval is valid, sync with interrupt timestamp */
> + if (valid) {
> + /* compute real fifo_period */
> + fifo_period = fifo_mult * ts->chip_period.val;
> + delta = it->lo - ts->timestamp;
> + while (delta >= (fifo_period * 3 / 2))
> + delta -= fifo_period;
> + /* compute maximal adjustment value */
> + m = INV_ICM42600_TIMESTAMP_MAX_PERIOD(ts->period) - ts->period;
> + if (delta > m)
> + delta = m;
> + else if (delta < -m)
> + delta = -m;
> + dev_dbg(ts->dev, "ts adj: %lld\n", delta);
> + ts->timestamp += delta;
> + }
> +}
> +
> +void inv_icm42600_timestamp_apply_odr(struct inv_icm42600_timestamp *ts,
> + uint32_t fifo_period, size_t fifo_nb,
> + unsigned int fifo_no)
> +{
> + int64_t interval;
> + uint32_t fifo_mult;
> +
> + ts->mult = ts->new_mult;
> + ts->period = ts->mult * ts->chip_period.val;
> + dev_dbg(ts->dev, "ts mult: %u\n", ts->mult);
> +
> + if (ts->timestamp != 0) {
> + /* compute real fifo period */
> + fifo_mult = fifo_period / INV_ICM42600_TIMESTAMP_PERIOD;
> + fifo_period = fifo_mult * ts->chip_period.val;
> + /* compute timestamp from current interrupt after ODR change */
> + interval = (int64_t)(fifo_nb - fifo_no) * (int64_t)fifo_period;
> + ts->timestamp = ts->it.up - interval;
> + dev_dbg(ts->dev, "ts new: %lld\n", ts->timestamp);
> + }
> +}
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.h b/drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.h
> new file mode 100644
> index 000000000000..c865e1a9a7c8
> --- /dev/null
> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.h
> @@ -0,0 +1,82 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2020 Invensense, Inc.
> + */
> +
> +#ifndef INV_ICM42600_TIMESTAMP_H_
> +#define INV_ICM42600_TIMESTAMP_H_
> +
> +#include <linux/device.h>
> +#include <linux/iio/iio.h>
> +
> +struct inv_icm42600_state;
> +
> +struct inv_icm42600_timestamp_interval {
> + int64_t lo;
> + int64_t up;
> +};
> +
> +struct inv_icm42600_timestamp_acc {
> + uint32_t val;
> + size_t idx;
> + uint32_t values[32];
> +};
> +
> +struct inv_icm42600_timestamp {
> + struct device *dev;
> + struct inv_icm42600_timestamp_interval it;
> + int64_t timestamp;
> + uint32_t mult;
> + uint32_t new_mult;
> + uint32_t period;
> + struct inv_icm42600_timestamp_acc chip_period;
> +};
> +
> +#define INV_ICM42600_TIMESTAMP_CHAN(_index) \
> + { \
> + .type = IIO_TIMESTAMP, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), \
> + .scan_index = _index, \
> + .scan_type = { \
> + .sign = 's', \
> + .realbits = 64, \
> + .storagebits = 64, \
> + }, \
> + }
> +
> +int inv_icm42600_timestamp_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask);
> +
> +int inv_icm42600_timestamp_init(struct inv_icm42600_state *st);
> +
> +void inv_icm42600_timestamp_update_odr(struct inv_icm42600_timestamp *ts,
> + int odr);
> +
> +void inv_icm42600_timestamp_interrupt(struct inv_icm42600_timestamp *ts,
> + uint32_t fifo_period, size_t fifo_nb,
> + size_t sensor_nb, int64_t timestamp);
> +
> +static inline int64_t
> +inv_icm42600_timestamp_get(struct inv_icm42600_timestamp *ts)

Perhaps timestamp_pop? Kind of indicates that you are destructively
retrieving a timetamp.

> +{
> + ts->timestamp += ts->period;
> + dev_dbg(ts->dev, "ts: %lld\n", ts->timestamp);
> +
> + return ts->timestamp;
> +}
> +
> +void inv_icm42600_timestamp_apply_odr(struct inv_icm42600_timestamp *ts,
> + uint32_t fifo_period, size_t fifo_nb,
> + unsigned int fifo_no);
> +
> +static inline void
> +inv_icm42600_timestamp_reset(struct inv_icm42600_timestamp *ts)
> +{
> + const struct inv_icm42600_timestamp_interval interval_init = {0LL, 0LL};
> +
> + ts->it = interval_init;
> + ts->timestamp = 0;
> +}
> +
> +#endif