Re: [PATCH 1/4] iio: imu: inv_icm42600: sleep before enabling FIFO data
Andy Shevchenko <[email protected]>
| Newsgroups | org.kernel.vger.linux-iio,org.kernel.vger.linux-kernel |
|---|---|
| Organization | Intel Finland Oy - BIC 0357606-4 - c/o Alberga Business Park, 6 krs, Bertel Jungin Aukio 5, 02600 Espoo |
| Message-ID | <[email protected]> |
On Fri, Aug 21, 2026 at 12:50:37PM +0000, Jean-Baptiste Maneyrol wrote: > >From: Andy Shevchenko <[email protected]> > >Sent: Friday, August 21, 2026 13:15 > >On Thu, Aug 20, 2026 at 09:02:37PM +0200, Jean-Baptiste Maneyrol via B4 Relay wrote: ... > >> + /* 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?) > > 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. Can we have some (short) comment in the code as well? ... > >> + /* 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