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

[Jean-Baptiste Maneyrol]

>
>
>________________________________________
>From: Andy Shevchenko <andriy.shevchenko@xxxxxxxxx>
>Sent: Friday, August 21, 2026 13:15
>To: Jean-Baptiste Maneyrol
>Cc: Jonathan Cameron; David Lechner; Nuno Sá; Andy Shevchenko; linux-iio@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Jean-Baptiste Maneyrol
>Subject: Re: [PATCH 1/4] iio: imu: inv_icm42600: sleep before enabling FIFO data
>
>On Thu, Aug 20, 2026 at 09: 02: 37PM +0200, Jean-Baptiste Maneyrol via B4 Relay wrote: > When turning accel or gyro on, a sleep is required for letting the > mechanical part stabilize before reporting correct data. Do the sleep > before
>ZjQcmQRYFpfptBannerStart
>This Message Is From an External Sender
>This message came from outside your organization.
>
>ZjQcmQRYFpfptBannerEnd
>
>On Thu, Aug 20, 2026 at 09:02:37PM +0200, Jean-Baptiste Maneyrol via B4 Relay wrote:
>
>> 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.
>
>...
>
>> +     /* sleep maximum required time */
>> +     sleep = max(sleep_accel, sleep_temp);
>> +     if (sleep) {
>> +             mutex_unlock(&st->lock);
>> +             msleep(sleep);
>> +             mutex_lock(&st->lock);
>> +     }
>
>Doesn't sounds correct. Here it breaks the atomicity. Is it a problem?
>(Imagine a few CPUs doing something with this driver, is any other thread may
> intervene the flow here?)

Hello Andy,

there is no risk here because we are inside the buffer enable function, and
there is an internal IIO mutex here preventing multiple calls. The driver mutex
is used to serialize access to the chip, mainly because we have 2 independent
sensors inside, accelerometer and gyroscope. We need to unlock it here to prevent
stopping data from the other sensor if it is enabled. For example, when we enable
gyroscope while accelerometer is running, we don't want to block the chip and
prevent sending of accelerometer data during 30ms. This is even more important
because the FIFO can easily overflow at high rates, and there would be data
loss.

Hope I'm clear enough in my explanations.

Thanks,
JB

>
>...
>
>> +     /* sleep maximum required time */
>> +     sleep = max(sleep_gyro, sleep_temp);
>> +     if (sleep) {
>> +             mutex_unlock(&st->lock);
>> +             msleep(sleep);
>> +             mutex_lock(&st->lock);
>> +     }
>
>Ditto.
>
>--
>With Best Regards,
>Andy Shevchenko
>
>