[PATCH v2 0/6] platform/chrome: cros_ec debugfs and sysfs updates

[PATCH v2 0/6] platform/chrome: cros_ec debugfs and sysfs updates

[Enric Balletbo i Serra]

Hi,

This is a second patchset that introduces some cros_ec debugfs and sysfs
updates. Some patches that were send before but never merged and some
others are new.

First patch was already sent [1] but did not receive feedback so I am
resending it. Needs to go through platform/chrome tree and can be
picked without dependencies.

Patches 2 to 4 are cleanups of current code, basically is the
implementation of the changes suggested by Andy Shevchenko on the first
series. Again these must go through platform/chrome tree.

Patch 5 is new, and like the others must go through platform/chrome tree.

Finally, patch 6 was already sent [2] but now needed to be reworked due
the split of the cros_ec_devs in two parts [3]. This patch touches
platform/chrome and mfd so we will probably need an immutable branch.

Best regards,
  Enric

[1] https://patchwork.kernel.org/patch/9894655/
[2] https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1488891.html
[3] https://lkml.org/lkml/2017/11/20/408

Changes since v1:
- Requested by Andy Shevchenko:
  - Introduce define to_cros_ec_dev.
  - Switch to DEVICE_ATTR_RO/RW variants. 
  - Drop unrelated changes on patch 5. Split in two patches, one for the
    pd feature and another one to use octal permissions.

Enric Balletbo i Serra (3):
  platform/chrome: cros_ec_sysfs: introduce to_cros_ec_dev define.
  platform/chrome: cros_ec_sysfs: use permission-specific DEVICE_ATTR
    variants
  platform/chrome: cros_ec_debugfs: Use octal permissions '0444'

Gwendal Grignou (2):
  platform/chrome: cros_ec_sysfs: Modify error handling
  platform/chrome: mfd/cros_ec_dev: Add sysfs entry to set keyboard wake
    lid angle

Shawn Nematbakhsh (1):
  platform/chrome: cros_ec_debugfs: Add PD port info to debugfs

 drivers/mfd/cros_ec_dev.c                 |  19 ++--
 drivers/platform/chrome/cros_ec_debugfs.c |  76 +++++++++++++++-
 drivers/platform/chrome/cros_ec_sysfs.c   | 141 +++++++++++++++++++++---------
 include/linux/mfd/cros_ec.h               |   1 +
 4 files changed, 187 insertions(+), 50 deletions(-)

-- 
2.16.1

[PATCH v2 1/6] platform/chrome: cros_ec_sysfs: Modify error handling

[Enric Balletbo i Serra]

From: Gwendal Grignou <gwendal@chromium.org>

When accessing a sysfs attribute, if the EC command fails, -EPROTO is
now returned instead of an error message as it is unlikely an app is
parsing the error message to do something meaningful.
Also, this patch makes use of cros_ec_cmd_xfer_status() instead of
cros_ec_cmd_xfer() so an error message is printed in the syslog.

Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
Changes since v1:
- None

 drivers/platform/chrome/cros_ec_sysfs.c | 25 ++++---------------------
 1 file changed, 4 insertions(+), 21 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_sysfs.c b/drivers/platform/chrome/cros_ec_sysfs.c
index da0a719d32f7..c03621e523a3 100644
--- a/drivers/platform/chrome/cros_ec_sysfs.c
+++ b/drivers/platform/chrome/cros_ec_sysfs.c
@@ -114,15 +114,9 @@ static ssize_t store_ec_reboot(struct device *dev,
 	msg->command = EC_CMD_REBOOT_EC + ec->cmd_offset;
 	msg->outsize = sizeof(*param);
 	msg->insize = 0;
-	ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
-	if (ret < 0) {
+	ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
+	if (ret < 0)
 		count = ret;
-		goto exit;
-	}
-	if (msg->result != EC_RES_SUCCESS) {
-		dev_dbg(ec->dev, "EC result %d\n", msg->result);
-		count = -EINVAL;
-	}
 exit:
 	kfree(msg);
 	return count;
@@ -150,17 +144,11 @@ static ssize_t show_ec_version(struct device *dev,
 	msg->command = EC_CMD_GET_VERSION + ec->cmd_offset;
 	msg->insize = sizeof(*r_ver);
 	msg->outsize = 0;
-	ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
+	ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
 	if (ret < 0) {
 		count = ret;
 		goto exit;
 	}
-	if (msg->result != EC_RES_SUCCESS) {
-		count = scnprintf(buf, PAGE_SIZE,
-				  "ERROR: EC returned %d\n", msg->result);
-		goto exit;
-	}
-
 	r_ver = (struct ec_response_get_version *)msg->data;
 	/* Strings should be null-terminated, but let's be sure. */
 	r_ver->version_string_ro[sizeof(r_ver->version_string_ro) - 1] = '\0';
@@ -255,14 +243,9 @@ static ssize_t show_ec_flashinfo(struct device *dev,
 	msg->command = EC_CMD_FLASH_INFO + ec->cmd_offset;
 	msg->insize = sizeof(*resp);
 	msg->outsize = 0;
-	ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
+	ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
 	if (ret < 0)
 		goto exit;
-	if (msg->result != EC_RES_SUCCESS) {
-		ret = scnprintf(buf, PAGE_SIZE,
-				"ERROR: EC returned %d\n", msg->result);
-		goto exit;
-	}
 
 	resp = (struct ec_response_flash_info *)msg->data;
 
-- 
2.16.1

[PATCH v2 2/6] platform/chrome: cros_ec_sysfs: introduce to_cros_ec_dev define.

[Enric Balletbo i Serra]

Add a define to get the cros_ec_dev from device and use it.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
Changes since v1:
- New in this series. Suggested by Andy Shevchenko.

 drivers/platform/chrome/cros_ec_sysfs.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_sysfs.c b/drivers/platform/chrome/cros_ec_sysfs.c
index c03621e523a3..85bb9580439e 100644
--- a/drivers/platform/chrome/cros_ec_sysfs.c
+++ b/drivers/platform/chrome/cros_ec_sysfs.c
@@ -34,6 +34,8 @@
 #include <linux/types.h>
 #include <linux/uaccess.h>
 
+#define to_cros_ec_dev(dev)  container_of(dev, struct cros_ec_dev, class_dev)
+
 /* Accessor functions */
 
 static ssize_t show_ec_reboot(struct device *dev,
@@ -70,8 +72,7 @@ static ssize_t store_ec_reboot(struct device *dev,
 	int got_cmd = 0, offset = 0;
 	int i;
 	int ret;
-	struct cros_ec_dev *ec = container_of(dev,
-					      struct cros_ec_dev, class_dev);
+	struct cros_ec_dev *ec = to_cros_ec_dev(dev);
 
 	msg = kmalloc(sizeof(*msg) + sizeof(*param), GFP_KERNEL);
 	if (!msg)
@@ -132,8 +133,7 @@ static ssize_t show_ec_version(struct device *dev,
 	struct cros_ec_command *msg;
 	int ret;
 	int count = 0;
-	struct cros_ec_dev *ec = container_of(dev,
-					      struct cros_ec_dev, class_dev);
+	struct cros_ec_dev *ec = to_cros_ec_dev(dev);
 
 	msg = kmalloc(sizeof(*msg) + EC_HOST_PARAM_SIZE, GFP_KERNEL);
 	if (!msg)
@@ -231,8 +231,7 @@ static ssize_t show_ec_flashinfo(struct device *dev,
 	struct ec_response_flash_info *resp;
 	struct cros_ec_command *msg;
 	int ret;
-	struct cros_ec_dev *ec = container_of(dev,
-					      struct cros_ec_dev, class_dev);
+	struct cros_ec_dev *ec = to_cros_ec_dev(dev);
 
 	msg = kmalloc(sizeof(*msg) + sizeof(*resp), GFP_KERNEL);
 	if (!msg)
-- 
2.16.1

[PATCH v2 3/6] platform/chrome: cros_ec_sysfs: use permission-specific DEVICE_ATTR variants

[Enric Balletbo i Serra]

Use DEVICE_ATTR variants for read/write attributes. This simplifies the
source code, improves readbility, and reduces the chance of
inconsistencies.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
Changes since v1:
- New in this series. Suggested by Andy Shevchenko.

 drivers/platform/chrome/cros_ec_sysfs.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_sysfs.c b/drivers/platform/chrome/cros_ec_sysfs.c
index 85bb9580439e..78ae0d3760e4 100644
--- a/drivers/platform/chrome/cros_ec_sysfs.c
+++ b/drivers/platform/chrome/cros_ec_sysfs.c
@@ -38,8 +38,8 @@
 
 /* Accessor functions */
 
-static ssize_t show_ec_reboot(struct device *dev,
-			      struct device_attribute *attr, char *buf)
+static ssize_t reboot_show(struct device *dev,
+			   struct device_attribute *attr, char *buf)
 {
 	int count = 0;
 
@@ -50,9 +50,9 @@ static ssize_t show_ec_reboot(struct device *dev,
 	return count;
 }
 
-static ssize_t store_ec_reboot(struct device *dev,
-			       struct device_attribute *attr,
-			       const char *buf, size_t count)
+static ssize_t reboot_store(struct device *dev,
+			    struct device_attribute *attr,
+			    const char *buf, size_t count)
 {
 	static const struct {
 		const char * const str;
@@ -123,8 +123,8 @@ static ssize_t store_ec_reboot(struct device *dev,
 	return count;
 }
 
-static ssize_t show_ec_version(struct device *dev,
-			       struct device_attribute *attr, char *buf)
+static ssize_t version_show(struct device *dev,
+			    struct device_attribute *attr, char *buf)
 {
 	static const char * const image_names[] = {"unknown", "RO", "RW"};
 	struct ec_response_get_version *r_ver;
@@ -225,8 +225,8 @@ static ssize_t show_ec_version(struct device *dev,
 	return count;
 }
 
-static ssize_t show_ec_flashinfo(struct device *dev,
-				 struct device_attribute *attr, char *buf)
+static ssize_t flashinfo_show(struct device *dev,
+			      struct device_attribute *attr, char *buf)
 {
 	struct ec_response_flash_info *resp;
 	struct cros_ec_command *msg;
@@ -260,9 +260,9 @@ static ssize_t show_ec_flashinfo(struct device *dev,
 
 /* Module initialization */
 
-static DEVICE_ATTR(reboot, S_IWUSR | S_IRUGO, show_ec_reboot, store_ec_reboot);
-static DEVICE_ATTR(version, S_IRUGO, show_ec_version, NULL);
-static DEVICE_ATTR(flashinfo, S_IRUGO, show_ec_flashinfo, NULL);
+static DEVICE_ATTR_RW(reboot);
+static DEVICE_ATTR_RO(version);
+static DEVICE_ATTR_RO(flashinfo);
 
 static struct attribute *__ec_attrs[] = {
 	&dev_attr_reboot.attr,
-- 
2.16.1

[PATCH v2 4/6] platform/chrome: cros_ec_debugfs: Use octal permissions '0444'

[Enric Balletbo i Serra]

Fixed the following checkpatch warning:

    WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider
    using octal permissions '0444'.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
Changes since v1:
- New in this series. Andy Shevchenko suggested use a separate patch for
  this.

 drivers/platform/chrome/cros_ec_debugfs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_debugfs.c b/drivers/platform/chrome/cros_ec_debugfs.c
index 0328856ec3a2..8be739b7385b 100644
--- a/drivers/platform/chrome/cros_ec_debugfs.c
+++ b/drivers/platform/chrome/cros_ec_debugfs.c
@@ -288,7 +288,7 @@ static int cros_ec_create_console_log(struct cros_ec_debugfs *debug_info)
 	init_waitqueue_head(&debug_info->log_wq);
 
 	if (!debugfs_create_file("console_log",
-				 S_IFREG | S_IRUGO,
+				 S_IFREG | 0444,
 				 debug_info->dir,
 				 debug_info,
 				 &cros_ec_console_log_fops))
@@ -341,7 +341,7 @@ static int cros_ec_create_panicinfo(struct cros_ec_debugfs *debug_info)
 	debug_info->panicinfo_blob.size = ret;
 
 	if (!debugfs_create_blob("panicinfo",
-				 S_IFREG | S_IRUGO,
+				 S_IFREG | 0444,
 				 debug_info->dir,
 				 &debug_info->panicinfo_blob)) {
 		ret = -ENOMEM;
-- 
2.16.1

[PATCH v2 5/6] platform/chrome: cros_ec_debugfs: Add PD port info to debugfs

[Enric Balletbo i Serra]

From: Shawn Nematbakhsh <shawnn@chromium.org>

Add info useful for debugging USB-PD port state.

Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
Changes since v1:
- Drop unrelated changes. Suggested by Andy Shevchenko.

 drivers/platform/chrome/cros_ec_debugfs.c | 72 +++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/drivers/platform/chrome/cros_ec_debugfs.c b/drivers/platform/chrome/cros_ec_debugfs.c
index 8be739b7385b..c62ee8e610a0 100644
--- a/drivers/platform/chrome/cros_ec_debugfs.c
+++ b/drivers/platform/chrome/cros_ec_debugfs.c
@@ -211,6 +211,58 @@ static int cros_ec_console_log_release(struct inode *inode, struct file *file)
 	return 0;
 }
 
+static ssize_t cros_ec_pdinfo_read(struct file *file,
+				   char __user *user_buf,
+				   size_t count,
+				   loff_t *ppos)
+{
+	char read_buf[EC_USB_PD_MAX_PORTS * 40], *p = read_buf;
+	struct cros_ec_debugfs *debug_info = file->private_data;
+	struct cros_ec_device *ec_dev = debug_info->ec->ec_dev;
+	struct {
+		struct cros_ec_command msg;
+		union {
+			struct ec_response_usb_pd_control_v1 resp;
+			struct ec_params_usb_pd_control params;
+		};
+	} __packed ec_buf;
+	struct cros_ec_command *msg;
+	struct ec_response_usb_pd_control_v1 *resp;
+	struct ec_params_usb_pd_control *params;
+	int i;
+
+	msg = &ec_buf.msg;
+	params = (struct ec_params_usb_pd_control *)msg->data;
+	resp = (struct ec_response_usb_pd_control_v1 *)msg->data;
+
+	msg->command = EC_CMD_USB_PD_CONTROL;
+	msg->version = 1;
+	msg->insize = sizeof(*resp);
+	msg->outsize = sizeof(*params);
+
+	/*
+	 * Read status from all PD ports until failure, typically caused
+	 * by attempting to read status on a port that doesn't exist.
+	 */
+	for (i = 0; i < EC_USB_PD_MAX_PORTS; ++i) {
+		params->port = i;
+		params->role = 0;
+		params->mux = 0;
+		params->swap = 0;
+
+		if (cros_ec_cmd_xfer_status(ec_dev, msg) < 0)
+			break;
+
+		p += scnprintf(p, sizeof(read_buf) + read_buf - p,
+			       "p%d: %s en:%.2x role:%.2x pol:%.2x\n", i,
+			       resp->state, resp->enabled, resp->role,
+			       resp->polarity);
+	}
+
+	return simple_read_from_buffer(user_buf, count, ppos,
+				       read_buf, p - read_buf);
+}
+
 const struct file_operations cros_ec_console_log_fops = {
 	.owner = THIS_MODULE,
 	.open = cros_ec_console_log_open,
@@ -220,6 +272,13 @@ const struct file_operations cros_ec_console_log_fops = {
 	.release = cros_ec_console_log_release,
 };
 
+const struct file_operations cros_ec_pdinfo_fops = {
+	.owner = THIS_MODULE,
+	.open = simple_open,
+	.read = cros_ec_pdinfo_read,
+	.llseek = default_llseek,
+};
+
 static int ec_read_version_supported(struct cros_ec_dev *ec)
 {
 	struct ec_params_get_cmd_versions_v1 *params;
@@ -355,6 +414,15 @@ static int cros_ec_create_panicinfo(struct cros_ec_debugfs *debug_info)
 	return ret;
 }
 
+static int cros_ec_create_pdinfo(struct cros_ec_debugfs *debug_info)
+{
+	if (!debugfs_create_file("pdinfo", 0444, debug_info->dir, debug_info,
+				 &cros_ec_pdinfo_fops))
+		return -ENOMEM;
+
+	return 0;
+}
+
 int cros_ec_debugfs_init(struct cros_ec_dev *ec)
 {
 	struct cros_ec_platform *ec_platform = dev_get_platdata(ec->dev);
@@ -379,6 +447,10 @@ int cros_ec_debugfs_init(struct cros_ec_dev *ec)
 	if (ret)
 		goto remove_debugfs;
 
+	ret = cros_ec_create_pdinfo(debug_info);
+	if (ret)
+		goto remove_debugfs;
+
 	ec->debug_info = debug_info;
 
 	return 0;
-- 
2.16.1

[PATCH v2 6/6] platform/chrome: mfd/cros_ec_dev: Add sysfs entry to set keyboard wake lid angle

[Enric Balletbo i Serra]

From: Gwendal Grignou <gwendal@chromium.org>

This adds a sysfs attribute (/sys/class/chromeos/cros_ec/kb_wake_angle)
used to set and get the keyboard wake lid angle. This attribute is
present only if 2 accelerometers are controlled by the EC.

This patch also moves the cros_ec features check before the device is
added so the features map obtained from the EC is ready on time.

Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
Changes since v1:
- Suggested by Andy Shevchenko:
  - Use the previous defined to_cros_ec_dev
  - Use one line when fits in 80 characters.
  - Use DEVICE_ATTR_RW variant.

 drivers/mfd/cros_ec_dev.c               | 19 ++++----
 drivers/platform/chrome/cros_ec_sysfs.c | 81 +++++++++++++++++++++++++++++++++
 include/linux/mfd/cros_ec.h             |  1 +
 3 files changed, 92 insertions(+), 9 deletions(-)

diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
index 9d4b74404f49..48eac38a7d62 100644
--- a/drivers/mfd/cros_ec_dev.c
+++ b/drivers/mfd/cros_ec_dev.c
@@ -485,15 +485,6 @@ static int ec_device_probe(struct platform_device *pdev)
 		goto failed;
 	}
 
-	retval = cdev_device_add(&ec->cdev, &ec->class_dev);
-	if (retval) {
-		dev_err(dev, "cdev_device_add failed => %d\n", retval);
-		goto failed;
-	}
-
-	if (cros_ec_debugfs_init(ec))
-		dev_warn(dev, "failed to create debugfs directory\n");
-
 	/* check whether this EC is a sensor hub. */
 	if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE))
 		cros_ec_sensors_register(ec);
@@ -514,6 +505,16 @@ static int ec_device_probe(struct platform_device *pdev)
 	/* Take control of the lightbar from the EC. */
 	lb_manual_suspend_ctrl(ec, 1);
 
+	/* We can now add the sysfs class, we know which parameter to show */
+	retval = cdev_device_add(&ec->cdev, &ec->class_dev);
+	if (retval) {
+		dev_err(dev, "cdev_device_add failed => %d\n", retval);
+		goto failed;
+	}
+
+	if (cros_ec_debugfs_init(ec))
+		dev_warn(dev, "failed to create debugfs directory\n");
+
 	return 0;
 
 failed:
diff --git a/drivers/platform/chrome/cros_ec_sysfs.c b/drivers/platform/chrome/cros_ec_sysfs.c
index 78ae0d3760e4..5a6db3fe213a 100644
--- a/drivers/platform/chrome/cros_ec_sysfs.c
+++ b/drivers/platform/chrome/cros_ec_sysfs.c
@@ -258,21 +258,102 @@ static ssize_t flashinfo_show(struct device *dev,
 	return ret;
 }
 
+/* Keyboard wake angle control */
+static ssize_t kb_wake_angle_show(struct device *dev,
+				  struct device_attribute *attr, char *buf)
+{
+	struct cros_ec_dev *ec = to_cros_ec_dev(dev);
+	struct ec_response_motion_sense *resp;
+	struct ec_params_motion_sense *param;
+	struct cros_ec_command *msg;
+	int ret;
+
+	msg = kmalloc(sizeof(*msg) + EC_HOST_PARAM_SIZE, GFP_KERNEL);
+	if (!msg)
+		return -ENOMEM;
+
+	param = (struct ec_params_motion_sense *)msg->data;
+	msg->command = EC_CMD_MOTION_SENSE_CMD + ec->cmd_offset;
+	msg->version = 2;
+	param->cmd = MOTIONSENSE_CMD_KB_WAKE_ANGLE;
+	param->kb_wake_angle.data = EC_MOTION_SENSE_NO_VALUE;
+	msg->outsize = sizeof(*param);
+	msg->insize = sizeof(*resp);
+
+	ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
+	if (ret < 0)
+		goto exit;
+
+	resp = (struct ec_response_motion_sense *)msg->data;
+	ret = scnprintf(buf, PAGE_SIZE, "%d\n", resp->kb_wake_angle.ret);
+exit:
+	kfree(msg);
+	return ret;
+}
+
+static ssize_t kb_wake_angle_store(struct device *dev,
+				   struct device_attribute *attr,
+				   const char *buf, size_t count)
+{
+	struct cros_ec_dev *ec = to_cros_ec_dev(dev);
+	struct ec_params_motion_sense *param;
+	struct cros_ec_command *msg;
+	u16 angle;
+	int ret;
+
+	ret = kstrtou16(buf, 0, &angle);
+	if (ret)
+		return ret;
+
+	msg = kmalloc(sizeof(*msg) + EC_HOST_PARAM_SIZE, GFP_KERNEL);
+	if (!msg)
+		return -ENOMEM;
+
+	param = (struct ec_params_motion_sense *)msg->data;
+	msg->command = EC_CMD_MOTION_SENSE_CMD + ec->cmd_offset;
+	msg->version = 2;
+	param->cmd = MOTIONSENSE_CMD_KB_WAKE_ANGLE;
+	param->kb_wake_angle.data = angle;
+	msg->outsize = sizeof(*param);
+	msg->insize = sizeof(struct ec_response_motion_sense);
+
+	ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
+	kfree(msg);
+	if (ret < 0)
+		return ret;
+	return count;
+}
+
 /* Module initialization */
 
 static DEVICE_ATTR_RW(reboot);
 static DEVICE_ATTR_RO(version);
 static DEVICE_ATTR_RO(flashinfo);
+static DEVICE_ATTR_RW(kb_wake_angle);
 
 static struct attribute *__ec_attrs[] = {
+	&dev_attr_kb_wake_angle.attr,
 	&dev_attr_reboot.attr,
 	&dev_attr_version.attr,
 	&dev_attr_flashinfo.attr,
 	NULL,
 };
 
+static umode_t cros_ec_ctrl_visible(struct kobject *kobj,
+				    struct attribute *a, int n)
+{
+	struct device *dev = container_of(kobj, struct device, kobj);
+	struct cros_ec_dev *ec = to_cros_ec_dev(dev);
+
+	if (a == &dev_attr_kb_wake_angle.attr && !ec->has_kb_wake_angle)
+		return 0;
+
+	return a->mode;
+}
+
 struct attribute_group cros_ec_attr_group = {
 	.attrs = __ec_attrs,
+	.is_visible = cros_ec_ctrl_visible,
 };
 EXPORT_SYMBOL(cros_ec_attr_group);
 
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index 804b3ddbf819..951c27333d46 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -191,6 +191,7 @@ struct cros_ec_dev {
 	struct cros_ec_device *ec_dev;
 	struct device *dev;
 	struct cros_ec_debugfs *debug_info;
+	bool has_kb_wake_angle;
 	u16 cmd_offset;
 	u32 features[2];
 };
-- 
2.16.1

Re: [PATCH v2 2/6] platform/chrome: cros_ec_sysfs: introduce to_cros_ec_dev define.

[Andy Shevchenko]

On Wed, Feb 21, 2018 at 7:07 PM, Enric Balletbo i Serra
<enric.balletbo@collabora.com> wrote:
> Add a define to get the cros_ec_dev from device and use it.
>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
> Changes since v1:
> - New in this series. Suggested by Andy Shevchenko.

We have an established tag

Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>

>
>  drivers/platform/chrome/cros_ec_sysfs.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_sysfs.c b/drivers/platform/chrome/cros_ec_sysfs.c
> index c03621e523a3..85bb9580439e 100644
> --- a/drivers/platform/chrome/cros_ec_sysfs.c
> +++ b/drivers/platform/chrome/cros_ec_sysfs.c
> @@ -34,6 +34,8 @@
>  #include <linux/types.h>
>  #include <linux/uaccess.h>
>
> +#define to_cros_ec_dev(dev)  container_of(dev, struct cros_ec_dev, class_dev)
> +
>  /* Accessor functions */
>
>  static ssize_t show_ec_reboot(struct device *dev,
> @@ -70,8 +72,7 @@ static ssize_t store_ec_reboot(struct device *dev,
>         int got_cmd = 0, offset = 0;
>         int i;
>         int ret;
> -       struct cros_ec_dev *ec = container_of(dev,
> -                                             struct cros_ec_dev, class_dev);
> +       struct cros_ec_dev *ec = to_cros_ec_dev(dev);
>
>         msg = kmalloc(sizeof(*msg) + sizeof(*param), GFP_KERNEL);
>         if (!msg)
> @@ -132,8 +133,7 @@ static ssize_t show_ec_version(struct device *dev,
>         struct cros_ec_command *msg;
>         int ret;
>         int count = 0;
> -       struct cros_ec_dev *ec = container_of(dev,
> -                                             struct cros_ec_dev, class_dev);
> +       struct cros_ec_dev *ec = to_cros_ec_dev(dev);
>
>         msg = kmalloc(sizeof(*msg) + EC_HOST_PARAM_SIZE, GFP_KERNEL);
>         if (!msg)
> @@ -231,8 +231,7 @@ static ssize_t show_ec_flashinfo(struct device *dev,
>         struct ec_response_flash_info *resp;
>         struct cros_ec_command *msg;
>         int ret;
> -       struct cros_ec_dev *ec = container_of(dev,
> -                                             struct cros_ec_dev, class_dev);
> +       struct cros_ec_dev *ec = to_cros_ec_dev(dev);
>
>         msg = kmalloc(sizeof(*msg) + sizeof(*resp), GFP_KERNEL);
>         if (!msg)
> --
> 2.16.1
>



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 3/6] platform/chrome: cros_ec_sysfs: use permission-specific DEVICE_ATTR variants

[Andy Shevchenko]

On Wed, Feb 21, 2018 at 7:07 PM, Enric Balletbo i Serra
<enric.balletbo@collabora.com> wrote:
> Use DEVICE_ATTR variants for read/write attributes. This simplifies the
> source code, improves readbility, and reduces the chance of
> inconsistencies.
>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
> Changes since v1:
> - New in this series. Suggested by Andy Shevchenko.

Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/6] platform/chrome: cros_ec_sysfs: Modify error handling

[Gwendal Grignou]

On Wed, Feb 21, 2018 at 9:07 AM, Enric Balletbo i Serra
<enric.balletbo@collabora.com> wrote:
> From: Gwendal Grignou <gwendal@chromium.org>
>
> When accessing a sysfs attribute, if the EC command fails, -EPROTO is
> now returned instead of an error message as it is unlikely an app is
> parsing the error message to do something meaningful.
> Also, this patch makes use of cros_ec_cmd_xfer_status() instead of
> cros_ec_cmd_xfer() so an error message is printed in the syslog.
>
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Reviewed-by: Gwendal Grignou <gwendal@chromium.org>
> ---
> Changes since v1:
> - None
>
>  drivers/platform/chrome/cros_ec_sysfs.c | 25 ++++---------------------
>  1 file changed, 4 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_sysfs.c b/drivers/platform/chrome/cros_ec_sysfs.c
> index da0a719d32f7..c03621e523a3 100644
> --- a/drivers/platform/chrome/cros_ec_sysfs.c
> +++ b/drivers/platform/chrome/cros_ec_sysfs.c
> @@ -114,15 +114,9 @@ static ssize_t store_ec_reboot(struct device *dev,
>         msg->command = EC_CMD_REBOOT_EC + ec->cmd_offset;
>         msg->outsize = sizeof(*param);
>         msg->insize = 0;
> -       ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
> -       if (ret < 0) {
> +       ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
> +       if (ret < 0)
>                 count = ret;
> -               goto exit;
> -       }
> -       if (msg->result != EC_RES_SUCCESS) {
> -               dev_dbg(ec->dev, "EC result %d\n", msg->result);
> -               count = -EINVAL;
> -       }
>  exit:
>         kfree(msg);
>         return count;
> @@ -150,17 +144,11 @@ static ssize_t show_ec_version(struct device *dev,
>         msg->command = EC_CMD_GET_VERSION + ec->cmd_offset;
>         msg->insize = sizeof(*r_ver);
>         msg->outsize = 0;
> -       ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
> +       ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
>         if (ret < 0) {
>                 count = ret;
>                 goto exit;
>         }
> -       if (msg->result != EC_RES_SUCCESS) {
> -               count = scnprintf(buf, PAGE_SIZE,
> -                                 "ERROR: EC returned %d\n", msg->result);
> -               goto exit;
> -       }
> -
>         r_ver = (struct ec_response_get_version *)msg->data;
>         /* Strings should be null-terminated, but let's be sure. */
>         r_ver->version_string_ro[sizeof(r_ver->version_string_ro) - 1] = '\0';
> @@ -255,14 +243,9 @@ static ssize_t show_ec_flashinfo(struct device *dev,
>         msg->command = EC_CMD_FLASH_INFO + ec->cmd_offset;
>         msg->insize = sizeof(*resp);
>         msg->outsize = 0;
> -       ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
> +       ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
>         if (ret < 0)
>                 goto exit;
> -       if (msg->result != EC_RES_SUCCESS) {
> -               ret = scnprintf(buf, PAGE_SIZE,
> -                               "ERROR: EC returned %d\n", msg->result);
> -               goto exit;
> -       }
>
>         resp = (struct ec_response_flash_info *)msg->data;
>
> --
> 2.16.1
>

Re: [PATCH v2 5/6] platform/chrome: cros_ec_debugfs: Add PD port info to debugfs

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3582 bytes --]

Hi Shawn,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on ljones-mfd/for-mfd-next]
[also build test ERROR on v4.16-rc3 next-20180228]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Enric-Balletbo-i-Serra/platform-chrome-cros_ec_sysfs-Modify-error-handling/20180223-053556
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/platform/chrome/cros_ec_debugfs.c: In function 'cros_ec_pdinfo_read':
>> drivers/platform/chrome/cros_ec_debugfs.c:219:16: error: 'EC_USB_PD_MAX_PORTS' undeclared (first use in this function); did you mean 'EC_CMD_USB_PD_PORTS'?
     char read_buf[EC_USB_PD_MAX_PORTS * 40], *p = read_buf;
                   ^~~~~~~~~~~~~~~~~~~
                   EC_CMD_USB_PD_PORTS
   drivers/platform/chrome/cros_ec_debugfs.c:219:16: note: each undeclared identifier is reported only once for each function it appears in
   drivers/platform/chrome/cros_ec_debugfs.c:219:7: warning: unused variable 'read_buf' [-Wunused-variable]
     char read_buf[EC_USB_PD_MAX_PORTS * 40], *p = read_buf;
          ^~~~~~~~
   drivers/platform/chrome/cros_ec_debugfs.c:264:1: warning: control reaches end of non-void function [-Wreturn-type]
    }
    ^

vim +219 drivers/platform/chrome/cros_ec_debugfs.c

   213	
   214	static ssize_t cros_ec_pdinfo_read(struct file *file,
   215					   char __user *user_buf,
   216					   size_t count,
   217					   loff_t *ppos)
   218	{
 > 219		char read_buf[EC_USB_PD_MAX_PORTS * 40], *p = read_buf;
   220		struct cros_ec_debugfs *debug_info = file->private_data;
   221		struct cros_ec_device *ec_dev = debug_info->ec->ec_dev;
   222		struct {
   223			struct cros_ec_command msg;
   224			union {
   225				struct ec_response_usb_pd_control_v1 resp;
   226				struct ec_params_usb_pd_control params;
   227			};
   228		} __packed ec_buf;
   229		struct cros_ec_command *msg;
   230		struct ec_response_usb_pd_control_v1 *resp;
   231		struct ec_params_usb_pd_control *params;
   232		int i;
   233	
   234		msg = &ec_buf.msg;
   235		params = (struct ec_params_usb_pd_control *)msg->data;
   236		resp = (struct ec_response_usb_pd_control_v1 *)msg->data;
   237	
   238		msg->command = EC_CMD_USB_PD_CONTROL;
   239		msg->version = 1;
   240		msg->insize = sizeof(*resp);
   241		msg->outsize = sizeof(*params);
   242	
   243		/*
   244		 * Read status from all PD ports until failure, typically caused
   245		 * by attempting to read status on a port that doesn't exist.
   246		 */
   247		for (i = 0; i < EC_USB_PD_MAX_PORTS; ++i) {
   248			params->port = i;
   249			params->role = 0;
   250			params->mux = 0;
   251			params->swap = 0;
   252	
   253			if (cros_ec_cmd_xfer_status(ec_dev, msg) < 0)
   254				break;
   255	
   256			p += scnprintf(p, sizeof(read_buf) + read_buf - p,
   257				       "p%d: %s en:%.2x role:%.2x pol:%.2x\n", i,
   258				       resp->state, resp->enabled, resp->role,
   259				       resp->polarity);
   260		}
   261	
   262		return simple_read_from_buffer(user_buf, count, ppos,
   263					       read_buf, p - read_buf);
   264	}
   265	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 62548 bytes --]

Re: [PATCH v2 5/6] platform/chrome: cros_ec_debugfs: Add PD port info to debugfs

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3547 bytes --]

Hi Shawn,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on ljones-mfd/for-mfd-next]
[also build test ERROR on v4.16-rc3 next-20180228]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Enric-Balletbo-i-Serra/platform-chrome-cros_ec_sysfs-Modify-error-handling/20180223-053556
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
config: x86_64-randconfig-s2-03010719 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/platform/chrome/cros_ec_debugfs.c: In function 'cros_ec_pdinfo_read':
>> drivers/platform/chrome/cros_ec_debugfs.c:219:16: error: 'EC_USB_PD_MAX_PORTS' undeclared (first use in this function)
     char read_buf[EC_USB_PD_MAX_PORTS * 40], *p = read_buf;
                   ^~~~~~~~~~~~~~~~~~~
   drivers/platform/chrome/cros_ec_debugfs.c:219:16: note: each undeclared identifier is reported only once for each function it appears in
   drivers/platform/chrome/cros_ec_debugfs.c:219:7: warning: unused variable 'read_buf' [-Wunused-variable]
     char read_buf[EC_USB_PD_MAX_PORTS * 40], *p = read_buf;
          ^~~~~~~~
   drivers/platform/chrome/cros_ec_debugfs.c:264:1: warning: control reaches end of non-void function [-Wreturn-type]
    }
    ^

vim +/EC_USB_PD_MAX_PORTS +219 drivers/platform/chrome/cros_ec_debugfs.c

   213	
   214	static ssize_t cros_ec_pdinfo_read(struct file *file,
   215					   char __user *user_buf,
   216					   size_t count,
   217					   loff_t *ppos)
   218	{
 > 219		char read_buf[EC_USB_PD_MAX_PORTS * 40], *p = read_buf;
   220		struct cros_ec_debugfs *debug_info = file->private_data;
   221		struct cros_ec_device *ec_dev = debug_info->ec->ec_dev;
   222		struct {
   223			struct cros_ec_command msg;
   224			union {
   225				struct ec_response_usb_pd_control_v1 resp;
   226				struct ec_params_usb_pd_control params;
   227			};
   228		} __packed ec_buf;
   229		struct cros_ec_command *msg;
   230		struct ec_response_usb_pd_control_v1 *resp;
   231		struct ec_params_usb_pd_control *params;
   232		int i;
   233	
   234		msg = &ec_buf.msg;
   235		params = (struct ec_params_usb_pd_control *)msg->data;
   236		resp = (struct ec_response_usb_pd_control_v1 *)msg->data;
   237	
   238		msg->command = EC_CMD_USB_PD_CONTROL;
   239		msg->version = 1;
   240		msg->insize = sizeof(*resp);
   241		msg->outsize = sizeof(*params);
   242	
   243		/*
   244		 * Read status from all PD ports until failure, typically caused
   245		 * by attempting to read status on a port that doesn't exist.
   246		 */
   247		for (i = 0; i < EC_USB_PD_MAX_PORTS; ++i) {
   248			params->port = i;
   249			params->role = 0;
   250			params->mux = 0;
   251			params->swap = 0;
   252	
   253			if (cros_ec_cmd_xfer_status(ec_dev, msg) < 0)
   254				break;
   255	
   256			p += scnprintf(p, sizeof(read_buf) + read_buf - p,
   257				       "p%d: %s en:%.2x role:%.2x pol:%.2x\n", i,
   258				       resp->state, resp->enabled, resp->role,
   259				       resp->polarity);
   260		}
   261	
   262		return simple_read_from_buffer(user_buf, count, ppos,
   263					       read_buf, p - read_buf);
   264	}
   265	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29320 bytes --]

Re: [PATCH v2 6/6] platform/chrome: mfd/cros_ec_dev: Add sysfs entry to set keyboard wake lid angle

[Lee Jones]

On Wed, 21 Feb 2018, Enric Balletbo i Serra wrote:

> From: Gwendal Grignou <gwendal@chromium.org>
> 
> This adds a sysfs attribute (/sys/class/chromeos/cros_ec/kb_wake_angle)
> used to set and get the keyboard wake lid angle. This attribute is
> present only if 2 accelerometers are controlled by the EC.
> 
> This patch also moves the cros_ec features check before the device is
> added so the features map obtained from the EC is ready on time.
> 
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
> Changes since v1:
> - Suggested by Andy Shevchenko:
>   - Use the previous defined to_cros_ec_dev
>   - Use one line when fits in 80 characters.
>   - Use DEVICE_ATTR_RW variant.
> 
>  drivers/platform/chrome/cros_ec_sysfs.c | 81 +++++++++++++++++++++++++++++++++

>  drivers/mfd/cros_ec_dev.c               | 19 ++++----
>  include/linux/mfd/cros_ec.h             |  1 +

Please send me a PR with this patch contained.

Acked-by: Lee Jones <lee.jones@linaro.org>

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

Re: [PATCH v2 6/6] platform/chrome: mfd/cros_ec_dev: Add sysfs entry to set keyboard wake lid angle

[Gwendal Grignou]

On Wed, Mar 7, 2018 at 8:28 AM Lee Jones <lee.jones@linaro.org> wrote:

> On Wed, 21 Feb 2018, Enric Balletbo i Serra wrote:

> > From: Gwendal Grignou <gwendal@chromium.org>
> >
> > This adds a sysfs attribute (/sys/class/chromeos/cros_ec/kb_wake_angle)
> > used to set and get the keyboard wake lid angle. This attribute is
> > present only if 2 accelerometers are controlled by the EC.
> >
> > This patch also moves the cros_ec features check before the device is
> > added so the features map obtained from the EC is ready on time.

Enric,

I found an error in the patch. During the merge, the code that set
has_kb_wake_angle in cros_ec_sensors_register() has been lost. I uploaded a
change (that also remove call for a non-existent cros-ec-angle platform
driver) at https://chromium-review.googlesource.com/#/c/866106.

Gwendal.
> >
> > Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> > Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> > ---
> > Changes since v1:
> > - Suggested by Andy Shevchenko:
> >   - Use the previous defined to_cros_ec_dev
> >   - Use one line when fits in 80 characters.
> >   - Use DEVICE_ATTR_RW variant.
> >
> >  drivers/platform/chrome/cros_ec_sysfs.c | 81
+++++++++++++++++++++++++++++++++

> >  drivers/mfd/cros_ec_dev.c               | 19 ++++----
> >  include/linux/mfd/cros_ec.h             |  1 +

> Please send me a PR with this patch contained.

> Acked-by: Lee Jones <lee.jones@linaro.org>

> --
> Lee Jones
> Linaro Services Technical Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog