Re: [PATCH 4/4] iio: imu: inv_icm42600: do not read FIFO count for watermark it

[Jean-Baptiste Maneyrol]

>
>
>________________________________________
>From: Jonathan Cameron <jic23@xxxxxxxxxx>
>Sent: Friday, August 21, 2026 02:28
>To: Jean-Baptiste Maneyrol via B4 Relay
>Cc: Jean-Baptiste Maneyrol; David Lechner; Nuno Sá; Andy Shevchenko; linux-iio@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Jean-Baptiste Maneyrol
>Subject: Re: [PATCH 4/4] iio: imu: inv_icm42600: do not read FIFO count for watermark it
>
>On Thu, 20 Aug 2026 21: 02: 40 +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> > > Optimize data reading
>ZjQcmQRYFpfptBannerStart
>This Message Is From an External Sender
>This message came from outside your organization.
>
>ZjQcmQRYFpfptBannerEnd
>
>On Thu, 20 Aug 2026 21:02:40 +0200
>Jean-Baptiste Maneyrol via B4 Relay <devnull+jean-baptiste.maneyrol.tdk.com@xxxxxxxxxx> wrote:
>
>> From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@xxxxxxx>
>>
>> Optimize data reading for high frequencies by not reading FIFO
>> count in case of watermark interrupt. We already know there is
>> watermark samples in the FIFO and we need to not read more than
>> watermark samples for timestamping mechanism. Let's just read FIFO
>> data directly without reading FIFO count in this case.
>
>Just to check
>
>Level interrupt?  Or one that will definitely get triggered again?
>
>The thinking on reading as much as possible is that for some devices
>we need to be very sure we drained them past the threshold or we
>don't get a fresh interrupt.  So if the threshold is set low (say 1)
>it could easily reach 2 before we get to handling the interrupt.
>Races are nasty anyway with those devices anyway but reading as
>much as possible helped.
>
>Do you have data on it being worth skipping the read? If we are
>at high frequencies I'd assume it is more useful to read more than
>the minimum just to reduce how often we read the fifo at all.
>
>Jonathan

Hello Jonathan,

because of our specific timestamp mechanism, we cannot read as much as we can.
It was already the case before this patch; we were limiting FIFO read to watermark.
It is required to preserve the timing between the watermark interrupts that
we are using to compute time elapsed in the chip frame.

Here it is only an optimization to avoid reading the FIFO count register.
Because we know we have at least watermark data since the interrupt occurred,
and we cannot read more otherwise we would break the timing between 2 interrupts.

With this optimization, I'm able for example to stream data at 1kHz using I2C. Otherwise
it is not possible on my test platform.

Hope I'm clear enough here.

Thanks,
JB

>
>
>>
>> Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@xxxxxxx>
>> ---
>>  drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c | 33 ++++++++++------------
>>  1 file changed, 15 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
>> index 1428f18408ce..1410096f6e6d 100644
>> --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
>> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
>> @@ -474,24 +474,21 @@ int inv_icm42600_buffer_fifo_read(struct inv_icm42600_state *st,
>>       st->fifo.nb.accel = 0;
>>       st->fifo.nb.total = 0;
>>
>> -     /* compute maximum FIFO read size (watermark for max = 0 interrupt case) */
>> -     if (max == 0)
>> -             max = st->fifo.watermark.value;
>> -     max_count = max * INV_ICM42600_FIFO_2SENSORS_PACKET_SIZE;
>> -
>> -     /* read FIFO count value */
>> -     raw_fifo_count = (__be16 *)st->buffer;
>> -     ret = regmap_bulk_read(st->map, INV_ICM42600_REG_FIFO_COUNT,
>> -                            raw_fifo_count, sizeof(*raw_fifo_count));
>> -     if (ret)
>> -             return ret;
>> -     st->fifo.count = be16_to_cpup(raw_fifo_count);
>> -
>> -     /* check and clamp FIFO count value */
>> -     if (st->fifo.count == 0)
>> -             return 0;
>> -     if (st->fifo.count > max_count)
>> -             st->fifo.count = max_count;
>> +     /* read watermark samples for interrupt case (max = 0) or read FIFO count */
>> +     if (max == 0) {
>> +             st->fifo.count = st->fifo.watermark.value *
>> +                              INV_ICM42600_FIFO_2SENSORS_PACKET_SIZE;
>> +     } else {
>> +             raw_fifo_count = (__be16 *)st->buffer;
>> +             ret = regmap_bulk_read(st->map, INV_ICM42600_REG_FIFO_COUNT,
>> +                                     raw_fifo_count, sizeof(*raw_fifo_count));
>> +             if (ret)
>> +                     return ret;
>> +             max_count = max * INV_ICM42600_FIFO_2SENSORS_PACKET_SIZE;
>> +             st->fifo.count = min(be16_to_cpup(raw_fifo_count), max_count);
>> +             if (st->fifo.count == 0)
>> +                     return 0;
>> +     }
>>
>>       /* read all FIFO data in internal buffer */
>>       ret = regmap_noinc_read(st->map, INV_ICM42600_REG_FIFO_DATA,
>>
>