2022-09-26 12:35:01

by Matti Vaittinen

[permalink] [raw]
Subject: bmc150-accel-core: potential timestamp calculation issue.

Hi dee Ho peeps!

I selected you as a recipient purely based on get_maintainer.pl. Please
drop me a note if the bmc150-accel-core.c does not feel like your
playground and I'll drop you from the CC if I send further mails for
this topic. Sorry in advance.

I was ruthlessly copying timestamp logic from the bmc150-accel-core.c
for my accelerometer driver. I noticed there may be a problem if FIFO is
not read empty:

The code computes time between two samples by

...
*
* To avoid this issue we compute the actual sample period
ourselves
* based on the timestamp delta between the last two flush
operations.
*/
sample_period = (data->timestamp - data->old_timestamp);
do_div(sample_period, count);
tstamp = data->timestamp - (count - 1) * sample_period;

Here the "count" is amount of samples bmc-150 reports there is in fifo.

As far as I understand, this works only if the old_timestamp match the
time when first sample in FIFO has been captured. This is true if the
previous 'flush' (where the old_timestamp is stored) has emptied the
FIFO - but as far as I understand the IIO does not guarantee the flush
reading all samples there are in the FIFO.

If the flush leaves - say 10 samples in FIFO, then at the next flush
there old_timestamp will be time when FIFO was previously flushed, but
this time does not match the first sample in FIFO, but the 6.th. Let's
say the sensor has collected 10 new samples between the flushes. This
would mean there is now 10 new and 5 old samples in FIFO, totaling 15
samples. Yet, the time difference computed by

sample_period = (data->timestamp - data->old_timestamp);

will reflect time between flushes - which means time of 10 samples. The
division which should compute time between two samples:

do_div(sample_period, count);

will now use incorrect amount of samples (10 + 6) and the sample period
will be too small. (Or is there something I don't quite understand).

I added following piece of code in the driver I am developing:

if (samples && count > samples) {
/*
* Here we leave some old samples to the buffer. We need to
* adjust the timestamp to match the first sample in
the buffer
* or we will miscalculate the sample_period at next round.
*/
data->timestamp -= (count - samples) * sample_period;
count = samples;
}

I can send this as a patch also to the bmc150-accel-core.c - but I'd
prefer someone who is familiar with this IC to verify my finding and
suggested fix :)

Yours,
-- Matti

--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~