Re: [PATCH] iio: cros_ec: Reapply range at resume

2020-05-31 Thread Jonathan Cameron
On Tue, 26 May 2020 21:35:17 -0700
Gwendal Grignou  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 
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(>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 = _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(>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(>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(>core, 0);
> + if (ret == 0)
> + st->core.range_updated = true;
>   break;
>   default:
>   ret = cros_ec_sensors_core_write(>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 = _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 

[PATCH] iio: cros_ec: Reapply range at resume

2020-05-26 Thread Gwendal Grignou
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 
---
 .../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(>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 = _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(>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(>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(>core, 0);
+   if (ret == 0)
+   st->core.range_updated = true;
break;
default:
ret = cros_ec_sensors_core_write(>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 = _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(>core, 0))
-   ret = -EIO;
+   ret = cros_ec_motion_send_host_cmd(>core, 0);
+   if (ret == 0) {
+