Re: [PATCH v3] iio: cros: Register FIFO callback after sensor is registered

[Jonathan Cameron <jic23@kernel.org>]

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>