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

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

[Enric Balletbo i Serra]

Hi,

This is a four patchset that introduces some cros_ec debugfs and sysfs
updates. This version is practically the same as version 3, it only
fixes the issue pointed by Gwendal to only allocate one extra sensor
intead of two.

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.

In Patch 5 I added a missing define, so now this patch can be picked
without any dependency, although now needs the ack from mfd and
platform/chrome maintainer as touches both sub-systems.

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.
This version also include a fix found by Gwendal.

Best regards,
  Enric

Changes in v4:
- [6/6] Allocate only one extra sensor (Gwendal)

Changes in v3:
- [2/6] Add Reviewed-by Andy Shevchenko
- [3/6] Add Reviewed-by Andy Shevchenko
- [4/6] Add Reviewed-by Andy Shevchenko
- [5/6] Add EC_USB_PD_MAX_PORTS define to avoid interdependencies.
- [5/6] Add Reviewed-by Andy Shevchenko
- [6/6] Add Reviewed-by Andy Shevchenko
- [6/6] Fix the code that has_kb_wake_angle in cros_ec_sensors_register().

Changes in v2:
- [2/6] Add a new define to get the cros_ec_dev.
- [3/6] Use DEVICE_ATTR variants.
- [4/6] Use octal permissions.
- [5/6] Drop unrelated changes.
- [6/6] Use DEVICE_ATTR_RW variant.
- [6/6] Use one line when fits in 80 characters.
- [6/6] Use the previous defined to_cros_ec_dev

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                 |  31 +++----
 drivers/platform/chrome/cros_ec_debugfs.c |  76 +++++++++++++++-
 drivers/platform/chrome/cros_ec_sysfs.c   | 141 +++++++++++++++++++++---------
 include/linux/mfd/cros_ec.h               |   2 +
 include/linux/mfd/cros_ec_commands.h      |   3 +
 5 files changed, 194 insertions(+), 59 deletions(-)

-- 
2.16.2

[PATCH v4 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 in v4: None
Changes in v3: None
Changes in v2: 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.2

[PATCH v4 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.

Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---

Changes in v4: None
Changes in v3:
- [2/6] Add Reviewed-by Andy Shevchenko

Changes in v2:
- [2/6] Add a new define to get the cros_ec_dev.

 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.2

[PATCH v4 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.

Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---

Changes in v4: None
Changes in v3:
- [3/6] Add Reviewed-by Andy Shevchenko

Changes in v2:
- [3/6] Use DEVICE_ATTR variants.

 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.2

[PATCH v4 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>
Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---

Changes in v4: None
Changes in v3:
- [4/6] Add Reviewed-by Andy Shevchenko

Changes in v2:
- [4/6] Use octal permissions.

 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 0e88e18362c1..539403022568 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.2

[PATCH v4 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>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---

Changes in v4: None
Changes in v3:
- [5/6] Add EC_USB_PD_MAX_PORTS define to avoid interdependencies.
- [5/6] Add Reviewed-by Andy Shevchenko

Changes in v2:
- [5/6] Drop unrelated changes.

 drivers/platform/chrome/cros_ec_debugfs.c | 72 +++++++++++++++++++++++++++++++
 include/linux/mfd/cros_ec_commands.h      |  3 ++
 2 files changed, 75 insertions(+)

diff --git a/drivers/platform/chrome/cros_ec_debugfs.c b/drivers/platform/chrome/cros_ec_debugfs.c
index 539403022568..cc265ed8deb7 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;
diff --git a/include/linux/mfd/cros_ec_commands.h b/include/linux/mfd/cros_ec_commands.h
index 2b96e630e3b6..f2edd9969b40 100644
--- a/include/linux/mfd/cros_ec_commands.h
+++ b/include/linux/mfd/cros_ec_commands.h
@@ -2948,6 +2948,9 @@ struct ec_response_usb_pd_control_v1 {
 
 #define EC_CMD_USB_PD_PORTS 0x102
 
+/* Maximum number of PD ports on a device, num_ports will be <= this */
+#define EC_USB_PD_MAX_PORTS 8
+
 struct ec_response_usb_pd_ports {
 	uint8_t num_ports;
 } __packed;
-- 
2.16.2

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

[Lee Jones]

On Fri, 23 Mar 2018, Enric Balletbo i Serra wrote:

> 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>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> ---
> 
> Changes in v4: None
> Changes in v3:
> - [5/6] Add EC_USB_PD_MAX_PORTS define to avoid interdependencies.
> - [5/6] Add Reviewed-by Andy Shevchenko
> 
> Changes in v2:
> - [5/6] Drop unrelated changes.
> 
>  drivers/platform/chrome/cros_ec_debugfs.c | 72 +++++++++++++++++++++++++++++++

>  include/linux/mfd/cros_ec_commands.h      |  3 ++

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

>  2 files changed, 75 insertions(+)

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

[PATCH v4 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>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---

Changes in v4:
- [6/6] Allocate only one extra sensor (Gwendal)

Changes in v3:
- [6/6] Add Reviewed-by Andy Shevchenko
- [6/6] Fix the code that has_kb_wake_angle in cros_ec_sensors_register().

Changes in v2:
- [6/6] Use DEVICE_ATTR_RW variant.
- [6/6] Use one line when fits in 80 characters.
- [6/6] Use the previous defined to_cros_ec_dev

 drivers/mfd/cros_ec_dev.c               | 31 ++++++-------
 drivers/platform/chrome/cros_ec_sysfs.c | 81 +++++++++++++++++++++++++++++++++
 include/linux/mfd/cros_ec.h             |  2 +
 3 files changed, 96 insertions(+), 18 deletions(-)

diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
index e4fafdd96e5e..eafd06f62a3a 100644
--- a/drivers/mfd/cros_ec_dev.c
+++ b/drivers/mfd/cros_ec_dev.c
@@ -305,8 +305,8 @@ static void cros_ec_sensors_register(struct cros_ec_dev *ec)
 
 	resp = (struct ec_response_motion_sense *)msg->data;
 	sensor_num = resp->dump.sensor_count;
-	/* Allocate 2 extra sensors in case lid angle or FIFO are needed */
-	sensor_cells = kzalloc(sizeof(struct mfd_cell) * (sensor_num + 2),
+	/* Allocate 1 extra sensors in FIFO are needed */
+	sensor_cells = kzalloc(sizeof(struct mfd_cell) * (sensor_num + 1),
 			       GFP_KERNEL);
 	if (sensor_cells == NULL)
 		goto error;
@@ -362,16 +362,10 @@ static void cros_ec_sensors_register(struct cros_ec_dev *ec)
 		sensor_type[resp->info.type]++;
 		id++;
 	}
-	if (sensor_type[MOTIONSENSE_TYPE_ACCEL] >= 2) {
-		sensor_platforms[id].sensor_num = sensor_num;
 
-		sensor_cells[id].name = "cros-ec-angle";
-		sensor_cells[id].id = 0;
-		sensor_cells[id].platform_data = &sensor_platforms[id];
-		sensor_cells[id].pdata_size =
-			sizeof(struct cros_ec_sensor_platform);
-		id++;
-	}
+	if (sensor_type[MOTIONSENSE_TYPE_ACCEL] >= 2)
+		ec->has_kb_wake_angle = true;
+
 	if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE_FIFO)) {
 		sensor_cells[id].name = "cros-ec-ring";
 		id++;
@@ -424,6 +418,14 @@ static int ec_device_probe(struct platform_device *pdev)
 		goto failed;
 	}
 
+	/* check whether this EC is a sensor hub. */
+	if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE))
+		cros_ec_sensors_register(ec);
+
+	/* 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);
@@ -433,13 +435,6 @@ static int ec_device_probe(struct platform_device *pdev)
 	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);
-
-	/* Take control of the lightbar from the EC. */
-	lb_manual_suspend_ctrl(ec, 1);
-
 	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 c61535979b8f..2d4e23c9ea0a 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -183,6 +183,7 @@ struct cros_ec_debugfs;
  * @ec_dev: cros_ec_device structure to talk to the physical device
  * @dev: pointer to the platform device
  * @debug_info: cros_ec_debugfs structure for debugging information
+ * @has_kb_wake_angle: true if at least 2 accelerometer are connected to the EC.
  * @cmd_offset: offset to apply for each command.
  */
 struct cros_ec_dev {
@@ -191,6 +192,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.2

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

[Lee Jones]

On Fri, 23 Mar 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>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> ---
> 
> Changes in v4:
> - [6/6] Allocate only one extra sensor (Gwendal)
> 
> Changes in v3:
> - [6/6] Add Reviewed-by Andy Shevchenko
> - [6/6] Fix the code that has_kb_wake_angle in cros_ec_sensors_register().
> 
> Changes in v2:
> - [6/6] Use DEVICE_ATTR_RW variant.
> - [6/6] Use one line when fits in 80 characters.
> - [6/6] Use the previous defined to_cros_ec_dev
> 
>  drivers/mfd/cros_ec_dev.c               | 31 ++++++-------
>  drivers/platform/chrome/cros_ec_sysfs.c | 81 +++++++++++++++++++++++++++++++++
>  include/linux/mfd/cros_ec.h             |  2 +
>  3 files changed, 96 insertions(+), 18 deletions(-)

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 v4 0/6] platform/chrome: cros_ec debugfs and sysfs updates

[Gwendal Grignou]

On Fri, Mar 23, 2018 at 10:42 AM Enric Balletbo i Serra <
enric.balletbo@collabora.com> wrote:

> Hi,

> This is a four patchset that introduces some cros_ec debugfs and sysfs
> updates. This version is practically the same as version 3, it only
> fixes the issue pointed by Gwendal to only allocate one extra sensor
> intead of two.

> 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.

> In Patch 5 I added a missing define, so now this patch can be picked
> without any dependency, although now needs the ack from mfd and
> platform/chrome maintainer as touches both sub-systems.

> 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.
> This version also include a fix found by Gwendal.

> Best regards,
>    Enric

> Changes in v4:
> - [6/6] Allocate only one extra sensor (Gwendal)

> Changes in v3:
> - [2/6] Add Reviewed-by Andy Shevchenko
> - [3/6] Add Reviewed-by Andy Shevchenko
> - [4/6] Add Reviewed-by Andy Shevchenko
> - [5/6] Add EC_USB_PD_MAX_PORTS define to avoid interdependencies.
> - [5/6] Add Reviewed-by Andy Shevchenko
> - [6/6] Add Reviewed-by Andy Shevchenko
> - [6/6] Fix the code that has_kb_wake_angle in cros_ec_sensors_register().

> Changes in v2:
> - [2/6] Add a new define to get the cros_ec_dev.
> - [3/6] Use DEVICE_ATTR variants.
> - [4/6] Use octal permissions.
> - [5/6] Drop unrelated changes.
> - [6/6] Use DEVICE_ATTR_RW variant.
> - [6/6] Use one line when fits in 80 characters.
> - [6/6] Use the previous defined to_cros_ec_dev

> 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                 |  31 +++----
>   drivers/platform/chrome/cros_ec_debugfs.c |  76 +++++++++++++++-
>   drivers/platform/chrome/cros_ec_sysfs.c   | 141
+++++++++++++++++++++---------
>   include/linux/mfd/cros_ec.h               |   2 +
>   include/linux/mfd/cros_ec_commands.h      |   3 +
>   5 files changed, 194 insertions(+), 59 deletions(-)

> --
> 2.16.2

The patchset looks good,
Tested-by: Gwendal Grignou <gwendal@chromium.org>