From: Jonathan Cameron <jic23@kernel.org>
To: Gwendal Grignou <gwendal@chromium.org>
Cc: groeck@chromium.org, bleung@chromium.org,
enric.balletbo@collabora.com, linux-iio@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] iio: cros_ec: Reapply range at resume
Date: Sun, 31 May 2020 14:56:13 +0100 [thread overview]
Message-ID: <20200531145613.353e4fbd@archlinux> (raw)
In-Reply-To: <20200527043517.240123-1-gwendal@chromium.org>
On Tue, 26 May 2020 21:35:17 -0700
Gwendal Grignou <gwendal@chromium.org> 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@chromium.org>
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[];
>
prev parent reply other threads:[~2020-05-31 13:56 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-27 4:35 [PATCH] iio: cros_ec: Reapply range at resume Gwendal Grignou
2020-05-31 13:56 ` Jonathan Cameron [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200531145613.353e4fbd@archlinux \
--to=jic23@kernel.org \
--cc=bleung@chromium.org \
--cc=enric.balletbo@collabora.com \
--cc=groeck@chromium.org \
--cc=gwendal@chromium.org \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).