diff options
| author | Enrico Granata <egranata@chromium.org> | 2018-12-06 17:47:07 -0800 |
|---|---|---|
| committer | ChromeOS Commit Bot <chromeos-commit-bot@chromium.org> | 2019-02-12 02:23:20 +0000 |
| commit | a34083addedd771eef22cf2b74c2b0c656bd9751 (patch) | |
| tree | 1edc476d5a4b0fa23ead24deecc89a559ef6bb46 | |
| parent | fb6bbccab8d61b7f816f871e632de846b61cb341 (diff) | |
| download | chrome-ec-a34083addedd771eef22cf2b74c2b0c656bd9751.tar.gz | |
bmi160: Keep timestamp and FIFO synchronized
The BMI160 will clear the FIFO watermark/full interrupt when the FIFO
crosses the threshold condition - which happens upon reading from it
in load_fifo(). As a result of this, and the fact that the soft IRQ
handler for the BMI160 tries to loop until it has finished serving all
interrupt causes, there are possibilities for a race condition such as
the following to occur:
read timestamp, value is t0
read interrupt cause: it's a watermark interrupt
load fifo(t0): this clears the interrupt, allowing a new one to happen
read timestamp, value is still t0
a watermark interrupt happens at t1
read interrupt cause: watermark
load fifo(t0)
At this point, the fifo will process events at t1 as-if they had happened
at t0, which is bad and causes incorrect timestamps on the AP side.
This changes moves reading the timestamp value as close as possible
to load_fifo, and in a relative order where a new watermark interrupt
cannot happen, which ensures the timestamp and the FIFO data for it
are synchronized
BUG=b:120508077
TEST=on Bobba and Nocturne, observe that no events happen
where an identical "a" time matches two distinct (b,c) times
BRANCH=nocturne,bobba
Change-Id: I454b257366bccf2b9e4d78df5dc005a8ad7313a0
Signed-off-by: Enrico Granata <egranata@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1367013
Reviewed-by: Alexandru M Stan <amstan@chromium.org>
Reviewed-by: Diana Z <dzigterman@chromium.org>
(cherry picked from commit e5fc358df19e1c4147d35749b6ddf91e22cebbf2)
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/1387600
| -rw-r--r-- | driver/accelgyro_bmi160.c | 10 |
1 files changed, 2 insertions, 8 deletions
diff --git a/driver/accelgyro_bmi160.c b/driver/accelgyro_bmi160.c index ae0aae0991..e2bd7b0d96 100644 --- a/driver/accelgyro_bmi160.c +++ b/driver/accelgyro_bmi160.c @@ -1116,7 +1116,7 @@ static void irq_set_orientation(struct motion_sensor_t *s, */ static int irq_handler(struct motion_sensor_t *s, uint32_t *event) { - uint32_t interrupt, last_ts; + uint32_t interrupt; int rv; if ((s->type != MOTIONSENSE_TYPE_ACCEL) || @@ -1124,12 +1124,6 @@ static int irq_handler(struct motion_sensor_t *s, uint32_t *event) return EC_ERROR_NOT_HANDLED; do { - /* - * Collect timestamp before resetting the interrupt line: - * After reading, it is possible for the timestamp to change, - * before we can process the FIFO. - */ - last_ts = last_interrupt_timestamp; rv = raw_read32(s->port, s->addr, BMI160_INT_STATUS_0, &interrupt); /* @@ -1148,7 +1142,7 @@ static int irq_handler(struct motion_sensor_t *s, uint32_t *event) #endif #ifdef CONFIG_ACCEL_FIFO if (interrupt & (BMI160_FWM_INT | BMI160_FFULL_INT)) - load_fifo(s, last_ts); + load_fifo(s, last_interrupt_timestamp); #endif #ifdef CONFIG_BMI160_ORIENTATION_SENSOR irq_set_orientation(s, interrupt); |