On Tue, 26 May 2020 21:35:17 -0700
Gwendal Grignou <gwendal@xxxxxxxxxxxx> wrote:
> EC does not currently preserve range across sensor reinit.
> If sensor is powered down at suspend, it will default to the EC default
> range at resume, not the range set by the host.
>
> Save range if modified, and apply at resume.
>
> Signed-off-by: Gwendal Grignou <gwendal@xxxxxxxxxxxx>
one minor moan inline, but not important.
Applied to the togreg branch of iio.git and pushed out as testing.
There would be some logic to applying this as a fix and to stable.
If you like me to do that, let me know.
Thanks,
Jonathan
> ---
> .../common/cros_ec_sensors/cros_ec_sensors.c | 5 +++++
> .../cros_ec_sensors/cros_ec_sensors_core.c | 21 +++++++++++++++++++
> drivers/iio/light/cros_ec_light_prox.c | 6 +++++-
> drivers/iio/pressure/cros_ec_baro.c | 8 +++++--
> .../linux/iio/common/cros_ec_sensors_core.h | 11 +++++++++-
> 5 files changed, 47 insertions(+), 4 deletions(-)
>
> 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 a66941fdb3855..130ab8ce0269b 100644
> --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> @@ -200,6 +200,10 @@ static int cros_ec_sensors_write(struct iio_dev *indio_dev,
> st->core.param.sensor_range.roundup = 1;
>
> ret = cros_ec_motion_send_host_cmd(&st->core, 0);
> + if (ret == 0) {
> + st->core.range_updated = true;
> + st->core.curr_range = val;
> + }
> break;
> default:
> ret = cros_ec_sensors_core_write(
> @@ -315,6 +319,7 @@ MODULE_DEVICE_TABLE(platform, cros_ec_sensors_ids);
> static struct platform_driver cros_ec_sensors_platform_driver = {
> .driver = {
> .name = "cros-ec-sensors",
> + .pm = &cros_ec_sensors_pm_ops,
> },
> .probe = cros_ec_sensors_probe,
> .id_table = 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 c831915ca7e56..cda459b612067 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
> @@ -824,5 +824,26 @@ int cros_ec_sensors_core_write(struct cros_ec_sensors_core_state *st,
> }
> EXPORT_SYMBOL_GPL(cros_ec_sensors_core_write);
>
> +static int __maybe_unused cros_ec_sensors_resume(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> + struct cros_ec_sensors_core_state *st = iio_priv(indio_dev);
> + int ret = 0;
> +
> + if (st->range_updated) {
> + mutex_lock(&st->cmd_lock);
> + st->param.cmd = MOTIONSENSE_CMD_SENSOR_RANGE;
> + st->param.sensor_range.data = st->curr_range;
> + st->param.sensor_range.roundup = 1;
> + ret = cros_ec_motion_send_host_cmd(st, 0);
> + mutex_unlock(&st->cmd_lock);
> + }
> + return ret;
> +}
> +
> +SIMPLE_DEV_PM_OPS(cros_ec_sensors_pm_ops, NULL, cros_ec_sensors_resume);
> +EXPORT_SYMBOL_GPL(cros_ec_sensors_pm_ops);
> +
> MODULE_DESCRIPTION("ChromeOS EC sensor hub core functions");
> MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iio/light/cros_ec_light_prox.c b/drivers/iio/light/cros_ec_light_prox.c
> index 2198b50909ed0..fed79ba27fda5 100644
> --- a/drivers/iio/light/cros_ec_light_prox.c
> +++ b/drivers/iio/light/cros_ec_light_prox.c
> @@ -145,8 +145,11 @@ static int cros_ec_light_prox_write(struct iio_dev *indio_dev,
> break;
> case IIO_CHAN_INFO_CALIBSCALE:
> st->core.param.cmd = MOTIONSENSE_CMD_SENSOR_RANGE;
> - st->core.param.sensor_range.data = (val << 16) | (val2 / 100);
> + st->core.curr_range = (val << 16) | (val2 / 100);
> + st->core.param.sensor_range.data = st->core.curr_range;
> ret = cros_ec_motion_send_host_cmd(&st->core, 0);
> + if (ret == 0)
> + st->core.range_updated = true;
> break;
> default:
> ret = cros_ec_sensors_core_write(&st->core, chan, val, val2,
> @@ -256,6 +259,7 @@ MODULE_DEVICE_TABLE(platform, cros_ec_light_prox_ids);
> static struct platform_driver cros_ec_light_prox_platform_driver = {
> .driver = {
> .name = "cros-ec-light-prox",
> + .pm = &cros_ec_sensors_pm_ops,
> },
> .probe = cros_ec_light_prox_probe,
> .id_table = cros_ec_light_prox_ids,
> diff --git a/drivers/iio/pressure/cros_ec_baro.c b/drivers/iio/pressure/cros_ec_baro.c
> index c079b89600824..f0938b6fbba07 100644
> --- a/drivers/iio/pressure/cros_ec_baro.c
> +++ b/drivers/iio/pressure/cros_ec_baro.c
> @@ -96,8 +96,11 @@ static int cros_ec_baro_write(struct iio_dev *indio_dev,
> /* Always roundup, so caller gets at least what it asks for. */
> st->core.param.sensor_range.roundup = 1;
>
> - if (cros_ec_motion_send_host_cmd(&st->core, 0))
> - ret = -EIO;
> + ret = cros_ec_motion_send_host_cmd(&st->core, 0);
> + if (ret == 0) {
> + st->core.range_updated = true;
> + st->core.curr_range = val;
> + }
> break;
> default:
> ret = cros_ec_sensors_core_write(&st->core, chan, val, val2,
> @@ -199,6 +202,7 @@ MODULE_DEVICE_TABLE(platform, cros_ec_baro_ids);
> static struct platform_driver cros_ec_baro_platform_driver = {
> .driver = {
> .name = "cros-ec-baro",
> + .pm = &cros_ec_sensors_pm_ops,
> },
> .probe = cros_ec_baro_probe,
> .id_table = 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 7bc961defa87e..caa8bb279a346 100644
> --- a/include/linux/iio/common/cros_ec_sensors_core.h
> +++ b/include/linux/iio/common/cros_ec_sensors_core.h
> @@ -42,6 +42,10 @@ typedef irqreturn_t (*cros_ec_sensors_capture_t)(int irq, void *p);
> * @resp: motion sensor response structure
> * @type: type of motion sensor
> * @loc: location where the motion sensor is placed
> + * @range_updated: True if the range of the sensor has been
> + * updated.
> + * @curr_range: If updated, the current range value.
> + * It will be reapplied at every resume.
> * @calib: calibration parameters. Note that trigger
> * captured data will always provide the calibrated
> * data
> @@ -65,6 +69,9 @@ struct cros_ec_sensors_core_state {
> enum motionsensor_type type;
> enum motionsensor_location loc;
>
> + bool range_updated;
> + int curr_range;
> +
> struct calib_data {
> s16 offset;
> u16 scale;
> @@ -114,7 +121,9 @@ int cros_ec_sensors_core_write(struct cros_ec_sensors_core_state *st,
> struct iio_chan_spec const *chan,
> int val, int val2, long mask);
>
> -/* List of extended channel specification for all sensors */
> +extern const struct dev_pm_ops cros_ec_sensors_pm_ops;
> +
> +/* List of extended channel specification for all sensors. */
If I were being fussy... This change is unrelated and shouldn't be in this patch.
> extern const struct iio_chan_spec_ext_info cros_ec_sensors_ext_info[];
> extern const struct attribute *cros_ec_sensor_fifo_attributes[];
>
