Re: [PATCH 1/2] iio: invensense: better timestamp alignment when using watermark

[Jean-Baptiste Maneyrol]

>________________________________________
>From: Andy Shevchenko <andriy.shevchenko@xxxxxxxxx>
>Sent: Friday, July 17, 2026 20:06
>To: Jean-Baptiste Maneyrol
>Cc: Jonathan Cameron; David Lechner; Nuno Sá; Andy Shevchenko; linux-iio@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
>Subject: Re: [PATCH 1/2] iio: invensense: better timestamp alignment when using watermark
>
>On Fri, Jul 17, 2026 at 02: 39: 40PM +0200, Jean-Baptiste Maneyrol via B4 Relay wrote: > Current interrupt timestamp alignment only change the next coming > timestamp. For watermark where we have a batch of samples per > interrupt, it
>ZjQcmQRYFpfptBannerStart
>This Message Is From an External Sender
>This message came from outside your organization.
>
>ZjQcmQRYFpfptBannerEnd
>
>On Fri, Jul 17, 2026 at 02:39:40PM +0200, Jean-Baptiste Maneyrol via B4 Relay wrote:
>
>> Current interrupt timestamp alignment only change the next coming
>> timestamp. For watermark where we have a batch of samples per
>> interrupt, it doesn't manage to align timestamp since modification
>> is limited because of jitter.
>>
>> Implement a better version that instead modify the period to align
>> to interrupt timestamp. Period is now computed by align timestamp
>> if interrupt interval is valid, or we use the computed estimation.
>
>...
>
>> -/* compute jitter, min and max following jitter in per mille */
>> -#define INV_SENSORS_TIMESTAMP_JITTER(_val, _jitter)          \
>> -     (div_s64((_val) * (_jitter), 1000))
>> +/* compute min and max following jitter in per mille */
>>  #define INV_SENSORS_TIMESTAMP_MIN(_val, _jitter)             \
>>       (((_val) * (1000 - (_jitter))) / 1000)
>
>Side note: Just noticed "per mille" for which we now have a constant defined
>in units.h :-)
>
>...
>
>> -static void inv_align_timestamp_it(struct inv_sensors_timestamp *ts)
>> +static uint32_t inv_align_timestamp_it(struct inv_sensors_timestamp *ts,
>
>Please, use kernel types. u32 here. I think we used to have such a discussion
>a year or couple of years ago and become to an agreement that at some point
>the types should be changed in the driver before accepting any new changes
>to it.
>

Hello Andy,

you're right, I will add a preliminary patch to port all files to kernel types.

>> +                                    unsigned int sample_nb)
>
>...
>
>>       /* if interrupt interval is valid, sync with interrupt timestamp */
>> -     if (valid)
>
>Leave this line as is, just add assignment and 'else' branch.
>

Will do, no problem.

>> -             inv_align_timestamp_it(ts);
>> +     ts->period = valid ? inv_align_timestamp_it(ts, sample_nb) :
>> +                          ts->mult * ts->chip_period.val;
>>  }
>
>--
>With Best Regards,
>Andy Shevchenko
>
>

Thanks,
JB