Re: [PATCH v2 04/18] platform/mfd:iio: cros_ec: Register sensor through sensorhub
Hi Gwendal, On 21/10/19 18:00, Jonathan Cameron wrote: > On Sun, 20 Oct 2019 22:53:49 -0700 > Gwendal Grignou wrote: > >> - Remove duplicate code in mfd, since mfd just register >> cros_ec_sensorhub if at least one sensor is present >> - Change iio cros_ec driver to get the pointer to the cros_ec_dev >> through cros_ec_sensorhub. >> >> Signed-off-by: Gwendal Grignou > FWIW given I don't known the driver that well. > Looks good to me. > > Acked-by: Jonathan Cameron > The same from my side Acked-by: Enric Balletbo i Serra >> --- >> Changes in v2: >> - Remove unerelated changes. >> - Remove ec presence test in iio driver, done in cros_ec_sensorhub. >> >> drivers/iio/accel/cros_ec_accel_legacy.c | 6 - >> .../common/cros_ec_sensors/cros_ec_sensors.c | 6 - >> .../cros_ec_sensors/cros_ec_sensors_core.c| 4 +- >> drivers/iio/light/cros_ec_light_prox.c| 6 - >> drivers/mfd/cros_ec_dev.c | 203 ++ >> include/linux/platform_data/cros_ec_proto.h | 8 - >> .../linux/platform_data/cros_ec_sensorhub.h | 8 + >> 7 files changed, 23 insertions(+), 218 deletions(-) >> >> diff --git a/drivers/iio/accel/cros_ec_accel_legacy.c >> b/drivers/iio/accel/cros_ec_accel_legacy.c >> index fcc3f999e4827..65f85faf6f31d 100644 >> --- a/drivers/iio/accel/cros_ec_accel_legacy.c >> +++ b/drivers/iio/accel/cros_ec_accel_legacy.c >> @@ -163,16 +163,10 @@ static const struct iio_chan_spec >> cros_ec_accel_legacy_channels[] = { >> static int cros_ec_accel_legacy_probe(struct platform_device *pdev) >> { >> struct device *dev = >dev; >> -struct cros_ec_dev *ec = dev_get_drvdata(dev->parent); >> struct iio_dev *indio_dev; >> struct cros_ec_sensors_core_state *state; >> int ret; >> >> -if (!ec || !ec->ec_dev) { >> -dev_warn(>dev, "No EC device found.\n"); >> -return -EINVAL; >> -} >> - >> indio_dev = devm_iio_device_alloc(>dev, sizeof(*state)); >> if (!indio_dev) >> return -ENOMEM; >> 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 a6987726eeb8a..7dce044734678 100644 >> --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c >> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c >> @@ -222,17 +222,11 @@ static const struct iio_info ec_sensors_info = { >> static int cros_ec_sensors_probe(struct platform_device *pdev) >> { >> struct device *dev = >dev; >> -struct cros_ec_dev *ec_dev = dev_get_drvdata(dev->parent); >> struct iio_dev *indio_dev; >> struct cros_ec_sensors_state *state; >> struct iio_chan_spec *channel; >> int ret, i; >> >> -if (!ec_dev || !ec_dev->ec_dev) { >> -dev_warn(>dev, "No CROS EC device found.\n"); >> -return -EINVAL; >> -} >> - >> indio_dev = devm_iio_device_alloc(>dev, sizeof(*state)); >> if (!indio_dev) >> return -ENOMEM; >> 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 d2609e6feda4d..81a7f692de2f3 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 >> @@ -18,6 +18,7 @@ >> #include >> #include >> #include >> +#include >> #include >> >> static char *cros_ec_loc[] = { >> @@ -88,7 +89,8 @@ int cros_ec_sensors_core_init(struct platform_device *pdev, >> { >> struct device *dev = >dev; >> struct cros_ec_sensors_core_state *state = iio_priv(indio_dev); >> -struct cros_ec_dev *ec = dev_get_drvdata(pdev->dev.parent); >> +struct cros_ec_sensorhub *sensor_hub = dev_get_drvdata(dev->parent); >> +struct cros_ec_dev *ec = sensor_hub->ec; >> struct cros_ec_sensor_platform *sensor_platform = dev_get_platdata(dev); >> u32 ver_mask; >> int ret, i; >> diff --git a/drivers/iio/light/cros_ec_light_prox.c >> b/drivers/iio/light/cros_ec_light_prox.c >> index c5263b563fc19..d85a391e50c59 100644 >> --- a/drivers/iio/light/cros_ec_light_prox.c >> +++ b/drivers/iio/light/cros_ec_light_prox.c >> @@ -169,17 +169,11 @@ static const struct iio_info cros_ec_light_prox_info = >> { >> static int cros_ec_light_prox_probe(struct platform_device *pdev) >> { >> struct device *dev = >dev; >> -struct cros_ec_dev *ec_dev = dev_get_drvdata(dev->parent); >> struct iio_dev *indio_dev; >> struct cros_ec_light_prox_state *state; >> struct iio_chan_spec *channel; >> int ret; >> >> -if (!ec_dev || !ec_dev->ec_dev) { >> -dev_warn(dev, "No CROS EC device found.\n"); >> -return -EINVAL; >> -} >> - >> indio_dev = devm_iio_device_alloc(dev, sizeof(*state)); >> if (!indio_dev) >> return -ENOMEM; >> diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c >> index a35104e35cb4e..c4b977a5dd966
Re: [PATCH v2 04/18] platform/mfd:iio: cros_ec: Register sensor through sensorhub
On Sun, 20 Oct 2019 22:53:49 -0700 Gwendal Grignou wrote: > - Remove duplicate code in mfd, since mfd just register > cros_ec_sensorhub if at least one sensor is present > - Change iio cros_ec driver to get the pointer to the cros_ec_dev > through cros_ec_sensorhub. > > Signed-off-by: Gwendal Grignou FWIW given I don't known the driver that well. Looks good to me. Acked-by: Jonathan Cameron > --- > Changes in v2: > - Remove unerelated changes. > - Remove ec presence test in iio driver, done in cros_ec_sensorhub. > > drivers/iio/accel/cros_ec_accel_legacy.c | 6 - > .../common/cros_ec_sensors/cros_ec_sensors.c | 6 - > .../cros_ec_sensors/cros_ec_sensors_core.c| 4 +- > drivers/iio/light/cros_ec_light_prox.c| 6 - > drivers/mfd/cros_ec_dev.c | 203 ++ > include/linux/platform_data/cros_ec_proto.h | 8 - > .../linux/platform_data/cros_ec_sensorhub.h | 8 + > 7 files changed, 23 insertions(+), 218 deletions(-) > > diff --git a/drivers/iio/accel/cros_ec_accel_legacy.c > b/drivers/iio/accel/cros_ec_accel_legacy.c > index fcc3f999e4827..65f85faf6f31d 100644 > --- a/drivers/iio/accel/cros_ec_accel_legacy.c > +++ b/drivers/iio/accel/cros_ec_accel_legacy.c > @@ -163,16 +163,10 @@ static const struct iio_chan_spec > cros_ec_accel_legacy_channels[] = { > static int cros_ec_accel_legacy_probe(struct platform_device *pdev) > { > struct device *dev = >dev; > - struct cros_ec_dev *ec = dev_get_drvdata(dev->parent); > struct iio_dev *indio_dev; > struct cros_ec_sensors_core_state *state; > int ret; > > - if (!ec || !ec->ec_dev) { > - dev_warn(>dev, "No EC device found.\n"); > - return -EINVAL; > - } > - > indio_dev = devm_iio_device_alloc(>dev, sizeof(*state)); > if (!indio_dev) > return -ENOMEM; > 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 a6987726eeb8a..7dce044734678 100644 > --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c > +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c > @@ -222,17 +222,11 @@ static const struct iio_info ec_sensors_info = { > static int cros_ec_sensors_probe(struct platform_device *pdev) > { > struct device *dev = >dev; > - struct cros_ec_dev *ec_dev = dev_get_drvdata(dev->parent); > struct iio_dev *indio_dev; > struct cros_ec_sensors_state *state; > struct iio_chan_spec *channel; > int ret, i; > > - if (!ec_dev || !ec_dev->ec_dev) { > - dev_warn(>dev, "No CROS EC device found.\n"); > - return -EINVAL; > - } > - > indio_dev = devm_iio_device_alloc(>dev, sizeof(*state)); > if (!indio_dev) > return -ENOMEM; > 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 d2609e6feda4d..81a7f692de2f3 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 > @@ -18,6 +18,7 @@ > #include > #include > #include > +#include > #include > > static char *cros_ec_loc[] = { > @@ -88,7 +89,8 @@ int cros_ec_sensors_core_init(struct platform_device *pdev, > { > struct device *dev = >dev; > struct cros_ec_sensors_core_state *state = iio_priv(indio_dev); > - struct cros_ec_dev *ec = dev_get_drvdata(pdev->dev.parent); > + struct cros_ec_sensorhub *sensor_hub = dev_get_drvdata(dev->parent); > + struct cros_ec_dev *ec = sensor_hub->ec; > struct cros_ec_sensor_platform *sensor_platform = dev_get_platdata(dev); > u32 ver_mask; > int ret, i; > diff --git a/drivers/iio/light/cros_ec_light_prox.c > b/drivers/iio/light/cros_ec_light_prox.c > index c5263b563fc19..d85a391e50c59 100644 > --- a/drivers/iio/light/cros_ec_light_prox.c > +++ b/drivers/iio/light/cros_ec_light_prox.c > @@ -169,17 +169,11 @@ static const struct iio_info cros_ec_light_prox_info = { > static int cros_ec_light_prox_probe(struct platform_device *pdev) > { > struct device *dev = >dev; > - struct cros_ec_dev *ec_dev = dev_get_drvdata(dev->parent); > struct iio_dev *indio_dev; > struct cros_ec_light_prox_state *state; > struct iio_chan_spec *channel; > int ret; > > - if (!ec_dev || !ec_dev->ec_dev) { > - dev_warn(dev, "No CROS EC device found.\n"); > - return -EINVAL; > - } > - > indio_dev = devm_iio_device_alloc(dev, sizeof(*state)); > if (!indio_dev) > return -ENOMEM; > diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c > index a35104e35cb4e..c4b977a5dd966 100644 > --- a/drivers/mfd/cros_ec_dev.c > +++ b/drivers/mfd/cros_ec_dev.c > @@ -78,6 +78,10 @@ static const struct mfd_cell cros_ec_rtc_cells[] = { > { .name = "cros-ec-rtc", }, > }; > > +static