| Message ID | 20241017-invn-inv-sensors-timestamp-fix-switch-fifo-off-v1-1-1bcfa70a747b@tdk.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: invensense: fix multiple odr switch when FIFO is off | expand |
On Thu, 17 Oct 2024 16:06:28 +0200 Jean-Baptiste Maneyrol via B4 Relay <devnull+jean-baptiste.maneyrol.tdk.com@kernel.org> wrote: > From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com> > > When multiple ODR switch happens during FIFO off, the change could > not be taken into account if you get back to previous FIFO on value. > For example, if you run sensor buffer at 50Hz, stop, change to > 200Hz, then back to 50Hz and restart buffer, data will be timestamped > at 200Hz. This due to testing against mult and not new_mult. > > To prevent this, let's just run apply_odr automatically when FIFO is > off. It will also simplify driver code. > > Update inv_mpu6050 and inv_icm42600 to delete now useless apply_odr. > > Fixes: 95444b9eeb8c ("iio: invensense: fix odr switching to same value") > Cc: stable@vger.kernel.org > Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com> In at least some of the cases ts is no longer used: CHECK drivers/iio/common/inv_sensors/inv_sensors_timestamp.c drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c: In function ‘inv_icm42600_gyro_update_scan_mode’: drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c:103:39: warning: unused variable ‘ts’ [-Wunused-variable] 103 | struct inv_sensors_timestamp *ts = &gyro_st->ts; | ^~ drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c: In function ‘inv_icm42600_accel_update_scan_mode’: drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c:203:39: warning: unused variable ‘ts’ [-Wunused-variable] 203 | struct inv_sensors_timestamp *ts = &accel_st->ts; | ^~ So drop those as well for v2. > --- > drivers/iio/common/inv_sensors/inv_sensors_timestamp.c | 4 ++++ > drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c | 1 - > drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c | 1 - > drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c | 1 - > 4 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/iio/common/inv_sensors/inv_sensors_timestamp.c b/drivers/iio/common/inv_sensors/inv_sensors_timestamp.c > index f44458c380d92823ce2e7e5f78ca877ea4c06118..37d0bdaa8d824f79dcd2f341be7501d249926951 100644 > --- a/drivers/iio/common/inv_sensors/inv_sensors_timestamp.c > +++ b/drivers/iio/common/inv_sensors/inv_sensors_timestamp.c > @@ -70,6 +70,10 @@ int inv_sensors_timestamp_update_odr(struct inv_sensors_timestamp *ts, > if (mult != ts->mult) > ts->new_mult = mult; > > + /* When FIFO is off, directly apply the new ODR */ > + if (!fifo) > + inv_sensors_timestamp_apply_odr(ts, 0, 0, 0); > + > return 0; > } > EXPORT_SYMBOL_NS_GPL(inv_sensors_timestamp_update_odr, IIO_INV_SENSORS_TIMESTAMP); > diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c > index 56ac198142500a2e1fc40b62cdd465cc736d8bf0..d061a64ebbf71859a3bc44644a14137dff0f9efe 100644 > --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c > +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c > @@ -229,7 +229,6 @@ static int inv_icm42600_accel_update_scan_mode(struct iio_dev *indio_dev, > } > > /* update data FIFO write */ > - inv_sensors_timestamp_apply_odr(ts, 0, 0, 0); > ret = inv_icm42600_buffer_set_fifo_en(st, fifo_en | st->fifo.en); > > out_unlock: > diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c > index 938af5b640b00f58d2b8185f752c4755edfb0d25..f1e5a9648c4f5dd34f40136d02c72c90473eff37 100644 > --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c > +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c > @@ -128,7 +128,6 @@ static int inv_icm42600_gyro_update_scan_mode(struct iio_dev *indio_dev, > } > > /* update data FIFO write */ > - inv_sensors_timestamp_apply_odr(ts, 0, 0, 0); > ret = inv_icm42600_buffer_set_fifo_en(st, fifo_en | st->fifo.en); > > out_unlock: > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c > index 3bfeabab0ec4f6fa28fbbcd47afe92af5b8a58e2..5b1088cc3704f1ad1288a0d65b2f957b91455d7f 100644 > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c > @@ -112,7 +112,6 @@ int inv_mpu6050_prepare_fifo(struct inv_mpu6050_state *st, bool enable) > if (enable) { > /* reset timestamping */ > inv_sensors_timestamp_reset(&st->timestamp); > - inv_sensors_timestamp_apply_odr(&st->timestamp, 0, 0, 0); > /* reset FIFO */ > d = st->chip_config.user_ctrl | INV_MPU6050_BIT_FIFO_RST; > ret = regmap_write(st->map, st->reg->user_ctrl, d); > > --- > base-commit: c3e9df514041ec6c46be83801b1891392f4522f7 > change-id: 20241017-invn-inv-sensors-timestamp-fix-switch-fifo-off-3f29110e95d0 > > Best regards,
Hi Jean-Baptiste, kernel test robot noticed the following build warnings: [auto build test WARNING on c3e9df514041ec6c46be83801b1891392f4522f7] url: https://github.com/intel-lab-lkp/linux/commits/Jean-Baptiste-Maneyrol-via-B4-Relay/iio-invensense-fix-multiple-odr-switch-when-FIFO-is-off/20241017-220821 base: c3e9df514041ec6c46be83801b1891392f4522f7 patch link: https://lore.kernel.org/r/20241017-invn-inv-sensors-timestamp-fix-switch-fifo-off-v1-1-1bcfa70a747b%40tdk.com patch subject: [PATCH] iio: invensense: fix multiple odr switch when FIFO is off config: i386-allyesconfig (https://download.01.org/0day-ci/archive/20241021/202410210200.tPt6CQU4-lkp@intel.com/config) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241021/202410210200.tPt6CQU4-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202410210200.tPt6CQU4-lkp@intel.com/ All warnings (new ones prefixed by >>): drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c: In function 'inv_icm42600_gyro_update_scan_mode': >> drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c:103:39: warning: unused variable 'ts' [-Wunused-variable] 103 | struct inv_sensors_timestamp *ts = &gyro_st->ts; | ^~ -- drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c: In function 'inv_icm42600_accel_update_scan_mode': >> drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c:203:39: warning: unused variable 'ts' [-Wunused-variable] 203 | struct inv_sensors_timestamp *ts = &accel_st->ts; | ^~ Kconfig warnings: (for reference only) WARNING: unmet direct dependencies detected for GET_FREE_REGION Depends on [n]: SPARSEMEM [=n] Selected by [y]: - RESOURCE_KUNIT_TEST [=y] && RUNTIME_TESTING_MENU [=y] && KUNIT [=y] vim +/ts +103 drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 96 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 97 /* enable gyroscope sensor and FIFO write */ 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 98 static int inv_icm42600_gyro_update_scan_mode(struct iio_dev *indio_dev, 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 99 const unsigned long *scan_mask) 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 100 { 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 101 struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev); a1432b5b4f4c44 Jean-Baptiste Maneyrol 2024-04-22 102 struct inv_icm42600_sensor_state *gyro_st = iio_priv(indio_dev); a1432b5b4f4c44 Jean-Baptiste Maneyrol 2024-04-22 @103 struct inv_sensors_timestamp *ts = &gyro_st->ts; 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 104 struct inv_icm42600_sensor_conf conf = INV_ICM42600_SENSOR_CONF_INIT; 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 105 unsigned int fifo_en = 0; 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 106 unsigned int sleep_gyro = 0; 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 107 unsigned int sleep_temp = 0; 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 108 unsigned int sleep; 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 109 int ret; 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 110 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 111 mutex_lock(&st->lock); 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 112 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 113 if (*scan_mask & INV_ICM42600_SCAN_MASK_TEMP) { 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 114 /* enable temp sensor */ 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 115 ret = inv_icm42600_set_temp_conf(st, true, &sleep_temp); 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 116 if (ret) 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 117 goto out_unlock; 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 118 fifo_en |= INV_ICM42600_SENSOR_TEMP; 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 119 } 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 120 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 121 if (*scan_mask & INV_ICM42600_SCAN_MASK_GYRO_3AXIS) { 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 122 /* enable gyro sensor */ 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 123 conf.mode = INV_ICM42600_SENSOR_MODE_LOW_NOISE; 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 124 ret = inv_icm42600_set_gyro_conf(st, &conf, &sleep_gyro); 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 125 if (ret) 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 126 goto out_unlock; 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 127 fifo_en |= INV_ICM42600_SENSOR_GYRO; 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 128 } 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 129 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 130 /* update data FIFO write */ 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 131 ret = inv_icm42600_buffer_set_fifo_en(st, fifo_en | st->fifo.en); 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 132 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 133 out_unlock: 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 134 mutex_unlock(&st->lock); 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 135 /* sleep maximum required time */ c788b9e56acd46 Bragatheswaran Manickavel 2023-10-27 136 sleep = max(sleep_gyro, sleep_temp); 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 137 if (sleep) 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 138 msleep(sleep); 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 139 return ret; 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 140 } 7f85e42a6c54c0 Jean-Baptiste Maneyrol 2020-06-22 141
diff --git a/drivers/iio/common/inv_sensors/inv_sensors_timestamp.c b/drivers/iio/common/inv_sensors/inv_sensors_timestamp.c index f44458c380d92823ce2e7e5f78ca877ea4c06118..37d0bdaa8d824f79dcd2f341be7501d249926951 100644 --- a/drivers/iio/common/inv_sensors/inv_sensors_timestamp.c +++ b/drivers/iio/common/inv_sensors/inv_sensors_timestamp.c @@ -70,6 +70,10 @@ int inv_sensors_timestamp_update_odr(struct inv_sensors_timestamp *ts, if (mult != ts->mult) ts->new_mult = mult; + /* When FIFO is off, directly apply the new ODR */ + if (!fifo) + inv_sensors_timestamp_apply_odr(ts, 0, 0, 0); + return 0; } EXPORT_SYMBOL_NS_GPL(inv_sensors_timestamp_update_odr, IIO_INV_SENSORS_TIMESTAMP); diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c index 56ac198142500a2e1fc40b62cdd465cc736d8bf0..d061a64ebbf71859a3bc44644a14137dff0f9efe 100644 --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c @@ -229,7 +229,6 @@ static int inv_icm42600_accel_update_scan_mode(struct iio_dev *indio_dev, } /* update data FIFO write */ - inv_sensors_timestamp_apply_odr(ts, 0, 0, 0); ret = inv_icm42600_buffer_set_fifo_en(st, fifo_en | st->fifo.en); out_unlock: diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c index 938af5b640b00f58d2b8185f752c4755edfb0d25..f1e5a9648c4f5dd34f40136d02c72c90473eff37 100644 --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c @@ -128,7 +128,6 @@ static int inv_icm42600_gyro_update_scan_mode(struct iio_dev *indio_dev, } /* update data FIFO write */ - inv_sensors_timestamp_apply_odr(ts, 0, 0, 0); ret = inv_icm42600_buffer_set_fifo_en(st, fifo_en | st->fifo.en); out_unlock: diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c index 3bfeabab0ec4f6fa28fbbcd47afe92af5b8a58e2..5b1088cc3704f1ad1288a0d65b2f957b91455d7f 100644 --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c @@ -112,7 +112,6 @@ int inv_mpu6050_prepare_fifo(struct inv_mpu6050_state *st, bool enable) if (enable) { /* reset timestamping */ inv_sensors_timestamp_reset(&st->timestamp); - inv_sensors_timestamp_apply_odr(&st->timestamp, 0, 0, 0); /* reset FIFO */ d = st->chip_config.user_ctrl | INV_MPU6050_BIT_FIFO_RST; ret = regmap_write(st->map, st->reg->user_ctrl, d);