On Wed, 29 May 2024 15:47:17 +0000
inv.git-commit@xxxxxxx wrote:
> From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@xxxxxxx>
>
> Use IRQ_ONESHOT flag to ensure the timestamp is not updated in the
IRQF_ONESHOT
I fixed that up whilst applying.
> hard handler during the thread handler. And compute and use the
> effective watermark value that correspond to this first timestamp.
>
> This way we can ensure the timestamp is always corresponding to the
> value used by the timestamping mechanism. Otherwise, it is possible
> that between FIFO count read and FIFO processing the timestamp is
> overwritten in the hard handler.
>
> Fixes: ec74ae9fd37c ("iio: imu: inv_icm42600: add accurate timestamping")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@xxxxxxx>
Applied to the fixes-togreg branch of iio.git
Jonathan
> ---
> .../imu/inv_icm42600/inv_icm42600_buffer.c | 19 +++++++++++++++++--
> .../imu/inv_icm42600/inv_icm42600_buffer.h | 2 ++
> .../iio/imu/inv_icm42600/inv_icm42600_core.c | 1 +
> 3 files changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
> index 63b85ec88c13..a8cf74c84c3c 100644
> --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
> @@ -222,10 +222,15 @@ int inv_icm42600_buffer_update_watermark(struct inv_icm42600_state *st)
> latency_accel = period_accel * wm_accel;
>
> /* 0 value for watermark means that the sensor is turned off */
> + if (wm_gyro == 0 && wm_accel == 0)
> + return 0;
> +
> if (latency_gyro == 0) {
> watermark = wm_accel;
> + st->fifo.watermark.eff_accel = wm_accel;
> } else if (latency_accel == 0) {
> watermark = wm_gyro;
> + st->fifo.watermark.eff_gyro = wm_gyro;
> } else {
> /* compute the smallest latency that is a multiple of both */
> if (latency_gyro <= latency_accel)
> @@ -241,6 +246,13 @@ int inv_icm42600_buffer_update_watermark(struct inv_icm42600_state *st)
> watermark = latency / period;
> if (watermark < 1)
> watermark = 1;
> + /* update effective watermark */
> + st->fifo.watermark.eff_gyro = latency / period_gyro;
> + if (st->fifo.watermark.eff_gyro < 1)
> + st->fifo.watermark.eff_gyro = 1;
> + st->fifo.watermark.eff_accel = latency / period_accel;
> + if (st->fifo.watermark.eff_accel < 1)
> + st->fifo.watermark.eff_accel = 1;
> }
>
> /* compute watermark value in bytes */
> @@ -514,7 +526,7 @@ int inv_icm42600_buffer_fifo_parse(struct inv_icm42600_state *st)
> /* handle gyroscope timestamp and FIFO data parsing */
> if (st->fifo.nb.gyro > 0) {
> ts = &gyro_st->ts;
> - inv_sensors_timestamp_interrupt(ts, st->fifo.nb.gyro,
> + inv_sensors_timestamp_interrupt(ts, st->fifo.watermark.eff_gyro,
> st->timestamp.gyro);
> ret = inv_icm42600_gyro_parse_fifo(st->indio_gyro);
> if (ret)
> @@ -524,7 +536,7 @@ int inv_icm42600_buffer_fifo_parse(struct inv_icm42600_state *st)
> /* handle accelerometer timestamp and FIFO data parsing */
> if (st->fifo.nb.accel > 0) {
> ts = &accel_st->ts;
> - inv_sensors_timestamp_interrupt(ts, st->fifo.nb.accel,
> + inv_sensors_timestamp_interrupt(ts, st->fifo.watermark.eff_accel,
> st->timestamp.accel);
> ret = inv_icm42600_accel_parse_fifo(st->indio_accel);
> if (ret)
> @@ -577,6 +589,9 @@ int inv_icm42600_buffer_init(struct inv_icm42600_state *st)
> unsigned int val;
> int ret;
>
> + st->fifo.watermark.eff_gyro = 1;
> + st->fifo.watermark.eff_accel = 1;
> +
> /*
> * Default FIFO configuration (bits 7 to 5)
> * - use invalid value
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.h b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.h
> index 8b85ee333bf8..f6c85daf42b0 100644
> --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.h
> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.h
> @@ -32,6 +32,8 @@ struct inv_icm42600_fifo {
> struct {
> unsigned int gyro;
> unsigned int accel;
> + unsigned int eff_gyro;
> + unsigned int eff_accel;
> } watermark;
> size_t count;
> struct {
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
> index 96116a68ab29..62fdae530334 100644
> --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
> @@ -537,6 +537,7 @@ static int inv_icm42600_irq_init(struct inv_icm42600_state *st, int irq,
> if (ret)
> return ret;
>
> + irq_type |= IRQF_ONESHOT;
> return devm_request_threaded_irq(dev, irq, inv_icm42600_irq_timestamp,
> inv_icm42600_irq_handler, irq_type,
> "inv_icm42600", st);
