[PATCH] iio: cros_ec: Use platform sensor_num to unregister FIFO callback

[PATCH] iio: cros_ec: Use platform sensor_num to unregister FIFO callback

[Satyam Gupta]

cros_ec_sensors_core_register() registers the FIFO callback for the
sensor number found in the platform data. The matching cleanup action,
cros_ec_sensors_core_clean(), reads the number back from
st->param.info.sensor_num instead.

st->param is the EC host command buffer. Drivers fill it in under
cmd_lock before each command, while the cleanup action reads it without
that lock. It is also set only for physical sensors. If the field holds
another value when the device is unbound, the cleanup clears the wrong
slot and leaves the callback in place, still pointing to the iio_dev
that is about to be freed.

Use the platform data in the cleanup action too, so it always
unregisters the slot that cros_ec_sensors_core_register() registered.

Signed-off-by: Satyam Gupta <sgsatyam@google.com>
---
Tested on an Android desktop device (6.18, Intel) with two
accelerometers and a lid angle sensor: each sensor was unbound and
bound 20 times while a reader kept EC commands in flight, then FIFO
samples were streamed. Also tested on a ChromeOS device (6.6) with an
RGB light sensor, together with downstream changes to that driver.

 drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
index d03e40c91582..4d019132cd78 100644
--- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
+++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
@@ -216,11 +216,11 @@ static void cros_ec_sensors_core_clean(void *arg)
 	struct platform_device *pdev = (struct platform_device *)arg;
 	struct cros_ec_sensorhub *sensor_hub =
 		dev_get_drvdata(pdev->dev.parent);
-	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
-	struct cros_ec_sensors_core_state *st = iio_priv(indio_dev);
-	u8 sensor_num = st->param.info.sensor_num;
+	struct cros_ec_sensor_platform *sensor_platform =
+		dev_get_platdata(&pdev->dev);
 
-	cros_ec_sensorhub_unregister_push_data(sensor_hub, sensor_num);
+	cros_ec_sensorhub_unregister_push_data(sensor_hub,
+					       sensor_platform->sensor_num);
 }
 
 /**
-- 
2.56.0.398.g882cbad275-goog

Re: [PATCH] iio: cros_ec: Use platform sensor_num to unregister FIFO callback

[Andy Shevchenko]

On Thu, Nov 05, 2026 at 07:31:07AM +0000, Satyam Gupta wrote:
> cros_ec_sensors_core_register() registers the FIFO callback for the
> sensor number found in the platform data. The matching cleanup action,
> cros_ec_sensors_core_clean(), reads the number back from
> st->param.info.sensor_num instead.
> 
> st->param is the EC host command buffer. Drivers fill it in under
> cmd_lock before each command, while the cleanup action reads it without
> that lock. It is also set only for physical sensors. If the field holds
> another value when the device is unbound, the cleanup clears the wrong
> slot and leaves the callback in place, still pointing to the iio_dev
> that is about to be freed.
> 
> Use the platform data in the cleanup action too, so it always
> unregisters the slot that cros_ec_sensors_core_register() registered.

Sounds like a Fixes tag is good to have.

> Signed-off-by: Satyam Gupta <sgsatyam@google.com>
> ---
> Tested on an Android desktop device (6.18, Intel) with two
> accelerometers and a lid angle sensor: each sensor was unbound and
> bound 20 times while a reader kept EC commands in flight, then FIFO
> samples were streamed. Also tested on a ChromeOS device (6.6) with an
> RGB light sensor, together with downstream changes to that driver.
> 
>  drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c | 8 ++++----
> @@ -216,11 +216,11 @@ static void cros_ec_sensors_core_clean(void *arg)
>  	struct platform_device *pdev = (struct platform_device *)arg;
>  	struct cros_ec_sensorhub *sensor_hub =
>  		dev_get_drvdata(pdev->dev.parent);
> -	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> -	struct cros_ec_sensors_core_state *st = iio_priv(indio_dev);
> -	u8 sensor_num = st->param.info.sensor_num;
> +	struct cros_ec_sensor_platform *sensor_platform =
> +		dev_get_platdata(&pdev->dev);

With context I see it would be better written in the following form:

	struct platform_device *pdev = (struct platform_device *)arg;
	struct device *dev = &pdev->dev;
	struct cros_ec_sensorhub *sensor_hub = dev_get_drvdata(dev->parent);
	struct cros_ec_sensor_platform *pdata = dev_get_platdata(dev);

> -	cros_ec_sensorhub_unregister_push_data(sensor_hub, sensor_num);
> +	cros_ec_sensorhub_unregister_push_data(sensor_hub,
> +					       sensor_pdata->sensor_num);

	cros_ec_sensorhub_unregister_push_data(sensor_hub, pdata->sensor_num);

>  }

-- 
With Best Regards,
Andy Shevchenko