[PATCH v2 0/4] platform/chrome: cros_ec_lightbar - Add new features

[PATCH v2 0/4] platform/chrome: cros_ec_lightbar - Add new features

[Enric Balletbo i Serra]

Dear all,

The following patches taken from the ChromeOS 4.4 tree adds more features to
the cros_ec_lightbar driver. The patches were forward ported to current
mainline and were tested on a Chromebook Pixel 2015.

Best regards,

 Enric

Changes since v1:
 - Rebase on top of mainline.
 - Added Lee Jones ack for patch 1/4

Eric Caruso (3):
  platform/chrome: cros_ec_lightbar - Add lightbar program feature to
    sysfs
  platform/chrome: cros_ec_lightbar - Control of suspend/resume lightbar
    sequence
  platform/chrome: cros_ec_lightbar - Add userspace lightbar control bit
    to EC

Jeffery Yu (1):
  platform/chrome: cros_ec_lightbar - Avoid I2C xfer to EC during
    suspend

 drivers/platform/chrome/cros_ec_dev.c      |  34 +++++
 drivers/platform/chrome/cros_ec_dev.h      |   6 +
 drivers/platform/chrome/cros_ec_lightbar.c | 197 ++++++++++++++++++++++++++++-
 include/linux/mfd/cros_ec_commands.h       |  21 ++-
 4 files changed, 252 insertions(+), 6 deletions(-)

-- 
2.1.0

[PATCH v2 1/4] platform/chrome: cros_ec_lightbar - Add lightbar program feature to sysfs

[Enric Balletbo i Serra]

From: Eric Caruso <ejcaruso@chromium.org>

Add a program feature so we can upload and run programs for lightbar
sequences. We should be able to use this to shift sequences out of the
EC and save space there.

  $ cat <suitable program bin> > /sys/devices/.../cros_ec/program
  $ echo program > /sys/devices/.../cros_ec/sequence

Signed-off-by: Eric Caruso <ejcaruso@chromium.org>
Signed-off-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/platform/chrome/cros_ec_lightbar.c | 69 +++++++++++++++++++++++++++++-
 include/linux/mfd/cros_ec_commands.h       | 12 +++++-
 2 files changed, 79 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_lightbar.c b/drivers/platform/chrome/cros_ec_lightbar.c
index 8df3d44..2667505 100644
--- a/drivers/platform/chrome/cros_ec_lightbar.c
+++ b/drivers/platform/chrome/cros_ec_lightbar.c
@@ -295,7 +295,8 @@ static ssize_t led_rgb_store(struct device *dev, struct device_attribute *attr,
 
 static char const *seqname[] = {
 	"ERROR", "S5", "S3", "S0", "S5S3", "S3S0",
-	"S0S3", "S3S5", "STOP", "RUN", "PULSE", "TEST", "KONAMI",
+	"S0S3", "S3S5", "STOP", "RUN", "KONAMI",
+	"TAP", "PROGRAM",
 };
 
 static ssize_t sequence_show(struct device *dev,
@@ -390,6 +391,69 @@ static ssize_t sequence_store(struct device *dev, struct device_attribute *attr,
 	return ret;
 }
 
+static ssize_t program_store(struct device *dev, struct device_attribute *attr,
+			     const char *buf, size_t count)
+{
+	int extra_bytes, max_size, ret;
+	struct ec_params_lightbar *param;
+	struct cros_ec_command *msg;
+	struct cros_ec_dev *ec = container_of(dev, struct cros_ec_dev,
+					      class_dev);
+
+	/*
+	 * We might need to reject the program for size reasons. The EC
+	 * enforces a maximum program size, but we also don't want to try
+	 * and send a program that is too big for the protocol. In order
+	 * to ensure the latter, we also need to ensure we have extra bytes
+	 * to represent the rest of the packet.
+	 */
+	extra_bytes = sizeof(*param) - sizeof(param->set_program.data);
+	max_size = min(EC_LB_PROG_LEN, ec->ec_dev->max_request - extra_bytes);
+	if (count > max_size) {
+		dev_err(dev, "Program is %u bytes, too long to send (max: %u)",
+			(unsigned int)count, max_size);
+
+		return -EINVAL;
+	}
+
+	msg = alloc_lightbar_cmd_msg(ec);
+	if (!msg)
+		return -ENOMEM;
+
+	ret = lb_throttle();
+	if (ret)
+		goto exit;
+
+	dev_info(dev, "Copying %zu byte program to EC", count);
+
+	param = (struct ec_params_lightbar *)msg->data;
+	param->cmd = LIGHTBAR_CMD_SET_PROGRAM;
+
+	param->set_program.size = count;
+	memcpy(param->set_program.data, buf, count);
+
+	/*
+	 * We need to set the message size manually or else it will use
+	 * EC_LB_PROG_LEN. This might be too long, and the program
+	 * is unlikely to use all of the space.
+	 */
+	msg->outsize = count + extra_bytes;
+
+	ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
+	if (ret < 0)
+		goto exit;
+	if (msg->result != EC_RES_SUCCESS) {
+		ret = -EINVAL;
+		goto exit;
+	}
+
+	ret = count;
+exit:
+	kfree(msg);
+
+	return ret;
+}
+
 /* Module initialization */
 
 static DEVICE_ATTR_RW(interval_msec);
@@ -397,12 +461,15 @@ static DEVICE_ATTR_RO(version);
 static DEVICE_ATTR_WO(brightness);
 static DEVICE_ATTR_WO(led_rgb);
 static DEVICE_ATTR_RW(sequence);
+static DEVICE_ATTR_WO(program);
+
 static struct attribute *__lb_cmds_attrs[] = {
 	&dev_attr_interval_msec.attr,
 	&dev_attr_version.attr,
 	&dev_attr_brightness.attr,
 	&dev_attr_led_rgb.attr,
 	&dev_attr_sequence.attr,
+	&dev_attr_program.attr,
 	NULL,
 };
 
diff --git a/include/linux/mfd/cros_ec_commands.h b/include/linux/mfd/cros_ec_commands.h
index 73f7a62..efc78bd 100644
--- a/include/linux/mfd/cros_ec_commands.h
+++ b/include/linux/mfd/cros_ec_commands.h
@@ -1163,6 +1163,13 @@ struct lightbar_params_v1 {
 	struct rgb_s color[8];			/* 0-3 are Google colors */
 } __packed;
 
+/* Lightbar program */
+#define EC_LB_PROG_LEN 192
+struct lightbar_program {
+	uint8_t size;
+	uint8_t data[EC_LB_PROG_LEN];
+};
+
 struct ec_params_lightbar {
 	uint8_t cmd;		      /* Command (see enum lightbar_command) */
 	union {
@@ -1189,6 +1196,7 @@ struct ec_params_lightbar {
 
 		struct lightbar_params_v0 set_params_v0;
 		struct lightbar_params_v1 set_params_v1;
+		struct lightbar_program set_program;
 	};
 } __packed;
 
@@ -1221,7 +1229,8 @@ struct ec_response_lightbar {
 		struct {
 			/* no return params */
 		} off, on, init, set_brightness, seq, reg, set_rgb,
-			demo, set_params_v0, set_params_v1;
+			demo, set_params_v0, set_params_v1,
+			set_program;
 	};
 } __packed;
 
@@ -1245,6 +1254,7 @@ enum lightbar_command {
 	LIGHTBAR_CMD_GET_DEMO = 15,
 	LIGHTBAR_CMD_GET_PARAMS_V1 = 16,
 	LIGHTBAR_CMD_SET_PARAMS_V1 = 17,
+	LIGHTBAR_CMD_SET_PROGRAM = 18,
 	LIGHTBAR_NUM_CMDS
 };
 
-- 
2.9.3

[PATCH v2 2/4] platform/chrome: cros_ec_lightbar - Control of suspend/resume lightbar sequence

[Enric Balletbo i Serra]

From: Eric Caruso <ejcaruso@chromium.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@chromium.org>
Signed-off-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
 drivers/platform/chrome/cros_ec_dev.c      | 38 +++++++++++++
 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, 135 insertions(+), 5 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/chrome/cros_ec_dev.c
index ebe029d..ef04523 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>
 
@@ -463,6 +464,10 @@ static int ec_device_probe(struct platform_device *pdev)
 	if (cros_ec_check_features(ec, EC_FEATURE_RTC))
 		cros_ec_rtc_register(ec);
 
+	/* Take control of the lightbar from the EC. */
+	if (ec_has_lightbar(ec))
+		lb_manual_suspend_ctrl(ec, 1);
+
 	return 0;
 
 dev_reg_failed:
@@ -477,6 +482,11 @@ static int ec_device_probe(struct platform_device *pdev)
 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);
+
 	cdev_del(&ec->cdev);
 	device_unregister(&ec->class_dev);
 	return 0;
@@ -488,9 +498,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 efc78bd..64138fd 100644
--- a/include/linux/mfd/cros_ec_commands.h
+++ b/include/linux/mfd/cros_ec_commands.h
@@ -1176,7 +1176,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;
@@ -1194,6 +1194,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;
@@ -1230,7 +1234,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;
 
@@ -1255,6 +1259,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
 };
 
-- 
2.9.3

Re: [PATCH v2 2/4] platform/chrome: cros_ec_lightbar - Control of suspend/resume lightbar sequence

[Lee Jones]

On Wed, 11 Jan 2017, Enric Balletbo i Serra wrote:

> From: Eric Caruso <ejcaruso@chromium.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@chromium.org>
> Signed-off-by: Guenter Roeck <groeck@chromium.org>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
>  drivers/platform/chrome/cros_ec_dev.c      | 38 +++++++++++++
>  drivers/platform/chrome/cros_ec_dev.h      |  6 +++
>  drivers/platform/chrome/cros_ec_lightbar.c | 85 ++++++++++++++++++++++++++++--

>  include/linux/mfd/cros_ec_commands.h       | 11 +++-

Acked-by: Lee Jones <lee.jones@linaro.org>
  
>  4 files changed, 135 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/chrome/cros_ec_dev.c
> index ebe029d..ef04523 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>
>  
> @@ -463,6 +464,10 @@ static int ec_device_probe(struct platform_device *pdev)
>  	if (cros_ec_check_features(ec, EC_FEATURE_RTC))
>  		cros_ec_rtc_register(ec);
>  
> +	/* Take control of the lightbar from the EC. */
> +	if (ec_has_lightbar(ec))
> +		lb_manual_suspend_ctrl(ec, 1);
> +
>  	return 0;
>  
>  dev_reg_failed:
> @@ -477,6 +482,11 @@ static int ec_device_probe(struct platform_device *pdev)
>  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);
> +
>  	cdev_del(&ec->cdev);
>  	device_unregister(&ec->class_dev);
>  	return 0;
> @@ -488,9 +498,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 efc78bd..64138fd 100644
> --- a/include/linux/mfd/cros_ec_commands.h
> +++ b/include/linux/mfd/cros_ec_commands.h
> @@ -1176,7 +1176,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;
> @@ -1194,6 +1194,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;
> @@ -1230,7 +1234,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;
>  
> @@ -1255,6 +1259,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
>  };
>  

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH v2 3/4] platform/chrome: cros_ec_lightbar - Add userspace lightbar control bit to EC

[Enric Balletbo i Serra]

From: Eric Caruso <ejcaruso@chromium.org>

Some devices might want to turn off the lightbar if e.g. the
system turns the screen off due to idleness. This prevents the
kernel from going through its normal suspend/resume pathways.

Signed-off-by: Eric Caruso <ejcaruso@chromium.org>
Signed-off-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
 drivers/platform/chrome/cros_ec_lightbar.c | 38 ++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/drivers/platform/chrome/cros_ec_lightbar.c b/drivers/platform/chrome/cros_ec_lightbar.c
index 4df379d..e570c1e 100644
--- a/drivers/platform/chrome/cros_ec_lightbar.c
+++ b/drivers/platform/chrome/cros_ec_lightbar.c
@@ -38,6 +38,12 @@
 /* Rate-limit the lightbar interface to prevent DoS. */
 static unsigned long lb_interval_jiffies = 50 * HZ / 1000;
 
+/*
+ * Whether or not we have given userspace control of the lightbar.
+ * If this is true, we won't do anything during suspend/resume.
+ */
+static bool userspace_control;
+
 static ssize_t interval_msec_show(struct device *dev,
 				  struct device_attribute *attr, char *buf)
 {
@@ -407,11 +413,17 @@ int lb_manual_suspend_ctrl(struct cros_ec_dev *ec, uint8_t enable)
 
 int lb_suspend(struct cros_ec_dev *ec)
 {
+	if (userspace_control)
+		return 0;
+
 	return lb_send_empty_cmd(ec, LIGHTBAR_CMD_SUSPEND);
 }
 
 int lb_resume(struct cros_ec_dev *ec)
 {
+	if (userspace_control)
+		return 0;
+
 	return lb_send_empty_cmd(ec, LIGHTBAR_CMD_RESUME);
 }
 
@@ -528,6 +540,30 @@ static ssize_t program_store(struct device *dev, struct device_attribute *attr,
 	return ret;
 }
 
+static ssize_t userspace_control_show(struct device *dev,
+				      struct device_attribute *attr,
+				      char *buf)
+{
+	return scnprintf(buf, PAGE_SIZE, "%d\n", userspace_control);
+}
+
+static ssize_t userspace_control_store(struct device *dev,
+				       struct device_attribute *attr,
+				       const char *buf,
+				       size_t count)
+{
+	bool enable;
+	int ret;
+
+	ret = strtobool(buf, &enable);
+	if (ret < 0)
+		return ret;
+
+	userspace_control = enable;
+
+	return count;
+}
+
 /* Module initialization */
 
 static DEVICE_ATTR_RW(interval_msec);
@@ -536,6 +572,7 @@ static DEVICE_ATTR_WO(brightness);
 static DEVICE_ATTR_WO(led_rgb);
 static DEVICE_ATTR_RW(sequence);
 static DEVICE_ATTR_WO(program);
+static DEVICE_ATTR_RW(userspace_control);
 
 static struct attribute *__lb_cmds_attrs[] = {
 	&dev_attr_interval_msec.attr,
@@ -544,6 +581,7 @@ static struct attribute *__lb_cmds_attrs[] = {
 	&dev_attr_led_rgb.attr,
 	&dev_attr_sequence.attr,
 	&dev_attr_program.attr,
+	&dev_attr_userspace_control.attr,
 	NULL,
 };
 
-- 
2.9.3

[PATCH v2 4/4] platform/chrome: cros_ec_lightbar - Avoid I2C xfer to EC during suspend

[Enric Balletbo i Serra]

From: Jeffery Yu <jefferyy@nvidia.com>

A Mutex lock in cros_ec_cmd_xfer which may be held by frozen
Userspace thread during system suspending. So should not
call this routine in suspend thread.

Signed-off-by: Jeffery Yu <jefferyy@nvidia.com>
Signed-off-by: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
 drivers/platform/chrome/cros_ec_dev.c      | 12 ++++--------
 drivers/platform/chrome/cros_ec_lightbar.c | 13 +++++++++----
 2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/chrome/cros_ec_dev.c
index ef04523..df5894f 100644
--- a/drivers/platform/chrome/cros_ec_dev.c
+++ b/drivers/platform/chrome/cros_ec_dev.c
@@ -465,8 +465,7 @@ static int ec_device_probe(struct platform_device *pdev)
 		cros_ec_rtc_register(ec);
 
 	/* Take control of the lightbar from the EC. */
-	if (ec_has_lightbar(ec))
-		lb_manual_suspend_ctrl(ec, 1);
+	lb_manual_suspend_ctrl(ec, 1);
 
 	return 0;
 
@@ -484,8 +483,7 @@ 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);
+	lb_manual_suspend_ctrl(ec, 0);
 
 	cdev_del(&ec->cdev);
 	device_unregister(&ec->class_dev);
@@ -502,8 +500,7 @@ 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);
+	lb_suspend(ec);
 
 	return 0;
 }
@@ -512,8 +509,7 @@ 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);
+	lb_resume(ec);
 
 	return 0;
 }
diff --git a/drivers/platform/chrome/cros_ec_lightbar.c b/drivers/platform/chrome/cros_ec_lightbar.c
index e570c1e..fd2b047 100644
--- a/drivers/platform/chrome/cros_ec_lightbar.c
+++ b/drivers/platform/chrome/cros_ec_lightbar.c
@@ -43,6 +43,7 @@ static unsigned long lb_interval_jiffies = 50 * HZ / 1000;
  * If this is true, we won't do anything during suspend/resume.
  */
 static bool userspace_control;
+static struct cros_ec_dev *ec_with_lightbar;
 
 static ssize_t interval_msec_show(struct device *dev,
 				  struct device_attribute *attr, char *buf)
@@ -384,6 +385,9 @@ int lb_manual_suspend_ctrl(struct cros_ec_dev *ec, uint8_t enable)
 	struct cros_ec_command *msg;
 	int ret;
 
+	if (ec != ec_with_lightbar)
+		return 0;
+
 	msg = alloc_lightbar_cmd_msg(ec);
 	if (!msg)
 		return -ENOMEM;
@@ -413,7 +417,7 @@ int lb_manual_suspend_ctrl(struct cros_ec_dev *ec, uint8_t enable)
 
 int lb_suspend(struct cros_ec_dev *ec)
 {
-	if (userspace_control)
+	if (userspace_control || ec != ec_with_lightbar)
 		return 0;
 
 	return lb_send_empty_cmd(ec, LIGHTBAR_CMD_SUSPEND);
@@ -421,7 +425,7 @@ int lb_suspend(struct cros_ec_dev *ec)
 
 int lb_resume(struct cros_ec_dev *ec)
 {
-	if (userspace_control)
+	if (userspace_control || ec != ec_with_lightbar)
 		return 0;
 
 	return lb_send_empty_cmd(ec, LIGHTBAR_CMD_RESUME);
@@ -606,9 +610,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 (ec_has_lightbar(ec))
+	if (ec_has_lightbar(ec)) {
+		ec_with_lightbar = ec;
 		return a->mode;
-
+	}
 	return 0;
 }
 
-- 
2.9.3