From 311394248ca539c12b33d396b2944e03cd0d6bfc Mon Sep 17 00:00:00 2001 From: Gwendal Grignou Date: Fri, 23 Mar 2018 18:42:42 +0100 Subject: [PATCH 1/6] platform/chrome: cros_ec_sysfs: Modify error handling 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 Signed-off-by: Enric Balletbo i Serra Signed-off-by: Benson Leung --- 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; From 93afebbe694ecd0a22f065af2e2565e2dbd49025 Mon Sep 17 00:00:00 2001 From: Enric Balletbo i Serra Date: Fri, 23 Mar 2018 18:42:43 +0100 Subject: [PATCH 2/6] platform/chrome: cros_ec_sysfs: introduce to_cros_ec_dev define. Add a define to get the cros_ec_dev from device and use it. Suggested-by: Andy Shevchenko Signed-off-by: Enric Balletbo i Serra Reviewed-by: Andy Shevchenko Signed-off-by: Benson Leung --- 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 #include +#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) From 7d800c99ecfab57845c5ae29ed0aefb36c055c2b Mon Sep 17 00:00:00 2001 From: Enric Balletbo i Serra Date: Fri, 23 Mar 2018 18:42:44 +0100 Subject: [PATCH 3/6] platform/chrome: cros_ec_sysfs: use permission-specific DEVICE_ATTR variants 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 Signed-off-by: Enric Balletbo i Serra Reviewed-by: Andy Shevchenko Signed-off-by: Benson Leung --- 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 @@ exit: 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 @@ exit: 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 @@ exit: /* 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, From f63192800ebd01099d3487f7fef1dd0ee0b18b45 Mon Sep 17 00:00:00 2001 From: Enric Balletbo i Serra Date: Fri, 23 Mar 2018 18:42:45 +0100 Subject: [PATCH 4/6] platform/chrome: cros_ec_debugfs: Use octal permissions '0444' 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 Suggested-by: Andy Shevchenko Reviewed-by: Andy Shevchenko Signed-off-by: Benson Leung --- 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; From b082b2e1454c3e0217d7cf70f2211966c3d54301 Mon Sep 17 00:00:00 2001 From: Shawn Nematbakhsh Date: Fri, 23 Mar 2018 18:42:46 +0100 Subject: [PATCH 5/6] platform/chrome: cros_ec_debugfs: Add PD port info to debugfs Add info useful for debugging USB-PD port state. Signed-off-by: Shawn Nematbakhsh Signed-off-by: Enric Balletbo i Serra Reviewed-by: Andy Shevchenko Acked-by: Lee Jones Signed-off-by: Benson Leung --- 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 @@ free: 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; From c1d1e91aff3d1183d6b16a282c2575e3e006cee4 Mon Sep 17 00:00:00 2001 From: Gwendal Grignou Date: Fri, 23 Mar 2018 18:42:47 +0100 Subject: [PATCH 6/6] platform/chrome: mfd/cros_ec_dev: Add sysfs entry to set keyboard wake lid angle 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 Signed-off-by: Enric Balletbo i Serra Reviewed-by: Andy Shevchenko Acked-by: Lee Jones Signed-off-by: Benson Leung --- 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 @@ exit: 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]; };