From a841178445bb72a3d566b4e6ab9d19e9b002eb47 Mon Sep 17 00:00:00 2001 From: Javier Martinez Canillas Date: Tue, 9 Jun 2015 13:04:42 +0200 Subject: mfd: cros_ec: Use a zero-length array for command data Commit 1b84f2a4cd4a ("mfd: cros_ec: Use fixed size arrays to transfer data with the EC") modified the struct cros_ec_command fields to not use pointers for the input and output buffers and use fixed length arrays instead. This change was made because the cros_ec ioctl API uses that struct cros_ec_command to allow user-space to send commands to the EC and to get data from the EC. So using pointers made the API not 64-bit safe. Unfortunately this approach was not flexible enough for all the use-cases since there may be a need to send larger commands on newer versions of the EC command protocol. So to avoid to choose a constant length that it may be too big for most commands and thus wasting memory and CPU cycles on copy from and to user-space or having a size that is too small for some big commands, use a zero-length array that is both 64-bit safe and flexible. The same buffer is used for both output and input data so the maximum of these values should be used to allocate it. Suggested-by: Gwendal Grignou Signed-off-by: Javier Martinez Canillas Tested-by: Heiko Stuebner Acked-by: Lee Jones Acked-by: Olof Johansson Signed-off-by: Lee Jones --- drivers/platform/chrome/cros_ec_sysfs.c | 154 ++++++++++++++++++++------------ 1 file changed, 95 insertions(+), 59 deletions(-) (limited to 'drivers/platform/chrome/cros_ec_sysfs.c') diff --git a/drivers/platform/chrome/cros_ec_sysfs.c b/drivers/platform/chrome/cros_ec_sysfs.c index fb62ab6cc659..78ba82d670cb 100644 --- a/drivers/platform/chrome/cros_ec_sysfs.c +++ b/drivers/platform/chrome/cros_ec_sysfs.c @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -66,14 +67,19 @@ static ssize_t store_ec_reboot(struct device *dev, {"hibernate", EC_REBOOT_HIBERNATE, 0}, {"at-shutdown", -1, EC_REBOOT_FLAG_ON_AP_SHUTDOWN}, }; - struct cros_ec_command msg = { 0 }; - struct ec_params_reboot_ec *param = - (struct ec_params_reboot_ec *)msg.outdata; + struct cros_ec_command *msg; + struct ec_params_reboot_ec *param; int got_cmd = 0, offset = 0; int i; int ret; struct cros_ec_device *ec = dev_get_drvdata(dev); + msg = kmalloc(sizeof(*msg) + sizeof(*param), GFP_KERNEL); + if (!msg) + return -ENOMEM; + + param = (struct ec_params_reboot_ec *)msg->data; + param->flags = 0; while (1) { /* Find word to start scanning */ @@ -100,19 +106,26 @@ static ssize_t store_ec_reboot(struct device *dev, offset++; } - if (!got_cmd) - return -EINVAL; - - msg.command = EC_CMD_REBOOT_EC; - msg.outsize = sizeof(param); - ret = cros_ec_cmd_xfer(ec, &msg); - if (ret < 0) - return ret; - if (msg.result != EC_RES_SUCCESS) { - dev_dbg(ec->dev, "EC result %d\n", msg.result); - return -EINVAL; + if (!got_cmd) { + count = -EINVAL; + goto exit; } + msg->version = 0; + msg->command = EC_CMD_REBOOT_EC; + msg->outsize = sizeof(*param); + msg->insize = 0; + ret = cros_ec_cmd_xfer(ec, 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; } @@ -123,22 +136,32 @@ static ssize_t show_ec_version(struct device *dev, struct ec_response_get_version *r_ver; struct ec_response_get_chip_info *r_chip; struct ec_response_board_version *r_board; - struct cros_ec_command msg = { 0 }; + struct cros_ec_command *msg; int ret; int count = 0; struct cros_ec_device *ec = dev_get_drvdata(dev); + msg = kmalloc(sizeof(*msg) + EC_HOST_PARAM_SIZE, GFP_KERNEL); + if (!msg) + return -ENOMEM; + /* Get versions. RW may change. */ - msg.command = EC_CMD_GET_VERSION; - msg.insize = sizeof(*r_ver); - ret = cros_ec_cmd_xfer(ec, &msg); - if (ret < 0) - return ret; - if (msg.result != EC_RES_SUCCESS) - return scnprintf(buf, PAGE_SIZE, - "ERROR: EC returned %d\n", msg.result); + msg->version = 0; + msg->command = EC_CMD_GET_VERSION; + msg->insize = sizeof(*r_ver); + msg->outsize = 0; + ret = cros_ec_cmd_xfer(ec, 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.indata; + 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'; r_ver->version_string_rw[sizeof(r_ver->version_string_rw) - 1] = '\0'; @@ -152,33 +175,33 @@ static ssize_t show_ec_version(struct device *dev, image_names[r_ver->current_image] : "?")); /* Get build info. */ - msg.command = EC_CMD_GET_BUILD_INFO; - msg.insize = sizeof(msg.indata); - ret = cros_ec_cmd_xfer(ec, &msg); + msg->command = EC_CMD_GET_BUILD_INFO; + msg->insize = EC_HOST_PARAM_SIZE; + ret = cros_ec_cmd_xfer(ec, msg); if (ret < 0) count += scnprintf(buf + count, PAGE_SIZE - count, "Build info: XFER ERROR %d\n", ret); - else if (msg.result != EC_RES_SUCCESS) + else if (msg->result != EC_RES_SUCCESS) count += scnprintf(buf + count, PAGE_SIZE - count, - "Build info: EC error %d\n", msg.result); + "Build info: EC error %d\n", msg->result); else { - msg.indata[sizeof(msg.indata) - 1] = '\0'; + msg->data[sizeof(msg->data) - 1] = '\0'; count += scnprintf(buf + count, PAGE_SIZE - count, - "Build info: %s\n", msg.indata); + "Build info: %s\n", msg->data); } /* Get chip info. */ - msg.command = EC_CMD_GET_CHIP_INFO; - msg.insize = sizeof(*r_chip); - ret = cros_ec_cmd_xfer(ec, &msg); + msg->command = EC_CMD_GET_CHIP_INFO; + msg->insize = sizeof(*r_chip); + ret = cros_ec_cmd_xfer(ec, msg); if (ret < 0) count += scnprintf(buf + count, PAGE_SIZE - count, "Chip info: XFER ERROR %d\n", ret); - else if (msg.result != EC_RES_SUCCESS) + else if (msg->result != EC_RES_SUCCESS) count += scnprintf(buf + count, PAGE_SIZE - count, - "Chip info: EC error %d\n", msg.result); + "Chip info: EC error %d\n", msg->result); else { - r_chip = (struct ec_response_get_chip_info *)msg.indata; + r_chip = (struct ec_response_get_chip_info *)msg->data; r_chip->vendor[sizeof(r_chip->vendor) - 1] = '\0'; r_chip->name[sizeof(r_chip->name) - 1] = '\0'; @@ -192,23 +215,25 @@ static ssize_t show_ec_version(struct device *dev, } /* Get board version */ - msg.command = EC_CMD_GET_BOARD_VERSION; - msg.insize = sizeof(*r_board); - ret = cros_ec_cmd_xfer(ec, &msg); + msg->command = EC_CMD_GET_BOARD_VERSION; + msg->insize = sizeof(*r_board); + ret = cros_ec_cmd_xfer(ec, msg); if (ret < 0) count += scnprintf(buf + count, PAGE_SIZE - count, "Board version: XFER ERROR %d\n", ret); - else if (msg.result != EC_RES_SUCCESS) + else if (msg->result != EC_RES_SUCCESS) count += scnprintf(buf + count, PAGE_SIZE - count, - "Board version: EC error %d\n", msg.result); + "Board version: EC error %d\n", msg->result); else { - r_board = (struct ec_response_board_version *)msg.indata; + r_board = (struct ec_response_board_version *)msg->data; count += scnprintf(buf + count, PAGE_SIZE - count, "Board version: %d\n", r_board->board_version); } +exit: + kfree(msg); return count; } @@ -216,27 +241,38 @@ static ssize_t show_ec_flashinfo(struct device *dev, struct device_attribute *attr, char *buf) { struct ec_response_flash_info *resp; - struct cros_ec_command msg = { 0 }; + struct cros_ec_command *msg; int ret; struct cros_ec_device *ec = dev_get_drvdata(dev); + msg = kmalloc(sizeof(*msg) + sizeof(*resp), GFP_KERNEL); + if (!msg) + return -ENOMEM; + /* The flash info shouldn't ever change, but ask each time anyway. */ - msg.command = EC_CMD_FLASH_INFO; - msg.insize = sizeof(*resp); - ret = cros_ec_cmd_xfer(ec, &msg); + msg->version = 0; + msg->command = EC_CMD_FLASH_INFO; + msg->insize = sizeof(*resp); + msg->outsize = 0; + ret = cros_ec_cmd_xfer(ec, msg); if (ret < 0) - return ret; - if (msg.result != EC_RES_SUCCESS) - return scnprintf(buf, PAGE_SIZE, - "ERROR: EC returned %d\n", msg.result); - - resp = (struct ec_response_flash_info *)msg.indata; - - return scnprintf(buf, PAGE_SIZE, - "FlashSize %d\nWriteSize %d\n" - "EraseSize %d\nProtectSize %d\n", - resp->flash_size, resp->write_block_size, - resp->erase_block_size, resp->protect_block_size); + 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; + + ret = scnprintf(buf, PAGE_SIZE, + "FlashSize %d\nWriteSize %d\n" + "EraseSize %d\nProtectSize %d\n", + resp->flash_size, resp->write_block_size, + resp->erase_block_size, resp->protect_block_size); +exit: + kfree(msg); + return ret; } /* Module initialization */ -- cgit v1.2.1 From 57b33ff077beebb68481a2b6b8e5fe58ca998169 Mon Sep 17 00:00:00 2001 From: Gwendal Grignou Date: Tue, 9 Jun 2015 13:04:47 +0200 Subject: mfd: cros_ec: Support multiple EC in a system Chromebooks can have more than one Embedded Controller so the cros_ec device id has to be incremented for each EC registered. Add a new structure to represent multiple EC as different char devices (e.g: /dev/cros_ec, /dev/cros_pd). It connects to cros_ec_device and allows sysfs inferface for cros_pd. Also reduce number of allocated objects, make chromeos sysfs class object a static and add refcounting to prevent object deletion while command is in progress. Signed-off-by: Gwendal Grignou Reviewed-by: Dmitry Torokhov Signed-off-by: Javier Martinez Canillas Tested-by: Heiko Stuebner Acked-by: Lee Jones Acked-by: Olof Johansson Signed-off-by: Lee Jones --- drivers/platform/chrome/cros_ec_sysfs.c | 48 +++++++++++++-------------------- 1 file changed, 19 insertions(+), 29 deletions(-) (limited to 'drivers/platform/chrome/cros_ec_sysfs.c') diff --git a/drivers/platform/chrome/cros_ec_sysfs.c b/drivers/platform/chrome/cros_ec_sysfs.c index 78ba82d670cb..f3baf9973989 100644 --- a/drivers/platform/chrome/cros_ec_sysfs.c +++ b/drivers/platform/chrome/cros_ec_sysfs.c @@ -72,7 +72,8 @@ static ssize_t store_ec_reboot(struct device *dev, int got_cmd = 0, offset = 0; int i; int ret; - struct cros_ec_device *ec = dev_get_drvdata(dev); + struct cros_ec_dev *ec = container_of(dev, + struct cros_ec_dev, class_dev); msg = kmalloc(sizeof(*msg) + sizeof(*param), GFP_KERNEL); if (!msg) @@ -112,10 +113,10 @@ static ssize_t store_ec_reboot(struct device *dev, } msg->version = 0; - msg->command = EC_CMD_REBOOT_EC; + msg->command = EC_CMD_REBOOT_EC + ec->cmd_offset; msg->outsize = sizeof(*param); msg->insize = 0; - ret = cros_ec_cmd_xfer(ec, msg); + ret = cros_ec_cmd_xfer(ec->ec_dev, msg); if (ret < 0) { count = ret; goto exit; @@ -139,7 +140,8 @@ static ssize_t show_ec_version(struct device *dev, struct cros_ec_command *msg; int ret; int count = 0; - struct cros_ec_device *ec = dev_get_drvdata(dev); + struct cros_ec_dev *ec = container_of(dev, + struct cros_ec_dev, class_dev); msg = kmalloc(sizeof(*msg) + EC_HOST_PARAM_SIZE, GFP_KERNEL); if (!msg) @@ -147,10 +149,10 @@ static ssize_t show_ec_version(struct device *dev, /* Get versions. RW may change. */ msg->version = 0; - msg->command = EC_CMD_GET_VERSION; + msg->command = EC_CMD_GET_VERSION + ec->cmd_offset; msg->insize = sizeof(*r_ver); msg->outsize = 0; - ret = cros_ec_cmd_xfer(ec, msg); + ret = cros_ec_cmd_xfer(ec->ec_dev, msg); if (ret < 0) { count = ret; goto exit; @@ -175,9 +177,9 @@ static ssize_t show_ec_version(struct device *dev, image_names[r_ver->current_image] : "?")); /* Get build info. */ - msg->command = EC_CMD_GET_BUILD_INFO; + msg->command = EC_CMD_GET_BUILD_INFO + ec->cmd_offset; msg->insize = EC_HOST_PARAM_SIZE; - ret = cros_ec_cmd_xfer(ec, msg); + ret = cros_ec_cmd_xfer(ec->ec_dev, msg); if (ret < 0) count += scnprintf(buf + count, PAGE_SIZE - count, "Build info: XFER ERROR %d\n", ret); @@ -191,9 +193,9 @@ static ssize_t show_ec_version(struct device *dev, } /* Get chip info. */ - msg->command = EC_CMD_GET_CHIP_INFO; + msg->command = EC_CMD_GET_CHIP_INFO + ec->cmd_offset; msg->insize = sizeof(*r_chip); - ret = cros_ec_cmd_xfer(ec, msg); + ret = cros_ec_cmd_xfer(ec->ec_dev, msg); if (ret < 0) count += scnprintf(buf + count, PAGE_SIZE - count, "Chip info: XFER ERROR %d\n", ret); @@ -215,9 +217,9 @@ static ssize_t show_ec_version(struct device *dev, } /* Get board version */ - msg->command = EC_CMD_GET_BOARD_VERSION; + msg->command = EC_CMD_GET_BOARD_VERSION + ec->cmd_offset; msg->insize = sizeof(*r_board); - ret = cros_ec_cmd_xfer(ec, msg); + ret = cros_ec_cmd_xfer(ec->ec_dev, msg); if (ret < 0) count += scnprintf(buf + count, PAGE_SIZE - count, "Board version: XFER ERROR %d\n", ret); @@ -243,7 +245,8 @@ 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_device *ec = dev_get_drvdata(dev); + struct cros_ec_dev *ec = container_of(dev, + struct cros_ec_dev, class_dev); msg = kmalloc(sizeof(*msg) + sizeof(*resp), GFP_KERNEL); if (!msg) @@ -251,10 +254,10 @@ static ssize_t show_ec_flashinfo(struct device *dev, /* The flash info shouldn't ever change, but ask each time anyway. */ msg->version = 0; - msg->command = EC_CMD_FLASH_INFO; + msg->command = EC_CMD_FLASH_INFO + ec->cmd_offset; msg->insize = sizeof(*resp); msg->outsize = 0; - ret = cros_ec_cmd_xfer(ec, msg); + ret = cros_ec_cmd_xfer(ec->ec_dev, msg); if (ret < 0) goto exit; if (msg->result != EC_RES_SUCCESS) { @@ -288,20 +291,7 @@ static struct attribute *__ec_attrs[] = { NULL, }; -static struct attribute_group ec_attr_group = { +struct attribute_group cros_ec_attr_group = { .attrs = __ec_attrs, }; -void ec_dev_sysfs_init(struct cros_ec_device *ec) -{ - int error; - - error = sysfs_create_group(&ec->vdev->kobj, &ec_attr_group); - if (error) - pr_warn("failed to create group: %d\n", error); -} - -void ec_dev_sysfs_remove(struct cros_ec_device *ec) -{ - sysfs_remove_group(&ec->vdev->kobj, &ec_attr_group); -} -- cgit v1.2.1