Hello Jonathan,
the problem comes from the fifo rate setting code that is now using these states to compute if FIFO is on: fifo_on = st->chip_config.accl_fifo_enable || st->chip_config.gyro_fifo_enable || st->chip_config.magn_fifo_enable; result = inv_sensors_timestamp_update_odr(&st->timestamp, fifo_period, fifo_on); if (result) goto fifo_rate_fail_unlock;
It was not the case before using the inv_sensors_timestamp module.
If we don't set these states back to false, it is not possible to change frequency more than 1 time. This restriction comes from inv_sensors_timestamp module that is not able to handle accurately more than 1 frequency change when sensor FIFO is on. This restriction obviously doesn't exist when FIFO is off.
Thanks, JB
From: Jonathan Cameron jic23@kernel.org Sent: Saturday, February 24, 2024 17:22 To: INV Git Commit INV.git-commit@tdk.com Cc: lars@metafoo.de lars@metafoo.de; linux-iio@vger.kernel.org linux-iio@vger.kernel.org; stable@vger.kernel.org stable@vger.kernel.org; Jean-Baptiste Maneyrol Jean-Baptiste.Maneyrol@tdk.com Subject: Re: [PATCH v2] iio: imu: inv_mpu6050: fix frequency setting when chip is off On Mon, 19 Feb 2024 15: 47: 41 +0000 inv. git-commit@ tdk. com wrote: > From: Jean-Baptiste Maneyrol <jean-baptiste. maneyrol@ tdk. com> > > Track correctly FIFO state and apply ODR change before starting > the chip. Without the fix, ZjQcmQRYFpfptBannerStart This Message Is From an External Sender This message came from outside your organization. ZjQcmQRYFpfptBannerEnd On Mon, 19 Feb 2024 15:47:41 +0000 inv.git-commit@tdk.com wrote:
From: Jean-Baptiste Maneyrol jean-baptiste.maneyrol@tdk.com
Track correctly FIFO state and apply ODR change before starting the chip. Without the fix, you cannot change ODR more than 1 time when data buffering is off.
Hi Jean-Baptiste.
I think this patch needs a little more explanation. 1) Why has state changed such that we need to update these cached values? 2) What does that have to do with ODR - or is this two unrelated fixes?
I'm sure the fix is right - I just don't fully understand the issues!
Fixes: 111e1abd0045 ("iio: imu: inv_mpu6050: use the common inv_sensors timestamp module") Cc: stable@vger.kernel.org Signed-off-by: Jean-Baptiste Maneyrol jean-baptiste.maneyrol@tdk.com
V2: add missing stable tag
drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c index 676704f9151f..e6e6e94452a3 100644 --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c @@ -111,6 +111,7 @@ 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);@@ -184,6 +185,10 @@ static int inv_mpu6050_set_enable(struct iio_dev *indio_dev, bool enable) if (result) goto error_power_off; } else {
st->chip_config.gyro_fifo_enable = 0;st->chip_config.accl_fifo_enable = 0;st->chip_config.temp_fifo_enable = 0;st->chip_config.magn_fifo_enable = 0;
I think the write to actually do this is in prepare_fifo and it's not conditional on these, so why do we care?
Are these effectively paired with inv_scan_query in the enable path?
result = inv_mpu6050_prepare_fifo(st, false); if (result) goto error_power_off;-- 2.34.1