2015-06-02 08:11:56

by Javier Martinez Canillas

[permalink] [raw]
Subject: [PATCH v4 0/8] mfd: cros_ec: Add multi EC and proto v3 support

Hello,

Newer Chromebooks have more than one Embedded Controller (EC) in the
system. These additional ECs are connected through I2C with a host EC
which is the one that is connected to the Application Processor (AP)
through different transports (I2C, SPI or LPC).

So on these platforms, sub-processors are chained to each other:

AP <--> Host EC <--> Power Delivery (PD) EC

The AP sends commands to the additional EC through the host EC using
a set of passthru commands and the host redirects to the correct EC.

This is a v4 of a series that adds support for multiple EC in a system
and also for the protocol version 3 that is used on newer ECs.

Most patches were taken from the downstream ChromiumOS v3.14 tree with
fixes squashed, split to minimise the cross subsystem churn and changes
for mainline inclusion but were not modified functionality wise.

This version addresses a lot of issues pointed out by Lee Jones on the v3
posted before [0].

The patches are based on top of "[PATCH 0/2] mfd: cros_ec: Small cleanups"
[1] that were posted before and was already picked by Lee Jones.

Testing was done on some Chromebooks that have a single EC and support
protocol v2 such as the Exynos5250 Snow, Exynos5420 Peach Pit and Exynos5800
Peach Pi to be sure that no regressions were introduced for these machines.

The series were tested using a modified ectool [2] that supports the new
cros_ec IOCTL API. They were also tested on a x86 Pixel Chromebook 2 (Samus)
that uses the new protocol v3 and has 2 EC (cros_ec and cros_pd). But for
testing on Samus, also the posted "[PATCH 0/3] platform/chrome: Changes for
cros_ec_lpc and cros_ec_dev" series [3] are needed.

The series is composed of the following patches:

Alexandru M Stan (2):
mfd: cros_ec: spi: Add a DT property to delay asserting the CS
mfd: cros_ec: spi: Add delay for asserting CS

Gwendal Grignou (1):
mfd: cros_ec: Support multiple EC in a system

Javier Martinez Canillas (2):
mfd: cros_ec: Use a zero-length array for command data
mfd: cros_ec: Move protocol helpers out of the MFD driver

Stephen Barber (3):
mfd: cros_ec: rev cros_ec_commands.h
mfd: cros_ec: add proto v3 skeleton
mfd: cros_ec: add bus-specific proto v3 code

Documentation/devicetree/bindings/mfd/cros-ec.txt | 4 +
drivers/i2c/busses/Kconfig | 2 +-
drivers/i2c/busses/i2c-cros-ec-tunnel.c | 45 ++-
drivers/input/keyboard/Kconfig | 2 +-
drivers/input/keyboard/cros_ec_keyb.c | 31 +-
drivers/mfd/Kconfig | 6 +-
drivers/mfd/cros_ec.c | 158 ++++-----
drivers/mfd/cros_ec_i2c.c | 169 ++++++++-
drivers/mfd/cros_ec_spi.c | 407 +++++++++++++++++++---
drivers/platform/chrome/Kconfig | 9 +-
drivers/platform/chrome/Makefile | 1 +
drivers/platform/chrome/cros_ec_dev.c | 189 ++++++----
drivers/platform/chrome/cros_ec_dev.h | 7 -
drivers/platform/chrome/cros_ec_lightbar.c | 217 +++++++-----
drivers/platform/chrome/cros_ec_lpc.c | 84 ++++-
drivers/platform/chrome/cros_ec_proto.c | 382 ++++++++++++++++++++
drivers/platform/chrome/cros_ec_sysfs.c | 178 ++++++----
include/linux/mfd/cros_ec.h | 84 ++++-
include/linux/mfd/cros_ec_commands.h | 277 +++++++++++++--
19 files changed, 1803 insertions(+), 449 deletions(-)
create mode 100644 drivers/platform/chrome/cros_ec_proto.c

Patch #1 modifies the struct cros_ec_command to use a zero-length array for
the buffer used for EC input and output data.

Patch #2 synchronises the cros_ec_commands.h with a newer version of the
file in the EC firmware repository.

Patch #3 moves the EC communication protocol helper functions out of the MFD
driver.

Patch #4 adds the EC host command protocol v3 support to the cros_ec driver
and patch #5 adds the bus specific proto v3 support for I2C, SPI and LPC.

Patch #6 adds support to make multiple EC have a different device id and
also exposing a per EC character device interface.

Patch #7 adds support to the cros_ec_spi driver to specify a delay before
receiving SPI transfers to make sure that the EC has already waked up.

Since the changes are quite intrusive and affects all ChromeOS EC related
drivers, the patches should be merged through the MFD subsystem tree.

Best regards,
Javier

[0]: https://lkml.org/lkml/2015/5/9/73
[1]: https://lkml.org/lkml/2015/5/20/235
[2]: http://cgit.collabora.com/git/user/javier/ec.git/log/?h=mainline-ioctl-zero-length
[3]: https://lkml.org/lkml/2015/5/20/184


2015-06-02 08:12:12

by Javier Martinez Canillas

[permalink] [raw]
Subject: [PATCH v4 1/8] 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 <[email protected]>
Signed-off-by: Javier Martinez Canillas <[email protected]>
Tested-by: Heiko Stuebner <[email protected]>
Acked-by: Lee Jones <[email protected]>
---

Changes since v3:
- Added acked-by tag from Lee Jones.

Changes since v2:
- Set the version field in the cros_ec_command. Reported by Heiko Stuebner.
- Use kmalloc/kfree instead of pre-allocating using variables in the stack.
Suggested by Lee Jones.

Changes since v1:
- Add Heiko Stuebner Tested-by tag
- Removed a new blank line at EOF warning. Reported by Heiko Stuebner
- Use kmalloc instead of kzalloc when the message is later initialized
Suggested by Gwendal Grignou
- Pre-allocate struct cros_ec_command instead of dynamically allocate it
whenever is possible. Suggested by Gwendal Grignou
- Pre-allocate buffers for the usual cases and only allocate dynamically
in the heap for bigger sizes. Suggested by Gwendal Grignou
- Don't access the cros_ec_command received from user-space before doing
a copy_from_user. Suggested by Gwendal Grignou
- Only copy from user-space outsize bytes and copy_to_user insize bytes
Suggested by Gwendal Grignou
- ec_device_ioctl_xcmd() must return the numbers of bytes read and not 0
on success. Suggested by Gwendal Grignou
- Rename alloc_cmd_msg to alloc_lightbar_cmd_msg. Suggested by Gwendal Grignou
---
drivers/i2c/busses/i2c-cros-ec-tunnel.c | 45 ++++++---
drivers/input/keyboard/cros_ec_keyb.c | 29 ++++--
drivers/mfd/cros_ec.c | 28 ++++--
drivers/mfd/cros_ec_i2c.c | 4 +-
drivers/mfd/cros_ec_spi.c | 2 +-
drivers/platform/chrome/cros_ec_dev.c | 65 +++++++-----
drivers/platform/chrome/cros_ec_lightbar.c | 152 +++++++++++++++++++---------
drivers/platform/chrome/cros_ec_lpc.c | 8 +-
drivers/platform/chrome/cros_ec_sysfs.c | 154 ++++++++++++++++++-----------
include/linux/mfd/cros_ec.h | 6 +-
10 files changed, 318 insertions(+), 175 deletions(-)

diff --git a/drivers/i2c/busses/i2c-cros-ec-tunnel.c b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
index fa8dedd8c3a2..a0d95ff682ae 100644
--- a/drivers/i2c/busses/i2c-cros-ec-tunnel.c
+++ b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
@@ -182,8 +182,9 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg i2c_msgs[],
const u16 bus_num = bus->remote_bus;
int request_len;
int response_len;
+ int alloc_size;
int result;
- struct cros_ec_command msg = { };
+ struct cros_ec_command *msg;

request_len = ec_i2c_count_message(i2c_msgs, num);
if (request_len < 0) {
@@ -198,25 +199,39 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg i2c_msgs[],
return response_len;
}

- result = ec_i2c_construct_message(msg.outdata, i2c_msgs, num, bus_num);
- if (result)
- return result;
+ alloc_size = max(request_len, response_len);
+ msg = kmalloc(sizeof(*msg) + alloc_size, GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;

- msg.version = 0;
- msg.command = EC_CMD_I2C_PASSTHRU;
- msg.outsize = request_len;
- msg.insize = response_len;
+ result = ec_i2c_construct_message(msg->data, i2c_msgs, num, bus_num);
+ if (result) {
+ dev_err(dev, "Error constructing EC i2c message %d\n", result);
+ goto exit;
+ }

- result = cros_ec_cmd_xfer(bus->ec, &msg);
- if (result < 0)
- return result;
+ msg->version = 0;
+ msg->command = EC_CMD_I2C_PASSTHRU;
+ msg->outsize = request_len;
+ msg->insize = response_len;

- result = ec_i2c_parse_response(msg.indata, i2c_msgs, &num);
- if (result < 0)
- return result;
+ result = cros_ec_cmd_xfer(bus->ec, msg);
+ if (result < 0) {
+ dev_err(dev, "Error transferring EC i2c message %d\n", result);
+ goto exit;
+ }
+
+ result = ec_i2c_parse_response(msg->data, i2c_msgs, &num);
+ if (result < 0) {
+ dev_err(dev, "Error parsing EC i2c message %d\n", result);
+ goto exit;
+ }

/* Indicate success by saying how many messages were sent */
- return num;
+ result = num;
+exit:
+ kfree(msg);
+ return result;
}

static u32 ec_i2c_functionality(struct i2c_adapter *adap)
diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
index b50c5b8b8a4d..974154a74505 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -148,19 +148,28 @@ static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev,

static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t *kb_state)
{
- int ret;
- struct cros_ec_command msg = {
- .command = EC_CMD_MKBP_STATE,
- .insize = ckdev->cols,
- };
+ int ret = 0;
+ struct cros_ec_command *msg;

- ret = cros_ec_cmd_xfer(ckdev->ec, &msg);
- if (ret < 0)
- return ret;
+ msg = kmalloc(sizeof(*msg) + ckdev->cols, GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;

- memcpy(kb_state, msg.indata, ckdev->cols);
+ msg->version = 0;
+ msg->command = EC_CMD_MKBP_STATE;
+ msg->insize = ckdev->cols;
+ msg->outsize = 0;

- return 0;
+ ret = cros_ec_cmd_xfer(ckdev->ec, msg);
+ if (ret < 0) {
+ dev_err(ckdev->dev, "Error transferring EC message %d\n", ret);
+ goto exit;
+ }
+
+ memcpy(kb_state, msg->data, ckdev->cols);
+exit:
+ kfree(msg);
+ return ret;
}

static irqreturn_t cros_ec_keyb_irq(int irq, void *data)
diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index 1574a9352a6d..4a0f6dfcd376 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -41,7 +41,7 @@ int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
out[2] = msg->outsize;
csum = out[0] + out[1] + out[2];
for (i = 0; i < msg->outsize; i++)
- csum += out[EC_MSG_TX_HEADER_BYTES + i] = msg->outdata[i];
+ csum += out[EC_MSG_TX_HEADER_BYTES + i] = msg->data[i];
out[EC_MSG_TX_HEADER_BYTES + msg->outsize] = (uint8_t)(csum & 0xff);

return EC_MSG_TX_PROTO_BYTES + msg->outsize;
@@ -75,11 +75,20 @@ int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
ret = ec_dev->cmd_xfer(ec_dev, msg);
if (msg->result == EC_RES_IN_PROGRESS) {
int i;
- struct cros_ec_command status_msg = { };
+ struct cros_ec_command *status_msg;
struct ec_response_get_comms_status *status;

- status_msg.command = EC_CMD_GET_COMMS_STATUS;
- status_msg.insize = sizeof(*status);
+ status_msg = kmalloc(sizeof(*status_msg) + sizeof(*status),
+ GFP_KERNEL);
+ if (!status_msg) {
+ ret = -ENOMEM;
+ goto exit;
+ }
+
+ status_msg->version = 0;
+ status_msg->command = EC_CMD_GET_COMMS_STATUS;
+ status_msg->insize = sizeof(*status);
+ status_msg->outsize = 0;

/*
* Query the EC's status until it's no longer busy or
@@ -88,20 +97,23 @@ int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
for (i = 0; i < EC_COMMAND_RETRIES; i++) {
usleep_range(10000, 11000);

- ret = ec_dev->cmd_xfer(ec_dev, &status_msg);
+ ret = ec_dev->cmd_xfer(ec_dev, status_msg);
if (ret < 0)
break;

- msg->result = status_msg.result;
- if (status_msg.result != EC_RES_SUCCESS)
+ msg->result = status_msg->result;
+ if (status_msg->result != EC_RES_SUCCESS)
break;

status = (struct ec_response_get_comms_status *)
- status_msg.indata;
+ status_msg->data;
if (!(status->flags & EC_COMMS_STATUS_PROCESSING))
break;
}
+
+ kfree(status_msg);
}
+exit:
mutex_unlock(&ec_dev->lock);

return ret;
diff --git a/drivers/mfd/cros_ec_i2c.c b/drivers/mfd/cros_ec_i2c.c
index 82b4d6148698..fbf7819f5de5 100644
--- a/drivers/mfd/cros_ec_i2c.c
+++ b/drivers/mfd/cros_ec_i2c.c
@@ -76,7 +76,7 @@ static int cros_ec_cmd_xfer_i2c(struct cros_ec_device *ec_dev,
/* copy message payload and compute checksum */
sum = out_buf[0] + out_buf[1] + out_buf[2];
for (i = 0; i < msg->outsize; i++) {
- out_buf[3 + i] = msg->outdata[i];
+ out_buf[3 + i] = msg->data[i];
sum += out_buf[3 + i];
}
out_buf[3 + msg->outsize] = sum;
@@ -109,7 +109,7 @@ static int cros_ec_cmd_xfer_i2c(struct cros_ec_device *ec_dev,
/* copy response packet payload and compute checksum */
sum = in_buf[0] + in_buf[1];
for (i = 0; i < len; i++) {
- msg->indata[i] = in_buf[2 + i];
+ msg->data[i] = in_buf[2 + i];
sum += in_buf[2 + i];
}
dev_dbg(ec_dev->dev, "packet: %*ph, sum = %02x\n",
diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
index 27bd52e5e8b7..573730fec947 100644
--- a/drivers/mfd/cros_ec_spi.c
+++ b/drivers/mfd/cros_ec_spi.c
@@ -299,7 +299,7 @@ static int cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev,
for (i = 0; i < len; i++) {
sum += ptr[i + 2];
if (ec_msg->insize)
- ec_msg->indata[i] = ptr[i + 2];
+ ec_msg->data[i] = ptr[i + 2];
}
sum &= 0xff;

diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/chrome/cros_ec_dev.c
index 4232c8136939..14111e32658b 100644
--- a/drivers/platform/chrome/cros_ec_dev.c
+++ b/drivers/platform/chrome/cros_ec_dev.c
@@ -20,6 +20,7 @@
#include <linux/fs.h>
#include <linux/module.h>
#include <linux/platform_device.h>
+#include <linux/slab.h>
#include <linux/uaccess.h>

#include "cros_ec_dev.h"
@@ -36,28 +37,31 @@ static int ec_get_version(struct cros_ec_device *ec, char *str, int maxlen)
static const char * const current_image_name[] = {
"unknown", "read-only", "read-write", "invalid",
};
- struct cros_ec_command msg = {
- .version = 0,
- .command = EC_CMD_GET_VERSION,
- .outdata = { 0 },
- .outsize = 0,
- .indata = { 0 },
- .insize = sizeof(*resp),
- };
+ struct cros_ec_command *msg;
int ret;

- ret = cros_ec_cmd_xfer(ec, &msg);
+ msg = kmalloc(sizeof(*msg) + sizeof(*resp), GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
+
+ msg->version = 0;
+ msg->command = EC_CMD_GET_VERSION;
+ msg->insize = sizeof(*resp);
+ msg->outsize = 0;
+
+ ret = cros_ec_cmd_xfer(ec, msg);
if (ret < 0)
- return ret;
+ goto exit;

- if (msg.result != EC_RES_SUCCESS) {
+ if (msg->result != EC_RES_SUCCESS) {
snprintf(str, maxlen,
"%s\nUnknown EC version: EC returned %d\n",
- CROS_EC_DEV_VERSION, msg.result);
- return 0;
+ CROS_EC_DEV_VERSION, msg->result);
+ ret = -EINVAL;
+ goto exit;
}

- resp = (struct ec_response_get_version *)msg.indata;
+ resp = (struct ec_response_get_version *)msg->data;
if (resp->current_image >= ARRAY_SIZE(current_image_name))
resp->current_image = 3; /* invalid */

@@ -65,7 +69,10 @@ static int ec_get_version(struct cros_ec_device *ec, char *str, int maxlen)
resp->version_string_ro, resp->version_string_rw,
current_image_name[resp->current_image]);

- return 0;
+ ret = 0;
+exit:
+ kfree(msg);
+ return ret;
}

/* Device file ops */
@@ -110,20 +117,32 @@ static ssize_t ec_device_read(struct file *filp, char __user *buffer,
static long ec_device_ioctl_xcmd(struct cros_ec_device *ec, void __user *arg)
{
long ret;
- struct cros_ec_command s_cmd = { };
+ struct cros_ec_command u_cmd;
+ struct cros_ec_command *s_cmd;

- if (copy_from_user(&s_cmd, arg, sizeof(s_cmd)))
+ if (copy_from_user(&u_cmd, arg, sizeof(u_cmd)))
return -EFAULT;

- ret = cros_ec_cmd_xfer(ec, &s_cmd);
+ s_cmd = kmalloc(sizeof(*s_cmd) + max(u_cmd.outsize, u_cmd.insize),
+ GFP_KERNEL);
+ if (!s_cmd)
+ return -ENOMEM;
+
+ if (copy_from_user(s_cmd, arg, sizeof(*s_cmd) + u_cmd.outsize)) {
+ ret = -EFAULT;
+ goto exit;
+ }
+
+ ret = cros_ec_cmd_xfer(ec, s_cmd);
/* Only copy data to userland if data was received. */
if (ret < 0)
- return ret;
+ goto exit;

- if (copy_to_user(arg, &s_cmd, sizeof(s_cmd)))
- return -EFAULT;
-
- return 0;
+ if (copy_to_user(arg, s_cmd, sizeof(*s_cmd) + u_cmd.insize))
+ ret = -EFAULT;
+exit:
+ kfree(s_cmd);
+ return ret;
}

static long ec_device_ioctl_readmem(struct cros_ec_device *ec, void __user *arg)
diff --git a/drivers/platform/chrome/cros_ec_lightbar.c b/drivers/platform/chrome/cros_ec_lightbar.c
index b4ff47a9069a..560e5d41b7ae 100644
--- a/drivers/platform/chrome/cros_ec_lightbar.c
+++ b/drivers/platform/chrome/cros_ec_lightbar.c
@@ -31,6 +31,7 @@
#include <linux/sched.h>
#include <linux/types.h>
#include <linux/uaccess.h>
+#include <linux/slab.h>

#include "cros_ec_dev.h"

@@ -91,54 +92,79 @@ out:
return ret;
}

-#define INIT_MSG(P, R) { \
- .command = EC_CMD_LIGHTBAR_CMD, \
- .outsize = sizeof(*P), \
- .insize = sizeof(*R), \
- }
+static struct cros_ec_command *alloc_lightbar_cmd_msg(void)
+{
+ struct cros_ec_command *msg;
+ int len;
+
+ len = max(sizeof(struct ec_params_lightbar),
+ sizeof(struct ec_response_lightbar));
+
+ msg = kmalloc(sizeof(*msg) + len, GFP_KERNEL);
+ if (!msg)
+ return NULL;
+
+ msg->version = 0;
+ msg->command = EC_CMD_LIGHTBAR_CMD;
+ msg->outsize = sizeof(struct ec_params_lightbar);
+ msg->insize = sizeof(struct ec_response_lightbar);
+
+ return msg;
+}

static int get_lightbar_version(struct cros_ec_device *ec,
uint32_t *ver_ptr, uint32_t *flg_ptr)
{
struct ec_params_lightbar *param;
struct ec_response_lightbar *resp;
- struct cros_ec_command msg = INIT_MSG(param, resp);
+ struct cros_ec_command *msg;
int ret;

- param = (struct ec_params_lightbar *)msg.outdata;
- param->cmd = LIGHTBAR_CMD_VERSION;
- ret = cros_ec_cmd_xfer(ec, &msg);
- if (ret < 0)
+ msg = alloc_lightbar_cmd_msg();
+ if (!msg)
return 0;

- switch (msg.result) {
+ param = (struct ec_params_lightbar *)msg->data;
+ param->cmd = LIGHTBAR_CMD_VERSION;
+ ret = cros_ec_cmd_xfer(ec, msg);
+ if (ret < 0) {
+ ret = 0;
+ goto exit;
+ }
+
+ switch (msg->result) {
case EC_RES_INVALID_PARAM:
/* Pixel had no version command. */
if (ver_ptr)
*ver_ptr = 0;
if (flg_ptr)
*flg_ptr = 0;
- return 1;
+ ret = 1;
+ goto exit;

case EC_RES_SUCCESS:
- resp = (struct ec_response_lightbar *)msg.indata;
+ resp = (struct ec_response_lightbar *)msg->data;

/* Future devices w/lightbars should implement this command */
if (ver_ptr)
*ver_ptr = resp->version.num;
if (flg_ptr)
*flg_ptr = resp->version.flags;
- return 1;
+ ret = 1;
+ goto exit;
}

/* Anything else (ie, EC_RES_INVALID_COMMAND) - no lightbar */
- return 0;
+ ret = 0;
+exit:
+ kfree(msg);
+ return ret;
}

static ssize_t version_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
- uint32_t version, flags;
+ uint32_t version = 0, flags = 0;
struct cros_ec_device *ec = dev_get_drvdata(dev);
int ret;

@@ -158,8 +184,7 @@ static ssize_t brightness_store(struct device *dev,
const char *buf, size_t count)
{
struct ec_params_lightbar *param;
- struct ec_response_lightbar *resp;
- struct cros_ec_command msg = INIT_MSG(param, resp);
+ struct cros_ec_command *msg;
int ret;
unsigned int val;
struct cros_ec_device *ec = dev_get_drvdata(dev);
@@ -167,21 +192,30 @@ static ssize_t brightness_store(struct device *dev,
if (kstrtouint(buf, 0, &val))
return -EINVAL;

- param = (struct ec_params_lightbar *)msg.outdata;
+ msg = alloc_lightbar_cmd_msg();
+ if (!msg)
+ return -ENOMEM;
+
+ param = (struct ec_params_lightbar *)msg->data;
param->cmd = LIGHTBAR_CMD_BRIGHTNESS;
param->brightness.num = val;
ret = lb_throttle();
if (ret)
- return ret;
+ goto exit;

- ret = cros_ec_cmd_xfer(ec, &msg);
+ ret = cros_ec_cmd_xfer(ec, msg);
if (ret < 0)
- return ret;
+ goto exit;

- if (msg.result != EC_RES_SUCCESS)
- return -EINVAL;
+ if (msg->result != EC_RES_SUCCESS) {
+ ret = -EINVAL;
+ goto exit;
+ }

- return count;
+ ret = count;
+exit:
+ kfree(msg);
+ return ret;
}


@@ -196,12 +230,15 @@ static ssize_t led_rgb_store(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
struct ec_params_lightbar *param;
- struct ec_response_lightbar *resp;
- struct cros_ec_command msg = INIT_MSG(param, resp);
+ struct cros_ec_command *msg;
struct cros_ec_device *ec = dev_get_drvdata(dev);
unsigned int val[4];
int ret, i = 0, j = 0, ok = 0;

+ msg = alloc_lightbar_cmd_msg();
+ if (!msg)
+ return -ENOMEM;
+
do {
/* Skip any whitespace */
while (*buf && isspace(*buf))
@@ -215,7 +252,7 @@ static ssize_t led_rgb_store(struct device *dev, struct device_attribute *attr,
return -EINVAL;

if (i == 4) {
- param = (struct ec_params_lightbar *)msg.outdata;
+ param = (struct ec_params_lightbar *)msg->data;
param->cmd = LIGHTBAR_CMD_RGB;
param->rgb.led = val[0];
param->rgb.red = val[1];
@@ -231,12 +268,14 @@ static ssize_t led_rgb_store(struct device *dev, struct device_attribute *attr,
return ret;
}

- ret = cros_ec_cmd_xfer(ec, &msg);
+ ret = cros_ec_cmd_xfer(ec, msg);
if (ret < 0)
- return ret;
+ goto exit;

- if (msg.result != EC_RES_SUCCESS)
- return -EINVAL;
+ if (msg->result != EC_RES_SUCCESS) {
+ ret = -EINVAL;
+ goto exit;
+ }

i = 0;
ok = 1;
@@ -248,6 +287,8 @@ static ssize_t led_rgb_store(struct device *dev, struct device_attribute *attr,

} while (*buf);

+exit:
+ kfree(msg);
return (ok && i == 0) ? count : -EINVAL;
}

@@ -261,42 +302,55 @@ static ssize_t sequence_show(struct device *dev,
{
struct ec_params_lightbar *param;
struct ec_response_lightbar *resp;
- struct cros_ec_command msg = INIT_MSG(param, resp);
+ struct cros_ec_command *msg;
int ret;
struct cros_ec_device *ec = dev_get_drvdata(dev);

- param = (struct ec_params_lightbar *)msg.outdata;
+ msg = alloc_lightbar_cmd_msg();
+ if (!msg)
+ return -ENOMEM;
+
+ param = (struct ec_params_lightbar *)msg->data;
param->cmd = LIGHTBAR_CMD_GET_SEQ;
ret = lb_throttle();
if (ret)
- return ret;
+ goto exit;

- ret = cros_ec_cmd_xfer(ec, &msg);
+ ret = cros_ec_cmd_xfer(ec, msg);
if (ret < 0)
- return ret;
+ goto exit;

- if (msg.result != EC_RES_SUCCESS)
- return scnprintf(buf, PAGE_SIZE,
- "ERROR: EC returned %d\n", msg.result);
+ if (msg->result != EC_RES_SUCCESS) {
+ ret = scnprintf(buf, PAGE_SIZE,
+ "ERROR: EC returned %d\n", msg->result);
+ goto exit;
+ }

- resp = (struct ec_response_lightbar *)msg.indata;
+ resp = (struct ec_response_lightbar *)msg->data;
if (resp->get_seq.num >= ARRAY_SIZE(seqname))
- return scnprintf(buf, PAGE_SIZE, "%d\n", resp->get_seq.num);
+ ret = scnprintf(buf, PAGE_SIZE, "%d\n", resp->get_seq.num);
else
- return scnprintf(buf, PAGE_SIZE, "%s\n",
- seqname[resp->get_seq.num]);
+ ret = scnprintf(buf, PAGE_SIZE, "%s\n",
+ seqname[resp->get_seq.num]);
+
+exit:
+ kfree(msg);
+ return ret;
}

static ssize_t sequence_store(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
struct ec_params_lightbar *param;
- struct ec_response_lightbar *resp;
- struct cros_ec_command msg = INIT_MSG(param, resp);
+ struct cros_ec_command *msg;
unsigned int num;
int ret, len;
struct cros_ec_device *ec = dev_get_drvdata(dev);

+ msg = alloc_lightbar_cmd_msg();
+ if (!msg)
+ return -ENOMEM;
+
for (len = 0; len < count; len++)
if (!isalnum(buf[len]))
break;
@@ -311,18 +365,18 @@ static ssize_t sequence_store(struct device *dev, struct device_attribute *attr,
return ret;
}

- param = (struct ec_params_lightbar *)msg.outdata;
+ param = (struct ec_params_lightbar *)msg->data;
param->cmd = LIGHTBAR_CMD_SEQ;
param->seq.num = num;
ret = lb_throttle();
if (ret)
return ret;

- ret = cros_ec_cmd_xfer(ec, &msg);
+ ret = cros_ec_cmd_xfer(ec, msg);
if (ret < 0)
return ret;

- if (msg.result != EC_RES_SUCCESS)
+ if (msg->result != EC_RES_SUCCESS)
return -EINVAL;

return count;
diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index ea75af35c995..214ae7fef984 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -73,8 +73,8 @@ static int cros_ec_cmd_xfer_lpc(struct cros_ec_device *ec,

/* Copy data and update checksum */
for (i = 0; i < msg->outsize; i++) {
- outb(msg->outdata[i], EC_LPC_ADDR_HOST_PARAM + i);
- csum += msg->outdata[i];
+ outb(msg->data[i], EC_LPC_ADDR_HOST_PARAM + i);
+ csum += msg->data[i];
}

/* Finalize checksum and write args */
@@ -119,8 +119,8 @@ static int cros_ec_cmd_xfer_lpc(struct cros_ec_device *ec,

/* Read response and update checksum */
for (i = 0; i < args.data_size; i++) {
- msg->indata[i] = inb(EC_LPC_ADDR_HOST_PARAM + i);
- csum += msg->indata[i];
+ msg->data[i] = inb(EC_LPC_ADDR_HOST_PARAM + i);
+ csum += msg->data[i];
}

/* Verify checksum */
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 <linux/module.h>
#include <linux/platform_device.h>
#include <linux/printk.h>
+#include <linux/slab.h>
#include <linux/stat.h>
#include <linux/types.h>
#include <linux/uaccess.h>
@@ -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 */
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index 14cf522123dd..7eee38abd02a 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -42,8 +42,7 @@ enum {
* @outsize: Outgoing length in bytes
* @insize: Max number of bytes to accept from EC
* @result: EC's response to the command (separate from communication failure)
- * @outdata: Outgoing data to EC
- * @indata: Where to put the incoming data from EC
+ * @data: Where to put the incoming data from EC and outgoing data to EC
*/
struct cros_ec_command {
uint32_t version;
@@ -51,8 +50,7 @@ struct cros_ec_command {
uint32_t outsize;
uint32_t insize;
uint32_t result;
- uint8_t outdata[EC_PROTO2_MAX_PARAM_SIZE];
- uint8_t indata[EC_PROTO2_MAX_PARAM_SIZE];
+ uint8_t data[0];
};

/**
--
2.1.4

2015-06-02 08:12:49

by Javier Martinez Canillas

[permalink] [raw]
Subject: [PATCH v4 2/8] mfd: cros_ec: rev cros_ec_commands.h

From: Stephen Barber <[email protected]>

Update cros_ec_commands.h to the latest version in the EC
firmware sources and add power domain and passthru commands.

Also, update lightbar to use new command names.

Signed-off-by: Stephen Barber <[email protected]>
Reviewed-by: Randall Spangler <[email protected]>
Signed-off-by: Javier Martinez Canillas <[email protected]>
Tested-by: Heiko Stuebner <[email protected]>
Reviewed-by: Gwendal Grignou <[email protected]>
Tested-by: Gwendal Grignou <[email protected]>
Acked-by: Lee Jones <[email protected]>
---

Changes since v3: None

Changes since v2: None

Changes since v1:
- Added Gwendal Grignou and Heiko Stuebner Tested-by tags
- Added Gwendal Grignou Reviewed-by tag
- Added Lee Jones Acked-by tag
---
drivers/platform/chrome/cros_ec_lightbar.c | 14 +-
include/linux/mfd/cros_ec_commands.h | 277 ++++++++++++++++++++++++++---
2 files changed, 262 insertions(+), 29 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_lightbar.c b/drivers/platform/chrome/cros_ec_lightbar.c
index 560e5d41b7ae..6e1986a2dce1 100644
--- a/drivers/platform/chrome/cros_ec_lightbar.c
+++ b/drivers/platform/chrome/cros_ec_lightbar.c
@@ -197,8 +197,8 @@ static ssize_t brightness_store(struct device *dev,
return -ENOMEM;

param = (struct ec_params_lightbar *)msg->data;
- param->cmd = LIGHTBAR_CMD_BRIGHTNESS;
- param->brightness.num = val;
+ param->cmd = LIGHTBAR_CMD_SET_BRIGHTNESS;
+ param->set_brightness.num = val;
ret = lb_throttle();
if (ret)
goto exit;
@@ -253,11 +253,11 @@ static ssize_t led_rgb_store(struct device *dev, struct device_attribute *attr,

if (i == 4) {
param = (struct ec_params_lightbar *)msg->data;
- param->cmd = LIGHTBAR_CMD_RGB;
- param->rgb.led = val[0];
- param->rgb.red = val[1];
- param->rgb.green = val[2];
- param->rgb.blue = val[3];
+ param->cmd = LIGHTBAR_CMD_SET_RGB;
+ param->set_rgb.led = val[0];
+ param->set_rgb.red = val[1];
+ param->set_rgb.green = val[2];
+ param->set_rgb.blue = val[3];
/*
* Throttle only the first of every four transactions,
* so that the user can update all four LEDs at once.
diff --git a/include/linux/mfd/cros_ec_commands.h b/include/linux/mfd/cros_ec_commands.h
index a49cd41feea7..13b630c10d4c 100644
--- a/include/linux/mfd/cros_ec_commands.h
+++ b/include/linux/mfd/cros_ec_commands.h
@@ -515,7 +515,7 @@ struct ec_host_response {
/*
* Notes on commands:
*
- * Each command is an 8-byte command value. Commands which take params or
+ * Each command is an 16-bit command value. Commands which take params or
* return response data specify structs for that data. If no struct is
* specified, the command does not input or output data, respectively.
* Parameter/response length is implicit in the structs. Some underlying
@@ -966,7 +966,7 @@ struct rgb_s {
/* List of tweakable parameters. NOTE: It's __packed so it can be sent in a
* host command, but the alignment is the same regardless. Keep it that way.
*/
-struct lightbar_params {
+struct lightbar_params_v0 {
/* Timing */
int32_t google_ramp_up;
int32_t google_ramp_down;
@@ -1000,32 +1000,81 @@ struct lightbar_params {
struct rgb_s color[8]; /* 0-3 are Google colors */
} __packed;

+struct lightbar_params_v1 {
+ /* Timing */
+ int32_t google_ramp_up;
+ int32_t google_ramp_down;
+ int32_t s3s0_ramp_up;
+ int32_t s0_tick_delay[2]; /* AC=0/1 */
+ int32_t s0a_tick_delay[2]; /* AC=0/1 */
+ int32_t s0s3_ramp_down;
+ int32_t s3_sleep_for;
+ int32_t s3_ramp_up;
+ int32_t s3_ramp_down;
+ int32_t tap_tick_delay;
+ int32_t tap_display_time;
+
+ /* Tap-for-battery params */
+ uint8_t tap_pct_red;
+ uint8_t tap_pct_green;
+ uint8_t tap_seg_min_on;
+ uint8_t tap_seg_max_on;
+ uint8_t tap_seg_osc;
+ uint8_t tap_idx[3];
+
+ /* Oscillation */
+ uint8_t osc_min[2]; /* AC=0/1 */
+ uint8_t osc_max[2]; /* AC=0/1 */
+ uint8_t w_ofs[2]; /* AC=0/1 */
+
+ /* Brightness limits based on the backlight and AC. */
+ uint8_t bright_bl_off_fixed[2]; /* AC=0/1 */
+ uint8_t bright_bl_on_min[2]; /* AC=0/1 */
+ uint8_t bright_bl_on_max[2]; /* AC=0/1 */
+
+ /* Battery level thresholds */
+ uint8_t battery_threshold[LB_BATTERY_LEVELS - 1];
+
+ /* Map [AC][battery_level] to color index */
+ uint8_t s0_idx[2][LB_BATTERY_LEVELS]; /* AP is running */
+ uint8_t s3_idx[2][LB_BATTERY_LEVELS]; /* AP is sleeping */
+
+ /* Color palette */
+ struct rgb_s color[8]; /* 0-3 are Google colors */
+} __packed;
+
struct ec_params_lightbar {
uint8_t cmd; /* Command (see enum lightbar_command) */
union {
struct {
/* no args */
- } dump, off, on, init, get_seq, get_params, version;
+ } dump, off, on, init, get_seq, get_params_v0, get_params_v1,
+ version, get_brightness, get_demo;

- struct num {
+ struct {
uint8_t num;
- } brightness, seq, demo;
+ } set_brightness, seq, demo;

- struct reg {
+ struct {
uint8_t ctrl, reg, value;
} reg;

- struct rgb {
+ struct {
uint8_t led, red, green, blue;
- } rgb;
+ } set_rgb;
+
+ struct {
+ uint8_t led;
+ } get_rgb;

- struct lightbar_params set_params;
+ struct lightbar_params_v0 set_params_v0;
+ struct lightbar_params_v1 set_params_v1;
};
} __packed;

struct ec_response_lightbar {
union {
- struct dump {
+ struct {
struct {
uint8_t reg;
uint8_t ic0;
@@ -1033,20 +1082,26 @@ struct ec_response_lightbar {
} vals[23];
} dump;

- struct get_seq {
+ struct {
uint8_t num;
- } get_seq;
+ } get_seq, get_brightness, get_demo;

- struct lightbar_params get_params;
+ struct lightbar_params_v0 get_params_v0;
+ struct lightbar_params_v1 get_params_v1;

- struct version {
+ struct {
uint32_t num;
uint32_t flags;
} version;

struct {
+ uint8_t red, green, blue;
+ } get_rgb;
+
+ struct {
/* no return params */
- } off, on, init, brightness, seq, reg, rgb, demo, set_params;
+ } off, on, init, set_brightness, seq, reg, set_rgb,
+ demo, set_params_v0, set_params_v1;
};
} __packed;

@@ -1056,15 +1111,20 @@ enum lightbar_command {
LIGHTBAR_CMD_OFF = 1,
LIGHTBAR_CMD_ON = 2,
LIGHTBAR_CMD_INIT = 3,
- LIGHTBAR_CMD_BRIGHTNESS = 4,
+ LIGHTBAR_CMD_SET_BRIGHTNESS = 4,
LIGHTBAR_CMD_SEQ = 5,
LIGHTBAR_CMD_REG = 6,
- LIGHTBAR_CMD_RGB = 7,
+ LIGHTBAR_CMD_SET_RGB = 7,
LIGHTBAR_CMD_GET_SEQ = 8,
LIGHTBAR_CMD_DEMO = 9,
- LIGHTBAR_CMD_GET_PARAMS = 10,
- LIGHTBAR_CMD_SET_PARAMS = 11,
+ LIGHTBAR_CMD_GET_PARAMS_V0 = 10,
+ LIGHTBAR_CMD_SET_PARAMS_V0 = 11,
LIGHTBAR_CMD_VERSION = 12,
+ LIGHTBAR_CMD_GET_BRIGHTNESS = 13,
+ LIGHTBAR_CMD_GET_RGB = 14,
+ LIGHTBAR_CMD_GET_DEMO = 15,
+ LIGHTBAR_CMD_GET_PARAMS_V1 = 16,
+ LIGHTBAR_CMD_SET_PARAMS_V1 = 17,
LIGHTBAR_NUM_CMDS
};

@@ -1421,8 +1481,40 @@ struct ec_response_rtc {
/*****************************************************************************/
/* Port80 log access */

+/* Maximum entries that can be read/written in a single command */
+#define EC_PORT80_SIZE_MAX 32
+
/* Get last port80 code from previous boot */
#define EC_CMD_PORT80_LAST_BOOT 0x48
+#define EC_CMD_PORT80_READ 0x48
+
+enum ec_port80_subcmd {
+ EC_PORT80_GET_INFO = 0,
+ EC_PORT80_READ_BUFFER,
+};
+
+struct ec_params_port80_read {
+ uint16_t subcmd;
+ union {
+ struct {
+ uint32_t offset;
+ uint32_t num_entries;
+ } read_buffer;
+ };
+} __packed;
+
+struct ec_response_port80_read {
+ union {
+ struct {
+ uint32_t writes;
+ uint32_t history_size;
+ uint32_t last_boot;
+ } get_info;
+ struct {
+ uint16_t codes[EC_PORT80_SIZE_MAX];
+ } data;
+ };
+} __packed;

struct ec_response_port80_last_boot {
uint16_t code;
@@ -1782,6 +1874,7 @@ struct ec_params_gpio_set {
/* Get GPIO value */
#define EC_CMD_GPIO_GET 0x93

+/* Version 0 of input params and response */
struct ec_params_gpio_get {
char name[32];
} __packed;
@@ -1789,6 +1882,38 @@ struct ec_response_gpio_get {
uint8_t val;
} __packed;

+/* Version 1 of input params and response */
+struct ec_params_gpio_get_v1 {
+ uint8_t subcmd;
+ union {
+ struct {
+ char name[32];
+ } get_value_by_name;
+ struct {
+ uint8_t index;
+ } get_info;
+ };
+} __packed;
+
+struct ec_response_gpio_get_v1 {
+ union {
+ struct {
+ uint8_t val;
+ } get_value_by_name, get_count;
+ struct {
+ uint8_t val;
+ char name[32];
+ uint32_t flags;
+ } get_info;
+ };
+} __packed;
+
+enum gpio_get_subcmd {
+ EC_GPIO_GET_BY_NAME = 0,
+ EC_GPIO_GET_COUNT = 1,
+ EC_GPIO_GET_INFO = 2,
+};
+
/*****************************************************************************/
/* I2C commands. Only available when flash write protect is unlocked. */

@@ -1857,13 +1982,21 @@ struct ec_params_charge_control {
/*****************************************************************************/

/*
- * Cut off battery power output if the battery supports.
+ * Cut off battery power immediately or after the host has shut down.
*
- * For unsupported battery, just don't implement this command and lets EC
- * return EC_RES_INVALID_COMMAND.
+ * return EC_RES_INVALID_COMMAND if unsupported by a board/battery.
+ * EC_RES_SUCCESS if the command was successful.
+ * EC_RES_ERROR if the cut off command failed.
*/
+
#define EC_CMD_BATTERY_CUT_OFF 0x99

+#define EC_BATTERY_CUTOFF_FLAG_AT_SHUTDOWN (1 << 0)
+
+struct ec_params_battery_cutoff {
+ uint8_t flags;
+} __packed;
+
/*****************************************************************************/
/* USB port mux control. */

@@ -2142,6 +2275,32 @@ struct ec_params_sb_wr_block {
} __packed;

/*****************************************************************************/
+/* Battery vendor parameters
+ *
+ * Get or set vendor-specific parameters in the battery. Implementations may
+ * differ between boards or batteries. On a set operation, the response
+ * contains the actual value set, which may be rounded or clipped from the
+ * requested value.
+ */
+
+#define EC_CMD_BATTERY_VENDOR_PARAM 0xb4
+
+enum ec_battery_vendor_param_mode {
+ BATTERY_VENDOR_PARAM_MODE_GET = 0,
+ BATTERY_VENDOR_PARAM_MODE_SET,
+};
+
+struct ec_params_battery_vendor_param {
+ uint32_t param;
+ uint32_t value;
+ uint8_t mode;
+} __packed;
+
+struct ec_response_battery_vendor_param {
+ uint32_t value;
+} __packed;
+
+/*****************************************************************************/
/* System commands */

/*
@@ -2338,6 +2497,80 @@ struct ec_params_reboot_ec {

/*****************************************************************************/
/*
+ * PD commands
+ *
+ * These commands are for PD MCU communication.
+ */
+
+/* EC to PD MCU exchange status command */
+#define EC_CMD_PD_EXCHANGE_STATUS 0x100
+
+/* Status of EC being sent to PD */
+struct ec_params_pd_status {
+ int8_t batt_soc; /* battery state of charge */
+} __packed;
+
+/* Status of PD being sent back to EC */
+struct ec_response_pd_status {
+ int8_t status; /* PD MCU status */
+ uint32_t curr_lim_ma; /* input current limit */
+} __packed;
+
+/* Set USB type-C port role and muxes */
+#define EC_CMD_USB_PD_CONTROL 0x101
+
+enum usb_pd_control_role {
+ USB_PD_CTRL_ROLE_NO_CHANGE = 0,
+ USB_PD_CTRL_ROLE_TOGGLE_ON = 1, /* == AUTO */
+ USB_PD_CTRL_ROLE_TOGGLE_OFF = 2,
+ USB_PD_CTRL_ROLE_FORCE_SINK = 3,
+ USB_PD_CTRL_ROLE_FORCE_SOURCE = 4,
+};
+
+enum usb_pd_control_mux {
+ USB_PD_CTRL_MUX_NO_CHANGE = 0,
+ USB_PD_CTRL_MUX_NONE = 1,
+ USB_PD_CTRL_MUX_USB = 2,
+ USB_PD_CTRL_MUX_DP = 3,
+ USB_PD_CTRL_MUX_DOCK = 4,
+ USB_PD_CTRL_MUX_AUTO = 5,
+};
+
+struct ec_params_usb_pd_control {
+ uint8_t port;
+ uint8_t role;
+ uint8_t mux;
+} __packed;
+
+/*****************************************************************************/
+/*
+ * Passthru commands
+ *
+ * Some platforms have sub-processors chained to each other. For example.
+ *
+ * AP <--> EC <--> PD MCU
+ *
+ * The top 2 bits of the command number are used to indicate which device the
+ * command is intended for. Device 0 is always the device receiving the
+ * command; other device mapping is board-specific.
+ *
+ * When a device receives a command to be passed to a sub-processor, it passes
+ * it on with the device number set back to 0. This allows the sub-processor
+ * to remain blissfully unaware of whether the command originated on the next
+ * device up the chain, or was passed through from the AP.
+ *
+ * In the above example, if the AP wants to send command 0x0002 to the PD MCU,
+ * AP sends command 0x4002 to the EC
+ * EC sends command 0x0002 to the PD MCU
+ * EC forwards PD MCU response back to the AP
+ */
+
+/* Offset and max command number for sub-device n */
+#define EC_CMD_PASSTHRU_OFFSET(n) (0x4000 * (n))
+#define EC_CMD_PASSTHRU_MAX(n) (EC_CMD_PASSTHRU_OFFSET(n) + 0x3fff)
+
+/*****************************************************************************/
+/*
* Deprecated constants. These constants have been renamed for clarity. The
* meaning and size has not changed. Programs that use the old names should
* switch to the new names soon, as the old names may not be carried forward
--
2.1.4

2015-06-02 08:12:42

by Javier Martinez Canillas

[permalink] [raw]
Subject: [PATCH v4 3/8] mfd: cros_ec: Move protocol helpers out of the MFD driver

The MFD driver should only have the logic to instantiate its child devices
and setup any shared resources that will be used by the subdevices drivers.

The cros_ec MFD is more complex than expected since it also has helpers to
communicate with the EC. So the driver will only get more bigger as other
protocols are supported in the future. So move the communication protocol
helpers to its own driver as drivers/platform/chrome/cros_ec_proto.c.

Suggested-by: Lee Jones <[email protected]>
Signed-off-by: Javier Martinez Canillas <[email protected]>
Tested-by: Heiko Stuebner <[email protected]>
Acked-by: Lee Jones <[email protected]>
---

Changes since v3:
- Added tested-by tag from Heiko Stuebner.
- Added acked-by tag from Lee Jones.

Changes since v2: None, new patch.
---
drivers/i2c/busses/Kconfig | 2 +-
drivers/input/keyboard/Kconfig | 2 +-
drivers/mfd/Kconfig | 6 +-
drivers/mfd/cros_ec.c | 96 --------------------------
drivers/platform/chrome/Kconfig | 9 ++-
drivers/platform/chrome/Makefile | 1 +
drivers/platform/chrome/cros_ec_proto.c | 115 ++++++++++++++++++++++++++++++++
7 files changed, 129 insertions(+), 102 deletions(-)
create mode 100644 drivers/platform/chrome/cros_ec_proto.c

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 2255af23b9c7..5f1c1c4f5d87 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -1103,7 +1103,7 @@ config I2C_SIBYTE

config I2C_CROS_EC_TUNNEL
tristate "ChromeOS EC tunnel I2C bus"
- depends on MFD_CROS_EC
+ depends on CROS_EC_PROTO
help
If you say yes here you get an I2C bus that will tunnel i2c commands
through to the other side of the ChromeOS EC to the i2c bus
diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index 106fbac7f8c5..e8eb60c6d83e 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -677,7 +677,7 @@ config KEYBOARD_W90P910
config KEYBOARD_CROS_EC
tristate "ChromeOS EC keyboard"
select INPUT_MATRIXKMAP
- depends on MFD_CROS_EC
+ depends on CROS_EC_PROTO
help
Say Y here to enable the matrix keyboard used by ChromeOS devices
and implemented on the ChromeOS EC. You must enable one bus option
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index d5ad04dad081..cf3b86d441f7 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -94,6 +94,8 @@ config MFD_AXP20X
config MFD_CROS_EC
tristate "ChromeOS Embedded Controller"
select MFD_CORE
+ select CHROME_PLATFORMS
+ select CROS_EC_PROTO
help
If you say Y here you get support for the ChromeOS Embedded
Controller (EC) providing keyboard, battery and power services.
@@ -102,7 +104,7 @@ config MFD_CROS_EC

config MFD_CROS_EC_I2C
tristate "ChromeOS Embedded Controller (I2C)"
- depends on MFD_CROS_EC && I2C
+ depends on MFD_CROS_EC && CROS_EC_PROTO && I2C

help
If you say Y here, you get support for talking to the ChromeOS
@@ -112,7 +114,7 @@ config MFD_CROS_EC_I2C

config MFD_CROS_EC_SPI
tristate "ChromeOS Embedded Controller (SPI)"
- depends on MFD_CROS_EC && SPI && OF
+ depends on MFD_CROS_EC && CROS_EC_PROTO && SPI && OF

---help---
If you say Y here, you get support for talking to the ChromeOS EC
diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index 4a0f6dfcd376..d857f6a2b57b 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -23,102 +23,6 @@
#include <linux/module.h>
#include <linux/mfd/core.h>
#include <linux/mfd/cros_ec.h>
-#include <linux/mfd/cros_ec_commands.h>
-#include <linux/delay.h>
-
-#define EC_COMMAND_RETRIES 50
-
-int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
- struct cros_ec_command *msg)
-{
- uint8_t *out;
- int csum, i;
-
- BUG_ON(msg->outsize > EC_PROTO2_MAX_PARAM_SIZE);
- out = ec_dev->dout;
- out[0] = EC_CMD_VERSION0 + msg->version;
- out[1] = msg->command;
- out[2] = msg->outsize;
- csum = out[0] + out[1] + out[2];
- for (i = 0; i < msg->outsize; i++)
- csum += out[EC_MSG_TX_HEADER_BYTES + i] = msg->data[i];
- out[EC_MSG_TX_HEADER_BYTES + msg->outsize] = (uint8_t)(csum & 0xff);
-
- return EC_MSG_TX_PROTO_BYTES + msg->outsize;
-}
-EXPORT_SYMBOL(cros_ec_prepare_tx);
-
-int cros_ec_check_result(struct cros_ec_device *ec_dev,
- struct cros_ec_command *msg)
-{
- switch (msg->result) {
- case EC_RES_SUCCESS:
- return 0;
- case EC_RES_IN_PROGRESS:
- dev_dbg(ec_dev->dev, "command 0x%02x in progress\n",
- msg->command);
- return -EAGAIN;
- default:
- dev_dbg(ec_dev->dev, "command 0x%02x returned %d\n",
- msg->command, msg->result);
- return 0;
- }
-}
-EXPORT_SYMBOL(cros_ec_check_result);
-
-int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
- struct cros_ec_command *msg)
-{
- int ret;
-
- mutex_lock(&ec_dev->lock);
- ret = ec_dev->cmd_xfer(ec_dev, msg);
- if (msg->result == EC_RES_IN_PROGRESS) {
- int i;
- struct cros_ec_command *status_msg;
- struct ec_response_get_comms_status *status;
-
- status_msg = kmalloc(sizeof(*status_msg) + sizeof(*status),
- GFP_KERNEL);
- if (!status_msg) {
- ret = -ENOMEM;
- goto exit;
- }
-
- status_msg->version = 0;
- status_msg->command = EC_CMD_GET_COMMS_STATUS;
- status_msg->insize = sizeof(*status);
- status_msg->outsize = 0;
-
- /*
- * Query the EC's status until it's no longer busy or
- * we encounter an error.
- */
- for (i = 0; i < EC_COMMAND_RETRIES; i++) {
- usleep_range(10000, 11000);
-
- ret = ec_dev->cmd_xfer(ec_dev, status_msg);
- if (ret < 0)
- break;
-
- msg->result = status_msg->result;
- if (status_msg->result != EC_RES_SUCCESS)
- break;
-
- status = (struct ec_response_get_comms_status *)
- status_msg->data;
- if (!(status->flags & EC_COMMS_STATUS_PROCESSING))
- break;
- }
-
- kfree(status_msg);
- }
-exit:
- mutex_unlock(&ec_dev->lock);
-
- return ret;
-}
-EXPORT_SYMBOL(cros_ec_cmd_xfer);

static const struct mfd_cell cros_devs[] = {
{
diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
index 2a6531a5fde8..cb1329919527 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -40,7 +40,7 @@ config CHROMEOS_PSTORE

config CROS_EC_CHARDEV
tristate "Chrome OS Embedded Controller userspace device interface"
- depends on MFD_CROS_EC
+ depends on CROS_EC_PROTO
---help---
This driver adds support to talk with the ChromeOS EC from userspace.

@@ -49,7 +49,7 @@ config CROS_EC_CHARDEV

config CROS_EC_LPC
tristate "ChromeOS Embedded Controller (LPC)"
- depends on MFD_CROS_EC && (X86 || COMPILE_TEST)
+ depends on MFD_CROS_EC && CROS_EC_PROTO && (X86 || COMPILE_TEST)
help
If you say Y here, you get support for talking to the ChromeOS EC
over an LPC bus. This uses a simple byte-level protocol with a
@@ -59,4 +59,9 @@ config CROS_EC_LPC
To compile this driver as a module, choose M here: the
module will be called cros_ec_lpc.

+config CROS_EC_PROTO
+ bool
+ help
+ ChromeOS EC communication protocol helpers.
+
endif # CHROMEOS_PLATFORMS
diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
index bd8d8601e875..4a11b010f5d8 100644
--- a/drivers/platform/chrome/Makefile
+++ b/drivers/platform/chrome/Makefile
@@ -4,3 +4,4 @@ obj-$(CONFIG_CHROMEOS_PSTORE) += chromeos_pstore.o
cros_ec_devs-objs := cros_ec_dev.o cros_ec_sysfs.o cros_ec_lightbar.o
obj-$(CONFIG_CROS_EC_CHARDEV) += cros_ec_devs.o
obj-$(CONFIG_CROS_EC_LPC) += cros_ec_lpc.o
+obj-$(CONFIG_CROS_EC_PROTO) += cros_ec_proto.o
diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
new file mode 100644
index 000000000000..58e98a24fd08
--- /dev/null
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -0,0 +1,115 @@
+/*
+ * ChromeOS EC communication protocol helper functions
+ *
+ * Copyright (C) 2015 Google, Inc
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/mfd/cros_ec.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+
+#define EC_COMMAND_RETRIES 50
+
+int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
+ struct cros_ec_command *msg)
+{
+ uint8_t *out;
+ int csum, i;
+
+ BUG_ON(msg->outsize > EC_PROTO2_MAX_PARAM_SIZE);
+ out = ec_dev->dout;
+ out[0] = EC_CMD_VERSION0 + msg->version;
+ out[1] = msg->command;
+ out[2] = msg->outsize;
+ csum = out[0] + out[1] + out[2];
+ for (i = 0; i < msg->outsize; i++)
+ csum += out[EC_MSG_TX_HEADER_BYTES + i] = msg->data[i];
+ out[EC_MSG_TX_HEADER_BYTES + msg->outsize] = (uint8_t)(csum & 0xff);
+
+ return EC_MSG_TX_PROTO_BYTES + msg->outsize;
+}
+EXPORT_SYMBOL(cros_ec_prepare_tx);
+
+int cros_ec_check_result(struct cros_ec_device *ec_dev,
+ struct cros_ec_command *msg)
+{
+ switch (msg->result) {
+ case EC_RES_SUCCESS:
+ return 0;
+ case EC_RES_IN_PROGRESS:
+ dev_dbg(ec_dev->dev, "command 0x%02x in progress\n",
+ msg->command);
+ return -EAGAIN;
+ default:
+ dev_dbg(ec_dev->dev, "command 0x%02x returned %d\n",
+ msg->command, msg->result);
+ return 0;
+ }
+}
+EXPORT_SYMBOL(cros_ec_check_result);
+
+int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
+ struct cros_ec_command *msg)
+{
+ int ret;
+
+ mutex_lock(&ec_dev->lock);
+ ret = ec_dev->cmd_xfer(ec_dev, msg);
+ if (msg->result == EC_RES_IN_PROGRESS) {
+ int i;
+ struct cros_ec_command *status_msg;
+ struct ec_response_get_comms_status *status;
+
+ status_msg = kmalloc(sizeof(*status_msg) + sizeof(*status),
+ GFP_KERNEL);
+ if (!status_msg) {
+ ret = -ENOMEM;
+ goto exit;
+ }
+
+ status_msg->version = 0;
+ status_msg->command = EC_CMD_GET_COMMS_STATUS;
+ status_msg->insize = sizeof(*status);
+ status_msg->outsize = 0;
+
+ /*
+ * Query the EC's status until it's no longer busy or
+ * we encounter an error.
+ */
+ for (i = 0; i < EC_COMMAND_RETRIES; i++) {
+ usleep_range(10000, 11000);
+
+ ret = ec_dev->cmd_xfer(ec_dev, status_msg);
+ if (ret < 0)
+ break;
+
+ msg->result = status_msg->result;
+ if (status_msg->result != EC_RES_SUCCESS)
+ break;
+
+ status = (struct ec_response_get_comms_status *)
+ status_msg->data;
+ if (!(status->flags & EC_COMMS_STATUS_PROCESSING))
+ break;
+ }
+
+ kfree(status_msg);
+ }
+exit:
+ mutex_unlock(&ec_dev->lock);
+
+ return ret;
+}
+EXPORT_SYMBOL(cros_ec_cmd_xfer);
--
2.1.4

2015-06-02 08:12:31

by Javier Martinez Canillas

[permalink] [raw]
Subject: [PATCH v4 4/8] mfd: cros_ec: add proto v3 skeleton

From: Stephen Barber <[email protected]>

Add support in cros_ec.c to handle EC host command protocol v3.
For v3+, probe for maximum shared protocol version and max
request, response, and passthrough sizes. For now, this will
always fall back to v2, since there is no bus-specific code
for handling proto v3 packets.

Signed-off-by: Stephen Barber <[email protected]>
Signed-off-by: Javier Martinez Canillas <[email protected]>
Reviewed-by: Gwendal Grignou <[email protected]>
Tested-by: Gwendal Grignou <[email protected]>
Tested-by: Heiko Stuebner <[email protected]>
Acked-by: Lee Jones <[email protected]>
---

Changes since v3:
- Added tested-by from Heiko Stuebner.
- Added acked-by tag from Lee Jones.

Changes since v2:
- Add the helpers to the drivers/platform/chrome/cros_ec_proto.c driver
instead of drivers/mfd/cros_ec.c. Suggested by Lee Jones.
- Rename the proto probe functions for proto_query since probe has a
special meaning in Linux so is confusing.

Changes since v1:
- Squash change https://chromium-review.googlesource.com/#/c/262870/ in
the patch. Suggested by Gwendal Grignou
- Add Reviewed-by and Tested-by tags from Gwendal Grignou
---
drivers/mfd/cros_ec.c | 23 ++-
drivers/mfd/cros_ec_i2c.c | 4 +
drivers/mfd/cros_ec_spi.c | 7 +-
drivers/platform/chrome/cros_ec_lpc.c | 4 +
drivers/platform/chrome/cros_ec_proto.c | 339 ++++++++++++++++++++++++++++----
include/linux/mfd/cros_ec.h | 28 ++-
6 files changed, 355 insertions(+), 50 deletions(-)

diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index d857f6a2b57b..08d82bfc5268 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -36,19 +36,22 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
struct device *dev = ec_dev->dev;
int err = 0;

- if (ec_dev->din_size) {
- ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL);
- if (!ec_dev->din)
- return -ENOMEM;
- }
- if (ec_dev->dout_size) {
- ec_dev->dout = devm_kzalloc(dev, ec_dev->dout_size, GFP_KERNEL);
- if (!ec_dev->dout)
- return -ENOMEM;
- }
+ ec_dev->max_request = sizeof(struct ec_params_hello);
+ ec_dev->max_response = sizeof(struct ec_response_get_protocol_info);
+ ec_dev->max_passthru = 0;
+
+ ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL);
+ if (!ec_dev->din)
+ return -ENOMEM;
+
+ ec_dev->dout = devm_kzalloc(dev, ec_dev->dout_size, GFP_KERNEL);
+ if (!ec_dev->dout)
+ return -ENOMEM;

mutex_init(&ec_dev->lock);

+ cros_ec_query_all(ec_dev);
+
err = mfd_add_devices(dev, 0, cros_devs,
ARRAY_SIZE(cros_devs),
NULL, ec_dev->irq, NULL);
diff --git a/drivers/mfd/cros_ec_i2c.c b/drivers/mfd/cros_ec_i2c.c
index fbf7819f5de5..b400bfa2772a 100644
--- a/drivers/mfd/cros_ec_i2c.c
+++ b/drivers/mfd/cros_ec_i2c.c
@@ -143,8 +143,12 @@ static int cros_ec_i2c_probe(struct i2c_client *client,
ec_dev->priv = client;
ec_dev->irq = client->irq;
ec_dev->cmd_xfer = cros_ec_cmd_xfer_i2c;
+ ec_dev->pkt_xfer = NULL;
ec_dev->ec_name = client->name;
ec_dev->phys_name = client->adapter->name;
+ ec_dev->din_size = sizeof(struct ec_host_response) +
+ sizeof(struct ec_response_get_protocol_info);
+ ec_dev->dout_size = sizeof(struct ec_host_request);

err = cros_ec_register(ec_dev);
if (err) {
diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
index 573730fec947..04da2f288ef8 100644
--- a/drivers/mfd/cros_ec_spi.c
+++ b/drivers/mfd/cros_ec_spi.c
@@ -361,10 +361,13 @@ static int cros_ec_spi_probe(struct spi_device *spi)
ec_dev->priv = ec_spi;
ec_dev->irq = spi->irq;
ec_dev->cmd_xfer = cros_ec_cmd_xfer_spi;
+ ec_dev->pkt_xfer = NULL;
ec_dev->ec_name = ec_spi->spi->modalias;
ec_dev->phys_name = dev_name(&ec_spi->spi->dev);
- ec_dev->din_size = EC_MSG_BYTES + EC_MSG_PREAMBLE_COUNT;
- ec_dev->dout_size = EC_MSG_BYTES;
+ ec_dev->din_size = EC_MSG_PREAMBLE_COUNT +
+ sizeof(struct ec_host_response) +
+ sizeof(struct ec_response_get_protocol_info);
+ ec_dev->dout_size = sizeof(struct ec_host_request);

err = cros_ec_register(ec_dev);
if (err) {
diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index 214ae7fef984..06c5790b2c28 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -205,7 +205,11 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
ec_dev->ec_name = pdev->name;
ec_dev->phys_name = dev_name(dev);
ec_dev->cmd_xfer = cros_ec_cmd_xfer_lpc;
+ ec_dev->pkt_xfer = NULL;
ec_dev->cmd_readmem = cros_ec_lpc_readmem;
+ ec_dev->din_size = sizeof(struct ec_host_response) +
+ sizeof(struct ec_response_get_protocol_info);
+ ec_dev->dout_size = sizeof(struct ec_host_request);

ret = cros_ec_register(ec_dev);
if (ret) {
diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index 58e98a24fd08..990308ca384f 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -22,11 +22,100 @@

#define EC_COMMAND_RETRIES 50

+static int prepare_packet(struct cros_ec_device *ec_dev,
+ struct cros_ec_command *msg)
+{
+ struct ec_host_request *request;
+ u8 *out;
+ int i;
+ u8 csum = 0;
+
+ BUG_ON(ec_dev->proto_version != EC_HOST_REQUEST_VERSION);
+ BUG_ON(msg->outsize + sizeof(*request) > ec_dev->dout_size);
+
+ out = ec_dev->dout;
+ request = (struct ec_host_request *)out;
+ request->struct_version = EC_HOST_REQUEST_VERSION;
+ request->checksum = 0;
+ request->command = msg->command;
+ request->command_version = msg->version;
+ request->reserved = 0;
+ request->data_len = msg->outsize;
+
+ for (i = 0; i < sizeof(*request); i++)
+ csum += out[i];
+
+ /* Copy data and update checksum */
+ memcpy(out + sizeof(*request), msg->data, msg->outsize);
+ for (i = 0; i < msg->outsize; i++)
+ csum += msg->data[i];
+
+ request->checksum = -csum;
+
+ return sizeof(*request) + msg->outsize;
+}
+
+static int send_command(struct cros_ec_device *ec_dev,
+ struct cros_ec_command *msg)
+{
+ int ret;
+
+ if (ec_dev->proto_version > 2)
+ ret = ec_dev->pkt_xfer(ec_dev, msg);
+ else
+ ret = ec_dev->cmd_xfer(ec_dev, msg);
+
+ if (msg->result == EC_RES_IN_PROGRESS) {
+ int i;
+ struct cros_ec_command *status_msg;
+ struct ec_response_get_comms_status *status;
+
+ status_msg = kmalloc(sizeof(*status_msg) + sizeof(*status),
+ GFP_KERNEL);
+ if (!status_msg)
+ return -ENOMEM;
+
+ status_msg->version = 0;
+ status_msg->command = EC_CMD_GET_COMMS_STATUS;
+ status_msg->insize = sizeof(*status);
+ status_msg->outsize = 0;
+
+ /*
+ * Query the EC's status until it's no longer busy or
+ * we encounter an error.
+ */
+ for (i = 0; i < EC_COMMAND_RETRIES; i++) {
+ usleep_range(10000, 11000);
+
+ ret = ec_dev->cmd_xfer(ec_dev, status_msg);
+ if (ret < 0)
+ break;
+
+ msg->result = status_msg->result;
+ if (status_msg->result != EC_RES_SUCCESS)
+ break;
+
+ status = (struct ec_response_get_comms_status *)
+ status_msg->data;
+ if (!(status->flags & EC_COMMS_STATUS_PROCESSING))
+ break;
+ }
+
+ kfree(status_msg);
+ }
+
+ return ret;
+}
+
int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
struct cros_ec_command *msg)
{
- uint8_t *out;
- int csum, i;
+ u8 *out;
+ u8 csum;
+ int i;
+
+ if (ec_dev->proto_version > 2)
+ return prepare_packet(ec_dev, msg);

BUG_ON(msg->outsize > EC_PROTO2_MAX_PARAM_SIZE);
out = ec_dev->dout;
@@ -36,7 +125,7 @@ int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
csum = out[0] + out[1] + out[2];
for (i = 0; i < msg->outsize; i++)
csum += out[EC_MSG_TX_HEADER_BYTES + i] = msg->data[i];
- out[EC_MSG_TX_HEADER_BYTES + msg->outsize] = (uint8_t)(csum & 0xff);
+ out[EC_MSG_TX_HEADER_BYTES + msg->outsize] = csum;

return EC_MSG_TX_PROTO_BYTES + msg->outsize;
}
@@ -60,54 +149,232 @@ int cros_ec_check_result(struct cros_ec_device *ec_dev,
}
EXPORT_SYMBOL(cros_ec_check_result);

-int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
- struct cros_ec_command *msg)
+static int cros_ec_host_command_proto_query(struct cros_ec_device *ec_dev,
+ int devidx,
+ struct cros_ec_command *msg)
{
+ /*
+ * Try using v3+ to query for supported protocols. If this
+ * command fails, fall back to v2. Returns the highest protocol
+ * supported by the EC.
+ * Also sets the max request/response/passthru size.
+ */
int ret;

- mutex_lock(&ec_dev->lock);
- ret = ec_dev->cmd_xfer(ec_dev, msg);
- if (msg->result == EC_RES_IN_PROGRESS) {
- int i;
- struct cros_ec_command *status_msg;
- struct ec_response_get_comms_status *status;
+ if (!ec_dev->pkt_xfer)
+ return -EPROTONOSUPPORT;

- status_msg = kmalloc(sizeof(*status_msg) + sizeof(*status),
- GFP_KERNEL);
- if (!status_msg) {
- ret = -ENOMEM;
- goto exit;
- }
+ memset(msg, 0, sizeof(*msg));
+ msg->command = EC_CMD_PASSTHRU_OFFSET(devidx) | EC_CMD_GET_PROTOCOL_INFO;
+ msg->insize = sizeof(struct ec_response_get_protocol_info);

- status_msg->version = 0;
- status_msg->command = EC_CMD_GET_COMMS_STATUS;
- status_msg->insize = sizeof(*status);
- status_msg->outsize = 0;
+ ret = send_command(ec_dev, msg);
+
+ if (ret < 0) {
+ dev_dbg(ec_dev->dev,
+ "failed to check for EC[%d] protocol version: %d\n",
+ devidx, ret);
+ return ret;
+ }
+
+ if (devidx > 0 && msg->result == EC_RES_INVALID_COMMAND)
+ return -ENODEV;
+ else if (msg->result != EC_RES_SUCCESS)
+ return msg->result;
+
+ return 0;
+}
+
+static int cros_ec_host_command_proto_query_v2(struct cros_ec_device *ec_dev)
+{
+ struct cros_ec_command *msg;
+ struct ec_params_hello *hello_params;
+ struct ec_response_hello *hello_response;
+ int ret;
+ int len = max(sizeof(*hello_params), sizeof(*hello_response));
+
+ msg = kmalloc(sizeof(*msg) + len, GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
+
+ msg->version = 0;
+ msg->command = EC_CMD_HELLO;
+ hello_params = (struct ec_params_hello *)msg->data;
+ msg->outsize = sizeof(*hello_params);
+ hello_response = (struct ec_response_hello *)msg->data;
+ msg->insize = sizeof(*hello_response);
+
+ hello_params->in_data = 0xa0b0c0d0;
+
+ ret = send_command(ec_dev, msg);
+
+ if (ret < 0) {
+ dev_dbg(ec_dev->dev,
+ "EC failed to respond to v2 hello: %d\n",
+ ret);
+ goto exit;
+ } else if (msg->result != EC_RES_SUCCESS) {
+ dev_err(ec_dev->dev,
+ "EC responded to v2 hello with error: %d\n",
+ msg->result);
+ ret = msg->result;
+ goto exit;
+ } else if (hello_response->out_data != 0xa1b2c3d4) {
+ dev_err(ec_dev->dev,
+ "EC responded to v2 hello with bad result: %u\n",
+ hello_response->out_data);
+ ret = -EBADMSG;
+ goto exit;
+ }
+
+ ret = 0;
+
+ exit:
+ kfree(msg);
+ return ret;
+}
+
+int cros_ec_query_all(struct cros_ec_device *ec_dev)
+{
+ struct device *dev = ec_dev->dev;
+ struct cros_ec_command *proto_msg;
+ struct ec_response_get_protocol_info *proto_info;
+ int ret;
+
+ proto_msg = kzalloc(sizeof(*proto_msg) + sizeof(*proto_info),
+ GFP_KERNEL);
+ if (!proto_msg)
+ return -ENOMEM;
+
+ /* First try sending with proto v3. */
+ ec_dev->proto_version = 3;
+ ret = cros_ec_host_command_proto_query(ec_dev, 0, proto_msg);
+
+ if (ret == 0) {
+ proto_info = (struct ec_response_get_protocol_info *)
+ proto_msg->data;
+ ec_dev->max_request = proto_info->max_request_packet_size -
+ sizeof(struct ec_host_request);
+ ec_dev->max_response = proto_info->max_response_packet_size -
+ sizeof(struct ec_host_response);
+ ec_dev->proto_version =
+ min(EC_HOST_REQUEST_VERSION,
+ fls(proto_info->protocol_versions) - 1);
+ dev_dbg(ec_dev->dev,
+ "using proto v%u\n",
+ ec_dev->proto_version);
+
+ ec_dev->din_size = ec_dev->max_response +
+ sizeof(struct ec_host_response) +
+ EC_MAX_RESPONSE_OVERHEAD;
+ ec_dev->dout_size = ec_dev->max_request +
+ sizeof(struct ec_host_request) +
+ EC_MAX_REQUEST_OVERHEAD;

/*
- * Query the EC's status until it's no longer busy or
- * we encounter an error.
+ * Check for PD
*/
- for (i = 0; i < EC_COMMAND_RETRIES; i++) {
- usleep_range(10000, 11000);
+ ret = cros_ec_host_command_proto_query(ec_dev, 1, proto_msg);

- ret = ec_dev->cmd_xfer(ec_dev, status_msg);
- if (ret < 0)
- break;
+ if (ret) {
+ dev_dbg(ec_dev->dev, "no PD chip found: %d\n", ret);
+ ec_dev->max_passthru = 0;
+ } else {
+ dev_dbg(ec_dev->dev, "found PD chip\n");
+ ec_dev->max_passthru =
+ proto_info->max_request_packet_size -
+ sizeof(struct ec_host_request);
+ }
+ } else {
+ /* Try querying with a v2 hello message. */
+ ec_dev->proto_version = 2;
+ ret = cros_ec_host_command_proto_query_v2(ec_dev);

- msg->result = status_msg->result;
- if (status_msg->result != EC_RES_SUCCESS)
- break;
+ if (ret == 0) {
+ /* V2 hello succeeded. */
+ dev_dbg(ec_dev->dev, "falling back to proto v2\n");

- status = (struct ec_response_get_comms_status *)
- status_msg->data;
- if (!(status->flags & EC_COMMS_STATUS_PROCESSING))
- break;
+ ec_dev->max_request = EC_PROTO2_MAX_PARAM_SIZE;
+ ec_dev->max_response = EC_PROTO2_MAX_PARAM_SIZE;
+ ec_dev->max_passthru = 0;
+ ec_dev->pkt_xfer = NULL;
+ ec_dev->din_size = EC_MSG_BYTES;
+ ec_dev->dout_size = EC_MSG_BYTES;
+ } else {
+ /*
+ * It's possible for a test to occur too early when
+ * the EC isn't listening. If this happens, we'll
+ * test later when the first command is run.
+ */
+ ec_dev->proto_version = EC_PROTO_VERSION_UNKNOWN;
+ dev_dbg(ec_dev->dev, "EC query failed: %d\n", ret);
+ goto exit;
}
+ }

- kfree(status_msg);
+ devm_kfree(dev, ec_dev->din);
+ devm_kfree(dev, ec_dev->dout);
+
+ ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL);
+ if (!ec_dev->din) {
+ ret = -ENOMEM;
+ goto exit;
}
+
+ ec_dev->dout = devm_kzalloc(dev, ec_dev->dout_size, GFP_KERNEL);
+ if (!ec_dev->dout) {
+ devm_kfree(dev, ec_dev->din);
+ ret = -ENOMEM;
+ goto exit;
+ }
+
exit:
+ kfree(proto_msg);
+ return ret;
+}
+EXPORT_SYMBOL(cros_ec_query_all);
+
+int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
+ struct cros_ec_command *msg)
+{
+ int ret;
+
+ mutex_lock(&ec_dev->lock);
+ if (ec_dev->proto_version == EC_PROTO_VERSION_UNKNOWN) {
+ ret = cros_ec_query_all(ec_dev);
+ if (ret) {
+ dev_err(ec_dev->dev,
+ "EC version unknown and query failed; aborting command\n");
+ mutex_unlock(&ec_dev->lock);
+ return ret;
+ }
+ }
+
+ if (msg->insize > ec_dev->max_response) {
+ dev_dbg(ec_dev->dev, "clamping message receive buffer\n");
+ msg->insize = ec_dev->max_response;
+ }
+
+ if (msg->command < EC_CMD_PASSTHRU_OFFSET(1)) {
+ if (msg->outsize > ec_dev->max_request) {
+ dev_err(ec_dev->dev,
+ "request of size %u is too big (max: %u)\n",
+ msg->outsize,
+ ec_dev->max_request);
+ mutex_unlock(&ec_dev->lock);
+ return -EMSGSIZE;
+ }
+ } else {
+ if (msg->outsize > ec_dev->max_passthru) {
+ dev_err(ec_dev->dev,
+ "passthru rq of size %u is too big (max: %u)\n",
+ msg->outsize,
+ ec_dev->max_passthru);
+ mutex_unlock(&ec_dev->lock);
+ return -EMSGSIZE;
+ }
+ }
+ ret = send_command(ec_dev, msg);
mutex_unlock(&ec_dev->lock);

return ret;
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index 7eee38abd02a..59d909434efd 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -22,6 +22,15 @@
#include <linux/mutex.h>

/*
+ * Max bus-specific overhead incurred by request/responses.
+ * I2C requires 1 additional byte for requests.
+ * I2C requires 2 additional bytes for responses.
+ * */
+#define EC_PROTO_VERSION_UNKNOWN 0
+#define EC_MAX_REQUEST_OVERHEAD 1
+#define EC_MAX_RESPONSE_OVERHEAD 2
+
+/*
* Command interface between EC and AP, for LPC, I2C and SPI interfaces.
*/
enum {
@@ -88,6 +97,7 @@ struct cros_ec_command {
* Returns the number of bytes received if the communication succeeded, but
* that doesn't mean the EC was happy with the command. The caller
* should check msg.result for the EC's result code.
+ * @pkt_xfer: send packet to EC and get response
* @lock: one transaction at a time
*/
struct cros_ec_device {
@@ -104,15 +114,21 @@ struct cros_ec_device {
unsigned int bytes, void *dest);

/* These are used to implement the platform-specific interface */
+ u16 max_request;
+ u16 max_response;
+ u16 max_passthru;
+ u16 proto_version;
void *priv;
int irq;
- uint8_t *din;
- uint8_t *dout;
+ u8 *din;
+ u8 *dout;
int din_size;
int dout_size;
bool wake_enabled;
int (*cmd_xfer)(struct cros_ec_device *ec,
struct cros_ec_command *msg);
+ int (*pkt_xfer)(struct cros_ec_device *ec,
+ struct cros_ec_command *msg);
struct mutex lock;
};

@@ -194,4 +210,12 @@ int cros_ec_remove(struct cros_ec_device *ec_dev);
*/
int cros_ec_register(struct cros_ec_device *ec_dev);

+/**
+ * cros_ec_register - Query the protocol version supported by the ChromeOS EC
+ *
+ * @ec_dev: Device to register
+ * @return 0 if ok, -ve on error
+ */
+int cros_ec_query_all(struct cros_ec_device *ec_dev);
+
#endif /* __LINUX_MFD_CROS_EC_H */
--
2.1.4

2015-06-02 08:12:59

by Javier Martinez Canillas

[permalink] [raw]
Subject: [PATCH v4 5/8] mfd: cros_ec: add bus-specific proto v3 code

From: Stephen Barber <[email protected]>

Add proto v3 support to the SPI, I2C, and LPC.

Signed-off-by: Stephen Barber <[email protected]>
Signed-off-by: Javier Martinez Canillas <[email protected]>
Tested-by: Heiko Stuebner <[email protected]>
Reviewed-by: Gwendal Grignou <[email protected]>
Tested-by: Gwendal Grignou <[email protected]>
Acked-by: Lee Jones <[email protected]>
---

Changes since v3:
- Added acked-by tag from Lee Jones.

Changes since v2: None

Changes since v1:
- Added Gwendal Grignou Reviewed-by and Tested-by tags
---
drivers/mfd/cros_ec_i2c.c | 166 ++++++++++++++-
drivers/mfd/cros_ec_spi.c | 382 +++++++++++++++++++++++++++++-----
drivers/platform/chrome/cros_ec_lpc.c | 73 ++++++-
include/linux/mfd/cros_ec.h | 6 +
4 files changed, 569 insertions(+), 58 deletions(-)

diff --git a/drivers/mfd/cros_ec_i2c.c b/drivers/mfd/cros_ec_i2c.c
index b400bfa2772a..22e8a4ae1711 100644
--- a/drivers/mfd/cros_ec_i2c.c
+++ b/drivers/mfd/cros_ec_i2c.c
@@ -13,6 +13,7 @@
* GNU General Public License for more details.
*/

+#include <linux/delay.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/i2c.h>
@@ -22,6 +23,32 @@
#include <linux/platform_device.h>
#include <linux/slab.h>

+/**
+ * Request format for protocol v3
+ * byte 0 0xda (EC_COMMAND_PROTOCOL_3)
+ * byte 1-8 struct ec_host_request
+ * byte 10- response data
+ */
+struct ec_host_request_i2c {
+ /* Always 0xda to backward compatible with v2 struct */
+ uint8_t command_protocol;
+ struct ec_host_request ec_request;
+} __packed;
+
+
+/*
+ * Response format for protocol v3
+ * byte 0 result code
+ * byte 1 packet_length
+ * byte 2-9 struct ec_host_response
+ * byte 10- response data
+ */
+struct ec_host_response_i2c {
+ uint8_t result;
+ uint8_t packet_length;
+ struct ec_host_response ec_response;
+} __packed;
+
static inline struct cros_ec_device *to_ec_dev(struct device *dev)
{
struct i2c_client *client = to_i2c_client(dev);
@@ -29,6 +56,134 @@ static inline struct cros_ec_device *to_ec_dev(struct device *dev)
return i2c_get_clientdata(client);
}

+static int cros_ec_pkt_xfer_i2c(struct cros_ec_device *ec_dev,
+ struct cros_ec_command *msg)
+{
+ struct i2c_client *client = ec_dev->priv;
+ int ret = -ENOMEM;
+ int i;
+ int packet_len;
+ u8 *out_buf = NULL;
+ u8 *in_buf = NULL;
+ u8 sum;
+ struct i2c_msg i2c_msg[2];
+ struct ec_host_response *ec_response;
+ struct ec_host_request_i2c *ec_request_i2c;
+ struct ec_host_response_i2c *ec_response_i2c;
+ int request_header_size = sizeof(struct ec_host_request_i2c);
+ int response_header_size = sizeof(struct ec_host_response_i2c);
+
+ i2c_msg[0].addr = client->addr;
+ i2c_msg[0].flags = 0;
+ i2c_msg[1].addr = client->addr;
+ i2c_msg[1].flags = I2C_M_RD;
+
+ packet_len = msg->insize + response_header_size;
+ BUG_ON(packet_len > ec_dev->din_size);
+ in_buf = ec_dev->din;
+ i2c_msg[1].len = packet_len;
+ i2c_msg[1].buf = (char *) in_buf;
+
+ packet_len = msg->outsize + request_header_size;
+ BUG_ON(packet_len > ec_dev->dout_size);
+ out_buf = ec_dev->dout;
+ i2c_msg[0].len = packet_len;
+ i2c_msg[0].buf = (char *) out_buf;
+
+ /* create request data */
+ ec_request_i2c = (struct ec_host_request_i2c *) out_buf;
+ ec_request_i2c->command_protocol = EC_COMMAND_PROTOCOL_3;
+
+ ec_dev->dout++;
+ ret = cros_ec_prepare_tx(ec_dev, msg);
+ ec_dev->dout--;
+
+ /* send command to EC and read answer */
+ ret = i2c_transfer(client->adapter, i2c_msg, 2);
+ if (ret < 0) {
+ dev_dbg(ec_dev->dev, "i2c transfer failed: %d\n", ret);
+ goto done;
+ } else if (ret != 2) {
+ dev_err(ec_dev->dev, "failed to get response: %d\n", ret);
+ ret = -EIO;
+ goto done;
+ }
+
+ ec_response_i2c = (struct ec_host_response_i2c *) in_buf;
+ msg->result = ec_response_i2c->result;
+ ec_response = &ec_response_i2c->ec_response;
+
+ switch (msg->result) {
+ case EC_RES_SUCCESS:
+ break;
+ case EC_RES_IN_PROGRESS:
+ ret = -EAGAIN;
+ dev_dbg(ec_dev->dev, "command 0x%02x in progress\n",
+ msg->command);
+ goto done;
+
+ default:
+ dev_dbg(ec_dev->dev, "command 0x%02x returned %d\n",
+ msg->command, msg->result);
+ /*
+ * When we send v3 request to v2 ec, ec won't recognize the
+ * 0xda (EC_COMMAND_PROTOCOL_3) and will return with status
+ * EC_RES_INVALID_COMMAND with zero data length.
+ *
+ * In case of invalid command for v3 protocol the data length
+ * will be at least sizeof(struct ec_host_response)
+ */
+ if (ec_response_i2c->result == EC_RES_INVALID_COMMAND &&
+ ec_response_i2c->packet_length == 0) {
+ ret = -EPROTONOSUPPORT;
+ goto done;
+ }
+ }
+
+ if (ec_response_i2c->packet_length < sizeof(struct ec_host_response)) {
+ dev_err(ec_dev->dev,
+ "response of %u bytes too short; not a full header\n",
+ ec_response_i2c->packet_length);
+ ret = -EBADMSG;
+ goto done;
+ }
+
+ if (msg->insize < ec_response->data_len) {
+ dev_err(ec_dev->dev,
+ "response data size is too large: expected %u, got %u\n",
+ msg->insize,
+ ec_response->data_len);
+ ret = -EMSGSIZE;
+ goto done;
+ }
+
+ /* copy response packet payload and compute checksum */
+ sum = 0;
+ for (i = 0; i < sizeof(struct ec_host_response); i++)
+ sum += ((u8 *)ec_response)[i];
+
+ memcpy(msg->data,
+ in_buf + response_header_size,
+ ec_response->data_len);
+ for (i = 0; i < ec_response->data_len; i++)
+ sum += msg->data[i];
+
+ /* All bytes should sum to zero */
+ if (sum) {
+ dev_err(ec_dev->dev, "bad packet checksum\n");
+ ret = -EBADMSG;
+ goto done;
+ }
+
+ ret = ec_response->data_len;
+
+done:
+ if (msg->command == EC_CMD_REBOOT_EC)
+ msleep(EC_REBOOT_DELAY_MS);
+
+ return ret;
+}
+
static int cros_ec_cmd_xfer_i2c(struct cros_ec_device *ec_dev,
struct cros_ec_command *msg)
{
@@ -121,9 +276,12 @@ static int cros_ec_cmd_xfer_i2c(struct cros_ec_device *ec_dev,
}

ret = len;
- done:
+done:
kfree(in_buf);
kfree(out_buf);
+ if (msg->command == EC_CMD_REBOOT_EC)
+ msleep(EC_REBOOT_DELAY_MS);
+
return ret;
}

@@ -143,12 +301,12 @@ static int cros_ec_i2c_probe(struct i2c_client *client,
ec_dev->priv = client;
ec_dev->irq = client->irq;
ec_dev->cmd_xfer = cros_ec_cmd_xfer_i2c;
- ec_dev->pkt_xfer = NULL;
+ ec_dev->pkt_xfer = cros_ec_pkt_xfer_i2c;
ec_dev->ec_name = client->name;
ec_dev->phys_name = client->adapter->name;
- ec_dev->din_size = sizeof(struct ec_host_response) +
+ ec_dev->din_size = sizeof(struct ec_host_response_i2c) +
sizeof(struct ec_response_get_protocol_info);
- ec_dev->dout_size = sizeof(struct ec_host_request);
+ ec_dev->dout_size = sizeof(struct ec_host_request_i2c);

err = cros_ec_register(ec_dev);
if (err) {
diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
index 04da2f288ef8..4e6f2f6b1095 100644
--- a/drivers/mfd/cros_ec_spi.c
+++ b/drivers/mfd/cros_ec_spi.c
@@ -65,12 +65,6 @@
*/
#define EC_SPI_RECOVERY_TIME_NS (200 * 1000)

-/*
- * The EC is unresponsive for a time after a reboot command. Add a
- * simple delay to make sure that the bus stays locked.
- */
-#define EC_REBOOT_DELAY_MS 50
-
/**
* struct cros_ec_spi - information about a SPI-connected EC
*
@@ -87,7 +81,7 @@ struct cros_ec_spi {
};

static void debug_packet(struct device *dev, const char *name, u8 *ptr,
- int len)
+ int len)
{
#ifdef DEBUG
int i;
@@ -100,6 +94,172 @@ static void debug_packet(struct device *dev, const char *name, u8 *ptr,
#endif
}

+static int terminate_request(struct cros_ec_device *ec_dev)
+{
+ struct cros_ec_spi *ec_spi = ec_dev->priv;
+ struct spi_message msg;
+ struct spi_transfer trans;
+ int ret;
+
+ /*
+ * Turn off CS, possibly adding a delay to ensure the rising edge
+ * doesn't come too soon after the end of the data.
+ */
+ spi_message_init(&msg);
+ memset(&trans, 0, sizeof(trans));
+ trans.delay_usecs = ec_spi->end_of_msg_delay;
+ spi_message_add_tail(&trans, &msg);
+
+ ret = spi_sync(ec_spi->spi, &msg);
+
+ /* Reset end-of-response timer */
+ ec_spi->last_transfer_ns = ktime_get_ns();
+ if (ret < 0) {
+ dev_err(ec_dev->dev,
+ "cs-deassert spi transfer failed: %d\n",
+ ret);
+ }
+
+ return ret;
+}
+
+/**
+ * receive_n_bytes - receive n bytes from the EC.
+ *
+ * Assumes buf is a pointer into the ec_dev->din buffer
+ */
+static int receive_n_bytes(struct cros_ec_device *ec_dev, u8 *buf, int n)
+{
+ struct cros_ec_spi *ec_spi = ec_dev->priv;
+ struct spi_transfer trans;
+ struct spi_message msg;
+ int ret;
+
+ BUG_ON(buf - ec_dev->din + n > ec_dev->din_size);
+
+ memset(&trans, 0, sizeof(trans));
+ trans.cs_change = 1;
+ trans.rx_buf = buf;
+ trans.len = n;
+
+ spi_message_init(&msg);
+ spi_message_add_tail(&trans, &msg);
+ ret = spi_sync(ec_spi->spi, &msg);
+ if (ret < 0)
+ dev_err(ec_dev->dev, "spi transfer failed: %d\n", ret);
+
+ return ret;
+}
+
+/**
+ * cros_ec_spi_receive_packet - Receive a packet from the EC.
+ *
+ * This function has two phases: reading the preamble bytes (since if we read
+ * data from the EC before it is ready to send, we just get preamble) and
+ * reading the actual message.
+ *
+ * The received data is placed into ec_dev->din.
+ *
+ * @ec_dev: ChromeOS EC device
+ * @need_len: Number of message bytes we need to read
+ */
+static int cros_ec_spi_receive_packet(struct cros_ec_device *ec_dev,
+ int need_len)
+{
+ struct ec_host_response *response;
+ u8 *ptr, *end;
+ int ret;
+ unsigned long deadline;
+ int todo;
+
+ BUG_ON(EC_MSG_PREAMBLE_COUNT > ec_dev->din_size);
+
+ /* Receive data until we see the header byte */
+ deadline = jiffies + msecs_to_jiffies(EC_MSG_DEADLINE_MS);
+ while (true) {
+ unsigned long start_jiffies = jiffies;
+
+ ret = receive_n_bytes(ec_dev,
+ ec_dev->din,
+ EC_MSG_PREAMBLE_COUNT);
+ if (ret < 0)
+ return ret;
+
+ ptr = ec_dev->din;
+ for (end = ptr + EC_MSG_PREAMBLE_COUNT; ptr != end; ptr++) {
+ if (*ptr == EC_SPI_FRAME_START) {
+ dev_dbg(ec_dev->dev, "msg found at %zd\n",
+ ptr - ec_dev->din);
+ break;
+ }
+ }
+ if (ptr != end)
+ break;
+
+ /*
+ * Use the time at the start of the loop as a timeout. This
+ * gives us one last shot at getting the transfer and is useful
+ * in case we got context switched out for a while.
+ */
+ if (time_after(start_jiffies, deadline)) {
+ dev_warn(ec_dev->dev, "EC failed to respond in time\n");
+ return -ETIMEDOUT;
+ }
+ }
+
+ /*
+ * ptr now points to the header byte. Copy any valid data to the
+ * start of our buffer
+ */
+ todo = end - ++ptr;
+ BUG_ON(todo < 0 || todo > ec_dev->din_size);
+ todo = min(todo, need_len);
+ memmove(ec_dev->din, ptr, todo);
+ ptr = ec_dev->din + todo;
+ dev_dbg(ec_dev->dev, "need %d, got %d bytes from preamble\n",
+ need_len, todo);
+ need_len -= todo;
+
+ /* If the entire response struct wasn't read, get the rest of it. */
+ if (todo < sizeof(*response)) {
+ ret = receive_n_bytes(ec_dev, ptr, sizeof(*response) - todo);
+ if (ret < 0)
+ return -EBADMSG;
+ ptr += (sizeof(*response) - todo);
+ todo = sizeof(*response);
+ }
+
+ response = (struct ec_host_response *)ec_dev->din;
+
+ /* Abort if data_len is too large. */
+ if (response->data_len > ec_dev->din_size)
+ return -EMSGSIZE;
+
+ /* Receive data until we have it all */
+ while (need_len > 0) {
+ /*
+ * We can't support transfers larger than the SPI FIFO size
+ * unless we have DMA. We don't have DMA on the ISP SPI ports
+ * for Exynos. We need a way of asking SPI driver for
+ * maximum-supported transfer size.
+ */
+ todo = min(need_len, 256);
+ dev_dbg(ec_dev->dev, "loop, todo=%d, need_len=%d, ptr=%zd\n",
+ todo, need_len, ptr - ec_dev->din);
+
+ ret = receive_n_bytes(ec_dev, ptr, todo);
+ if (ret < 0)
+ return ret;
+
+ ptr += todo;
+ need_len -= todo;
+ }
+
+ dev_dbg(ec_dev->dev, "loop done, ptr=%zd\n", ptr - ec_dev->din);
+
+ return 0;
+}
+
/**
* cros_ec_spi_receive_response - Receive a response from the EC.
*
@@ -115,34 +275,27 @@ static void debug_packet(struct device *dev, const char *name, u8 *ptr,
static int cros_ec_spi_receive_response(struct cros_ec_device *ec_dev,
int need_len)
{
- struct cros_ec_spi *ec_spi = ec_dev->priv;
- struct spi_transfer trans;
- struct spi_message msg;
u8 *ptr, *end;
int ret;
unsigned long deadline;
int todo;

+ BUG_ON(EC_MSG_PREAMBLE_COUNT > ec_dev->din_size);
+
/* Receive data until we see the header byte */
deadline = jiffies + msecs_to_jiffies(EC_MSG_DEADLINE_MS);
while (true) {
unsigned long start_jiffies = jiffies;

- memset(&trans, 0, sizeof(trans));
- trans.cs_change = 1;
- trans.rx_buf = ptr = ec_dev->din;
- trans.len = EC_MSG_PREAMBLE_COUNT;
-
- spi_message_init(&msg);
- spi_message_add_tail(&trans, &msg);
- ret = spi_sync(ec_spi->spi, &msg);
- if (ret < 0) {
- dev_err(ec_dev->dev, "spi transfer failed: %d\n", ret);
+ ret = receive_n_bytes(ec_dev,
+ ec_dev->din,
+ EC_MSG_PREAMBLE_COUNT);
+ if (ret < 0)
return ret;
- }

+ ptr = ec_dev->din;
for (end = ptr + EC_MSG_PREAMBLE_COUNT; ptr != end; ptr++) {
- if (*ptr == EC_MSG_HEADER) {
+ if (*ptr == EC_SPI_FRAME_START) {
dev_dbg(ec_dev->dev, "msg found at %zd\n",
ptr - ec_dev->din);
break;
@@ -187,21 +340,9 @@ static int cros_ec_spi_receive_response(struct cros_ec_device *ec_dev,
dev_dbg(ec_dev->dev, "loop, todo=%d, need_len=%d, ptr=%zd\n",
todo, need_len, ptr - ec_dev->din);

- memset(&trans, 0, sizeof(trans));
- trans.cs_change = 1;
- trans.rx_buf = ptr;
- trans.len = todo;
- spi_message_init(&msg);
- spi_message_add_tail(&trans, &msg);
-
- /* send command to EC and read answer */
- BUG_ON((u8 *)trans.rx_buf - ec_dev->din + todo >
- ec_dev->din_size);
- ret = spi_sync(ec_spi->spi, &msg);
- if (ret < 0) {
- dev_err(ec_dev->dev, "spi transfer failed: %d\n", ret);
+ ret = receive_n_bytes(ec_dev, ptr, todo);
+ if (ret < 0)
return ret;
- }

debug_packet(ec_dev->dev, "interim", ptr, todo);
ptr += todo;
@@ -214,6 +355,128 @@ static int cros_ec_spi_receive_response(struct cros_ec_device *ec_dev,
}

/**
+ * cros_ec_pkt_xfer_spi - Transfer a packet over SPI and receive the reply
+ *
+ * @ec_dev: ChromeOS EC device
+ * @ec_msg: Message to transfer
+ */
+static int cros_ec_pkt_xfer_spi(struct cros_ec_device *ec_dev,
+ struct cros_ec_command *ec_msg)
+{
+ struct ec_host_request *request;
+ struct ec_host_response *response;
+ struct cros_ec_spi *ec_spi = ec_dev->priv;
+ struct spi_transfer trans;
+ struct spi_message msg;
+ int i, len;
+ u8 *ptr;
+ u8 *rx_buf;
+ u8 sum;
+ int ret = 0, final_ret;
+
+ len = cros_ec_prepare_tx(ec_dev, ec_msg);
+ request = (struct ec_host_request *)ec_dev->dout;
+ dev_dbg(ec_dev->dev, "prepared, len=%d\n", len);
+
+ /* If it's too soon to do another transaction, wait */
+ if (ec_spi->last_transfer_ns) {
+ unsigned long delay; /* The delay completed so far */
+
+ delay = ktime_get_ns() - ec_spi->last_transfer_ns;
+ if (delay < EC_SPI_RECOVERY_TIME_NS)
+ ndelay(EC_SPI_RECOVERY_TIME_NS - delay);
+ }
+
+ rx_buf = kzalloc(len, GFP_KERNEL);
+ if (!rx_buf) {
+ ret = -ENOMEM;
+ goto exit;
+ }
+
+ /* Transmit phase - send our message */
+ memset(&trans, 0, sizeof(trans));
+ trans.tx_buf = ec_dev->dout;
+ trans.rx_buf = rx_buf;
+ trans.len = len;
+ trans.cs_change = 1;
+ spi_message_init(&msg);
+ spi_message_add_tail(&trans, &msg);
+ ret = spi_sync(ec_spi->spi, &msg);
+
+ /* Get the response */
+ if (!ret) {
+ /* Verify that EC can process command */
+ for (i = 0; i < len; i++) {
+ switch (rx_buf[i]) {
+ case EC_SPI_PAST_END:
+ case EC_SPI_RX_BAD_DATA:
+ case EC_SPI_NOT_READY:
+ ret = -EAGAIN;
+ ec_msg->result = EC_RES_IN_PROGRESS;
+ default:
+ break;
+ }
+ if (ret)
+ break;
+ }
+ if (!ret)
+ ret = cros_ec_spi_receive_packet(ec_dev,
+ ec_msg->insize + sizeof(*response));
+ } else {
+ dev_err(ec_dev->dev, "spi transfer failed: %d\n", ret);
+ }
+
+ final_ret = terminate_request(ec_dev);
+ if (!ret)
+ ret = final_ret;
+ if (ret < 0)
+ goto exit;
+
+ ptr = ec_dev->din;
+
+ /* check response error code */
+ response = (struct ec_host_response *)ptr;
+ ec_msg->result = response->result;
+
+ ret = cros_ec_check_result(ec_dev, ec_msg);
+ if (ret)
+ goto exit;
+
+ len = response->data_len;
+ sum = 0;
+ if (len > ec_msg->insize) {
+ dev_err(ec_dev->dev, "packet too long (%d bytes, expected %d)",
+ len, ec_msg->insize);
+ ret = -EMSGSIZE;
+ goto exit;
+ }
+
+ for (i = 0; i < sizeof(*response); i++)
+ sum += ptr[i];
+
+ /* copy response packet payload and compute checksum */
+ memcpy(ec_msg->data, ptr + sizeof(*response), len);
+ for (i = 0; i < len; i++)
+ sum += ec_msg->data[i];
+
+ if (sum) {
+ dev_err(ec_dev->dev,
+ "bad packet checksum, calculated %x\n",
+ sum);
+ ret = -EBADMSG;
+ goto exit;
+ }
+
+ ret = len;
+exit:
+ kfree(rx_buf);
+ if (ec_msg->command == EC_CMD_REBOOT_EC)
+ msleep(EC_REBOOT_DELAY_MS);
+
+ return ret;
+}
+
+/**
* cros_ec_cmd_xfer_spi - Transfer a message over SPI and receive the reply
*
* @ec_dev: ChromeOS EC device
@@ -227,6 +490,7 @@ static int cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev,
struct spi_message msg;
int i, len;
u8 *ptr;
+ u8 *rx_buf;
int sum;
int ret = 0, final_ret;

@@ -242,10 +506,17 @@ static int cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev,
ndelay(EC_SPI_RECOVERY_TIME_NS - delay);
}

+ rx_buf = kzalloc(len, GFP_KERNEL);
+ if (!rx_buf) {
+ ret = -ENOMEM;
+ goto exit;
+ }
+
/* Transmit phase - send our message */
debug_packet(ec_dev->dev, "out", ec_dev->dout, len);
memset(&trans, 0, sizeof(trans));
trans.tx_buf = ec_dev->dout;
+ trans.rx_buf = rx_buf;
trans.len = len;
trans.cs_change = 1;
spi_message_init(&msg);
@@ -254,29 +525,32 @@ static int cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev,

/* Get the response */
if (!ret) {
- ret = cros_ec_spi_receive_response(ec_dev,
- ec_msg->insize + EC_MSG_TX_PROTO_BYTES);
+ /* Verify that EC can process command */
+ for (i = 0; i < len; i++) {
+ switch (rx_buf[i]) {
+ case EC_SPI_PAST_END:
+ case EC_SPI_RX_BAD_DATA:
+ case EC_SPI_NOT_READY:
+ ret = -EAGAIN;
+ ec_msg->result = EC_RES_IN_PROGRESS;
+ default:
+ break;
+ }
+ if (ret)
+ break;
+ }
+ if (!ret)
+ ret = cros_ec_spi_receive_response(ec_dev,
+ ec_msg->insize + EC_MSG_TX_PROTO_BYTES);
} else {
dev_err(ec_dev->dev, "spi transfer failed: %d\n", ret);
}

- /*
- * Turn off CS, possibly adding a delay to ensure the rising edge
- * doesn't come too soon after the end of the data.
- */
- spi_message_init(&msg);
- memset(&trans, 0, sizeof(trans));
- trans.delay_usecs = ec_spi->end_of_msg_delay;
- spi_message_add_tail(&trans, &msg);
-
- final_ret = spi_sync(ec_spi->spi, &msg);
- ec_spi->last_transfer_ns = ktime_get_ns();
+ final_ret = terminate_request(ec_dev);
if (!ret)
ret = final_ret;
- if (ret < 0) {
- dev_err(ec_dev->dev, "spi transfer failed: %d\n", ret);
+ if (ret < 0)
goto exit;
- }

ptr = ec_dev->din;

@@ -315,6 +589,7 @@ static int cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev,

ret = len;
exit:
+ kfree(rx_buf);
if (ec_msg->command == EC_CMD_REBOOT_EC)
msleep(EC_REBOOT_DELAY_MS);

@@ -361,7 +636,7 @@ static int cros_ec_spi_probe(struct spi_device *spi)
ec_dev->priv = ec_spi;
ec_dev->irq = spi->irq;
ec_dev->cmd_xfer = cros_ec_cmd_xfer_spi;
- ec_dev->pkt_xfer = NULL;
+ ec_dev->pkt_xfer = cros_ec_pkt_xfer_spi;
ec_dev->ec_name = ec_spi->spi->modalias;
ec_dev->phys_name = dev_name(&ec_spi->spi->dev);
ec_dev->din_size = EC_MSG_PREAMBLE_COUNT +
@@ -369,6 +644,7 @@ static int cros_ec_spi_probe(struct spi_device *spi)
sizeof(struct ec_response_get_protocol_info);
ec_dev->dout_size = sizeof(struct ec_host_request);

+
err = cros_ec_register(ec_dev);
if (err) {
dev_err(dev, "cannot register EC\n");
diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index 06c5790b2c28..92b633324aaa 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -46,6 +46,77 @@ static int ec_response_timed_out(void)
return 1;
}

+static int cros_ec_pkt_xfer_lpc(struct cros_ec_device *ec,
+ struct cros_ec_command *msg)
+{
+ struct ec_host_request *request;
+ struct ec_host_response response;
+ u8 sum = 0;
+ int i;
+ int ret = 0;
+ u8 *dout;
+
+ ret = cros_ec_prepare_tx(ec, msg);
+
+ /* Write buffer */
+ for (i = 0; i < ret; i++)
+ outb(ec->dout[i], EC_LPC_ADDR_HOST_PACKET + i);
+
+ request = (struct ec_host_request *)ec->dout;
+
+ /* Here we go */
+ outb(EC_COMMAND_PROTOCOL_3, EC_LPC_ADDR_HOST_CMD);
+
+ if (ec_response_timed_out()) {
+ dev_warn(ec->dev, "EC responsed timed out\n");
+ ret = -EIO;
+ goto done;
+ }
+
+ /* Check result */
+ msg->result = inb(EC_LPC_ADDR_HOST_DATA);
+ ret = cros_ec_check_result(ec, msg);
+ if (ret)
+ goto done;
+
+ /* Read back response */
+ dout = (u8 *)&response;
+ for (i = 0; i < sizeof(response); i++) {
+ dout[i] = inb(EC_LPC_ADDR_HOST_PACKET + i);
+ sum += dout[i];
+ }
+
+ msg->result = response.result;
+
+ if (response.data_len > msg->insize) {
+ dev_err(ec->dev,
+ "packet too long (%d bytes, expected %d)",
+ response.data_len, msg->insize);
+ ret = -EMSGSIZE;
+ goto done;
+ }
+
+ /* Read response and process checksum */
+ for (i = 0; i < response.data_len; i++) {
+ msg->data[i] =
+ inb(EC_LPC_ADDR_HOST_PACKET + sizeof(response) + i);
+ sum += msg->data[i];
+ }
+
+ if (sum) {
+ dev_err(ec->dev,
+ "bad packet checksum %02x\n",
+ response.checksum);
+ ret = -EBADMSG;
+ goto done;
+ }
+
+ /* Return actual amount of data received */
+ ret = response.data_len;
+done:
+ return ret;
+}
+
static int cros_ec_cmd_xfer_lpc(struct cros_ec_device *ec,
struct cros_ec_command *msg)
{
@@ -205,7 +276,7 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
ec_dev->ec_name = pdev->name;
ec_dev->phys_name = dev_name(dev);
ec_dev->cmd_xfer = cros_ec_cmd_xfer_lpc;
- ec_dev->pkt_xfer = NULL;
+ ec_dev->pkt_xfer = cros_ec_pkt_xfer_lpc;
ec_dev->cmd_readmem = cros_ec_lpc_readmem;
ec_dev->din_size = sizeof(struct ec_host_response) +
sizeof(struct ec_response_get_protocol_info);
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index 59d909434efd..92e13aaa450c 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -22,6 +22,12 @@
#include <linux/mutex.h>

/*
+ * The EC is unresponsive for a time after a reboot command. Add a
+ * simple delay to make sure that the bus stays locked.
+ */
+#define EC_REBOOT_DELAY_MS 50
+
+/*
* Max bus-specific overhead incurred by request/responses.
* I2C requires 1 additional byte for requests.
* I2C requires 2 additional bytes for responses.
--
2.1.4

2015-06-02 08:14:06

by Javier Martinez Canillas

[permalink] [raw]
Subject: [PATCH v4 6/8] mfd: cros_ec: Support multiple EC in a system

From: Gwendal Grignou <[email protected]>

Chromebooks can have more than one Embedded Controller so the
cros_ec device id has to be incremented for each EC registered.

Add code to handle multiple EC. First ec found is cros-ec0,
second cros-ec1 and so on.

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 <[email protected]>
Reviewed-by: Dmitry Torokhov <[email protected]>
Signed-off-by: Javier Martinez Canillas <[email protected]>
---

Changes since v3:
- Add defines for the EC and PD index constants.
- Remove cros_ec_dev_register() and declare the mfd_cells as static structs.
Suggested by Lee Jones.
- Add a new line before the return statement in cros_ec_dev_register().
Suggested by Lee Jones.

Changes since v2: None

Changes since v1:
- Squash patch that adds support to represent EC's as different
char devices (e.g: /dev/cros_ec, /dev/cros_pd):
https://chromium-review.googlesource.com/#/c/217297/
Suggested by Gwendal Grignou
- Use cros_ec instead of cros-ec in the subject line to be consistent.
Suggested by Gwendal Grignou
---
drivers/input/keyboard/cros_ec_keyb.c | 2 +-
drivers/mfd/cros_ec.c | 59 +++++++++++--
drivers/mfd/cros_ec_i2c.c | 1 -
drivers/mfd/cros_ec_spi.c | 1 -
drivers/platform/chrome/cros_ec_dev.c | 128 ++++++++++++++++++++---------
drivers/platform/chrome/cros_ec_dev.h | 7 --
drivers/platform/chrome/cros_ec_lightbar.c | 75 +++++++++--------
drivers/platform/chrome/cros_ec_lpc.c | 1 -
drivers/platform/chrome/cros_ec_sysfs.c | 48 +++++------
include/linux/mfd/cros_ec.h | 44 ++++++++--
10 files changed, 240 insertions(+), 126 deletions(-)

diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
index 974154a74505..b01966dc7eb3 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -275,7 +275,7 @@ static int cros_ec_keyb_probe(struct platform_device *pdev)
ckdev->dev = dev;
dev_set_drvdata(&pdev->dev, ckdev);

- idev->name = ec->ec_name;
+ idev->name = CROS_EC_DEV_NAME;
idev->phys = ec->phys_name;
__set_bit(EV_REP, idev->evbit);

diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index 08d82bfc5268..4fbb4ce8f81e 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -24,11 +24,29 @@
#include <linux/mfd/core.h>
#include <linux/mfd/cros_ec.h>

-static const struct mfd_cell cros_devs[] = {
- {
- .name = "cros-ec-ctl",
- .id = PLATFORM_DEVID_AUTO,
- },
+#define CROS_EC_DEV_EC_INDEX 0
+#define CROS_EC_DEV_PD_INDEX 1
+
+struct cros_ec_platform ec_p = {
+ .cmd_offset = EC_CMD_PASSTHRU_OFFSET(CROS_EC_DEV_EC_INDEX),
+};
+
+struct cros_ec_platform pd_p = {
+ .cmd_offset = EC_CMD_PASSTHRU_OFFSET(CROS_EC_DEV_PD_INDEX),
+};
+
+struct mfd_cell ec_cell = {
+ .name = "cros-ec-ctl",
+ .id = PLATFORM_DEVID_AUTO,
+ .platform_data = &ec_p,
+ .pdata_size = sizeof(ec_p),
+};
+
+struct mfd_cell ec_pd_cell = {
+ .name = "cros-ec-ctl",
+ .id = PLATFORM_DEVID_AUTO,
+ .platform_data = &pd_p,
+ .pdata_size = sizeof(pd_p),
};

int cros_ec_register(struct cros_ec_device *ec_dev)
@@ -52,14 +70,39 @@ int cros_ec_register(struct cros_ec_device *ec_dev)

cros_ec_query_all(ec_dev);

- err = mfd_add_devices(dev, 0, cros_devs,
- ARRAY_SIZE(cros_devs),
+ if (IS_ENABLED(CONFIG_OF) && dev->of_node)
+ ec_p.ec_name = of_get_property(dev->of_node, "devname",
+ NULL);
+
+ if (!ec_p.ec_name)
+ ec_p.ec_name = CROS_EC_DEV_NAME;
+
+ err = mfd_add_devices(ec_dev->dev, CROS_EC_DEV_EC_INDEX, &ec_cell, 1,
NULL, ec_dev->irq, NULL);
if (err) {
- dev_err(dev, "failed to add mfd devices\n");
+ dev_err(dev, "failed to add ec\n");
return err;
}

+ if (ec_dev->max_passthru) {
+ /*
+ * Register a PD device as well on top of this device.
+ * We make the following assumptions:
+ * - behind an EC, we have a pd
+ * - only one device added.
+ * - the EC is responsive at init time (it is not true for a
+ * sensor hub.
+ */
+ pd_p.ec_name = CROS_EC_DEV_PD_NAME;
+
+ err = mfd_add_devices(ec_dev->dev, CROS_EC_DEV_PD_INDEX,
+ &ec_pd_cell, 1, NULL, ec_dev->irq, NULL);
+ if (err) {
+ dev_err(dev, "failed to add pd ec\n");
+ return err;
+ }
+ }
+
if (IS_ENABLED(CONFIG_OF) && dev->of_node) {
err = of_platform_populate(dev->of_node, NULL, NULL, dev);
if (err) {
diff --git a/drivers/mfd/cros_ec_i2c.c b/drivers/mfd/cros_ec_i2c.c
index 22e8a4ae1711..b9a0963ca5c3 100644
--- a/drivers/mfd/cros_ec_i2c.c
+++ b/drivers/mfd/cros_ec_i2c.c
@@ -302,7 +302,6 @@ static int cros_ec_i2c_probe(struct i2c_client *client,
ec_dev->irq = client->irq;
ec_dev->cmd_xfer = cros_ec_cmd_xfer_i2c;
ec_dev->pkt_xfer = cros_ec_pkt_xfer_i2c;
- ec_dev->ec_name = client->name;
ec_dev->phys_name = client->adapter->name;
ec_dev->din_size = sizeof(struct ec_host_response_i2c) +
sizeof(struct ec_response_get_protocol_info);
diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
index 4e6f2f6b1095..faba03e2f1ef 100644
--- a/drivers/mfd/cros_ec_spi.c
+++ b/drivers/mfd/cros_ec_spi.c
@@ -637,7 +637,6 @@ static int cros_ec_spi_probe(struct spi_device *spi)
ec_dev->irq = spi->irq;
ec_dev->cmd_xfer = cros_ec_cmd_xfer_spi;
ec_dev->pkt_xfer = cros_ec_pkt_xfer_spi;
- ec_dev->ec_name = ec_spi->spi->modalias;
ec_dev->phys_name = dev_name(&ec_spi->spi->dev);
ec_dev->din_size = EC_MSG_PREAMBLE_COUNT +
sizeof(struct ec_host_response) +
diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/chrome/cros_ec_dev.c
index 14111e32658b..4b636cc84171 100644
--- a/drivers/platform/chrome/cros_ec_dev.c
+++ b/drivers/platform/chrome/cros_ec_dev.c
@@ -27,11 +27,22 @@

/* Device variables */
#define CROS_MAX_DEV 128
-static struct class *cros_class;
static int ec_major;

+static const struct attribute_group *cros_ec_groups[] = {
+ &cros_ec_attr_group,
+ &cros_ec_lightbar_attr_group,
+ NULL,
+};
+
+static struct class cros_class = {
+ .owner = THIS_MODULE,
+ .name = "chromeos",
+ .dev_groups = cros_ec_groups,
+};
+
/* Basic communication */
-static int ec_get_version(struct cros_ec_device *ec, char *str, int maxlen)
+static int ec_get_version(struct cros_ec_dev *ec, char *str, int maxlen)
{
struct ec_response_get_version *resp;
static const char * const current_image_name[] = {
@@ -49,7 +60,7 @@ static int ec_get_version(struct cros_ec_device *ec, char *str, int maxlen)
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;

@@ -78,8 +89,10 @@ exit:
/* Device file ops */
static int ec_device_open(struct inode *inode, struct file *filp)
{
- filp->private_data = container_of(inode->i_cdev,
- struct cros_ec_device, cdev);
+ struct cros_ec_dev *ec = container_of(inode->i_cdev,
+ struct cros_ec_dev, cdev);
+ filp->private_data = ec;
+ nonseekable_open(inode, filp);
return 0;
}

@@ -91,7 +104,7 @@ static int ec_device_release(struct inode *inode, struct file *filp)
static ssize_t ec_device_read(struct file *filp, char __user *buffer,
size_t length, loff_t *offset)
{
- struct cros_ec_device *ec = filp->private_data;
+ struct cros_ec_dev *ec = filp->private_data;
char msg[sizeof(struct ec_response_get_version) +
sizeof(CROS_EC_DEV_VERSION)];
size_t count;
@@ -114,7 +127,7 @@ static ssize_t ec_device_read(struct file *filp, char __user *buffer,
}

/* Ioctls */
-static long ec_device_ioctl_xcmd(struct cros_ec_device *ec, void __user *arg)
+static long ec_device_ioctl_xcmd(struct cros_ec_dev *ec, void __user *arg)
{
long ret;
struct cros_ec_command u_cmd;
@@ -133,7 +146,8 @@ static long ec_device_ioctl_xcmd(struct cros_ec_device *ec, void __user *arg)
goto exit;
}

- ret = cros_ec_cmd_xfer(ec, s_cmd);
+ s_cmd->command += ec->cmd_offset;
+ ret = cros_ec_cmd_xfer(ec->ec_dev, s_cmd);
/* Only copy data to userland if data was received. */
if (ret < 0)
goto exit;
@@ -145,19 +159,21 @@ exit:
return ret;
}

-static long ec_device_ioctl_readmem(struct cros_ec_device *ec, void __user *arg)
+static long ec_device_ioctl_readmem(struct cros_ec_dev *ec, void __user *arg)
{
+ struct cros_ec_device *ec_dev = ec->ec_dev;
struct cros_ec_readmem s_mem = { };
long num;

/* Not every platform supports direct reads */
- if (!ec->cmd_readmem)
+ if (!ec_dev->cmd_readmem)
return -ENOTTY;

if (copy_from_user(&s_mem, arg, sizeof(s_mem)))
return -EFAULT;

- num = ec->cmd_readmem(ec, s_mem.offset, s_mem.bytes, s_mem.buffer);
+ num = ec_dev->cmd_readmem(ec_dev, s_mem.offset, s_mem.bytes,
+ s_mem.buffer);
if (num <= 0)
return num;

@@ -170,7 +186,7 @@ static long ec_device_ioctl_readmem(struct cros_ec_device *ec, void __user *arg)
static long ec_device_ioctl(struct file *filp, unsigned int cmd,
unsigned long arg)
{
- struct cros_ec_device *ec = filp->private_data;
+ struct cros_ec_dev *ec = filp->private_data;

if (_IOC_TYPE(cmd) != CROS_EC_DEV_IOC)
return -ENOTTY;
@@ -193,45 +209,81 @@ static const struct file_operations fops = {
.unlocked_ioctl = ec_device_ioctl,
};

+static void __remove(struct device *dev)
+{
+ struct cros_ec_dev *ec = container_of(dev, struct cros_ec_dev,
+ class_dev);
+ kfree(ec);
+}
+
static int ec_device_probe(struct platform_device *pdev)
{
- struct cros_ec_device *ec = dev_get_drvdata(pdev->dev.parent);
- int retval = -ENOTTY;
- dev_t devno = MKDEV(ec_major, 0);
+ int retval = -ENOMEM;
+ struct device *dev = &pdev->dev;
+ struct cros_ec_platform *ec_platform = dev_get_platdata(dev);
+ dev_t devno = MKDEV(ec_major, pdev->id);
+ struct cros_ec_dev *ec = kzalloc(sizeof(*ec), GFP_KERNEL);
+
+ if (!ec)
+ return retval;

- /* Instantiate it (and remember the EC) */
+ dev_set_drvdata(dev, ec);
+ ec->ec_dev = dev_get_drvdata(dev->parent);
+ ec->dev = dev;
+ ec->cmd_offset = ec_platform->cmd_offset;
+ device_initialize(&ec->class_dev);
cdev_init(&ec->cdev, &fops);

+ /*
+ * Add the character device
+ * Link cdev to the class device to be sure device is not used
+ * before unbinding it.
+ */
+ ec->cdev.kobj.parent = &ec->class_dev.kobj;
retval = cdev_add(&ec->cdev, devno, 1);
if (retval) {
- dev_err(&pdev->dev, ": failed to add character device\n");
- return retval;
+ dev_err(dev, ": failed to add character device\n");
+ goto cdev_add_failed;
}

- ec->vdev = device_create(cros_class, NULL, devno, ec,
- CROS_EC_DEV_NAME);
- if (IS_ERR(ec->vdev)) {
- retval = PTR_ERR(ec->vdev);
- dev_err(&pdev->dev, ": failed to create device\n");
- cdev_del(&ec->cdev);
- return retval;
+ /*
+ * Add the class device
+ * Link to the character device for creating the /dev entry
+ * in devtmpfs.
+ */
+ ec->class_dev.devt = ec->cdev.dev;
+ ec->class_dev.class = &cros_class;
+ ec->class_dev.parent = dev;
+ ec->class_dev.release = __remove;
+
+ retval = dev_set_name(&ec->class_dev, "%s", ec_platform->ec_name);
+ if (retval) {
+ dev_err(dev, "dev_set_name failed => %d\n", retval);
+ goto set_named_failed;
}

- /* Initialize extra interfaces */
- ec_dev_sysfs_init(ec);
- ec_dev_lightbar_init(ec);
+ retval = device_add(&ec->class_dev);
+ if (retval) {
+ dev_err(dev, "device_register failed => %d\n", retval);
+ goto dev_reg_failed;
+ }

return 0;
+
+dev_reg_failed:
+set_named_failed:
+ dev_set_drvdata(dev, NULL);
+ cdev_del(&ec->cdev);
+cdev_add_failed:
+ kfree(ec);
+ return retval;
}

static int ec_device_remove(struct platform_device *pdev)
{
- struct cros_ec_device *ec = dev_get_drvdata(pdev->dev.parent);
-
- ec_dev_lightbar_remove(ec);
- ec_dev_sysfs_remove(ec);
- device_destroy(cros_class, MKDEV(ec_major, 0));
+ struct cros_ec_dev *ec = dev_get_drvdata(&pdev->dev);
cdev_del(&ec->cdev);
+ device_unregister(&ec->class_dev);
return 0;
}

@@ -254,10 +306,10 @@ static int __init cros_ec_dev_init(void)
int ret;
dev_t dev = 0;

- cros_class = class_create(THIS_MODULE, "chromeos");
- if (IS_ERR(cros_class)) {
+ ret = class_register(&cros_class);
+ if (ret) {
pr_err(CROS_EC_DEV_NAME ": failed to register device class\n");
- return PTR_ERR(cros_class);
+ return ret;
}

/* Get a range of minor numbers (starting with 0) to work with */
@@ -279,7 +331,7 @@ static int __init cros_ec_dev_init(void)
failed_devreg:
unregister_chrdev_region(MKDEV(ec_major, 0), CROS_MAX_DEV);
failed_chrdevreg:
- class_destroy(cros_class);
+ class_unregister(&cros_class);
return ret;
}

@@ -287,7 +339,7 @@ static void __exit cros_ec_dev_exit(void)
{
platform_driver_unregister(&cros_ec_dev_driver);
unregister_chrdev(ec_major, CROS_EC_DEV_NAME);
- class_destroy(cros_class);
+ class_unregister(&cros_class);
}

module_init(cros_ec_dev_init);
diff --git a/drivers/platform/chrome/cros_ec_dev.h b/drivers/platform/chrome/cros_ec_dev.h
index 45d67f7e518c..bfd2c84c3571 100644
--- a/drivers/platform/chrome/cros_ec_dev.h
+++ b/drivers/platform/chrome/cros_ec_dev.h
@@ -24,7 +24,6 @@
#include <linux/types.h>
#include <linux/mfd/cros_ec.h>

-#define CROS_EC_DEV_NAME "cros_ec"
#define CROS_EC_DEV_VERSION "1.0.0"

/*
@@ -44,10 +43,4 @@ struct cros_ec_readmem {
#define CROS_EC_DEV_IOCXCMD _IOWR(CROS_EC_DEV_IOC, 0, struct cros_ec_command)
#define CROS_EC_DEV_IOCRDMEM _IOWR(CROS_EC_DEV_IOC, 1, struct cros_ec_readmem)

-void ec_dev_sysfs_init(struct cros_ec_device *);
-void ec_dev_sysfs_remove(struct cros_ec_device *);
-
-void ec_dev_lightbar_init(struct cros_ec_device *);
-void ec_dev_lightbar_remove(struct cros_ec_device *);
-
#endif /* _CROS_EC_DEV_H_ */
diff --git a/drivers/platform/chrome/cros_ec_lightbar.c b/drivers/platform/chrome/cros_ec_lightbar.c
index 6e1986a2dce1..144e09df9b84 100644
--- a/drivers/platform/chrome/cros_ec_lightbar.c
+++ b/drivers/platform/chrome/cros_ec_lightbar.c
@@ -92,7 +92,7 @@ out:
return ret;
}

-static struct cros_ec_command *alloc_lightbar_cmd_msg(void)
+static struct cros_ec_command *alloc_lightbar_cmd_msg(struct cros_ec_dev *ec)
{
struct cros_ec_command *msg;
int len;
@@ -105,14 +105,14 @@ static struct cros_ec_command *alloc_lightbar_cmd_msg(void)
return NULL;

msg->version = 0;
- msg->command = EC_CMD_LIGHTBAR_CMD;
+ msg->command = EC_CMD_LIGHTBAR_CMD + ec->cmd_offset;
msg->outsize = sizeof(struct ec_params_lightbar);
msg->insize = sizeof(struct ec_response_lightbar);

return msg;
}

-static int get_lightbar_version(struct cros_ec_device *ec,
+static int get_lightbar_version(struct cros_ec_dev *ec,
uint32_t *ver_ptr, uint32_t *flg_ptr)
{
struct ec_params_lightbar *param;
@@ -120,13 +120,13 @@ static int get_lightbar_version(struct cros_ec_device *ec,
struct cros_ec_command *msg;
int ret;

- msg = alloc_lightbar_cmd_msg();
+ msg = alloc_lightbar_cmd_msg(ec);
if (!msg)
return 0;

param = (struct ec_params_lightbar *)msg->data;
param->cmd = LIGHTBAR_CMD_VERSION;
- ret = cros_ec_cmd_xfer(ec, msg);
+ ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
if (ret < 0) {
ret = 0;
goto exit;
@@ -165,7 +165,8 @@ static ssize_t version_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
uint32_t version = 0, flags = 0;
- struct cros_ec_device *ec = dev_get_drvdata(dev);
+ struct cros_ec_dev *ec = container_of(dev,
+ struct cros_ec_dev, class_dev);
int ret;

ret = lb_throttle();
@@ -187,12 +188,13 @@ static ssize_t brightness_store(struct device *dev,
struct cros_ec_command *msg;
int ret;
unsigned int val;
- struct cros_ec_device *ec = dev_get_drvdata(dev);
+ struct cros_ec_dev *ec = container_of(dev,
+ struct cros_ec_dev, class_dev);

if (kstrtouint(buf, 0, &val))
return -EINVAL;

- msg = alloc_lightbar_cmd_msg();
+ msg = alloc_lightbar_cmd_msg(ec);
if (!msg)
return -ENOMEM;

@@ -203,7 +205,7 @@ static ssize_t brightness_store(struct device *dev,
if (ret)
goto exit;

- ret = cros_ec_cmd_xfer(ec, msg);
+ ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
if (ret < 0)
goto exit;

@@ -231,11 +233,12 @@ static ssize_t led_rgb_store(struct device *dev, struct device_attribute *attr,
{
struct ec_params_lightbar *param;
struct cros_ec_command *msg;
- struct cros_ec_device *ec = dev_get_drvdata(dev);
+ struct cros_ec_dev *ec = container_of(dev,
+ struct cros_ec_dev, class_dev);
unsigned int val[4];
int ret, i = 0, j = 0, ok = 0;

- msg = alloc_lightbar_cmd_msg();
+ msg = alloc_lightbar_cmd_msg(ec);
if (!msg)
return -ENOMEM;

@@ -268,7 +271,7 @@ static ssize_t led_rgb_store(struct device *dev, struct device_attribute *attr,
return ret;
}

- ret = cros_ec_cmd_xfer(ec, msg);
+ ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
if (ret < 0)
goto exit;

@@ -304,9 +307,10 @@ static ssize_t sequence_show(struct device *dev,
struct ec_response_lightbar *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 = alloc_lightbar_cmd_msg();
+ msg = alloc_lightbar_cmd_msg(ec);
if (!msg)
return -ENOMEM;

@@ -316,7 +320,7 @@ static ssize_t sequence_show(struct device *dev,
if (ret)
goto exit;

- ret = cros_ec_cmd_xfer(ec, msg);
+ ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
if (ret < 0)
goto exit;

@@ -345,9 +349,10 @@ static ssize_t sequence_store(struct device *dev, struct device_attribute *attr,
struct cros_ec_command *msg;
unsigned int num;
int ret, len;
- struct cros_ec_device *ec = dev_get_drvdata(dev);
+ struct cros_ec_dev *ec = container_of(dev,
+ struct cros_ec_dev, class_dev);

- msg = alloc_lightbar_cmd_msg();
+ msg = alloc_lightbar_cmd_msg(ec);
if (!msg)
return -ENOMEM;

@@ -372,7 +377,7 @@ static ssize_t sequence_store(struct device *dev, struct device_attribute *attr,
if (ret)
return ret;

- ret = cros_ec_cmd_xfer(ec, msg);
+ ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
if (ret < 0)
return ret;

@@ -397,25 +402,27 @@ static struct attribute *__lb_cmds_attrs[] = {
&dev_attr_sequence.attr,
NULL,
};
-static struct attribute_group lb_cmds_attr_group = {
- .name = "lightbar",
- .attrs = __lb_cmds_attrs,
-};

-void ec_dev_lightbar_init(struct cros_ec_device *ec)
+static umode_t cros_ec_lightbar_attrs_are_visible(struct kobject *kobj,
+ struct attribute *a, int n)
{
- int ret = 0;
+ struct device *dev = container_of(kobj, struct device, kobj);
+ struct cros_ec_dev *ec = container_of(dev,
+ struct cros_ec_dev, class_dev);
+ struct platform_device *pdev = container_of(ec->dev,
+ struct platform_device, dev);
+ if (pdev->id != 0)
+ return 0;

/* Only instantiate this stuff if the EC has a lightbar */
- if (!get_lightbar_version(ec, NULL, NULL))
- return;
-
- ret = sysfs_create_group(&ec->vdev->kobj, &lb_cmds_attr_group);
- if (ret)
- pr_warn("sysfs_create_group() failed: %d\n", ret);
+ if (get_lightbar_version(ec, NULL, NULL))
+ return a->mode;
+ else
+ return 0;
}

-void ec_dev_lightbar_remove(struct cros_ec_device *ec)
-{
- sysfs_remove_group(&ec->vdev->kobj, &lb_cmds_attr_group);
-}
+struct attribute_group cros_ec_lightbar_attr_group = {
+ .name = "lightbar",
+ .attrs = __lb_cmds_attrs,
+ .is_visible = cros_ec_lightbar_attrs_are_visible,
+};
diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index 92b633324aaa..f9a245465fd0 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -273,7 +273,6 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)

platform_set_drvdata(pdev, ec_dev);
ec_dev->dev = dev;
- ec_dev->ec_name = pdev->name;
ec_dev->phys_name = dev_name(dev);
ec_dev->cmd_xfer = cros_ec_cmd_xfer_lpc;
ec_dev->pkt_xfer = cros_ec_pkt_xfer_lpc;
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);
-}
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index 92e13aaa450c..da72671a42fa 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -17,10 +17,14 @@
#define __LINUX_MFD_CROS_EC_H

#include <linux/cdev.h>
+#include <linux/device.h>
#include <linux/notifier.h>
#include <linux/mfd/cros_ec_commands.h>
#include <linux/mutex.h>

+#define CROS_EC_DEV_NAME "cros_ec"
+#define CROS_EC_DEV_PD_NAME "cros_pd"
+
/*
* The EC is unresponsive for a time after a reboot command. Add a
* simple delay to make sure that the bus stays locked.
@@ -71,11 +75,8 @@ struct cros_ec_command {
/**
* struct cros_ec_device - Information about a ChromeOS EC device
*
- * @ec_name: name of EC device (e.g. 'chromeos-ec')
* @phys_name: name of physical comms layer (e.g. 'i2c-4')
* @dev: Device pointer for physical comms device
- * @vdev: Device pointer for virtual comms device
- * @cdev: Character device structure for virtual comms device
* @was_wake_device: true if this device was set to wake the system from
* sleep at the last suspend
* @cmd_readmem: direct read of the EC memory-mapped region, if supported
@@ -87,6 +88,7 @@ struct cros_ec_command {
*
* @priv: Private data
* @irq: Interrupt to use
+ * @id: Device id
* @din: input buffer (for data from EC)
* @dout: output buffer (for data to EC)
* \note
@@ -109,11 +111,8 @@ struct cros_ec_command {
struct cros_ec_device {

/* These are used by other drivers that want to talk to the EC */
- const char *ec_name;
const char *phys_name;
struct device *dev;
- struct device *vdev;
- struct cdev cdev;
bool was_wake_device;
struct class *cros_class;
int (*cmd_readmem)(struct cros_ec_device *ec, unsigned int offset,
@@ -138,6 +137,35 @@ struct cros_ec_device {
struct mutex lock;
};

+/* struct cros_ec_platform - ChromeOS EC platform information
+ *
+ * @ec_name: name of EC device (e.g. 'cros-ec', 'cros-pd', ...)
+ * used in /dev/ and sysfs.
+ * @cmd_offset: offset to apply for each command. Set when
+ * registering a devicde behind another one.
+ */
+struct cros_ec_platform {
+ const char *ec_name;
+ u16 cmd_offset;
+};
+
+/*
+ * struct cros_ec_dev - ChromeOS EC device entry point
+ *
+ * @class_dev: Device structure used in sysfs
+ * @cdev: Character device structure in /dev
+ * @ec_dev: cros_ec_device structure to talk to the physical device
+ * @dev: pointer to the platform device
+ * @cmd_offset: offset to apply for each command.
+ */
+struct cros_ec_dev {
+ struct device class_dev;
+ struct cdev cdev;
+ struct cros_ec_device *ec_dev;
+ struct device *dev;
+ u16 cmd_offset;
+};
+
/**
* cros_ec_suspend - Handle a suspend operation for the ChromeOS EC device
*
@@ -224,4 +252,8 @@ int cros_ec_register(struct cros_ec_device *ec_dev);
*/
int cros_ec_query_all(struct cros_ec_device *ec_dev);

+/* sysfs stuff */
+extern struct attribute_group cros_ec_attr_group;
+extern struct attribute_group cros_ec_lightbar_attr_group;
+
#endif /* __LINUX_MFD_CROS_EC_H */
--
2.1.4

2015-06-02 08:13:58

by Javier Martinez Canillas

[permalink] [raw]
Subject: [PATCH v4 7/8] mfd: cros_ec: spi: Add a DT property to delay asserting the CS

From: Alexandru M Stan <[email protected]>

Some ECs need a little time for waking up before they can accept
SPI data at a high speed. Add a "google,cros-ec-spi-pre-delay"
property to the DT binding to configure this.

If this property isn't set, then no delay will be added. However,
if set it will cause a delay equal to the value passed to it to
be inserted at the beginning of a transaction.

Signed-off-by: Alexandru M Stan <[email protected]>
Reviewed-by: Doug Anderson <[email protected]>
Signed-off-by: Chris Zhong <[email protected]>
Signed-off-by: Javier Martinez Canillas <[email protected]>
Tested-by: Heiko Stuebner <[email protected]>
Acked-by: Lee Jones <[email protected]>
---

Changes since v3:
- Split DT binding and driver change as suggested by Lee Jones.
- Add tested-by tag from Heiko Stuebner
- Add acked-by tag from Lee Jones.

Changes since v2: None

Changes since v1: None, new patch
---
Documentation/devicetree/bindings/mfd/cros-ec.txt | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/cros-ec.txt b/Documentation/devicetree/bindings/mfd/cros-ec.txt
index 8009c3d87f33..1777916e9e28 100644
--- a/Documentation/devicetree/bindings/mfd/cros-ec.txt
+++ b/Documentation/devicetree/bindings/mfd/cros-ec.txt
@@ -18,6 +18,10 @@ Required properties (SPI):
- reg: SPI chip select

Optional properties (SPI):
+- google,cros-ec-spi-pre-delay: Some implementations of the EC need a little
+ time to wake up from sleep before they can receive SPI transfers at a high
+ clock rate. This property specifies the delay, in usecs, between the
+ assertion of the CS to the start of the first clock pulse.
- google,cros-ec-spi-msg-delay: Some implementations of the EC require some
additional processing time in order to accept new transactions. If the delay
between transactions is not long enough the EC may not be able to respond
--
2.1.4

2015-06-02 08:13:53

by Javier Martinez Canillas

[permalink] [raw]
Subject: [PATCH v4 8/8] mfd: cros_ec: spi: Add delay for asserting CS

From: Alexandru M Stan <[email protected]>

Some ECs need a little time for waking up before they can accept
SPI data at a high speed. This is configurable via a DT property
"google,cros-ec-spi-pre-delay".

Use this DT property to cause a delay before the beginning of a
SPI transaction to make sure that the EC has already woken up.

Signed-off-by: Alexandru M Stan <[email protected]>
Reviewed-by: Doug Anderson <[email protected]>
Signed-off-by: Chris Zhong <[email protected]>
Signed-off-by: Javier Martinez Canillas <[email protected]>
Tested-by: Heiko Stuebner <[email protected]>
Acked-by: Lee Jones <[email protected]>
---

Changes since v3:
- New patch, split DT binding and driver implementation as suggested
by Lee Jones.
- Add tested-by tag from Heiko Stuebner.
- Add acked-by tag from Lee Jones.
---
drivers/mfd/cros_ec_spi.c | 21 +++++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
index faba03e2f1ef..16f228dc243f 100644
--- a/drivers/mfd/cros_ec_spi.c
+++ b/drivers/mfd/cros_ec_spi.c
@@ -71,12 +71,15 @@
* @spi: SPI device we are connected to
* @last_transfer_ns: time that we last finished a transfer, or 0 if there
* if no record
+ * @start_of_msg_delay: used to set the delay_usecs on the spi_transfer that
+ * is sent when we want to turn on CS at the start of a transaction.
* @end_of_msg_delay: used to set the delay_usecs on the spi_transfer that
* is sent when we want to turn off CS at the end of a transaction.
*/
struct cros_ec_spi {
struct spi_device *spi;
s64 last_transfer_ns;
+ unsigned int start_of_msg_delay;
unsigned int end_of_msg_delay;
};

@@ -366,7 +369,7 @@ static int cros_ec_pkt_xfer_spi(struct cros_ec_device *ec_dev,
struct ec_host_request *request;
struct ec_host_response *response;
struct cros_ec_spi *ec_spi = ec_dev->priv;
- struct spi_transfer trans;
+ struct spi_transfer trans, trans_delay;
struct spi_message msg;
int i, len;
u8 *ptr;
@@ -393,13 +396,23 @@ static int cros_ec_pkt_xfer_spi(struct cros_ec_device *ec_dev,
goto exit;
}

+ /*
+ * Leave a gap between CS assertion and clocking of data to allow the
+ * EC time to wakeup.
+ */
+ spi_message_init(&msg);
+ if (ec_spi->start_of_msg_delay) {
+ memset(&trans_delay, 0, sizeof(trans_delay));
+ trans_delay.delay_usecs = ec_spi->start_of_msg_delay;
+ spi_message_add_tail(&trans_delay, &msg);
+ }
+
/* Transmit phase - send our message */
memset(&trans, 0, sizeof(trans));
trans.tx_buf = ec_dev->dout;
trans.rx_buf = rx_buf;
trans.len = len;
trans.cs_change = 1;
- spi_message_init(&msg);
spi_message_add_tail(&trans, &msg);
ret = spi_sync(ec_spi->spi, &msg);

@@ -602,6 +615,10 @@ static void cros_ec_spi_dt_probe(struct cros_ec_spi *ec_spi, struct device *dev)
u32 val;
int ret;

+ ret = of_property_read_u32(np, "google,cros-ec-spi-pre-delay", &val);
+ if (!ret)
+ ec_spi->start_of_msg_delay = val;
+
ret = of_property_read_u32(np, "google,cros-ec-spi-msg-delay", &val);
if (!ret)
ec_spi->end_of_msg_delay = val;
--
2.1.4

2015-06-02 21:16:11

by Heiko Stuebner

[permalink] [raw]
Subject: Re: [PATCH v4 0/8] mfd: cros_ec: Add multi EC and proto v3 support

Am Dienstag, 2. Juni 2015, 10:11:03 schrieb Javier Martinez Canillas:
> Hello,
>
> Newer Chromebooks have more than one Embedded Controller (EC) in the
> system. These additional ECs are connected through I2C with a host EC
> which is the one that is connected to the Application Processor (AP)
> through different transports (I2C, SPI or LPC).
>
> So on these platforms, sub-processors are chained to each other:
>
> AP <--> Host EC <--> Power Delivery (PD) EC
>
> The AP sends commands to the additional EC through the host EC using
> a set of passthru commands and the host redirects to the correct EC.
>
> This is a v4 of a series that adds support for multiple EC in a system
> and also for the protocol version 3 that is used on newer ECs.
>
> Most patches were taken from the downstream ChromiumOS v3.14 tree with
> fixes squashed, split to minimise the cross subsystem churn and changes
> for mainline inclusion but were not modified functionality wise.
>
> This version addresses a lot of issues pointed out by Lee Jones on the v3
> posted before [0].
>
> The patches are based on top of "[PATCH 0/2] mfd: cros_ec: Small cleanups"
> [1] that were posted before and was already picked by Lee Jones.
>
> Testing was done on some Chromebooks that have a single EC and support
> protocol v2 such as the Exynos5250 Snow, Exynos5420 Peach Pit and Exynos5800
> Peach Pi to be sure that no regressions were introduced for these machines.

I just gave this a try on veyron and everything still works as expected.

All patches except "[PATCH v4 6/8] mfd: cros_ec: Support multiple EC in a
system" already have Tested-by tags, so this patch now is also

Tested-by: Heiko Stuebner <[email protected]>


Heiko

2015-06-03 06:45:50

by Javier Martinez Canillas

[permalink] [raw]
Subject: Re: [PATCH v4 0/8] mfd: cros_ec: Add multi EC and proto v3 support

Hello Heiko,

On 06/02/2015 11:15 PM, Heiko St?bner wrote:
> Am Dienstag, 2. Juni 2015, 10:11:03 schrieb Javier Martinez Canillas:
>> Hello,
>>
>> Newer Chromebooks have more than one Embedded Controller (EC) in the
>> system. These additional ECs are connected through I2C with a host EC
>> which is the one that is connected to the Application Processor (AP)
>> through different transports (I2C, SPI or LPC).
>>
>> So on these platforms, sub-processors are chained to each other:
>>
>> AP <--> Host EC <--> Power Delivery (PD) EC
>>
>> The AP sends commands to the additional EC through the host EC using
>> a set of passthru commands and the host redirects to the correct EC.
>>
>> This is a v4 of a series that adds support for multiple EC in a system
>> and also for the protocol version 3 that is used on newer ECs.
>>
>> Most patches were taken from the downstream ChromiumOS v3.14 tree with
>> fixes squashed, split to minimise the cross subsystem churn and changes
>> for mainline inclusion but were not modified functionality wise.
>>
>> This version addresses a lot of issues pointed out by Lee Jones on the v3
>> posted before [0].
>>
>> The patches are based on top of "[PATCH 0/2] mfd: cros_ec: Small cleanups"
>> [1] that were posted before and was already picked by Lee Jones.
>>
>> Testing was done on some Chromebooks that have a single EC and support
>> protocol v2 such as the Exynos5250 Snow, Exynos5420 Peach Pit and Exynos5800
>> Peach Pi to be sure that no regressions were introduced for these machines.
>
> I just gave this a try on veyron and everything still works as expected.
>
> All patches except "[PATCH v4 6/8] mfd: cros_ec: Support multiple EC in a
> system" already have Tested-by tags, so this patch now is also
>
> Tested-by: Heiko Stuebner <[email protected]>
>

Thanks a lot for testing the series again.

Now let's wait for Olof to review/ack/provide feedback on patches #1-#6
that touches drivers/platform/chrome, so Lee can merge through his tree.

>
> Heiko
>

Best regards,
Javier

2015-06-03 08:53:58

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v4 6/8] mfd: cros_ec: Support multiple EC in a system

On Tue, 02 Jun 2015, Javier Martinez Canillas wrote:

> From: Gwendal Grignou <[email protected]>
>
> Chromebooks can have more than one Embedded Controller so the
> cros_ec device id has to be incremented for each EC registered.
>
> Add code to handle multiple EC. First ec found is cros-ec0,
> second cros-ec1 and so on.
>
> 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 <[email protected]>
> Reviewed-by: Dmitry Torokhov <[email protected]>
> Signed-off-by: Javier Martinez Canillas <[email protected]>
> ---
>
> Changes since v3:
> - Add defines for the EC and PD index constants.
> - Remove cros_ec_dev_register() and declare the mfd_cells as static structs.
> Suggested by Lee Jones.
> - Add a new line before the return statement in cros_ec_dev_register().
> Suggested by Lee Jones.
>
> Changes since v2: None
>
> Changes since v1:
> - Squash patch that adds support to represent EC's as different
> char devices (e.g: /dev/cros_ec, /dev/cros_pd):
> https://chromium-review.googlesource.com/#/c/217297/
> Suggested by Gwendal Grignou
> - Use cros_ec instead of cros-ec in the subject line to be consistent.
> Suggested by Gwendal Grignou
> ---
> drivers/input/keyboard/cros_ec_keyb.c | 2 +-
> drivers/mfd/cros_ec.c | 59 +++++++++++--
> drivers/mfd/cros_ec_i2c.c | 1 -
> drivers/mfd/cros_ec_spi.c | 1 -
> drivers/platform/chrome/cros_ec_dev.c | 128 ++++++++++++++++++++---------
> drivers/platform/chrome/cros_ec_dev.h | 7 --
> drivers/platform/chrome/cros_ec_lightbar.c | 75 +++++++++--------
> drivers/platform/chrome/cros_ec_lpc.c | 1 -
> drivers/platform/chrome/cros_ec_sysfs.c | 48 +++++------
> include/linux/mfd/cros_ec.h | 44 ++++++++--
> 10 files changed, 240 insertions(+), 126 deletions(-)
>
> diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
> index 974154a74505..b01966dc7eb3 100644
> --- a/drivers/input/keyboard/cros_ec_keyb.c
> +++ b/drivers/input/keyboard/cros_ec_keyb.c
> @@ -275,7 +275,7 @@ static int cros_ec_keyb_probe(struct platform_device *pdev)
> ckdev->dev = dev;
> dev_set_drvdata(&pdev->dev, ckdev);
>
> - idev->name = ec->ec_name;
> + idev->name = CROS_EC_DEV_NAME;
> idev->phys = ec->phys_name;
> __set_bit(EV_REP, idev->evbit);
>
> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
> index 08d82bfc5268..4fbb4ce8f81e 100644
> --- a/drivers/mfd/cros_ec.c
> +++ b/drivers/mfd/cros_ec.c
> @@ -24,11 +24,29 @@
> #include <linux/mfd/core.h>
> #include <linux/mfd/cros_ec.h>
>
> -static const struct mfd_cell cros_devs[] = {
> - {
> - .name = "cros-ec-ctl",
> - .id = PLATFORM_DEVID_AUTO,
> - },
> +#define CROS_EC_DEV_EC_INDEX 0
> +#define CROS_EC_DEV_PD_INDEX 1
> +
> +struct cros_ec_platform ec_p = {
> + .cmd_offset = EC_CMD_PASSTHRU_OFFSET(CROS_EC_DEV_EC_INDEX),
> +};
> +
> +struct cros_ec_platform pd_p = {
> + .cmd_offset = EC_CMD_PASSTHRU_OFFSET(CROS_EC_DEV_PD_INDEX),
> +};
> +
> +struct mfd_cell ec_cell = {
> + .name = "cros-ec-ctl",
> + .id = PLATFORM_DEVID_AUTO,
> + .platform_data = &ec_p,
> + .pdata_size = sizeof(ec_p),
> +};
> +
> +struct mfd_cell ec_pd_cell = {
> + .name = "cros-ec-ctl",
> + .id = PLATFORM_DEVID_AUTO,
> + .platform_data = &pd_p,
> + .pdata_size = sizeof(pd_p),
> };
>
> int cros_ec_register(struct cros_ec_device *ec_dev)
> @@ -52,14 +70,39 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
>
> cros_ec_query_all(ec_dev);
>
> - err = mfd_add_devices(dev, 0, cros_devs,
> - ARRAY_SIZE(cros_devs),
> + if (IS_ENABLED(CONFIG_OF) && dev->of_node)
> + ec_p.ec_name = of_get_property(dev->of_node, "devname",
> + NULL);

Has this binding already been accepted? We don't normally allow
"device name"properties in DT.

> + if (!ec_p.ec_name)
> + ec_p.ec_name = CROS_EC_DEV_NAME;
> +
> + err = mfd_add_devices(ec_dev->dev, CROS_EC_DEV_EC_INDEX, &ec_cell, 1,
> NULL, ec_dev->irq, NULL);

Take a look at how 'id' (second argument) is handled in mfd-core, then
try to figure out why what you're doing is not correct.

> if (err) {
> - dev_err(dev, "failed to add mfd devices\n");
> + dev_err(dev, "failed to add ec\n");
> return err;
> }

[...]

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

2015-06-03 10:12:33

by Javier Martinez Canillas

[permalink] [raw]
Subject: Re: [PATCH v4 6/8] mfd: cros_ec: Support multiple EC in a system

Hello Lee,

Thanks a lot for your feedback.

On 06/03/2015 10:53 AM, Lee Jones wrote:
> On Tue, 02 Jun 2015, Javier Martinez Canillas wrote:
>
>> From: Gwendal Grignou <[email protected]>
>>
>> Chromebooks can have more than one Embedded Controller so the
>> cros_ec device id has to be incremented for each EC registered.
>>
>> Add code to handle multiple EC. First ec found is cros-ec0,
>> second cros-ec1 and so on.
>>
>> 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 <[email protected]>
>> Reviewed-by: Dmitry Torokhov <[email protected]>
>> Signed-off-by: Javier Martinez Canillas <[email protected]>
>> ---
>>
>> Changes since v3:
>> - Add defines for the EC and PD index constants.
>> - Remove cros_ec_dev_register() and declare the mfd_cells as static structs.
>> Suggested by Lee Jones.
>> - Add a new line before the return statement in cros_ec_dev_register().
>> Suggested by Lee Jones.
>>
>> Changes since v2: None
>>
>> Changes since v1:
>> - Squash patch that adds support to represent EC's as different
>> char devices (e.g: /dev/cros_ec, /dev/cros_pd):
>> https://chromium-review.googlesource.com/#/c/217297/
>> Suggested by Gwendal Grignou
>> - Use cros_ec instead of cros-ec in the subject line to be consistent.
>> Suggested by Gwendal Grignou
>> ---
>> drivers/input/keyboard/cros_ec_keyb.c | 2 +-
>> drivers/mfd/cros_ec.c | 59 +++++++++++--
>> drivers/mfd/cros_ec_i2c.c | 1 -
>> drivers/mfd/cros_ec_spi.c | 1 -
>> drivers/platform/chrome/cros_ec_dev.c | 128 ++++++++++++++++++++---------
>> drivers/platform/chrome/cros_ec_dev.h | 7 --
>> drivers/platform/chrome/cros_ec_lightbar.c | 75 +++++++++--------
>> drivers/platform/chrome/cros_ec_lpc.c | 1 -
>> drivers/platform/chrome/cros_ec_sysfs.c | 48 +++++------
>> include/linux/mfd/cros_ec.h | 44 ++++++++--
>> 10 files changed, 240 insertions(+), 126 deletions(-)
>>
>> diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
>> index 974154a74505..b01966dc7eb3 100644
>> --- a/drivers/input/keyboard/cros_ec_keyb.c
>> +++ b/drivers/input/keyboard/cros_ec_keyb.c
>> @@ -275,7 +275,7 @@ static int cros_ec_keyb_probe(struct platform_device *pdev)
>> ckdev->dev = dev;
>> dev_set_drvdata(&pdev->dev, ckdev);
>>
>> - idev->name = ec->ec_name;
>> + idev->name = CROS_EC_DEV_NAME;
>> idev->phys = ec->phys_name;
>> __set_bit(EV_REP, idev->evbit);
>>
>> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
>> index 08d82bfc5268..4fbb4ce8f81e 100644
>> --- a/drivers/mfd/cros_ec.c
>> +++ b/drivers/mfd/cros_ec.c
>> @@ -24,11 +24,29 @@
>> #include <linux/mfd/core.h>
>> #include <linux/mfd/cros_ec.h>
>>
>> -static const struct mfd_cell cros_devs[] = {
>> - {
>> - .name = "cros-ec-ctl",
>> - .id = PLATFORM_DEVID_AUTO,
>> - },
>> +#define CROS_EC_DEV_EC_INDEX 0
>> +#define CROS_EC_DEV_PD_INDEX 1
>> +
>> +struct cros_ec_platform ec_p = {
>> + .cmd_offset = EC_CMD_PASSTHRU_OFFSET(CROS_EC_DEV_EC_INDEX),
>> +};
>> +
>> +struct cros_ec_platform pd_p = {
>> + .cmd_offset = EC_CMD_PASSTHRU_OFFSET(CROS_EC_DEV_PD_INDEX),
>> +};
>> +
>> +struct mfd_cell ec_cell = {
>> + .name = "cros-ec-ctl",
>> + .id = PLATFORM_DEVID_AUTO,
>> + .platform_data = &ec_p,
>> + .pdata_size = sizeof(ec_p),
>> +};
>> +
>> +struct mfd_cell ec_pd_cell = {
>> + .name = "cros-ec-ctl",
>> + .id = PLATFORM_DEVID_AUTO,
>> + .platform_data = &pd_p,
>> + .pdata_size = sizeof(pd_p),
>> };
>>
>> int cros_ec_register(struct cros_ec_device *ec_dev)
>> @@ -52,14 +70,39 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
>>
>> cros_ec_query_all(ec_dev);
>>
>> - err = mfd_add_devices(dev, 0, cros_devs,
>> - ARRAY_SIZE(cros_devs),
>> + if (IS_ENABLED(CONFIG_OF) && dev->of_node)
>> + ec_p.ec_name = of_get_property(dev->of_node, "devname",
>> + NULL);
>
> Has this binding already been accepted? We don't normally allow
> "device name"properties in DT.
>

Sigh, I now noticed that I missed the DT binding change when originally
forward porting the patches from the downstream ChromiumOS tree. I'm so
sorry about that.

ChromiumOS tree has in Documentation/devicetree/bindings/mfd/cros-ec.txt:

Optional properties (All):
- devname: name of the EC. Can be based on its function: for instance,
for Sensor Hub, 'cros_sh', for Power Delivery MCU 'cros_pd'.
If not present, 'cros_ec' can be assumed.

When you say that normally device names are not allowed in DT, do you mean
that "devname" is a too generic property name? (which I totally agree) or
that DT bindings shouldn't have a property to change a device's name for
something more descriptive?

Because for example, the regulator bindings has a "regulator-name" property.
If you agree I can change the property to "google,cros-ec-name" which should
be more precise.

>> + if (!ec_p.ec_name)
>> + ec_p.ec_name = CROS_EC_DEV_NAME;
>> +
>> + err = mfd_add_devices(ec_dev->dev, CROS_EC_DEV_EC_INDEX, &ec_cell, 1,
>> NULL, ec_dev->irq, NULL);
>
> Take a look at how 'id' (second argument) is handled in mfd-core, then
> try to figure out why what you're doing is not correct.
>

I see, so I should not set the struct mfd_cell .id to PLATFORM_DEVID_AUTO and
just pass PLATFORM_DEVID_AUTO to mfd_add_devices() so platform_device_add()
automatically allocates a device ID and sets pdev->id_auto = true.

It was only working because CROS_EC_DEV_EC_INDEX was zero so id + cell->id
was still PLATFORM_DEVID_AUTO but for CROS_EC_DEV_PD_INDEX, it was set to
PLATFORM_DEVID_NONE...

I'll change that, thanks for pointing it out.

>> if (err) {
>> - dev_err(dev, "failed to add mfd devices\n");
>> + dev_err(dev, "failed to add ec\n");
>> return err;
>> }
>
> [...]
>

Best regards,
Javier

2015-06-03 11:27:53

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v4 6/8] mfd: cros_ec: Support multiple EC in a system

On Wed, 03 Jun 2015, Javier Martinez Canillas wrote:

> Hello Lee,
>
> Thanks a lot for your feedback.
>
> On 06/03/2015 10:53 AM, Lee Jones wrote:
> > On Tue, 02 Jun 2015, Javier Martinez Canillas wrote:
> >
> >> From: Gwendal Grignou <[email protected]>
> >>
> >> Chromebooks can have more than one Embedded Controller so the
> >> cros_ec device id has to be incremented for each EC registered.
> >>
> >> Add code to handle multiple EC. First ec found is cros-ec0,
> >> second cros-ec1 and so on.
> >>
> >> 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 <[email protected]>
> >> Reviewed-by: Dmitry Torokhov <[email protected]>
> >> Signed-off-by: Javier Martinez Canillas <[email protected]>
> >> ---
> >>
> >> Changes since v3:
> >> - Add defines for the EC and PD index constants.
> >> - Remove cros_ec_dev_register() and declare the mfd_cells as static structs.
> >> Suggested by Lee Jones.
> >> - Add a new line before the return statement in cros_ec_dev_register().
> >> Suggested by Lee Jones.
> >>
> >> Changes since v2: None
> >>
> >> Changes since v1:
> >> - Squash patch that adds support to represent EC's as different
> >> char devices (e.g: /dev/cros_ec, /dev/cros_pd):
> >> https://chromium-review.googlesource.com/#/c/217297/
> >> Suggested by Gwendal Grignou
> >> - Use cros_ec instead of cros-ec in the subject line to be consistent.
> >> Suggested by Gwendal Grignou
> >> ---
> >> drivers/input/keyboard/cros_ec_keyb.c | 2 +-
> >> drivers/mfd/cros_ec.c | 59 +++++++++++--
> >> drivers/mfd/cros_ec_i2c.c | 1 -
> >> drivers/mfd/cros_ec_spi.c | 1 -
> >> drivers/platform/chrome/cros_ec_dev.c | 128 ++++++++++++++++++++---------
> >> drivers/platform/chrome/cros_ec_dev.h | 7 --
> >> drivers/platform/chrome/cros_ec_lightbar.c | 75 +++++++++--------
> >> drivers/platform/chrome/cros_ec_lpc.c | 1 -
> >> drivers/platform/chrome/cros_ec_sysfs.c | 48 +++++------
> >> include/linux/mfd/cros_ec.h | 44 ++++++++--
> >> 10 files changed, 240 insertions(+), 126 deletions(-)
> >>
> >> diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
> >> index 974154a74505..b01966dc7eb3 100644
> >> --- a/drivers/input/keyboard/cros_ec_keyb.c
> >> +++ b/drivers/input/keyboard/cros_ec_keyb.c
> >> @@ -275,7 +275,7 @@ static int cros_ec_keyb_probe(struct platform_device *pdev)
> >> ckdev->dev = dev;
> >> dev_set_drvdata(&pdev->dev, ckdev);
> >>
> >> - idev->name = ec->ec_name;
> >> + idev->name = CROS_EC_DEV_NAME;
> >> idev->phys = ec->phys_name;
> >> __set_bit(EV_REP, idev->evbit);
> >>
> >> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
> >> index 08d82bfc5268..4fbb4ce8f81e 100644
> >> --- a/drivers/mfd/cros_ec.c
> >> +++ b/drivers/mfd/cros_ec.c
> >> @@ -24,11 +24,29 @@
> >> #include <linux/mfd/core.h>
> >> #include <linux/mfd/cros_ec.h>
> >>
> >> -static const struct mfd_cell cros_devs[] = {
> >> - {
> >> - .name = "cros-ec-ctl",
> >> - .id = PLATFORM_DEVID_AUTO,
> >> - },
> >> +#define CROS_EC_DEV_EC_INDEX 0
> >> +#define CROS_EC_DEV_PD_INDEX 1
> >> +
> >> +struct cros_ec_platform ec_p = {
> >> + .cmd_offset = EC_CMD_PASSTHRU_OFFSET(CROS_EC_DEV_EC_INDEX),
> >> +};
> >> +
> >> +struct cros_ec_platform pd_p = {
> >> + .cmd_offset = EC_CMD_PASSTHRU_OFFSET(CROS_EC_DEV_PD_INDEX),
> >> +};
> >> +
> >> +struct mfd_cell ec_cell = {
> >> + .name = "cros-ec-ctl",
> >> + .id = PLATFORM_DEVID_AUTO,
> >> + .platform_data = &ec_p,
> >> + .pdata_size = sizeof(ec_p),
> >> +};
> >> +
> >> +struct mfd_cell ec_pd_cell = {
> >> + .name = "cros-ec-ctl",
> >> + .id = PLATFORM_DEVID_AUTO,
> >> + .platform_data = &pd_p,
> >> + .pdata_size = sizeof(pd_p),
> >> };
> >>
> >> int cros_ec_register(struct cros_ec_device *ec_dev)
> >> @@ -52,14 +70,39 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
> >>
> >> cros_ec_query_all(ec_dev);
> >>
> >> - err = mfd_add_devices(dev, 0, cros_devs,
> >> - ARRAY_SIZE(cros_devs),
> >> + if (IS_ENABLED(CONFIG_OF) && dev->of_node)
> >> + ec_p.ec_name = of_get_property(dev->of_node, "devname",
> >> + NULL);
> >
> > Has this binding already been accepted? We don't normally allow
> > "device name"properties in DT.
> >
>
> Sigh, I now noticed that I missed the DT binding change when originally
> forward porting the patches from the downstream ChromiumOS tree. I'm so
> sorry about that.
>
> ChromiumOS tree has in Documentation/devicetree/bindings/mfd/cros-ec.txt:
>
> Optional properties (All):
> - devname: name of the EC. Can be based on its function: for instance,
> for Sensor Hub, 'cros_sh', for Power Delivery MCU 'cros_pd'.
> If not present, 'cros_ec' can be assumed.
>
> When you say that normally device names are not allowed in DT, do you mean
> that "devname" is a too generic property name? (which I totally agree) or
> that DT bindings shouldn't have a property to change a device's name for
> something more descriptive?
>
> Because for example, the regulator bindings has a "regulator-name" property.
> If you agree I can change the property to "google,cros-ec-name" which should
> be more precise.

I mean, no names of devices are allowed in Device Tree:

`git grep name -- arch/arm/boot/dts/ | grep dev`

> >> + if (!ec_p.ec_name)
> >> + ec_p.ec_name = CROS_EC_DEV_NAME;
> >> +
> >> + err = mfd_add_devices(ec_dev->dev, CROS_EC_DEV_EC_INDEX, &ec_cell, 1,
> >> NULL, ec_dev->irq, NULL);
> >
> > Take a look at how 'id' (second argument) is handled in mfd-core, then
> > try to figure out why what you're doing is not correct.
> >
>
> I see, so I should not set the struct mfd_cell .id to PLATFORM_DEVID_AUTO and
> just pass PLATFORM_DEVID_AUTO to mfd_add_devices() so platform_device_add()
> automatically allocates a device ID and sets pdev->id_auto = true.
>
> It was only working because CROS_EC_DEV_EC_INDEX was zero so id + cell->id
> was still PLATFORM_DEVID_AUTO but for CROS_EC_DEV_PD_INDEX, it was set to
> PLATFORM_DEVID_NONE...
>
> I'll change that, thanks for pointing it out.

No problem

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

2015-06-03 11:49:28

by Javier Martinez Canillas

[permalink] [raw]
Subject: Re: [PATCH v4 6/8] mfd: cros_ec: Support multiple EC in a system

Hello Lee,

On 06/03/2015 01:27 PM, Lee Jones wrote:
> On Wed, 03 Jun 2015, Javier Martinez Canillas wrote:

[snip]

>> >>
>> >> - err = mfd_add_devices(dev, 0, cros_devs,
>> >> - ARRAY_SIZE(cros_devs),
>> >> + if (IS_ENABLED(CONFIG_OF) && dev->of_node)
>> >> + ec_p.ec_name = of_get_property(dev->of_node, "devname",
>> >> + NULL);
>> >
>> > Has this binding already been accepted? We don't normally allow
>> > "device name"properties in DT.
>> >
>>
>> Sigh, I now noticed that I missed the DT binding change when originally
>> forward porting the patches from the downstream ChromiumOS tree. I'm so
>> sorry about that.
>>
>> ChromiumOS tree has in Documentation/devicetree/bindings/mfd/cros-ec.txt:
>>
>> Optional properties (All):
>> - devname: name of the EC. Can be based on its function: for instance,
>> for Sensor Hub, 'cros_sh', for Power Delivery MCU 'cros_pd'.
>> If not present, 'cros_ec' can be assumed.
>>
>> When you say that normally device names are not allowed in DT, do you mean
>> that "devname" is a too generic property name? (which I totally agree) or
>> that DT bindings shouldn't have a property to change a device's name for
>> something more descriptive?
>>
>> Because for example, the regulator bindings has a "regulator-name" property.
>> If you agree I can change the property to "google,cros-ec-name" which should
>> be more precise.
>
> I mean, no names of devices are allowed in Device Tree:
>
> `git grep name -- arch/arm/boot/dts/ | grep dev`
>

Ok, but do you agree on having a "google,cros-ec-name" (with no dev on it)
property to define the resource name just like regulators have "regulator-name"
that changes what is shown in /sys/kernel/debug/regulator/$name?

Otherwise all the EC's will be named cros_ec or regardless of their function
and this is shown in both /dev/$name and /sys/class/chromeos/$name.

Best regards,
Javier