iio: imu: inv_icm42600: fix timestamp reset
Subsystems (3)
PATCH 1/1 iio: imu: inv_icm42600: fix timestamp reset
From: Jean-Baptiste Maneyrol <[email protected]>
Timestamp reset is not done in the correct place. It must be done before enabling buffer. The reason is that interrupt timestamping is always happening when the chip is on, even if the corresponding sensor is off. When the sensor restarts, timestamp is wrong if you don't do a reset first.
--- drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c +5 -5
@@ -275,9 +275,14 @@ static int inv_icm42600_buffer_preenable(struct iio_dev *indio_dev) {struct inv_icm42600_state *st =iio_device_get_drvdata (indio_dev);struct device *dev =regmap_get_device (st->map); +struct inv_icm42600_timestamp *ts =iio_priv (indio_dev);pm_runtime_get_sync (dev); +mutex_lock (&st->lock); +inv_icm42600_timestamp_reset (ts); +mutex_unlock (&st->lock); +return 0 ; }
@@ -375,7 +380,6 @@ static int inv_icm42600_buffer_postdisable(struct iio_dev *indio_dev)struct device *dev =regmap_get_device (st->map);unsigned int sensor;unsigned int *watermark; -struct inv_icm42600_timestamp *ts;struct inv_icm42600_sensor_conf conf =INV_ICM42600_SENSOR_CONF_INIT ;unsigned int sleep_temp =0 ;unsigned int sleep_sensor =0 ;
@@ -385,11 +389,9 @@ static int inv_icm42600_buffer_postdisable(struct iio_dev *indio_dev)if (indio_dev == st->indio_gyro) { sensor =INV_ICM42600_SENSOR_GYRO ; watermark = &st->fifo.watermark.gyro; - ts =iio_priv (st->indio_gyro); }else if (indio_dev == st->indio_accel) { sensor =INV_ICM42600_SENSOR_ACCEL ; watermark = &st->fifo.watermark.accel; - ts =iio_priv (st->indio_accel); }else {return -EINVAL ; }
@@ -417,8 +419,6 @@ static int inv_icm42600_buffer_postdisable(struct iio_dev *indio_dev)if (!st->fifo.on) ret =inv_icm42600_set_temp_conf (st, false, &sleep_temp); -inv_icm42600_timestamp_reset (ts); - out_unlock:mutex_unlock (&st->lock);
Signature · 1 line
2.34.1
Jonathan Cameron
maintainerQuoted 11 lines · On Wed, 3 May 2023 20:44:10 +0000 [email protected] wrote:
From: Jean-Baptiste Maneyrol <[email protected]>
Timestamp reset is not done in the correct place. It must be done before enabling buffer. The reason is that interrupt timestamping is always happening when the chip is on, even if the corresponding sensor is off. When the sensor restarts, timestamp is wrong if you don't do a reset first.
Fixes ec74ae9fd37c ("iio: imu: inv_icm42600: add accurate timestamping")Signed-off-by Jean-Baptiste Maneyrol <[email protected]>Signed-off-by <[email protected]>
What's this sign off?
Quoted 1 line
Cc <[email protected]>
Otherwise patch looks fine.
Jonathan
Quoted 52 lines
--- drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c +5
-5@@ -275,9 +275,14 @@ static int inv_icm42600_buffer_preenable(struct iio_dev *indio_dev) {struct inv_icm42600_state *st =iio_device_get_drvdata (indio_dev);struct device *dev =regmap_get_device (st->map); +struct inv_icm42600_timestamp *ts =iio_priv (indio_dev);pm_runtime_get_sync (dev); +mutex_lock (&st->lock); +inv_icm42600_timestamp_reset (ts); +mutex_unlock (&st->lock); +return 0 ; }@@ -375,7 +380,6 @@ static int inv_icm42600_buffer_postdisable(struct iio_dev *indio_dev)struct device *dev =regmap_get_device (st->map);unsigned int sensor;unsigned int *watermark; -struct inv_icm42600_timestamp *ts;struct inv_icm42600_sensor_conf conf =INV_ICM42600_SENSOR_CONF_INIT ;unsigned int sleep_temp =0 ;unsigned int sleep_sensor =0 ;@@ -385,11 +389,9 @@ static int inv_icm42600_buffer_postdisable(struct iio_dev *indio_dev)if (indio_dev == st->indio_gyro) { sensor =INV_ICM42600_SENSOR_GYRO ; watermark = &st->fifo.watermark.gyro; - ts =iio_priv (st->indio_gyro); }else if (indio_dev == st->indio_accel) { sensor =INV_ICM42600_SENSOR_ACCEL ; watermark = &st->fifo.watermark.accel; - ts =iio_priv (st->indio_accel); }else {return -EINVAL ; }@@ -417,8 +419,6 @@ static int inv_icm42600_buffer_postdisable(struct iio_dev *indio_dev)if (!st->fifo.on) ret =inv_icm42600_set_temp_conf (st, false, &sleep_temp); -inv_icm42600_timestamp_reset (ts); - out_unlock:mutex_unlock (&st->lock);
Jean-Baptiste Maneyrol
maintainerHi Jonathan,
the signed off is for the email address we are using for sending email to the mailing list.
We cannot use our personal email address, so we are obliged to use a dedicated service account for sending email with the git send-email tool.
Should I delete the signed-off corresponding to that email address?
Thanks,
JB
From: Jonathan Cameron <[email protected]>
Sent: Saturday, May 6, 2023 20:10
To: INV Git Commit <[email protected]>
Cc: [email protected] <[email protected]>; [email protected] <[email protected]>; Jean-Baptiste Maneyrol <[email protected]>; [email protected] <[email protected]>
Subject: Re: [PATCH] iio: imu: inv_icm42600: fix timestamp reset
[CAUTION] This is an EXTERNAL email. Do not click links or open attachments unless you recognize the sender and know the content is safe.
======================================================================
Quoted 11 lines · On Wed, 3 May 2023 20:44:10 +0000 [email protected] wrote:
From: Jean-Baptiste Maneyrol <[email protected]>
Timestamp reset is not done in the correct place. It must be done before enabling buffer. The reason is that interrupt timestamping is always happening when the chip is on, even if the corresponding sensor is off. When the sensor restarts, timestamp is wrong if you don't do a reset first.
Fixes ec74ae9fd37c ("iio: imu: inv_icm42600: add accurate timestamping")Signed-off-by Jean-Baptiste Maneyrol <[email protected]>Signed-off-by <[email protected]>
What's this sign off?
Quoted 1 line
Cc <[email protected]>
Otherwise patch looks fine.
Jonathan
Quoted 52 lines
---
drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c| 10 +++++-----1 file changed, 5 insertions(+), 5 deletions(-)drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c +1
-0@@ -275,9 +275,14 @@ static int inv_icm42600_buffer_preenable(struct iio_dev *indio_dev) { struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev); struct device *dev = regmap_get_device(st->map); + struct inv_icm42600_timestamp *ts = iio_priv(indio_dev);
pm_runtime_get_sync(dev);
+mutex_lock(&st->lock);
+inv_icm42600_timestamp_reset(ts);
+mutex_unlock(&st->lock);
+
return 0;
}drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c +0
-3@@ -375,7 +380,6 @@ static int inv_icm42600_buffer_postdisable(struct iio_dev *indio_dev) struct device *dev = regmap_get_device(st->map); unsigned int sensor; unsigned int *watermark; - struct inv_icm42600_timestamp *ts; struct inv_icm42600_sensor_conf conf = INV_ICM42600_SENSOR_CONF_INIT; unsigned int sleep_temp = 0; unsigned int sleep_sensor = 0;@@ -385,11 +389,9 @@ static int inv_icm42600_buffer_postdisable(struct iio_dev *indio_dev) if (indio_dev == st->indio_gyro) { sensor = INV_ICM42600_SENSOR_GYRO; watermark = &st->fifo.watermark.gyro; - ts = iio_priv(st->indio_gyro); } else if (indio_dev == st->indio_accel) { sensor = INV_ICM42600_SENSOR_ACCEL; watermark = &st->fifo.watermark.accel; - ts = iio_priv(st->indio_accel); } else { return -EINVAL; }@@ -417,8 +419,6 @@ static int inv_icm42600_buffer_postdisable(struct iio_dev *indio_dev) if (!st->fifo.on) ret = inv_icm42600_set_temp_conf(st, false, &sleep_temp);
-inv_icm42600_timestamp_reset(ts);
-
out_unlock:
mutex_unlock(&st->lock);
Greg KH
maintainerQuoted 3 lines · Jean-Baptiste Maneyrol
Hi Jonathan,
the signed off is for the email address we are using for sending email to the mailing list.
Please read the documentation for what Signed-off-by: means, it is NOT for email aliases, it is a legal statement.
Please remove it when you resend.
thanks,
greg k-h