Re: [PATCH v2] platform/chrome: sensorhub: Fix dropped timestamp events and log spam

On Wed, Jul 15, 2026 at 11:45 AM Tzung-Bi Shih <[email protected]> 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 <[email protected]>
> ---
> 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/[email protected]
> ---
>  .../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