[PATCH] platform/chrome: sensorhub: Support devices without FIFO_INT_ENABLE

[PATCH] platform/chrome: sensorhub: Support devices without FIFO_INT_ENABLE

[Diogo Ivo]

Some devices (such as Smaug) report having MOTION_SENSE_FIFO but do not
support controlling the behaviour of the FIFO interrupt via the
FIFO_INT_ENABLE command and in these cases the interrupt is always
enabled. However, currently the code assumes that if MOTION_SENSE_FIFO
is supported then so is FIFO_INT_ENABLE, and when it tries to enable
the interrupt via this command and an unsupported device reports a
failure it then propagates this into failing the sensors probe.

In order to fix this, since the EC does not provide a way to determine
a priori if there is support for FIFO_INT_ENABLE send this command
unconditionally and carefully interpret the return value -EINVAL as a
device where FIFO_INT_ENABLE is not present and the interrupt is always
enabled.

Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
---
 drivers/platform/chrome/cros_ec_sensorhub_ring.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/platform/chrome/cros_ec_sensorhub_ring.c b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
index 1205219515d6..d487ace7d172 100644
--- a/drivers/platform/chrome/cros_ec_sensorhub_ring.c
+++ b/drivers/platform/chrome/cros_ec_sensorhub_ring.c
@@ -129,6 +129,18 @@ int cros_ec_sensorhub_ring_fifo_enable(struct cros_ec_sensorhub *sensorhub,
 	/* We expect to receive a payload of 4 bytes, ignore. */
 	if (ret > 0)
 		ret = 0;
+	/*
+	 * Some platforms (such as Smaug) don't support the FIFO_INT_ENABLE
+	 * command and the interrupt is always enabled. Since the EC protocol
+	 * does not provide a way to discover if the command is supported a
+	 * priori interpret -EINVAL as not supported and proceed normally.
+	 *
+	 * N.B: there is no danger of -EINVAL meaning any other invalid
+	 * parameter since fifo_int_enable.enable is a bool and can never
+	 * be in an invalid range.
+	 */
+	else if (ret == -EINVAL)
+		ret = 0;
 
 	return ret;
 }

---
base-commit: 50e3a23e86817c295b5a188663c24054ec5932ed
change-id: 20251104-diogo-smaug_ec_sensorhub-3ddca5b0d359

Best regards,
-- 
Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>

Re: [PATCH] platform/chrome: sensorhub: Support devices without FIFO_INT_ENABLE

[Tzung-Bi Shih]

On Wed, Nov 12, 2025 at 04:00:12PM +0000, Diogo Ivo wrote:
> In order to fix this, since the EC does not provide a way to determine
> a priori if there is support for FIFO_INT_ENABLE send this command
> unconditionally and carefully interpret the return value -EINVAL as a
> device where FIFO_INT_ENABLE is not present and the interrupt is always
> enabled.

Thanks for looking into this.

Simplified the commit message and some side notes:
- -EOPNOTSUPP[1] for learning if a EC command is unsupported.
- FIFO_INT_ENABLE is a sub-command of EC_CMD_MOTION_SENSE_CMD.
- It likely falls into [2].

[1] https://elixir.bootlin.com/linux/v6.17/source/drivers/platform/chrome/cros_ec_proto.c#L22
[2] https://crrev.com/30c5bc8e822436e6a74b36adc24d71293a5ba9c2/common/motion_sense.c#1438


Changed the commit message and code comment, and applied to

    https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git for-next

[1/1] platform/chrome: sensorhub: Support devices without FIFO_INT_ENABLE
      commit: 52075d2c7595cfd6c1ce211a9cd138a9b067c448

Thanks!

Re: [PATCH] platform/chrome: sensorhub: Support devices without FIFO_INT_ENABLE

[Diogo Ivo]

On 11/14/25 03:19, Tzung-Bi Shih wrote:
> On Wed, Nov 12, 2025 at 04:00:12PM +0000, Diogo Ivo wrote:
>> In order to fix this, since the EC does not provide a way to determine
>> a priori if there is support for FIFO_INT_ENABLE send this command
>> unconditionally and carefully interpret the return value -EINVAL as a
>> device where FIFO_INT_ENABLE is not present and the interrupt is always
>> enabled.
> 
> Thanks for looking into this.
> 
> Simplified the commit message and some side notes:
> - -EOPNOTSUPP[1] for learning if a EC command is unsupported.
> - FIFO_INT_ENABLE is a sub-command of EC_CMD_MOTION_SENSE_CMD.
> - It likely falls into [2].

In the case of Smaug the subcommand FIFO_INT_ENABLE is not present in
the switch (in->cmd) statement and thus we return EC_RES_INVALID_PARAM
from host_cmd_motion_sense() [3]. My reasoning for the workaround was
that on the EC code EC_FEATURE_MOTION_SENSE_FIFO is dependent on
CONFIG_ACCEL_FIFO [4] and the kernel checks for this EC_FEATURE when
handling a possible sensorhub [5], meaning that when we call
FIFO_INT_ENABLE then CONFIG_ACCEL_INFO is defined in EC code and we are
not hitting [2].

Thanks,
Diogo

> [1] https://elixir.bootlin.com/linux/v6.17/source/drivers/platform/chrome/cros_ec_proto.c#L22
> [2] https://crrev.com/30c5bc8e822436e6a74b36adc24d71293a5ba9c2/common/motion_sense.c#1438

[3]: 
https://chromium.googlesource.com/chromiumos/platform/ec/+/refs/heads/firmware-smaug-7900.B/common/motion_sense.c#914
[4]: 
https://chromium.googlesource.com/chromiumos/platform/ec/+/refs/heads/main/common/ec_features.c#88
[5]: 
https://elixir.bootlin.com/linux/v6.17.7/source/drivers/platform/chrome/cros_ec_sensorhub.c#L204

> Changed the commit message and code comment, and applied to
> 
>      https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git for-next
> 
> [1/1] platform/chrome: sensorhub: Support devices without FIFO_INT_ENABLE
>        commit: 52075d2c7595cfd6c1ce211a9cd138a9b067c448
> 
> Thanks!