| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <38c12794-30da-c30f-eacd-3950634de437@google.com>
Date: Fri, 23 Jun 2017 15:08:15 -0700
From: Benson Leung <bleung@...gle.com>
To: Enric Balletbo i Serra <enric.balletbo@...labora.com>,
olof@...om.net, bleung@...omium.org, linux-kernel@...r.kernel.org
Cc: lee.jones@...aro.org, Eric Caruso <ejcaruso@...omium.org>,
Guenter Roeck <groeck@...omium.org>
Subject: Re: [PATCH RESEND 11/13] platform/chrome: cros_ec_lightbar - Control
of suspend/resume lightbar sequence
Hi Enric,
On 05/16/2017 09:13 AM, Enric Balletbo i Serra wrote:
> From: Eric Caruso <ejcaruso@...omium.org>
>
> Don't let EC control suspend/resume sequence. If the EC controls the
> lightbar and sets the sequence when it notices the chipset transitioning
> 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.
>
> Signed-off-by: Eric Caruso <ejcaruso@...omium.org>
> Signed-off-by: Guenter Roeck <groeck@...omium.org>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@...labora.com>
> Acked-by: Lee Jones <lee.jones@...aro.org>
Signed-off-by: Benson Leung <bleung@...omium.org>
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(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/chrome/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 <linux/mfd/core.h>
> #include <linux/module.h>
> #include <linux/platform_device.h>
> +#include <linux/pm.h>
> #include <linux/slab.h>
> #include <linux/uaccess.h>
>
> @@ -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);
>
> + /* Take control of the lightbar from the EC. */
> + if (ec_has_lightbar(ec))
> + lb_manual_suspend_ctrl(ec, 1);
> +
> return 0;
>
> failed:
> @@ -446,6 +451,10 @@ static int ec_device_remove(struct platform_device *pdev)
> {
> struct cros_ec_dev *ec = dev_get_drvdata(&pdev->dev);
>
> + /* Let the EC take over the lightbar again. */
> + if (ec_has_lightbar(ec))
> + lb_manual_suspend_ctrl(ec, 0);
> +
> cros_ec_debugfs_remove(ec);
>
> cdev_del(&ec->cdev);
> @@ -459,9 +468,37 @@ static const struct platform_device_id cros_ec_id[] = {
> };
> MODULE_DEVICE_TABLE(platform, cros_ec_id);
>
> +static int ec_device_suspend(struct device *dev)
> +{
> + struct cros_ec_dev *ec = 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 = 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 = {
> +#ifdef CONFIG_PM_SLEEP
> + .suspend = ec_device_suspend,
> + .resume = ec_device_resume,
> +#endif
> +};
> +
> static struct platform_driver cros_ec_dev_driver = {
> .driver = {
> .name = "cros-ec-ctl",
> + .pm = &cros_ec_dev_pm_ops,
> },
> .probe = ec_device_probe,
> .remove = ec_device_remove,
> diff --git a/drivers/platform/chrome/cros_ec_dev.h b/drivers/platform/chrome/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)
>
> +/* 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 enable);
> +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/platform/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;
> }
>
> +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 = alloc_lightbar_cmd_msg(ec);
> + if (!msg)
> + return -ENOMEM;
> +
> + param = (struct ec_params_lightbar *)msg->data;
> + param->cmd = cmd;
> +
> + ret = lb_throttle();
> + if (ret)
> + goto error;
> +
> + ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
> + if (ret < 0)
> + goto error;
> + if (msg->result != EC_RES_SUCCESS) {
> + ret = -EINVAL;
> + goto error;
> + }
> + ret = 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 = alloc_lightbar_cmd_msg(ec);
> + if (!msg)
> + return -ENOMEM;
> +
> + param = (struct ec_params_lightbar *)msg->data;
> +
> + param->cmd = LIGHTBAR_CMD_MANUAL_SUSPEND_CTRL;
> + param->manual_suspend_ctrl.enable = enable;
> +
> + ret = lb_throttle();
> + if (ret)
> + goto error;
> +
> + ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
> + if (ret < 0)
> + goto error;
> + if (msg->result != EC_RES_SUCCESS) {
> + ret = -EINVAL;
> + goto error;
> + }
> + ret = 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_attribute *attr,
> const char *buf, size_t count)
> {
> @@ -473,6 +547,11 @@ static struct attribute *__lb_cmds_attrs[] = {
> NULL,
> };
>
> +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;
>
> /* 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;
> }
>
> struct attribute_group cros_ec_lightbar_attr_group = {
> diff --git a/include/linux/mfd/cros_ec_commands.h b/include/linux/mfd/cros_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;
>
> struct {
> uint8_t num;
> @@ -1193,6 +1193,10 @@ struct ec_params_lightbar {
> uint8_t led;
> } get_rgb;
>
> + 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;
>
> @@ -1254,6 +1258,9 @@ enum lightbar_command {
> LIGHTBAR_CMD_GET_PARAMS_V1 = 16,
> LIGHTBAR_CMD_SET_PARAMS_V1 = 17,
> LIGHTBAR_CMD_SET_PROGRAM = 18,
> + LIGHTBAR_CMD_MANUAL_SUSPEND_CTRL = 19,
> + LIGHTBAR_CMD_SUSPEND = 20,
> + LIGHTBAR_CMD_RESUME = 21,
> LIGHTBAR_NUM_CMDS
> };
>
>
--
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@...gle.com
Chromium OS Project
bleung@...omium.org
Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)
Powered by blists - more mailing lists