* [PATCH] iio: imu: inv_icm42600: stabilized timestamp in interrupt
@ 2024-05-29 15:47 inv.git-commit
2024-06-02 10:58 ` Jonathan Cameron
0 siblings, 1 reply; 2+ messages in thread
From: inv.git-commit @ 2024-05-29 15:47 UTC (permalink / raw)
To: jic23; +Cc: lars, linux-iio, stable, Jean-Baptiste Maneyrol
From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
Use IRQ_ONESHOT flag to ensure the timestamp is not updated in the
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>
---
.../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);
--
2.34.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] iio: imu: inv_icm42600: stabilized timestamp in interrupt
2024-05-29 15:47 [PATCH] iio: imu: inv_icm42600: stabilized timestamp in interrupt inv.git-commit
@ 2024-06-02 10:58 ` Jonathan Cameron
0 siblings, 0 replies; 2+ messages in thread
From: Jonathan Cameron @ 2024-06-02 10:58 UTC (permalink / raw)
To: inv.git-commit; +Cc: lars, linux-iio, stable, Jean-Baptiste Maneyrol
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);
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-06-02 10:58 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-29 15:47 [PATCH] iio: imu: inv_icm42600: stabilized timestamp in interrupt inv.git-commit
2024-06-02 10:58 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).