[PATCH v6 0/7] platform/chrome: cros_ec_sensorhub: Timestamp wrap and drift filter fixes

[PATCH v6 0/7] platform/chrome: cros_ec_sensorhub: Timestamp wrap and drift filter fixes

[Satyam Gupta]

This series fixes how the 32-bit EC timestamps are unwrapped, and
several issues in the filter that converts them to the AP timebase. They
caused sensor timestamps to be off by a whole overflow period (about 71.6
minutes) after long gaps such as suspend, Out Of Order (OOO) timestamps,
and large gaps between sensor samples during batching.

Patches 1 and 2 handle the EC timestamp wrap across long gaps. Patch 3
fixes an off-by-one in the filter history shift. Patches 4 to 6 make the
drift estimate robust to interrupt jitter and to switches between
streaming and batching. Patch 7 drops samples whose timestamp goes
backward instead of re-timing them.

Drift between the AP and the sensor hub, measured from the
cros_ec_sensorhub_timestamp trace event over 10 to 20 minutes:
- Intel ISH (Panther Lake device): about 16 to 21 PPM
- standalone EC (MediaTek-based device): about 2900 PPM

Tested on both devices with the Android CTS sensor tests
(CtsSensorTestCases, 440 tests, all pass), and across a natural wrap of
the 32-bit EC timestamp with sensors running. On the MediaTek-based
device, also tested two 40 minute gaps without FIFO interrupt, longer
than half the wrap period, after streaming the accelerometer at 100 Hz:
with the FIFO interrupt disabled as is done on suspend, and with the
sensors also stopped, as the EC does in suspend, so that samples stayed
in the FIFO across the gap. In both cases the FIFO interrupt timestamp
was unwrapped one period off, patch 2 reset the filter, and the samples
read after the gap got timestamps matching their age, including the 40
minute old ones. Replaying the same traces through the current code
gives timestamps off by up to 71.5 minutes.

Changes in v6:
- Patch 1: rework "Fix AP/EC TS wrap". v5 compensated for missed
  overflows on resume and only detected forward wraps, but the sample
  and interrupt timestamps were still unwrapped separately and could end
  up a period apart, which the forward-only rule never corrected. Derive
  the sample timestamp from the interrupt timestamp instead, which also
  covers samples that stayed in the FIFO during a long suspend.
- Patch 2: new. Reset the filter when the EC and AP elapsed times
  disagree.
- Patch 3: describe the off-by-one precisely and add a Fixes: tag.
- Patch 4: fix the unit mismatch in MAX_M pointed out by Tzung-Bi: the
  slope is in ns per us, so it is 1000 times the unitless drift. With
  the fix, a MAX_DRIFT_PPM of 5000 is really 5000 PPM (v5 clamped at
  5 PPM), which covers the drift measured above.
- Patch 5: drop the unnecessary "m = 0" and use -dx instead of abs(dx)
  so that backward timestamps are not treated as valid intervals.
- Patch 7: reset batch_state[].last_len when dropping a sample, so the
  next batch does not compute its sample period across the gap.
- Rebase on chrome-platform for-next.

v5: https://lore.kernel.org/all/20261014074306.2500909-1-sgsatyam@google.com/
v4: https://lore.kernel.org/all/20261013105753.1370713-1-sgsatyam@google.com/

Satyam Gupta (2):
  platform/chrome: cros_ec_sensorhub: Unwrap sample TS relative to FIFO
    TS
  platform/chrome: cros_ec_sensorhub: Reset TS filter on EC/AP time
    mismatch

Scott Collyer (5):
  platform/chrome: cros_ec_sensorhub: Fix TS filter history shift index
  platform/chrome: cros_ec_sensorhub: Clamp clock drift slope to
    MAX_DRIFT_PPM
  platform/chrome: cros_ec_sensorhub: Reset TS filter on ODR or mode
    changes
  platform/chrome: cros_ec_sensorhub: Increase TS_HISTORY_BORED_US to 2s
  platform/chrome: cros_ec_sensorhub: Drop sensor samples with past
    timestamps

 .../platform/chrome/cros_ec_sensorhub_ring.c  | 114 ++++++++++++++----
 .../linux/platform_data/cros_ec_sensorhub.h   |   2 -
 2 files changed, 93 insertions(+), 23 deletions(-)

-- 
2.56.0.307.g5fe7bd680c-goog

[PATCH v6 1/7] platform/chrome: cros_ec_sensorhub: Unwrap sample TS relative to FIFO TS

[Satyam Gupta]

The EC reports the sample timestamp (a) and the FIFO interrupt timestamp
(b) as 32-bit microsecond counters, which wrap every 71.6 minutes. They
are unwrapped by cros_ec_sensor_ring_fix_overflow() with separate states,
assuming that two consecutive values are less than half a period apart.

After a longer gap without events, for instance across suspend, that
assumption no longer holds, and as a and b are not taken at the same
time, one of them may be unwrapped to the next period while the other is
not. The AP timestamp of a sample is computed from b - a, so the sample
timestamps are then off by a whole period, about 71.6 minutes, until
another gap happens to realign them.

Derive a from the unwrapped b and the raw difference b - a instead, so
that a and b always fall in the same period. A sample can be taken long
before its FIFO interrupt, when it stays in the FIFO during a suspend,
but only slightly after it, when it reaches the FIFO before the FIFO is
read. So treat b - a as unsigned, offset by FIFO_TS_SLACK_US for the
latter case: a is then unwrapped correctly for samples up to almost a
whole period older than b.

Signed-off-by: Satyam Gupta <sgsatyam@google.com>
---
 .../platform/chrome/cros_ec_sensorhub_ring.c  | 21 ++++++++++++++++---
 .../linux/platform_data/cros_ec_sensorhub.h   |  2 --
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_sensorhub_ring.c b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
index d92b602..7eb8841 100644
--- a/drivers/platform/chrome/cros_ec_sensorhub_ring.c
+++ b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
@@ -35,6 +35,13 @@
 /* To measure by how much the filter is overshooting, if it happens. */
 #define FUTURE_TS_ANALYTICS_COUNT_MAX 100
 
+/*
+ * A sample can reach the FIFO after the FIFO interrupt and before the FIFO
+ * is read, so its timestamp (a) can be slightly later than the interrupt
+ * one (b). This is how much later it is allowed to be.
+ */
+#define FIFO_TS_SLACK_US (1 * USEC_PER_SEC)
+
 static inline int
 cros_sensorhub_send_sample(struct cros_ec_sensorhub *sensorhub,
 			   struct cros_ec_sensors_ring_sample *sample)
@@ -441,14 +448,22 @@ cros_ec_sensor_ring_process_event(struct cros_ec_sensorhub *sensorhub,
 		(MOTIONSENSE_SENSOR_FLAG_ODR | MOTIONSENSE_SENSOR_FLAG_FLUSH);
 
 	if (in->flags & MOTIONSENSE_SENSOR_FLAG_TIMESTAMP && !async_flags) {
-		s64 a = in->timestamp;
 		s64 b = fifo_info->timestamp;
 		s64 c = fifo_timestamp;
+		s64 a;
 
-		cros_ec_sensor_ring_fix_overflow(&a, 1LL << 32,
-					  &sensorhub->overflow_a);
 		cros_ec_sensor_ring_fix_overflow(&b, 1LL << 32,
 					  &sensorhub->overflow_b);
+		/*
+		 * Derive a from b rather than unwrapping it on its own, so
+		 * that both always fall in the same overflow period. a is at
+		 * most FIFO_TS_SLACK_US later than b, but can be up to almost
+		 * a whole period earlier, when the sample stayed in the FIFO
+		 * while the FIFO interrupt was disabled, e.g. in suspend.
+		 */
+		a = b + FIFO_TS_SLACK_US -
+		    (u32)(fifo_info->timestamp + FIFO_TS_SLACK_US -
+			  in->timestamp);
 
 		if (sensorhub->tight_timestamps) {
 			cros_ec_sensor_ring_ts_filter_update(
diff --git a/include/linux/platform_data/cros_ec_sensorhub.h b/include/linux/platform_data/cros_ec_sensorhub.h
index 0ecce6a..96eaad2 100644
--- a/include/linux/platform_data/cros_ec_sensorhub.h
+++ b/include/linux/platform_data/cros_ec_sensorhub.h
@@ -127,7 +127,6 @@ struct cros_ec_sensors_ts_batch_state {
  * @fifo_info: Copy of FIFO information coming from the EC.
  * @fifo_size: Size of the ring.
  * @batch_state: Per sensor information of the last batches received.
- * @overflow_a: For handling timestamp overflow for a time (sensor events)
  * @overflow_b: For handling timestamp overflow for b time (ec interrupts)
  * @filter: Medium fileter structure.
  * @tight_timestamps: Set to truen when EC support tight timestamping:
@@ -164,7 +163,6 @@ struct cros_ec_sensorhub {
 
 	struct cros_ec_sensors_ts_batch_state *batch_state;
 
-	struct cros_ec_sensors_ec_overflow_state overflow_a;
 	struct cros_ec_sensors_ec_overflow_state overflow_b;
 
 	struct cros_ec_sensors_ts_filter_state filter;
-- 
2.56.0.307.g5fe7bd680c-goog

[PATCH v6 2/7] platform/chrome: cros_ec_sensorhub: Reset TS filter on EC/AP time mismatch

[Satyam Gupta]

The timestamp filter history is only emptied when no interrupt happened
for TS_HISTORY_BORED_US of EC time. After a long gap without events, for
instance across suspend, b may be unwrapped to the wrong overflow period
and appear to have advanced very little, while the AP time advanced by
the whole gap. The filter then keeps a history that no longer matches
the offset between the EC and the AP, and predicts timestamps off by up
to an overflow period until that history is replaced.

Also empty the filter when the EC time and the AP time elapsed between
two interrupts differ by more than TS_HISTORY_BORED_US.

Signed-off-by: Satyam Gupta <sgsatyam@google.com>
---
 drivers/platform/chrome/cros_ec_sensorhub_ring.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/platform/chrome/cros_ec_sensorhub_ring.c b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
index 7eb8841..076a65a 100644
--- a/drivers/platform/chrome/cros_ec_sensorhub_ring.c
+++ b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
@@ -294,6 +294,14 @@ cros_ec_sensor_ring_ts_filter_update(struct cros_ec_sensors_ts_filter_state
 	if (-dx > TS_HISTORY_BORED_US)
 		state->history_len = 0;
 
+	/*
+	 * Also empty it if the EC and the AP disagree on the elapsed time
+	 * (dy, in ns), for instance when b was unwrapped to the wrong
+	 * overflow period after a long gap without events.
+	 */
+	if (abs(dy) > TS_HISTORY_BORED_US * NSEC_PER_USEC)
+		state->history_len = 0;
+
 	/* Move everything over, also update offset to all absolute coords .*/
 	for (i = state->history_len - 1; i >= 1; i--) {
 		state->x_history[i] = state->x_history[i - 1] + dx;
-- 
2.56.0.307.g5fe7bd680c-goog

[PATCH v6 3/7] platform/chrome: cros_ec_sensorhub: Fix TS filter history shift index

[Satyam Gupta]

From: Scott Collyer <scollyer@google.com>

cros_ec_sensor_ring_ts_filter_update() shifts the history arrays by one
entry before storing the new IRQ timestamp pair at index 0, and then
increments history_len. The shift loop starts at history_len - 1, so the
oldest valid entry is overwritten instead of moved, and the entry that
becomes valid when history_len is incremented holds stale data. For
example, after the second update, x_history[1] does not hold the first
IRQ timestamp.

Start the loop at history_len, bounded by
CROS_EC_SENSORHUB_TS_HISTORY_SIZE - 1 so that the oldest entry is
dropped once the arrays are full.

Fixes: 93fe48a58590 ("platform/chrome: cros_ec_sensorhub: Add median filter")
Signed-off-by: Scott Collyer <scollyer@google.com>
Signed-off-by: Satyam Gupta <sgsatyam@google.com>
---
 drivers/platform/chrome/cros_ec_sensorhub_ring.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_sensorhub_ring.c b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
index 076a65a..c5331d9 100644
--- a/drivers/platform/chrome/cros_ec_sensorhub_ring.c
+++ b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
@@ -277,7 +277,7 @@ cros_ec_sensor_ring_ts_filter_update(struct cros_ec_sensors_ts_filter_state
 	s64 m; /* stored as *M_PRECISION */
 	s64 *m_history_copy = state->temp_buf;
 	s64 *error = state->temp_buf;
-	int i;
+	int i, start;
 
 	/* we trust b the most, that'll be our independent variable */
 	x = b;
@@ -303,7 +303,9 @@ cros_ec_sensor_ring_ts_filter_update(struct cros_ec_sensors_ts_filter_state
 		state->history_len = 0;
 
 	/* Move everything over, also update offset to all absolute coords .*/
-	for (i = state->history_len - 1; i >= 1; i--) {
+	start = min_t(int, state->history_len,
+		      CROS_EC_SENSORHUB_TS_HISTORY_SIZE - 1);
+	for (i = start; i >= 1; i--) {
 		state->x_history[i] = state->x_history[i - 1] + dx;
 		state->y_history[i] = state->y_history[i - 1] + dy;
 
-- 
2.56.0.307.g5fe7bd680c-goog

[PATCH v6 4/7] platform/chrome: cros_ec_sensorhub: Clamp clock drift slope to MAX_DRIFT_PPM

[Satyam Gupta]

From: Scott Collyer <scollyer@google.com>

Interrupt jitter can produce slopes between consecutive IRQ timestamp
pairs that are far larger than any real clock drift between the EC and
the AP. When such outliers dominate the history, they corrupt the median
filter used to convert EC timestamps to the AP timebase and lead to Out
Of Order (OOO) sensor events.

Clamp the slope to +/- MAX_DRIFT_PPM Parts Per Million (PPM). The slope
is computed from a time delta in ns over a time delta in us, so it is
1000 times the unitless drift; take that into account when converting
the limit to fixed point.

Set the limit to 5000 PPM. The drift measured between the AP and the
sensor hub is about 16 PPM on a device with sensors behind the Intel ISH,
but about 2900 PPM on a MediaTek-based device with sensors behind a
standalone EC.

Signed-off-by: Scott Collyer <scollyer@google.com>
Signed-off-by: Satyam Gupta <sgsatyam@google.com>
---
 drivers/platform/chrome/cros_ec_sensorhub_ring.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/platform/chrome/cros_ec_sensorhub_ring.c b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
index c5331d9..46f44c6 100644
--- a/drivers/platform/chrome/cros_ec_sensorhub_ring.c
+++ b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
@@ -23,6 +23,14 @@
 /* Precision of fixed point for the m values from the filter */
 #define M_PRECISION BIT(23)
 
+/*
+ * Maximum clock drift between the EC and the AP, in Parts Per Million
+ * (PPM). m is computed from a time delta in ns over a time delta in us,
+ * so it is 1000 times the unitless drift: 1 PPM is M_PRECISION / 1000.
+ */
+#define MAX_DRIFT_PPM 5000LL
+#define MAX_M ((s64)(MAX_DRIFT_PPM * M_PRECISION / 1000))
+
 /* Only activate the filter once we have at least this many elements. */
 #define TS_HISTORY_THRESHOLD 8
 
@@ -289,6 +297,12 @@ cros_ec_sensor_ring_ts_filter_update(struct cros_ec_sensors_ts_filter_state
 		return; /* we already have this irq in the history */
 	dy = (state->y_history[0] + state->y_offset) - y;
 	m = div64_s64(dy * M_PRECISION, dx);
+	/*
+	 * Clamp the slope to the maximum expected clock drift. Interrupt
+	 * jitter can produce slopes much larger than any real clock drift,
+	 * which would otherwise corrupt the median filter history.
+	 */
+	m = clamp(m, -MAX_M, MAX_M);
 
 	/* Empty filter if we haven't seen any action in a while. */
 	if (-dx > TS_HISTORY_BORED_US)
-- 
2.56.0.307.g5fe7bd680c-goog

[PATCH v6 5/7] platform/chrome: cros_ec_sensorhub: Reset TS filter on ODR or mode changes

[Satyam Gupta]

From: Scott Collyer <scollyer@google.com>

When a sensor changes Output Data Rate (ODR), or switches between active
streaming and low-frequency batching, the interval between EC interrupts
changes abruptly. Slopes measured over very different intervals then get
mixed in the same median filter history.

Reset the timestamp filter history when the interval between interrupts
changes by more than 10x compared to the previous interval.

Signed-off-by: Scott Collyer <scollyer@google.com>
Signed-off-by: Satyam Gupta <sgsatyam@google.com>
---
 drivers/platform/chrome/cros_ec_sensorhub_ring.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/platform/chrome/cros_ec_sensorhub_ring.c b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
index 46f44c6..cec33a7 100644
--- a/drivers/platform/chrome/cros_ec_sensorhub_ring.c
+++ b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
@@ -304,6 +304,20 @@ cros_ec_sensor_ring_ts_filter_update(struct cros_ec_sensors_ts_filter_state
 	 */
 	m = clamp(m, -MAX_M, MAX_M);
 
+	/*
+	 * Detect ODR (Output Data Rate) or mode changes (e.g., transition from
+	 * active streaming to low-frequency batching). If the interval between
+	 * interrupts changes by more than 10x in either direction, reset the
+	 * filter history to avoid mixing slopes measured over very different
+	 * intervals.
+	 */
+	if (state->history_len > 1) {
+		s64 prev_dx = -state->x_history[1];
+
+		if (prev_dx > 0 && (-dx > 10 * prev_dx || prev_dx > 10 * -dx))
+			state->history_len = 0;
+	}
+
 	/* Empty filter if we haven't seen any action in a while. */
 	if (-dx > TS_HISTORY_BORED_US)
 		state->history_len = 0;
-- 
2.56.0.307.g5fe7bd680c-goog

[PATCH v6 6/7] platform/chrome: cros_ec_sensorhub: Increase TS_HISTORY_BORED_US to 2s

[Satyam Gupta]

From: Scott Collyer <scollyer@google.com>

Increase the TS_HISTORY_BORED_US gap threshold from 0.5 seconds
(500,000 us) to 2.0 seconds (2,000,000 us) to accommodate normal sensor
batching flush intervals (which can be 1 to 2 seconds long) without
repeatedly tearing down the timestamp filter, while still resetting the
filter across actual long idle gaps or suspend.

Signed-off-by: Scott Collyer <scollyer@google.com>
Signed-off-by: Satyam Gupta <sgsatyam@google.com>
---
 drivers/platform/chrome/cros_ec_sensorhub_ring.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_sensorhub_ring.c b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
index cec33a7..ef20aac 100644
--- a/drivers/platform/chrome/cros_ec_sensorhub_ring.c
+++ b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
@@ -36,9 +36,13 @@
 
 /*
  * If we don't have any history entries for this long, empty the filter to
- * make sure there are no big discontinuities.
+ * make sure there are no big discontinuities (e.g. after suspend or long idle).
+ *
+ * This is set to 2 seconds to accommodate normal batching flush intervals
+ * (which can be up to 1-2 seconds) without constantly resetting the filter,
+ * while still ensuring a reset occurs during actual long gaps or suspend.
  */
-#define TS_HISTORY_BORED_US 500000
+#define TS_HISTORY_BORED_US 2000000
 
 /* To measure by how much the filter is overshooting, if it happens. */
 #define FUTURE_TS_ANALYTICS_COUNT_MAX 100
-- 
2.56.0.307.g5fe7bd680c-goog

[PATCH v6 7/7] platform/chrome: cros_ec_sensorhub: Drop sensor samples with past timestamps

[Satyam Gupta]

From: Scott Collyer <scollyer@google.com>

When a sample timestamp is earlier than the newest timestamp seen for
that sensor, the sample is clamped to the last batch timestamp so that
the batch spreading logic re-times it. This makes the spreading logic
treat it as part of the previous batch, which produces stale timestamps
downstream.

Drop such Out Of Order (OOO) samples instead. The previous batch length
then no longer covers the interval up to the next batch, so reset it to
prevent the next batch from computing its sample period across the gap.

Rename cros_ec_sensor_ring_check_for_past_timestamp() to
cros_ec_sensor_ring_is_past_timestamp() and make it return true for past
timestamps.

Signed-off-by: Scott Collyer <scollyer@google.com>
Signed-off-by: Satyam Gupta <sgsatyam@google.com>
---
 .../platform/chrome/cros_ec_sensorhub_ring.c  | 43 +++++++++++++------
 1 file changed, 29 insertions(+), 14 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_sensorhub_ring.c b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
index ef20aac..bc3f120 100644
--- a/drivers/platform/chrome/cros_ec_sensorhub_ring.c
+++ b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
@@ -440,23 +440,29 @@ cros_ec_sensor_ring_fix_overflow(s64 *ts,
 	state->last = *ts;
 }
 
-static void
-cros_ec_sensor_ring_check_for_past_timestamp(struct cros_ec_sensorhub
-					     *sensorhub,
-					     struct cros_ec_sensors_ring_sample
-					     *sample)
+static bool
+cros_ec_sensor_ring_is_past_timestamp(struct cros_ec_sensorhub *sensorhub,
+				      struct cros_ec_sensors_ring_sample *sample)
 {
 	const u8 sensor_id = sample->sensor_id;
 
-	/* If this event is earlier than one we saw before... */
+	/*
+	 * If this event is strictly earlier than one we saw before, it is truly
+	 * out-of-order and must be dropped to preserve monotonicity.
+	 *
+	 * We intentionally allow duplicate timestamps (==) to pass through here
+	 * because the EC/ISH may report a batch of samples with the same raw
+	 * timestamp. These duplicates are required by the downstream spreading
+	 * logic (cros_ec_sensor_ring_spread_add) to interpolate unique,
+	 * monotonic timestamps for each sample in the batch.
+	 */
 	if (sensorhub->batch_state[sensor_id].newest_sensor_event >
 	    sample->timestamp)
-		/* mark it for spreading. */
-		sample->timestamp =
-			sensorhub->batch_state[sensor_id].last_ts;
-	else
-		sensorhub->batch_state[sensor_id].newest_sensor_event =
-			sample->timestamp;
+		return true;
+
+	sensorhub->batch_state[sensor_id].newest_sensor_event =
+		sample->timestamp;
+	return false;
 }
 
 /**
@@ -608,8 +614,17 @@ cros_ec_sensor_ring_process_event(struct cros_ec_sensorhub *sensorhub,
 	for (axis = 0; axis < 3; axis++)
 		out->vector[axis] = in->data[axis];
 
-	if (sensorhub->tight_timestamps)
-		cros_ec_sensor_ring_check_for_past_timestamp(sensorhub, out);
+	if (sensorhub->tight_timestamps &&
+	    cros_ec_sensor_ring_is_past_timestamp(sensorhub, out)) {
+		/*
+		 * Drop the sample. The previous batch length no longer
+		 * covers the interval up to the next batch, so reset it to
+		 * prevent the next batch from computing its sample period
+		 * across the gap.
+		 */
+		sensorhub->batch_state[out->sensor_id].last_len = 0;
+		return false;
+	}
 	return true;
 }
 
-- 
2.56.0.307.g5fe7bd680c-goog