Re: [PATCH 1/4] iio: imu: inv_icm42600: sleep before enabling FIFO data

>
>
>________________________________________
>From: Jonathan Cameron <[email protected]>
>Sent: Friday, August 21, 2026 02:06
>To: Jean-Baptiste Maneyrol via B4 Relay
>Cc: Jean-Baptiste Maneyrol; David Lechner; Nuno Sá; Andy Shevchenko; [email protected]; [email protected]; Jean-Baptiste Maneyrol
>Subject: Re: [PATCH 1/4] iio: imu: inv_icm42600: sleep before enabling FIFO data
>
>On Thu, 20 Aug 2026 21: 02: 37 +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 turning accel or
>ZjQcmQRYFpfptBannerStart
>This Message Is From an External Sender
>This message came from outside your organization.
>
>ZjQcmQRYFpfptBannerEnd
>
>On Thu, 20 Aug 2026 21:02:37 +0200
>Jean-Baptiste Maneyrol via B4 Relay <[email protected]> wrote:
>
>> From: Jean-Baptiste Maneyrol <[email protected]>
>>
>> When turning accel or gyro on, a sleep is required for letting the
>> mechanical part stabilize before reporting correct data. Do the sleep
>> before enabling FIFO data to prevent push of incorrect data. This is
>> required if the FIFO is already on because a sensor is already running.
>>
>> Signed-off-by: Jean-Baptiste Maneyrol <[email protected]>
>
>I would like a note in here on why the lock dance is
>useful. What sort of actions will be waiting on that lock and
>make sense in the interval?
>
>Other than that looks fine to me and does indeed seem logical to
>do this before the fifo starts
>
>Jonathan

Hello Jonathan,

the lock dance is required to prevent blocking the other sensor. We don't want
to prevent data coming from accelerometer when enabling gyroscope for example.
This is even more important that at high rates the FIFO can easily overflow
when sleeping, resulting in data loss.

I will add some more explanations in the commit message for v2.

Thanks,
JB

>
>
>> ---
>>  drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c | 12 ++++++++----
>>  drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c  | 12 ++++++++----
>>  2 files changed, 16 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
>> index 4b0e3cd8a506..d889c1dceec1 100644
>> --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
>> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
>> @@ -254,15 +254,19 @@ static int inv_icm42600_accel_update_scan_mode(struct iio_dev *indio_dev,
>>               fifo_en |= INV_ICM42600_SENSOR_ACCEL;
>>       }
>>
>> +     /* sleep maximum required time */
>> +     sleep = max(sleep_accel, sleep_temp);
>> +     if (sleep) {
>> +             mutex_unlock(&st->lock);
>> +             msleep(sleep);
>> +             mutex_lock(&st->lock);
>> +     }
>> +
>>       /* update data FIFO write */
>>       ret = inv_icm42600_buffer_set_fifo_en(st, fifo_en | st->fifo.en);
>>
>>  out_unlock:
>>       mutex_unlock(&st->lock);
>> -     /* sleep maximum required time */
>> -     sleep = max(sleep_accel, sleep_temp);
>> -     if (sleep)
>> -             msleep(sleep);
>>       return ret;
>>  }
>>
>> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c
>> index 253bf571439d..34037b9c1f82 100644
>> --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c
>> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c
>> @@ -126,15 +126,19 @@ static int inv_icm42600_gyro_update_scan_mode(struct iio_dev *indio_dev,
>>               fifo_en |= INV_ICM42600_SENSOR_GYRO;
>>       }
>>
>> +     /* sleep maximum required time */
>> +     sleep = max(sleep_gyro, sleep_temp);
>> +     if (sleep) {
>> +             mutex_unlock(&st->lock);
>> +             msleep(sleep);
>> +             mutex_lock(&st->lock);
>> +     }
>> +
>>       /* update data FIFO write */
>>       ret = inv_icm42600_buffer_set_fifo_en(st, fifo_en | st->fifo.en);
>>
>>  out_unlock:
>>       mutex_unlock(&st->lock);
>> -     /* sleep maximum required time */
>> -     sleep = max(sleep_gyro, sleep_temp);
>> -     if (sleep)
>> -             msleep(sleep);
>>       return ret;
>>  }
>>
>>
>
>