Re: [PATCH v2 2/3] iio: inv_sensors: better timestamp alignment when using watermark

[Jean-Baptiste Maneyrol]

>________________________________________
>From: Andy Shevchenko <andriy.shevchenko@xxxxxxxxx>
>Sent: Monday, July 20, 2026 13:33
>To: Jean-Baptiste Maneyrol
>Cc: Jonathan Cameron; David Lechner; Nuno Sá; Andy Shevchenko; linux-iio@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
>Subject: Re: [PATCH v2 2/3] iio: inv_sensors: better timestamp alignment when using watermark
>
>On Mon, Jul 20, 2026 at 12: 38: 33PM +0200, Jean-Baptiste Maneyrol via B4 Relay wrote: > Current interrupt timestamp alignment only changes the final timestamp. > When the watermark is in use, we have a batch of samples for each > interrupt. 
>ZjQcmQRYFpfptBannerStart
>This Message Is From an External Sender
>This message came from outside your organization.
>
>ZjQcmQRYFpfptBannerEnd
>
>On Mon, Jul 20, 2026 at 12:38:33PM +0200, Jean-Baptiste Maneyrol via B4 Relay wrote:
>
>> Current interrupt timestamp alignment only changes the final timestamp.
>> When the watermark is in use, we have a batch of samples for each
>> interrupt. The current code doesn't manage to align the timestamp
>> because the jitter is too high.
>>
>> Instead modify the estimated inter interrupt period and use that to
>> adjust the timestamp alignment over the batch in a linear fashion.
>
>...
>
>>  void inv_sensors_timestamp_interrupt(struct inv_sensors_timestamp *ts,
>
>>       if (sample_nb == 0)
>>               return;
>>
>> +     /* no previous data, compute theoretical value from interrupt */
>> +     if (ts->timestamp == 0) {
>> +             /* elapsed time: sensor period * sensor samples number */
>> +             interval = (s64)ts->period * (s64)sample_nb;
>
>The both variables are unsigned. Why do we cast them to the signed type?
>
Hello Andy,

the idea here is to have a (timestamp - interval) doing the difference on
variables of the same type, s64. I tried to avoid difference between signed
and unsigned values that can be problematic.

Interval is s64 for this reason, and that's why I cast the 2 u32 values to
s64 (which should not be a problem).

Is that OK like this or do you prefer I change to u64 (with implicit
conversion to s64 when assigning to interval)?

>> +             ts->timestamp = timestamp - interval;
>> +     }
>
>...
>
>> -     /* no previous data, compute theoretical value from interrupt */
>> -     if (ts->timestamp == 0) {
>> -             /* elapsed time: sensor period * sensor samples number */
>> -             interval = (s64)ts->period * (s64)sample_nb;
>
>Yeah, I see this is the original code, but consider revisiting.
>
>> -             ts->timestamp = it->up - interval;
>> -             return;
>> -     }
>
>--
>With Best Regards,
>Andy Shevchenko
>

Thanks,
JB