* [PATCH v2] platform/chrome: sensorhub: Fix dropped timestamp events and log spam
@ 2026-07-15 2:44 Tzung-Bi Shih
2026-07-15 7:15 ` Tomasz Figa
2026-07-16 2:43 ` Tzung-Bi Shih
0 siblings, 2 replies; 5+ messages in thread
From: Tzung-Bi Shih @ 2026-07-15 2:44 UTC (permalink / raw)
To: Benson Leung; +Cc: tzungbi, Bryam Vargas, chrome-platform, tfiga, suleiman
Commit 833740a2333c ("platform/chrome: sensorhub: Bound the EC-reported
sensor number") evaluated the `sensor_num` against the bounds limit even
for timestamp events. A timestamp event typically has a `sensor_num` of
0xff [1], causing the driver to flag it as invalid and skip to the next
event.
As a result, we'd see a flooding of "Invalid sensor number 255 from EC"
warning logs and these timestamp events were being dropped.
Move the bounds-check into cros_ec_sensor_ring_process_event() and
evaluate it only after standalone timestamp events have already been
processed and returned early.
[1] https://crrev.com/219ca6ef82ba266da788b673ee4ad50bd3ea1285/common/motion_sense_fifo.c#427
Fixes: 833740a2333c ("platform/chrome: sensorhub: Bound the EC-reported sensor number")
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
Bryam: sorry for reverting the decision (moving back the check to
cros_ec_sensor_ring_process_event()). I realized we need to prevent
dropping timestamp events and flooding the logs.
---
v2:
- Move the check to cros_ec_sensor_ring_process_event().
- Return earlier for standalone timestamp events.
v1: https://lore.kernel.org/all/20260714065202.3248017-1-tzungbi@kernel.org
---
.../platform/chrome/cros_ec_sensorhub_ring.c | 27 ++++++++++---------
1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_sensorhub_ring.c b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
index 92941924c347..d92b60213720 100644
--- a/drivers/platform/chrome/cros_ec_sensorhub_ring.c
+++ b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
@@ -475,6 +475,21 @@ cros_ec_sensor_ring_process_event(struct cros_ec_sensorhub *sensorhub,
fifo_timestamp,
*current_timestamp,
now);
+
+ /*
+ * A standalone timestamp event typically has a sensor_num of
+ * 0xff. Return early here to prevent it from hitting the
+ * bounds check below and spamming the logs.
+ */
+ return false;
+ }
+
+ /* Skip event if sensor_num from EC is out of bounds. */
+ if (in->sensor_num >= sensorhub->sensor_num) {
+ dev_warn_ratelimited(sensorhub->dev,
+ "Invalid sensor number %u from EC\n",
+ in->sensor_num);
+ return false;
}
if (in->flags & MOTIONSENSE_SENSOR_FLAG_ODR) {
@@ -502,10 +517,6 @@ cros_ec_sensor_ring_process_event(struct cros_ec_sensorhub *sensorhub,
return true;
}
- if (in->flags & MOTIONSENSE_SENSOR_FLAG_TIMESTAMP)
- /* If we just have a timestamp, skip this entry. */
- return false;
-
/* Regular sample */
out->sensor_id = in->sensor_num;
trace_cros_ec_sensorhub_data(in->sensor_num,
@@ -897,14 +908,6 @@ static void cros_ec_sensorhub_ring_handler(struct cros_ec_sensorhub *sensorhub)
for (in = sensorhub->resp->fifo_read.data, j = 0;
j < number_data; j++, in++) {
- /* Skip event if sensor_num from EC is out of bounds. */
- if (in->sensor_num >= sensorhub->sensor_num) {
- dev_warn_ratelimited(sensorhub->dev,
- "Invalid sensor number %u from EC\n",
- in->sensor_num);
- continue;
- }
-
if (cros_ec_sensor_ring_process_event(
sensorhub, fifo_info,
fifo_timestamp,
--
2.55.0.229.g6434b31f56-goog
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v2] platform/chrome: sensorhub: Fix dropped timestamp events and log spam
2026-07-15 2:44 [PATCH v2] platform/chrome: sensorhub: Fix dropped timestamp events and log spam Tzung-Bi Shih
@ 2026-07-15 7:15 ` Tomasz Figa
2026-07-15 9:28 ` Tzung-Bi Shih
2026-07-16 2:43 ` Tzung-Bi Shih
1 sibling, 1 reply; 5+ messages in thread
From: Tomasz Figa @ 2026-07-15 7:15 UTC (permalink / raw)
To: Tzung-Bi Shih; +Cc: Benson Leung, Bryam Vargas, chrome-platform, suleiman
On Wed, Jul 15, 2026 at 11:45 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> Commit 833740a2333c ("platform/chrome: sensorhub: Bound the EC-reported
> sensor number") evaluated the `sensor_num` against the bounds limit even
> for timestamp events. A timestamp event typically has a `sensor_num` of
> 0xff [1], causing the driver to flag it as invalid and skip to the next
> event.
>
> As a result, we'd see a flooding of "Invalid sensor number 255 from EC"
> warning logs and these timestamp events were being dropped.
>
> Move the bounds-check into cros_ec_sensor_ring_process_event() and
> evaluate it only after standalone timestamp events have already been
> processed and returned early.
>
> [1] https://crrev.com/219ca6ef82ba266da788b673ee4ad50bd3ea1285/common/motion_sense_fifo.c#427
>
> Fixes: 833740a2333c ("platform/chrome: sensorhub: Bound the EC-reported sensor number")
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> ---
> Bryam: sorry for reverting the decision (moving back the check to
> cros_ec_sensor_ring_process_event()). I realized we need to prevent
> dropping timestamp events and flooding the logs.
> ---
> v2:
> - Move the check to cros_ec_sensor_ring_process_event().
> - Return earlier for standalone timestamp events.
>
> v1: https://lore.kernel.org/all/20260714065202.3248017-1-tzungbi@kernel.org
> ---
> .../platform/chrome/cros_ec_sensorhub_ring.c | 27 ++++++++++---------
> 1 file changed, 15 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_sensorhub_ring.c b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
> index 92941924c347..d92b60213720 100644
> --- a/drivers/platform/chrome/cros_ec_sensorhub_ring.c
> +++ b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
> @@ -475,6 +475,21 @@ cros_ec_sensor_ring_process_event(struct cros_ec_sensorhub *sensorhub,
> fifo_timestamp,
> *current_timestamp,
> now);
> +
> + /*
> + * A standalone timestamp event typically has a sensor_num of
> + * 0xff. Return early here to prevent it from hitting the
> + * bounds check below and spamming the logs.
> + */
> + return false;
> + }
> +
> + /* Skip event if sensor_num from EC is out of bounds. */
> + if (in->sensor_num >= sensorhub->sensor_num) {
> + dev_warn_ratelimited(sensorhub->dev,
> + "Invalid sensor number %u from EC\n",
> + in->sensor_num);
> + return false;
> }
>
> if (in->flags & MOTIONSENSE_SENSOR_FLAG_ODR) {
> @@ -502,10 +517,6 @@ cros_ec_sensor_ring_process_event(struct cros_ec_sensorhub *sensorhub,
> return true;
> }
>
> - if (in->flags & MOTIONSENSE_SENSOR_FLAG_TIMESTAMP)
> - /* If we just have a timestamp, skip this entry. */
> - return false;
> -
Isn't this still needed for the case of
MOTIONSENSE_SENSOR_FLAG_TIMESTAMP and (MOTIONSENSE_SENSOR_FLAG_ODR
and/or MOTIONSENSE_SENSOR_FLAG_FLUSH)?
> /* Regular sample */
> out->sensor_id = in->sensor_num;
> trace_cros_ec_sensorhub_data(in->sensor_num,
> @@ -897,14 +908,6 @@ static void cros_ec_sensorhub_ring_handler(struct cros_ec_sensorhub *sensorhub)
>
> for (in = sensorhub->resp->fifo_read.data, j = 0;
> j < number_data; j++, in++) {
> - /* Skip event if sensor_num from EC is out of bounds. */
> - if (in->sensor_num >= sensorhub->sensor_num) {
> - dev_warn_ratelimited(sensorhub->dev,
> - "Invalid sensor number %u from EC\n",
> - in->sensor_num);
> - continue;
> - }
> -
> if (cros_ec_sensor_ring_process_event(
> sensorhub, fifo_info,
> fifo_timestamp,
> --
> 2.55.0.229.g6434b31f56-goog
>
Best,
Tomasz
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] platform/chrome: sensorhub: Fix dropped timestamp events and log spam
2026-07-15 7:15 ` Tomasz Figa
@ 2026-07-15 9:28 ` Tzung-Bi Shih
2026-07-15 9:30 ` Tomasz Figa
0 siblings, 1 reply; 5+ messages in thread
From: Tzung-Bi Shih @ 2026-07-15 9:28 UTC (permalink / raw)
To: Tomasz Figa; +Cc: Benson Leung, Bryam Vargas, chrome-platform, suleiman
On Wed, Jul 15, 2026 at 04:15:22PM +0900, Tomasz Figa wrote:
> On Wed, Jul 15, 2026 at 11:45 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
> >
> > Commit 833740a2333c ("platform/chrome: sensorhub: Bound the EC-reported
> > sensor number") evaluated the `sensor_num` against the bounds limit even
> > for timestamp events. A timestamp event typically has a `sensor_num` of
> > 0xff [1], causing the driver to flag it as invalid and skip to the next
> > event.
> >
> > As a result, we'd see a flooding of "Invalid sensor number 255 from EC"
> > warning logs and these timestamp events were being dropped.
> >
> > Move the bounds-check into cros_ec_sensor_ring_process_event() and
> > evaluate it only after standalone timestamp events have already been
> > processed and returned early.
> >
> > [1] https://crrev.com/219ca6ef82ba266da788b673ee4ad50bd3ea1285/common/motion_sense_fifo.c#427
> >
> > Fixes: 833740a2333c ("platform/chrome: sensorhub: Bound the EC-reported sensor number")
> > Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> > ---
> > Bryam: sorry for reverting the decision (moving back the check to
> > cros_ec_sensor_ring_process_event()). I realized we need to prevent
> > dropping timestamp events and flooding the logs.
> > ---
> > v2:
> > - Move the check to cros_ec_sensor_ring_process_event().
> > - Return earlier for standalone timestamp events.
> >
> > v1: https://lore.kernel.org/all/20260714065202.3248017-1-tzungbi@kernel.org
> > ---
> > .../platform/chrome/cros_ec_sensorhub_ring.c | 27 ++++++++++---------
> > 1 file changed, 15 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/platform/chrome/cros_ec_sensorhub_ring.c b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
> > index 92941924c347..d92b60213720 100644
> > --- a/drivers/platform/chrome/cros_ec_sensorhub_ring.c
> > +++ b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
> > @@ -475,6 +475,21 @@ cros_ec_sensor_ring_process_event(struct cros_ec_sensorhub *sensorhub,
> > fifo_timestamp,
> > *current_timestamp,
> > now);
> > +
> > + /*
> > + * A standalone timestamp event typically has a sensor_num of
> > + * 0xff. Return early here to prevent it from hitting the
> > + * bounds check below and spamming the logs.
> > + */
> > + return false;
> > + }
> > +
> > + /* Skip event if sensor_num from EC is out of bounds. */
> > + if (in->sensor_num >= sensorhub->sensor_num) {
> > + dev_warn_ratelimited(sensorhub->dev,
> > + "Invalid sensor number %u from EC\n",
> > + in->sensor_num);
> > + return false;
> > }
> >
> > if (in->flags & MOTIONSENSE_SENSOR_FLAG_ODR) {
> > @@ -502,10 +517,6 @@ cros_ec_sensor_ring_process_event(struct cros_ec_sensorhub *sensorhub,
> > return true;
> > }
> >
> > - if (in->flags & MOTIONSENSE_SENSOR_FLAG_TIMESTAMP)
> > - /* If we just have a timestamp, skip this entry. */
> > - return false;
> > -
>
> Isn't this still needed for the case of
> MOTIONSENSE_SENSOR_FLAG_TIMESTAMP and (MOTIONSENSE_SENSOR_FLAG_ODR
> and/or MOTIONSENSE_SENSOR_FLAG_FLUSH)?
The block serves for standalone MOTIONSENSE_SENSOR_FLAG_TIMESTAMP events.
If the flags contains ODR or FLUSH, it returns in other branches.
After applying the patch, the flow of cros_ec_sensor_ring_process_event()
looks like:
If TIMESTAMP but (!ODR and !FLUSH):
...
return <--- The patch merges the block and returns early.
Check bounds for sensor_num
If ODR:
...
return
If FLUSH:
...
return
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] platform/chrome: sensorhub: Fix dropped timestamp events and log spam
2026-07-15 9:28 ` Tzung-Bi Shih
@ 2026-07-15 9:30 ` Tomasz Figa
0 siblings, 0 replies; 5+ messages in thread
From: Tomasz Figa @ 2026-07-15 9:30 UTC (permalink / raw)
To: Tzung-Bi Shih; +Cc: Benson Leung, Bryam Vargas, chrome-platform, suleiman
On Wed, Jul 15, 2026 at 6:28 PM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> On Wed, Jul 15, 2026 at 04:15:22PM +0900, Tomasz Figa wrote:
> > On Wed, Jul 15, 2026 at 11:45 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
> > >
> > > Commit 833740a2333c ("platform/chrome: sensorhub: Bound the EC-reported
> > > sensor number") evaluated the `sensor_num` against the bounds limit even
> > > for timestamp events. A timestamp event typically has a `sensor_num` of
> > > 0xff [1], causing the driver to flag it as invalid and skip to the next
> > > event.
> > >
> > > As a result, we'd see a flooding of "Invalid sensor number 255 from EC"
> > > warning logs and these timestamp events were being dropped.
> > >
> > > Move the bounds-check into cros_ec_sensor_ring_process_event() and
> > > evaluate it only after standalone timestamp events have already been
> > > processed and returned early.
> > >
> > > [1] https://crrev.com/219ca6ef82ba266da788b673ee4ad50bd3ea1285/common/motion_sense_fifo.c#427
> > >
> > > Fixes: 833740a2333c ("platform/chrome: sensorhub: Bound the EC-reported sensor number")
> > > Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> > > ---
> > > Bryam: sorry for reverting the decision (moving back the check to
> > > cros_ec_sensor_ring_process_event()). I realized we need to prevent
> > > dropping timestamp events and flooding the logs.
> > > ---
> > > v2:
> > > - Move the check to cros_ec_sensor_ring_process_event().
> > > - Return earlier for standalone timestamp events.
> > >
> > > v1: https://lore.kernel.org/all/20260714065202.3248017-1-tzungbi@kernel.org
> > > ---
> > > .../platform/chrome/cros_ec_sensorhub_ring.c | 27 ++++++++++---------
> > > 1 file changed, 15 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/drivers/platform/chrome/cros_ec_sensorhub_ring.c b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
> > > index 92941924c347..d92b60213720 100644
> > > --- a/drivers/platform/chrome/cros_ec_sensorhub_ring.c
> > > +++ b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
> > > @@ -475,6 +475,21 @@ cros_ec_sensor_ring_process_event(struct cros_ec_sensorhub *sensorhub,
> > > fifo_timestamp,
> > > *current_timestamp,
> > > now);
> > > +
> > > + /*
> > > + * A standalone timestamp event typically has a sensor_num of
> > > + * 0xff. Return early here to prevent it from hitting the
> > > + * bounds check below and spamming the logs.
> > > + */
> > > + return false;
> > > + }
> > > +
> > > + /* Skip event if sensor_num from EC is out of bounds. */
> > > + if (in->sensor_num >= sensorhub->sensor_num) {
> > > + dev_warn_ratelimited(sensorhub->dev,
> > > + "Invalid sensor number %u from EC\n",
> > > + in->sensor_num);
> > > + return false;
> > > }
> > >
> > > if (in->flags & MOTIONSENSE_SENSOR_FLAG_ODR) {
> > > @@ -502,10 +517,6 @@ cros_ec_sensor_ring_process_event(struct cros_ec_sensorhub *sensorhub,
> > > return true;
> > > }
> > >
> > > - if (in->flags & MOTIONSENSE_SENSOR_FLAG_TIMESTAMP)
> > > - /* If we just have a timestamp, skip this entry. */
> > > - return false;
> > > -
> >
> > Isn't this still needed for the case of
> > MOTIONSENSE_SENSOR_FLAG_TIMESTAMP and (MOTIONSENSE_SENSOR_FLAG_ODR
> > and/or MOTIONSENSE_SENSOR_FLAG_FLUSH)?
>
> The block serves for standalone MOTIONSENSE_SENSOR_FLAG_TIMESTAMP events.
> If the flags contains ODR or FLUSH, it returns in other branches.
>
> After applying the patch, the flow of cros_ec_sensor_ring_process_event()
> looks like:
>
> If TIMESTAMP but (!ODR and !FLUSH):
> ...
> return <--- The patch merges the block and returns early.
>
> Check bounds for sensor_num
>
> If ODR:
> ...
> return
>
> If FLUSH:
> ...
> return
Ah, right, I didn't notice that those blocks already returned. Sorry
for the noise.
Reviewed-by: Tomasz Figa <tfiga@chromium.org>
Best,
Tomasz
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] platform/chrome: sensorhub: Fix dropped timestamp events and log spam
2026-07-15 2:44 [PATCH v2] platform/chrome: sensorhub: Fix dropped timestamp events and log spam Tzung-Bi Shih
2026-07-15 7:15 ` Tomasz Figa
@ 2026-07-16 2:43 ` Tzung-Bi Shih
1 sibling, 0 replies; 5+ messages in thread
From: Tzung-Bi Shih @ 2026-07-16 2:43 UTC (permalink / raw)
To: Benson Leung; +Cc: Bryam Vargas, chrome-platform, tfiga, suleiman
On Wed, Jul 15, 2026 at 02:44:53AM +0000, Tzung-Bi Shih wrote:
> Commit 833740a2333c ("platform/chrome: sensorhub: Bound the EC-reported
> sensor number") evaluated the `sensor_num` against the bounds limit even
> for timestamp events. A timestamp event typically has a `sensor_num` of
> 0xff [1], causing the driver to flag it as invalid and skip to the next
> event.
>
> As a result, we'd see a flooding of "Invalid sensor number 255 from EC"
> warning logs and these timestamp events were being dropped.
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git for-next
[1/1] platform/chrome: sensorhub: Fix dropped timestamp events and log spam
commit: 9a3f43b30373c61477d0d3ab52946c05f9492bf9
Thanks!
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-07-16 2:43 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-15 2:44 [PATCH v2] platform/chrome: sensorhub: Fix dropped timestamp events and log spam Tzung-Bi Shih
2026-07-15 7:15 ` Tomasz Figa
2026-07-15 9:28 ` Tzung-Bi Shih
2026-07-15 9:30 ` Tomasz Figa
2026-07-16 2:43 ` Tzung-Bi Shih
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox