Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754734AbdFWWIX (ORCPT ); Fri, 23 Jun 2017 18:08:23 -0400 Received: from mail-pg0-f50.google.com ([74.125.83.50]:34872 "EHLO mail-pg0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754340AbdFWWIU (ORCPT ); Fri, 23 Jun 2017 18:08:20 -0400 Subject: Re: [PATCH RESEND 11/13] platform/chrome: cros_ec_lightbar - Control of suspend/resume lightbar sequence To: Enric Balletbo i Serra , olof@lixom.net, bleung@chromium.org, linux-kernel@vger.kernel.org Cc: lee.jones@linaro.org, Eric Caruso , Guenter Roeck References: <20170516161319.13257-1-enric.balletbo@collabora.com> <20170516161319.13257-12-enric.balletbo@collabora.com> From: Benson Leung Organization: Google Message-ID: <38c12794-30da-c30f-eacd-3950634de437@google.com> Date: Fri, 23 Jun 2017 15:08:15 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1 MIME-Version: 1.0 In-Reply-To: <20170516161319.13257-12-enric.balletbo@collabora.com> Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="liV19q3NFuMKqV532CTnrKdEbqB9eX2mX" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 10215 Lines: 343 This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --liV19q3NFuMKqV532CTnrKdEbqB9eX2mX Content-Type: multipart/mixed; boundary="1K1P9OpfHO1r6ApQ95IWKXFRJ8qsPxgN4"; protected-headers="v1" From: Benson Leung To: Enric Balletbo i Serra , olof@lixom.net, bleung@chromium.org, linux-kernel@vger.kernel.org Cc: lee.jones@linaro.org, Eric Caruso , Guenter Roeck Message-ID: <38c12794-30da-c30f-eacd-3950634de437@google.com> Subject: Re: [PATCH RESEND 11/13] platform/chrome: cros_ec_lightbar - Control of suspend/resume lightbar sequence References: <20170516161319.13257-1-enric.balletbo@collabora.com> <20170516161319.13257-12-enric.balletbo@collabora.com> In-Reply-To: <20170516161319.13257-12-enric.balletbo@collabora.com> --1K1P9OpfHO1r6ApQ95IWKXFRJ8qsPxgN4 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: quoted-printable Hi Enric, On 05/16/2017 09:13 AM, Enric Balletbo i Serra wrote: > From: Eric Caruso >=20 > Don't let EC control suspend/resume sequence. If the EC controls the > lightbar and sets the sequence when it notices the chipset transitionin= g > between states, we can't make exceptions for cases where we don't want > to activate the lightbar. Instead, let's move the suspend/resume > notifications into the kernel so we can selectively play the sequences.= >=20 > Signed-off-by: Eric Caruso > Signed-off-by: Guenter Roeck > Signed-off-by: Enric Balletbo i Serra > Acked-by: Lee Jones Signed-off-by: Benson Leung Applied. Thanks. > --- > drivers/platform/chrome/cros_ec_dev.c | 37 +++++++++++++ > drivers/platform/chrome/cros_ec_dev.h | 6 +++ > drivers/platform/chrome/cros_ec_lightbar.c | 85 ++++++++++++++++++++++= ++++++-- > include/linux/mfd/cros_ec_commands.h | 11 +++- > 4 files changed, 134 insertions(+), 5 deletions(-) >=20 > diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/c= hrome/cros_ec_dev.c > index 20ce1c2..7c26223 100644 > --- a/drivers/platform/chrome/cros_ec_dev.c > +++ b/drivers/platform/chrome/cros_ec_dev.c > @@ -21,6 +21,7 @@ > #include > #include > #include > +#include > #include > #include > =20 > @@ -435,6 +436,10 @@ static int ec_device_probe(struct platform_device = *pdev) > if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE)) > cros_ec_sensors_register(ec); > =20 > + /* Take control of the lightbar from the EC. */ > + if (ec_has_lightbar(ec)) > + lb_manual_suspend_ctrl(ec, 1); > + > return 0; > =20 > failed: > @@ -446,6 +451,10 @@ static int ec_device_remove(struct platform_device= *pdev) > { > struct cros_ec_dev *ec =3D dev_get_drvdata(&pdev->dev); > =20 > + /* Let the EC take over the lightbar again. */ > + if (ec_has_lightbar(ec)) > + lb_manual_suspend_ctrl(ec, 0); > + > cros_ec_debugfs_remove(ec); > =20 > cdev_del(&ec->cdev); > @@ -459,9 +468,37 @@ static const struct platform_device_id cros_ec_id[= ] =3D { > }; > MODULE_DEVICE_TABLE(platform, cros_ec_id); > =20 > +static int ec_device_suspend(struct device *dev) > +{ > + struct cros_ec_dev *ec =3D dev_get_drvdata(dev); > + > + if (ec_has_lightbar(ec)) > + lb_suspend(ec); > + > + return 0; > +} > + > +static int ec_device_resume(struct device *dev) > +{ > + struct cros_ec_dev *ec =3D dev_get_drvdata(dev); > + > + if (ec_has_lightbar(ec)) > + lb_resume(ec); > + > + return 0; > +} > + > +static const struct dev_pm_ops cros_ec_dev_pm_ops =3D { > +#ifdef CONFIG_PM_SLEEP > + .suspend =3D ec_device_suspend, > + .resume =3D ec_device_resume, > +#endif > +}; > + > static struct platform_driver cros_ec_dev_driver =3D { > .driver =3D { > .name =3D "cros-ec-ctl", > + .pm =3D &cros_ec_dev_pm_ops, > }, > .probe =3D ec_device_probe, > .remove =3D ec_device_remove, > diff --git a/drivers/platform/chrome/cros_ec_dev.h b/drivers/platform/c= hrome/cros_ec_dev.h > index bfd2c84..45e9453 100644 > --- a/drivers/platform/chrome/cros_ec_dev.h > +++ b/drivers/platform/chrome/cros_ec_dev.h > @@ -43,4 +43,10 @@ struct cros_ec_readmem { > #define CROS_EC_DEV_IOCXCMD _IOWR(CROS_EC_DEV_IOC, 0, struct cros_ec= _command) > #define CROS_EC_DEV_IOCRDMEM _IOWR(CROS_EC_DEV_IOC, 1, struct cros_ec= _readmem) > =20 > +/* Lightbar utilities */ > +extern bool ec_has_lightbar(struct cros_ec_dev *ec); > +extern int lb_manual_suspend_ctrl(struct cros_ec_dev *ec, uint8_t enab= le); > +extern int lb_suspend(struct cros_ec_dev *ec); > +extern int lb_resume(struct cros_ec_dev *ec); > + > #endif /* _CROS_EC_DEV_H_ */ > diff --git a/drivers/platform/chrome/cros_ec_lightbar.c b/drivers/platf= orm/chrome/cros_ec_lightbar.c > index 2667505..4df379d 100644 > --- a/drivers/platform/chrome/cros_ec_lightbar.c > +++ b/drivers/platform/chrome/cros_ec_lightbar.c > @@ -341,6 +341,80 @@ static ssize_t sequence_show(struct device *dev, > return ret; > } > =20 > +static int lb_send_empty_cmd(struct cros_ec_dev *ec, uint8_t cmd) > +{ > + struct ec_params_lightbar *param; > + struct cros_ec_command *msg; > + int ret; > + > + msg =3D alloc_lightbar_cmd_msg(ec); > + if (!msg) > + return -ENOMEM; > + > + param =3D (struct ec_params_lightbar *)msg->data; > + param->cmd =3D cmd; > + > + ret =3D lb_throttle(); > + if (ret) > + goto error; > + > + ret =3D cros_ec_cmd_xfer(ec->ec_dev, msg); > + if (ret < 0) > + goto error; > + if (msg->result !=3D EC_RES_SUCCESS) { > + ret =3D -EINVAL; > + goto error; > + } > + ret =3D 0; > +error: > + kfree(msg); > + > + return ret; > +} > + > +int lb_manual_suspend_ctrl(struct cros_ec_dev *ec, uint8_t enable) > +{ > + struct ec_params_lightbar *param; > + struct cros_ec_command *msg; > + int ret; > + > + msg =3D alloc_lightbar_cmd_msg(ec); > + if (!msg) > + return -ENOMEM; > + > + param =3D (struct ec_params_lightbar *)msg->data; > + > + param->cmd =3D LIGHTBAR_CMD_MANUAL_SUSPEND_CTRL; > + param->manual_suspend_ctrl.enable =3D enable; > + > + ret =3D lb_throttle(); > + if (ret) > + goto error; > + > + ret =3D cros_ec_cmd_xfer(ec->ec_dev, msg); > + if (ret < 0) > + goto error; > + if (msg->result !=3D EC_RES_SUCCESS) { > + ret =3D -EINVAL; > + goto error; > + } > + ret =3D 0; > +error: > + kfree(msg); > + > + return ret; > +} > + > +int lb_suspend(struct cros_ec_dev *ec) > +{ > + return lb_send_empty_cmd(ec, LIGHTBAR_CMD_SUSPEND); > +} > + > +int lb_resume(struct cros_ec_dev *ec) > +{ > + return lb_send_empty_cmd(ec, LIGHTBAR_CMD_RESUME); > +} > + > static ssize_t sequence_store(struct device *dev, struct device_attrib= ute *attr, > const char *buf, size_t count) > { > @@ -473,6 +547,11 @@ static struct attribute *__lb_cmds_attrs[] =3D { > NULL, > }; > =20 > +bool ec_has_lightbar(struct cros_ec_dev *ec) > +{ > + return !!get_lightbar_version(ec, NULL, NULL); > +} > + > static umode_t cros_ec_lightbar_attrs_are_visible(struct kobject *kobj= , > struct attribute *a, int n) > { > @@ -489,10 +568,10 @@ static umode_t cros_ec_lightbar_attrs_are_visible= (struct kobject *kobj, > return 0; > =20 > /* Only instantiate this stuff if the EC has a lightbar */ > - if (get_lightbar_version(ec, NULL, NULL)) > + if (ec_has_lightbar(ec)) > return a->mode; > - else > - return 0; > + > + return 0; > } > =20 > struct attribute_group cros_ec_lightbar_attr_group =3D { > diff --git a/include/linux/mfd/cros_ec_commands.h b/include/linux/mfd/c= ros_ec_commands.h > index dbea580..190c8f4 100644 > --- a/include/linux/mfd/cros_ec_commands.h > +++ b/include/linux/mfd/cros_ec_commands.h > @@ -1175,7 +1175,7 @@ struct ec_params_lightbar { > struct { > /* no args */ > } dump, off, on, init, get_seq, get_params_v0, get_params_v1, > - version, get_brightness, get_demo; > + version, get_brightness, get_demo, suspend, resume; > =20 > struct { > uint8_t num; > @@ -1193,6 +1193,10 @@ struct ec_params_lightbar { > uint8_t led; > } get_rgb; > =20 > + struct { > + uint8_t enable; > + } manual_suspend_ctrl; > + > struct lightbar_params_v0 set_params_v0; > struct lightbar_params_v1 set_params_v1; > struct lightbar_program set_program; > @@ -1229,7 +1233,7 @@ struct ec_response_lightbar { > /* no return params */ > } off, on, init, set_brightness, seq, reg, set_rgb, > demo, set_params_v0, set_params_v1, > - set_program; > + set_program, manual_suspend_ctrl, suspend, resume; > }; > } __packed; > =20 > @@ -1254,6 +1258,9 @@ enum lightbar_command { > LIGHTBAR_CMD_GET_PARAMS_V1 =3D 16, > LIGHTBAR_CMD_SET_PARAMS_V1 =3D 17, > LIGHTBAR_CMD_SET_PROGRAM =3D 18, > + LIGHTBAR_CMD_MANUAL_SUSPEND_CTRL =3D 19, > + LIGHTBAR_CMD_SUSPEND =3D 20, > + LIGHTBAR_CMD_RESUME =3D 21, > LIGHTBAR_NUM_CMDS > }; > =20 >=20 --=20 Benson Leung Staff Software Engineer Chrome OS Kernel Google Inc. bleung@google.com Chromium OS Project bleung@chromium.org --1K1P9OpfHO1r6ApQ95IWKXFRJ8qsPxgN4-- --liV19q3NFuMKqV532CTnrKdEbqB9eX2mX Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBCgAGBQJZTZFPAAoJEB8J9XsKL+ZYPz4P/2LrrP6CdqzMTHGglmipfmxW waRGFfnAoGLVRqOPrK/AW0qrz1bEbWHawsRB91eraIUy2uOFOEVaYZAycvtz1ZMB vrf34q2rwKGoOAw8HDp/XltmLVNt4DusZ4r2r5Ax70AilhsTRx9y3rFh5dF7aa5F KVrPVwm/9m3AJRdJ+2/7flZy4NZRSwYn5YYoCn1PPauumpgeF9hlzNgZpdJ7wpHD epqFfdiAjIe0/vgzPIvM+arRw4Kobb8Q24N2NhS2JyaKMkLa/shHK65D7rViexMR k3C7A7WmzLwZ9Q9QOjOtZgJKTBc+3kSsWRrvNkMEjqNwbj1jXUHYJ8cNZL9K2HB2 0Ohb5o8FGe+v54BSNlYbdQVNrvtJQCoevYJwxO37pBCnqoQP9DFjakfoQFxH2BWX fgME6Vw2QEGpHux2AMOOd6AvF58ZixbyzJ22tpedWeAvszHGsjdVaQSPwUK/7xns 2EG5gC9BSmYi5RoKNuVzAH1lbZmV8flP0dwWEqsmZtzbvjzCU8GG9nENl5zJpuAg 3Ky+UykauSb3XYBuoN0FUx5ysLzTGj6ed6Of3wO0GoXFqd+lTljKln24WkDUQad7 Ta6B2gTxaGofPmw2lKOFulqwuNc3ch93hQ0eK3DBhzxiZbOxpfsXDpzWeBpvG76C hBuBOCMDXT8epvB2HCv6 =g8Wq -----END PGP SIGNATURE----- --liV19q3NFuMKqV532CTnrKdEbqB9eX2mX--