All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] iio: cros: Register FIFO callback after sensor is registered
@ 2022-07-11 14:47 Gwendal Grignou
  2022-07-11 18:45 ` Doug Anderson
  0 siblings, 1 reply; 4+ messages in thread
From: Gwendal Grignou @ 2022-07-11 14:47 UTC (permalink / raw)
  To: jic23, dianders, swboyd; +Cc: linux-iio, Gwendal Grignou

Instead of registering callback to process sensor events right at
initialization time, wait for the sensor to be register in the iio
subsystem.

Events can come at probe time (in case the kernel rebooted abruptly
without switching the sensor off for  instance), and be sent to IIO core
before the sensor is fully registered.

Reported-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
---
Changes since v2:
- Do not compare pointer with NULL,
- Invert logic to reduce indentation.
- Do not set local variable just before use.

Changes since v1:
- renamed from "iio: cros: Add cros_ec_sensors_core_register"
- Call devm_iio_device_register() inside cros_ec_sensors_core_register.

 drivers/iio/accel/cros_ec_accel_legacy.c      |  4 +-
 .../cros_ec_sensors/cros_ec_lid_angle.c       |  4 +-
 .../common/cros_ec_sensors/cros_ec_sensors.c  |  6 +-
 .../cros_ec_sensors/cros_ec_sensors_core.c    | 58 ++++++++++++++-----
 drivers/iio/light/cros_ec_light_prox.c        |  6 +-
 drivers/iio/pressure/cros_ec_baro.c           |  6 +-
 .../linux/iio/common/cros_ec_sensors_core.h   |  7 ++-
 7 files changed, 60 insertions(+), 31 deletions(-)

diff --git a/drivers/iio/accel/cros_ec_accel_legacy.c b/drivers/iio/accel/cros_ec_accel_legacy.c
index 1c0171f26e99e..0f403342b1fc0 100644
--- a/drivers/iio/accel/cros_ec_accel_legacy.c
+++ b/drivers/iio/accel/cros_ec_accel_legacy.c
@@ -215,7 +215,7 @@ static int cros_ec_accel_legacy_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	ret = cros_ec_sensors_core_init(pdev, indio_dev, true,
-					cros_ec_sensors_capture, NULL);
+					cros_ec_sensors_capture);
 	if (ret)
 		return ret;
 
@@ -235,7 +235,7 @@ static int cros_ec_accel_legacy_probe(struct platform_device *pdev)
 		state->sign[CROS_EC_SENSOR_Z] = -1;
 	}
 
-	return devm_iio_device_register(dev, indio_dev);
+	return cros_ec_sensors_core_register(dev, indio_dev, NULL);
 }
 
 static struct platform_driver cros_ec_accel_platform_driver = {
diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_lid_angle.c b/drivers/iio/common/cros_ec_sensors/cros_ec_lid_angle.c
index 9f780fafaed9f..119acb078af3b 100644
--- a/drivers/iio/common/cros_ec_sensors/cros_ec_lid_angle.c
+++ b/drivers/iio/common/cros_ec_sensors/cros_ec_lid_angle.c
@@ -98,7 +98,7 @@ static int cros_ec_lid_angle_probe(struct platform_device *pdev)
 	if (!indio_dev)
 		return -ENOMEM;
 
-	ret = cros_ec_sensors_core_init(pdev, indio_dev, false, NULL, NULL);
+	ret = cros_ec_sensors_core_init(pdev, indio_dev, false, NULL);
 	if (ret)
 		return ret;
 
@@ -114,7 +114,7 @@ static int cros_ec_lid_angle_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	return devm_iio_device_register(dev, indio_dev);
+	return cros_ec_sensors_core_register(dev, indio_dev, NULL);
 }
 
 static const struct platform_device_id cros_ec_lid_angle_ids[] = {
diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
index 61e07a7bb1995..66153b1850f10 100644
--- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
+++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
@@ -236,8 +236,7 @@ static int cros_ec_sensors_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	ret = cros_ec_sensors_core_init(pdev, indio_dev, true,
-					cros_ec_sensors_capture,
-					cros_ec_sensors_push_data);
+					cros_ec_sensors_capture);
 	if (ret)
 		return ret;
 
@@ -298,7 +297,8 @@ static int cros_ec_sensors_probe(struct platform_device *pdev)
 	else
 		state->core.read_ec_sensors_data = cros_ec_sensors_read_cmd;
 
-	return devm_iio_device_register(dev, indio_dev);
+	return cros_ec_sensors_core_register(dev, indio_dev,
+			cros_ec_sensors_push_data);
 }
 
 static const struct platform_device_id cros_ec_sensors_ids[] = {
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 e5ccedef13a80..05a28d353e343 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
@@ -228,21 +228,18 @@ static void cros_ec_sensors_core_clean(void *arg)
 
 /**
  * cros_ec_sensors_core_init() - basic initialization of the core structure
- * @pdev:		platform device created for the sensors
+ * @pdev:		platform device created for the sensor
  * @indio_dev:		iio device structure of the device
  * @physical_device:	true if the device refers to a physical device
  * @trigger_capture:    function pointer to call buffer is triggered,
  *    for backward compatibility.
- * @push_data:          function to call when cros_ec_sensorhub receives
- *    a sample for that sensor.
  *
  * Return: 0 on success, -errno on failure.
  */
 int cros_ec_sensors_core_init(struct platform_device *pdev,
 			      struct iio_dev *indio_dev,
 			      bool physical_device,
-			      cros_ec_sensors_capture_t trigger_capture,
-			      cros_ec_sensorhub_push_data_cb_t push_data)
+			      cros_ec_sensors_capture_t trigger_capture)
 {
 	struct device *dev = &pdev->dev;
 	struct cros_ec_sensors_core_state *state = iio_priv(indio_dev);
@@ -340,17 +337,6 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
 			if (ret)
 				return ret;
 
-			ret = cros_ec_sensorhub_register_push_data(
-					sensor_hub, sensor_platform->sensor_num,
-					indio_dev, push_data);
-			if (ret)
-				return ret;
-
-			ret = devm_add_action_or_reset(
-					dev, cros_ec_sensors_core_clean, pdev);
-			if (ret)
-				return ret;
-
 			/* Timestamp coming from FIFO are in ns since boot. */
 			ret = iio_device_set_clock(indio_dev, CLOCK_BOOTTIME);
 			if (ret)
@@ -372,6 +358,46 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
 }
 EXPORT_SYMBOL_GPL(cros_ec_sensors_core_init);
 
+/**
+ * cros_ec_sensors_core_register() - Register callback to FIFO and IIO when
+ * sensor is ready.
+ * It must be called at the end of the sensor probe routine.
+ * @dev:		device created for the sensor
+ * @indio_dev:		iio device structure of the device
+ * @push_data:          function to call when cros_ec_sensorhub receives
+ *    a sample for that sensor.
+ *
+ * Return: 0 on success, -errno on failure.
+ */
+int cros_ec_sensors_core_register(struct device *dev,
+				  struct iio_dev *indio_dev,
+				  cros_ec_sensorhub_push_data_cb_t push_data)
+{
+	struct cros_ec_sensor_platform *sensor_platform = dev_get_platdata(dev);
+	struct cros_ec_sensorhub *sensor_hub = dev_get_drvdata(dev->parent);
+	struct platform_device *pdev = to_platform_device(dev);
+	struct cros_ec_dev *ec = sensor_hub->ec;
+	int ret;
+
+	ret = devm_iio_device_register(dev, indio_dev);
+	if (ret)
+		return ret;
+
+	if (!push_data ||
+	    !cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE_FIFO))
+		return 0;
+
+	ret = cros_ec_sensorhub_register_push_data(
+			sensor_hub, sensor_platform->sensor_num,
+			indio_dev, push_data);
+	if (ret)
+		return ret;
+
+	return devm_add_action_or_reset(
+			dev, cros_ec_sensors_core_clean, pdev);
+}
+EXPORT_SYMBOL_GPL(cros_ec_sensors_core_register);
+
 /**
  * cros_ec_motion_send_host_cmd() - send motion sense host command
  * @state:		pointer to state information for device
diff --git a/drivers/iio/light/cros_ec_light_prox.c b/drivers/iio/light/cros_ec_light_prox.c
index e345e0f71b740..19e529c84e957 100644
--- a/drivers/iio/light/cros_ec_light_prox.c
+++ b/drivers/iio/light/cros_ec_light_prox.c
@@ -182,8 +182,7 @@ static int cros_ec_light_prox_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	ret = cros_ec_sensors_core_init(pdev, indio_dev, true,
-					cros_ec_sensors_capture,
-					cros_ec_sensors_push_data);
+					cros_ec_sensors_capture);
 	if (ret)
 		return ret;
 
@@ -239,7 +238,8 @@ static int cros_ec_light_prox_probe(struct platform_device *pdev)
 
 	state->core.read_ec_sensors_data = cros_ec_sensors_read_cmd;
 
-	return devm_iio_device_register(dev, indio_dev);
+	return cros_ec_sensors_core_register(dev, indio_dev,
+					     cros_ec_sensors_push_data);
 }
 
 static const struct platform_device_id cros_ec_light_prox_ids[] = {
diff --git a/drivers/iio/pressure/cros_ec_baro.c b/drivers/iio/pressure/cros_ec_baro.c
index 25217279f3507..2649c2f89e898 100644
--- a/drivers/iio/pressure/cros_ec_baro.c
+++ b/drivers/iio/pressure/cros_ec_baro.c
@@ -139,8 +139,7 @@ static int cros_ec_baro_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	ret = cros_ec_sensors_core_init(pdev, indio_dev, true,
-					cros_ec_sensors_capture,
-					cros_ec_sensors_push_data);
+					cros_ec_sensors_capture);
 	if (ret)
 		return ret;
 
@@ -185,7 +184,8 @@ static int cros_ec_baro_probe(struct platform_device *pdev)
 
 	state->core.read_ec_sensors_data = cros_ec_sensors_read_cmd;
 
-	return devm_iio_device_register(dev, indio_dev);
+	return cros_ec_sensors_core_register(dev, indio_dev,
+					     cros_ec_sensors_push_data);
 }
 
 static const struct platform_device_id cros_ec_baro_ids[] = {
diff --git a/include/linux/iio/common/cros_ec_sensors_core.h b/include/linux/iio/common/cros_ec_sensors_core.h
index a8259c8822f56..e72167b96d27e 100644
--- a/include/linux/iio/common/cros_ec_sensors_core.h
+++ b/include/linux/iio/common/cros_ec_sensors_core.h
@@ -93,8 +93,11 @@ int cros_ec_sensors_read_cmd(struct iio_dev *indio_dev, unsigned long scan_mask,
 struct platform_device;
 int cros_ec_sensors_core_init(struct platform_device *pdev,
 			      struct iio_dev *indio_dev, bool physical_device,
-			      cros_ec_sensors_capture_t trigger_capture,
-			      cros_ec_sensorhub_push_data_cb_t push_data);
+			      cros_ec_sensors_capture_t trigger_capture);
+
+int cros_ec_sensors_core_register(struct device *dev,
+				  struct iio_dev *indio_dev,
+				  cros_ec_sensorhub_push_data_cb_t push_data);
 
 irqreturn_t cros_ec_sensors_capture(int irq, void *p);
 int cros_ec_sensors_push_data(struct iio_dev *indio_dev,
-- 
2.37.0.rc0.161.g10f37bed90-goog


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH v3] iio: cros: Register FIFO callback after sensor is registered
  2022-07-11 14:47 [PATCH v3] iio: cros: Register FIFO callback after sensor is registered Gwendal Grignou
@ 2022-07-11 18:45 ` Doug Anderson
  2022-07-17 15:11   ` Jonathan Cameron
  0 siblings, 1 reply; 4+ messages in thread
From: Doug Anderson @ 2022-07-11 18:45 UTC (permalink / raw)
  To: Gwendal Grignou; +Cc: Jonathan Cameron, Stephen Boyd, linux-iio

Hi,

On Mon, Jul 11, 2022 at 7:47 AM Gwendal Grignou <gwendal@chromium.org> wrote:
>
> Instead of registering callback to process sensor events right at
> initialization time, wait for the sensor to be register in the iio
> subsystem.
>
> Events can come at probe time (in case the kernel rebooted abruptly
> without switching the sensor off for  instance), and be sent to IIO core
> before the sensor is fully registered.
>
> Reported-by: Douglas Anderson <dianders@chromium.org>
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> ---
> Changes since v2:
> - Do not compare pointer with NULL,
> - Invert logic to reduce indentation.
> - Do not set local variable just before use.
>
> Changes since v1:
> - renamed from "iio: cros: Add cros_ec_sensors_core_register"
> - Call devm_iio_device_register() inside cros_ec_sensors_core_register.
>
>  drivers/iio/accel/cros_ec_accel_legacy.c      |  4 +-
>  .../cros_ec_sensors/cros_ec_lid_angle.c       |  4 +-
>  .../common/cros_ec_sensors/cros_ec_sensors.c  |  6 +-
>  .../cros_ec_sensors/cros_ec_sensors_core.c    | 58 ++++++++++++++-----
>  drivers/iio/light/cros_ec_light_prox.c        |  6 +-
>  drivers/iio/pressure/cros_ec_baro.c           |  6 +-
>  .../linux/iio/common/cros_ec_sensors_core.h   |  7 ++-
>  7 files changed, 60 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/iio/accel/cros_ec_accel_legacy.c b/drivers/iio/accel/cros_ec_accel_legacy.c
> index 1c0171f26e99e..0f403342b1fc0 100644
> --- a/drivers/iio/accel/cros_ec_accel_legacy.c
> +++ b/drivers/iio/accel/cros_ec_accel_legacy.c
> @@ -215,7 +215,7 @@ static int cros_ec_accel_legacy_probe(struct platform_device *pdev)
>                 return -ENOMEM;
>
>         ret = cros_ec_sensors_core_init(pdev, indio_dev, true,
> -                                       cros_ec_sensors_capture, NULL);
> +                                       cros_ec_sensors_capture);
>         if (ret)
>                 return ret;
>
> @@ -235,7 +235,7 @@ static int cros_ec_accel_legacy_probe(struct platform_device *pdev)
>                 state->sign[CROS_EC_SENSOR_Z] = -1;
>         }
>
> -       return devm_iio_device_register(dev, indio_dev);
> +       return cros_ec_sensors_core_register(dev, indio_dev, NULL);
>  }
>
>  static struct platform_driver cros_ec_accel_platform_driver = {
> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_lid_angle.c b/drivers/iio/common/cros_ec_sensors/cros_ec_lid_angle.c
> index 9f780fafaed9f..119acb078af3b 100644
> --- a/drivers/iio/common/cros_ec_sensors/cros_ec_lid_angle.c
> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_lid_angle.c
> @@ -98,7 +98,7 @@ static int cros_ec_lid_angle_probe(struct platform_device *pdev)
>         if (!indio_dev)
>                 return -ENOMEM;
>
> -       ret = cros_ec_sensors_core_init(pdev, indio_dev, false, NULL, NULL);
> +       ret = cros_ec_sensors_core_init(pdev, indio_dev, false, NULL);
>         if (ret)
>                 return ret;
>
> @@ -114,7 +114,7 @@ static int cros_ec_lid_angle_probe(struct platform_device *pdev)
>         if (ret)
>                 return ret;
>
> -       return devm_iio_device_register(dev, indio_dev);
> +       return cros_ec_sensors_core_register(dev, indio_dev, NULL);
>  }
>
>  static const struct platform_device_id cros_ec_lid_angle_ids[] = {
> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> index 61e07a7bb1995..66153b1850f10 100644
> --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> @@ -236,8 +236,7 @@ static int cros_ec_sensors_probe(struct platform_device *pdev)
>                 return -ENOMEM;
>
>         ret = cros_ec_sensors_core_init(pdev, indio_dev, true,
> -                                       cros_ec_sensors_capture,
> -                                       cros_ec_sensors_push_data);
> +                                       cros_ec_sensors_capture);
>         if (ret)
>                 return ret;
>
> @@ -298,7 +297,8 @@ static int cros_ec_sensors_probe(struct platform_device *pdev)
>         else
>                 state->core.read_ec_sensors_data = cros_ec_sensors_read_cmd;
>
> -       return devm_iio_device_register(dev, indio_dev);
> +       return cros_ec_sensors_core_register(dev, indio_dev,
> +                       cros_ec_sensors_push_data);

Probably not worth spinning for, but now that the indentation was been
reduced (compared to v2) the above call doesn't need to be split
across 2 lines.

Reviewed-by: Douglas Anderson <dianders@chromium.org>

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH v3] iio: cros: Register FIFO callback after sensor is registered
  2022-07-11 18:45 ` Doug Anderson
@ 2022-07-17 15:11   ` Jonathan Cameron
  2022-07-17 16:50     ` Gwendal Grignou
  0 siblings, 1 reply; 4+ messages in thread
From: Jonathan Cameron @ 2022-07-17 15:11 UTC (permalink / raw)
  To: Doug Anderson; +Cc: Gwendal Grignou, Stephen Boyd, linux-iio

On Mon, 11 Jul 2022 11:45:36 -0700
Doug Anderson <dianders@chromium.org> wrote:

> Hi,
> 
> On Mon, Jul 11, 2022 at 7:47 AM Gwendal Grignou <gwendal@chromium.org> wrote:
> >
> > Instead of registering callback to process sensor events right at
> > initialization time, wait for the sensor to be register in the iio
> > subsystem.
> >
> > Events can come at probe time (in case the kernel rebooted abruptly
> > without switching the sensor off for  instance), and be sent to IIO core
> > before the sensor is fully registered.
> >
> > Reported-by: Douglas Anderson <dianders@chromium.org>
> > Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Applied to the togreg branch of iio.git which will be pushed out as testing
only until rc1 is out and I can rebase.

I'm assuming this doesn't need to go in quickly as a fix?  If it does
let me know (and add a fixes tag)

Thanks,

Jonathan

> > ---
> > Changes since v2:
> > - Do not compare pointer with NULL,
> > - Invert logic to reduce indentation.
> > - Do not set local variable just before use.
> >
> > Changes since v1:
> > - renamed from "iio: cros: Add cros_ec_sensors_core_register"
> > - Call devm_iio_device_register() inside cros_ec_sensors_core_register.
> >
> >  drivers/iio/accel/cros_ec_accel_legacy.c      |  4 +-
> >  .../cros_ec_sensors/cros_ec_lid_angle.c       |  4 +-
> >  .../common/cros_ec_sensors/cros_ec_sensors.c  |  6 +-
> >  .../cros_ec_sensors/cros_ec_sensors_core.c    | 58 ++++++++++++++-----
> >  drivers/iio/light/cros_ec_light_prox.c        |  6 +-
> >  drivers/iio/pressure/cros_ec_baro.c           |  6 +-
> >  .../linux/iio/common/cros_ec_sensors_core.h   |  7 ++-
> >  7 files changed, 60 insertions(+), 31 deletions(-)
> >
> > diff --git a/drivers/iio/accel/cros_ec_accel_legacy.c b/drivers/iio/accel/cros_ec_accel_legacy.c
> > index 1c0171f26e99e..0f403342b1fc0 100644
> > --- a/drivers/iio/accel/cros_ec_accel_legacy.c
> > +++ b/drivers/iio/accel/cros_ec_accel_legacy.c
> > @@ -215,7 +215,7 @@ static int cros_ec_accel_legacy_probe(struct platform_device *pdev)
> >                 return -ENOMEM;
> >
> >         ret = cros_ec_sensors_core_init(pdev, indio_dev, true,
> > -                                       cros_ec_sensors_capture, NULL);
> > +                                       cros_ec_sensors_capture);
> >         if (ret)
> >                 return ret;
> >
> > @@ -235,7 +235,7 @@ static int cros_ec_accel_legacy_probe(struct platform_device *pdev)
> >                 state->sign[CROS_EC_SENSOR_Z] = -1;
> >         }
> >
> > -       return devm_iio_device_register(dev, indio_dev);
> > +       return cros_ec_sensors_core_register(dev, indio_dev, NULL);
> >  }
> >
> >  static struct platform_driver cros_ec_accel_platform_driver = {
> > diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_lid_angle.c b/drivers/iio/common/cros_ec_sensors/cros_ec_lid_angle.c
> > index 9f780fafaed9f..119acb078af3b 100644
> > --- a/drivers/iio/common/cros_ec_sensors/cros_ec_lid_angle.c
> > +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_lid_angle.c
> > @@ -98,7 +98,7 @@ static int cros_ec_lid_angle_probe(struct platform_device *pdev)
> >         if (!indio_dev)
> >                 return -ENOMEM;
> >
> > -       ret = cros_ec_sensors_core_init(pdev, indio_dev, false, NULL, NULL);
> > +       ret = cros_ec_sensors_core_init(pdev, indio_dev, false, NULL);
> >         if (ret)
> >                 return ret;
> >
> > @@ -114,7 +114,7 @@ static int cros_ec_lid_angle_probe(struct platform_device *pdev)
> >         if (ret)
> >                 return ret;
> >
> > -       return devm_iio_device_register(dev, indio_dev);
> > +       return cros_ec_sensors_core_register(dev, indio_dev, NULL);
> >  }
> >
> >  static const struct platform_device_id cros_ec_lid_angle_ids[] = {
> > diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> > index 61e07a7bb1995..66153b1850f10 100644
> > --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> > +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> > @@ -236,8 +236,7 @@ static int cros_ec_sensors_probe(struct platform_device *pdev)
> >                 return -ENOMEM;
> >
> >         ret = cros_ec_sensors_core_init(pdev, indio_dev, true,
> > -                                       cros_ec_sensors_capture,
> > -                                       cros_ec_sensors_push_data);
> > +                                       cros_ec_sensors_capture);
> >         if (ret)
> >                 return ret;
> >
> > @@ -298,7 +297,8 @@ static int cros_ec_sensors_probe(struct platform_device *pdev)
> >         else
> >                 state->core.read_ec_sensors_data = cros_ec_sensors_read_cmd;
> >
> > -       return devm_iio_device_register(dev, indio_dev);
> > +       return cros_ec_sensors_core_register(dev, indio_dev,
> > +                       cros_ec_sensors_push_data);  
> 
> Probably not worth spinning for, but now that the indentation was been
> reduced (compared to v2) the above call doesn't need to be split
> across 2 lines.
> 
> Reviewed-by: Douglas Anderson <dianders@chromium.org>


^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH v3] iio: cros: Register FIFO callback after sensor is registered
  2022-07-17 15:11   ` Jonathan Cameron
@ 2022-07-17 16:50     ` Gwendal Grignou
  0 siblings, 0 replies; 4+ messages in thread
From: Gwendal Grignou @ 2022-07-17 16:50 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: Doug Anderson, Stephen Boyd, linux-iio

On Sun, Jul 17, 2022 at 5:01 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Mon, 11 Jul 2022 11:45:36 -0700
> Doug Anderson <dianders@chromium.org> wrote:
>
> > Hi,
> >
> > On Mon, Jul 11, 2022 at 7:47 AM Gwendal Grignou <gwendal@chromium.org> wrote:
> > >
> > > Instead of registering callback to process sensor events right at
> > > initialization time, wait for the sensor to be register in the iio
> > > subsystem.
> > >
> > > Events can come at probe time (in case the kernel rebooted abruptly
> > > without switching the sensor off for  instance), and be sent to IIO core
> > > before the sensor is fully registered.
> > >
> > > Reported-by: Douglas Anderson <dianders@chromium.org>
> > > Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> Applied to the togreg branch of iio.git which will be pushed out as testing
> only until rc1 is out and I can rebase.
>
> I'm assuming this doesn't need to go in quickly as a fix?  If it does
> let me know (and add a fixes tag)
We were lucky current drivers do not allocate channels after calling
cros_ec_sensors_core_init()
For cleanliness, we can add the following fixes tag:

Fixes: aa984f1ba4a4 ("iio: cros_ec: Register to cros_ec_sensorhub when
EC supports FIFO")

Thanks,
Gwendal
> Thanks,
>
> Jonathan
>
> > > ---
> > > Changes since v2:
> > > - Do not compare pointer with NULL,
> > > - Invert logic to reduce indentation.
> > > - Do not set local variable just before use.
> > >
> > > Changes since v1:
> > > - renamed from "iio: cros: Add cros_ec_sensors_core_register"
> > > - Call devm_iio_device_register() inside cros_ec_sensors_core_register.
> > >
> > >  drivers/iio/accel/cros_ec_accel_legacy.c      |  4 +-
> > >  .../cros_ec_sensors/cros_ec_lid_angle.c       |  4 +-
> > >  .../common/cros_ec_sensors/cros_ec_sensors.c  |  6 +-
> > >  .../cros_ec_sensors/cros_ec_sensors_core.c    | 58 ++++++++++++++-----
> > >  drivers/iio/light/cros_ec_light_prox.c        |  6 +-
> > >  drivers/iio/pressure/cros_ec_baro.c           |  6 +-
> > >  .../linux/iio/common/cros_ec_sensors_core.h   |  7 ++-
> > >  7 files changed, 60 insertions(+), 31 deletions(-)
> > >
> > > diff --git a/drivers/iio/accel/cros_ec_accel_legacy.c b/drivers/iio/accel/cros_ec_accel_legacy.c
> > > index 1c0171f26e99e..0f403342b1fc0 100644
> > > --- a/drivers/iio/accel/cros_ec_accel_legacy.c
> > > +++ b/drivers/iio/accel/cros_ec_accel_legacy.c
> > > @@ -215,7 +215,7 @@ static int cros_ec_accel_legacy_probe(struct platform_device *pdev)
> > >                 return -ENOMEM;
> > >
> > >         ret = cros_ec_sensors_core_init(pdev, indio_dev, true,
> > > -                                       cros_ec_sensors_capture, NULL);
> > > +                                       cros_ec_sensors_capture);
> > >         if (ret)
> > >                 return ret;
> > >
> > > @@ -235,7 +235,7 @@ static int cros_ec_accel_legacy_probe(struct platform_device *pdev)
> > >                 state->sign[CROS_EC_SENSOR_Z] = -1;
> > >         }
> > >
> > > -       return devm_iio_device_register(dev, indio_dev);
> > > +       return cros_ec_sensors_core_register(dev, indio_dev, NULL);
> > >  }
> > >
> > >  static struct platform_driver cros_ec_accel_platform_driver = {
> > > diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_lid_angle.c b/drivers/iio/common/cros_ec_sensors/cros_ec_lid_angle.c
> > > index 9f780fafaed9f..119acb078af3b 100644
> > > --- a/drivers/iio/common/cros_ec_sensors/cros_ec_lid_angle.c
> > > +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_lid_angle.c
> > > @@ -98,7 +98,7 @@ static int cros_ec_lid_angle_probe(struct platform_device *pdev)
> > >         if (!indio_dev)
> > >                 return -ENOMEM;
> > >
> > > -       ret = cros_ec_sensors_core_init(pdev, indio_dev, false, NULL, NULL);
> > > +       ret = cros_ec_sensors_core_init(pdev, indio_dev, false, NULL);
> > >         if (ret)
> > >                 return ret;
> > >
> > > @@ -114,7 +114,7 @@ static int cros_ec_lid_angle_probe(struct platform_device *pdev)
> > >         if (ret)
> > >                 return ret;
> > >
> > > -       return devm_iio_device_register(dev, indio_dev);
> > > +       return cros_ec_sensors_core_register(dev, indio_dev, NULL);
> > >  }
> > >
> > >  static const struct platform_device_id cros_ec_lid_angle_ids[] = {
> > > diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> > > index 61e07a7bb1995..66153b1850f10 100644
> > > --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> > > +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> > > @@ -236,8 +236,7 @@ static int cros_ec_sensors_probe(struct platform_device *pdev)
> > >                 return -ENOMEM;
> > >
> > >         ret = cros_ec_sensors_core_init(pdev, indio_dev, true,
> > > -                                       cros_ec_sensors_capture,
> > > -                                       cros_ec_sensors_push_data);
> > > +                                       cros_ec_sensors_capture);
> > >         if (ret)
> > >                 return ret;
> > >
> > > @@ -298,7 +297,8 @@ static int cros_ec_sensors_probe(struct platform_device *pdev)
> > >         else
> > >                 state->core.read_ec_sensors_data = cros_ec_sensors_read_cmd;
> > >
> > > -       return devm_iio_device_register(dev, indio_dev);
> > > +       return cros_ec_sensors_core_register(dev, indio_dev,
> > > +                       cros_ec_sensors_push_data);
> >
> > Probably not worth spinning for, but now that the indentation was been
> > reduced (compared to v2) the above call doesn't need to be split
> > across 2 lines.
> >
> > Reviewed-by: Douglas Anderson <dianders@chromium.org>
>

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-07-17 16:51 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-11 14:47 [PATCH v3] iio: cros: Register FIFO callback after sensor is registered Gwendal Grignou
2022-07-11 18:45 ` Doug Anderson
2022-07-17 15:11   ` Jonathan Cameron
2022-07-17 16:50     ` Gwendal Grignou

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.