| Message ID | 20240529154717.651863-1-inv.git-commit@tdk.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | iio: imu: inv_icm42600: stabilized timestamp in interrupt | expand |
On Wed, 29 May 2024 15:47:17 +0000 inv.git-commit@tdk.com wrote: > From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com> > > 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@vger.kernel.org > Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com> 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);
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);