[PATCH v2 0/10] Batch of cleanup patches for cros_ec

[PATCH v2 0/10] Batch of cleanup patches for cros_ec

[Doug Anderson]

This is a batch of cleanup patches picked from the ChromeOS 3.8 kernel
tree and applied to ToT.  Most of these patches were authored by Bill
Richardson (CCed).  Where appropriate I've squashed patches together,
though I have erred on the side of keeping patches logically distinct
rather than squashing into one big cleanup patch.

There is very little functionality added by this series, but this gets
us closer to how things look in the ChromeOS tree so we can add more
patches atop it.  In general I took the oldest patches from our tree
and stopped picking when I got to a reasonable patch size (10
patches).  There are about 5 more cleanup patches still in the
ChromeOS tree, then some more major functionality patches.

Note that I didn't take the "cros_ec_dev" userspace inteface, the
"LPC" implementation, the "vboot context" implementation, and patches
relating to exynos5250-spring when picking patches.  These bits are
very separate (and big!) and can be added and debated separately after
we've got cleanup in.  Whenever patches touched those pieces of the
code I ignored that part of the patch.  In general I did take cleanup
code that was intended to make it easier to later add these bits.

I have tested basic functionality of these patches on exynos5250-snow
and exynos5420-peach-pit.

Changes in v2:
- Include example printouts before/after in commit message.
- Removed unneeded "ret" variable.
- Added common function to cros_ec.c
- Changed to dev_dbg() as per http://crosreview.com/66726
- IRQs should be optional => move EC interrupt to keyboard.

Andrew Bresticker (1):
  mfd: cros_ec: move EC interrupt to cros_ec_keyb

Bill Richardson (8):
  mfd: cros_ec: Fix the comment on cros_ec_remove()
  mfd: cros_ec: Allow static din/dout buffers with cros_ec_register()
  mfd: cros_ec: Tweak struct cros_ec_device for clarity
  mfd: cros_ec: Use struct cros_ec_command to communicate with the EC
  mfd: cros_ec: cleanup: remove unused fields from struct cros_ec_device
  mfd: cros_ec: cleanup: Remove EC wrapper functions
  mfd: cros_ec: Check result code from EC messages
  mfd: cros_ec: ec_dev->cmd_xfer() returns number of bytes received from
    EC

Simon Glass (1):
  mdf: cros_ec: Detect in-progress commands

 drivers/i2c/busses/i2c-cros-ec-tunnel.c |  17 +++--
 drivers/input/keyboard/cros_ec_keyb.c   |  70 ++++++++++++--------
 drivers/mfd/cros_ec.c                   |  97 ++++++++--------------------
 drivers/mfd/cros_ec_i2c.c               |  37 +++++------
 drivers/mfd/cros_ec_spi.c               |  36 +++++------
 include/linux/mfd/cros_ec.h             | 110 +++++++++++++++++---------------
 6 files changed, 172 insertions(+), 195 deletions(-)

-- 
2.0.0.526.g5318336

[PATCH v2 01/10] mfd: cros_ec: Fix the comment on cros_ec_remove()

[Doug Anderson]

From: Bill Richardson <wfrichar@chromium.org>

This comment was incorrect, so update it.

Signed-off-by: Bill Richardson <wfrichar@chromium.org>
Signed-off-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Doug Anderson <dianders@chromium.org>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
Changes in v2: None

 include/linux/mfd/cros_ec.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index 887ef4f..7e9fe6e 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -148,8 +148,7 @@ int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
 /**
  * cros_ec_remove - Remove a ChromeOS EC
  *
- * Call this to deregister a ChromeOS EC. After this you should call
- * cros_ec_free().
+ * Call this to deregister a ChromeOS EC, then clean up any private data.
  *
  * @ec_dev: Device to register
  * @return 0 if ok, -ve on error
-- 
2.0.0.526.g5318336

[PATCH v2 02/10] mfd: cros_ec: Allow static din/dout buffers with cros_ec_register()

[Doug Anderson]

From: Bill Richardson <wfrichar@chromium.org>

The lower-level driver may want to provide its own buffers. If so,
there's no need to allocate new ones.  This already happens to work
just fine (since we check for size of 0 and use devm allocation), but
it's good to document it.

[dianders: Resolved conflicts; documented that no code changes needed
on mainline]

Signed-off-by: Bill Richardson <wfrichar@chromium.org>
Signed-off-by: Doug Anderson <dianders@chromium.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
Changes in v2: None

 include/linux/mfd/cros_ec.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index 7e9fe6e..2ee3190 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -68,8 +68,8 @@ struct cros_ec_msg {
  * We use this alignment to keep ARM and x86 happy. Probably word
  * alignment would be OK, there might be a small performance advantage
  * to using dword.
- * @din_size: size of din buffer
- * @dout_size: size of dout buffer
+ * @din_size: size of din buffer to allocate (zero to use static din)
+ * @dout_size: size of dout buffer to allocate (zero to use static dout)
  * @command_send: send a command
  * @command_recv: receive a command
  * @ec_name: name of EC device (e.g. 'chromeos-ec')
-- 
2.0.0.526.g5318336

[PATCH v2 03/10] mfd: cros_ec: Tweak struct cros_ec_device for clarity

[Doug Anderson]

From: Bill Richardson <wfrichar@chromium.org>

The members of struct cros_ec_device were improperly commented, and
intermixed the private and public sections. This is just cleanup to make it
more obvious what goes with what.

[dianders: left lock in the structure but gave it the name that will
eventually be used.]

Signed-off-by: Bill Richardson <wfrichar@chromium.org>
Signed-off-by: Doug Anderson <dianders@chromium.org>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
Changes in v2: None

 drivers/mfd/cros_ec.c       |  2 +-
 drivers/mfd/cros_ec_i2c.c   |  4 +--
 drivers/mfd/cros_ec_spi.c   | 10 +++----
 include/linux/mfd/cros_ec.h | 65 ++++++++++++++++++++++++---------------------
 4 files changed, 43 insertions(+), 38 deletions(-)

diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index 38fe9bf..04e053c 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -57,7 +57,7 @@ static int cros_ec_command_sendrecv(struct cros_ec_device *ec_dev,
 	msg.in_buf = in_buf;
 	msg.in_len = in_len;
 
-	return ec_dev->command_xfer(ec_dev, &msg);
+	return ec_dev->cmd_xfer(ec_dev, &msg);
 }
 
 static int cros_ec_command_recv(struct cros_ec_device *ec_dev,
diff --git a/drivers/mfd/cros_ec_i2c.c b/drivers/mfd/cros_ec_i2c.c
index 4f71be9..777e529 100644
--- a/drivers/mfd/cros_ec_i2c.c
+++ b/drivers/mfd/cros_ec_i2c.c
@@ -29,7 +29,7 @@ static inline struct cros_ec_device *to_ec_dev(struct device *dev)
 	return i2c_get_clientdata(client);
 }
 
-static int cros_ec_command_xfer(struct cros_ec_device *ec_dev,
+static int cros_ec_cmd_xfer_i2c(struct cros_ec_device *ec_dev,
 				struct cros_ec_msg *msg)
 {
 	struct i2c_client *client = ec_dev->priv;
@@ -136,7 +136,7 @@ static int cros_ec_i2c_probe(struct i2c_client *client,
 	ec_dev->dev = dev;
 	ec_dev->priv = client;
 	ec_dev->irq = client->irq;
-	ec_dev->command_xfer = cros_ec_command_xfer;
+	ec_dev->cmd_xfer = cros_ec_cmd_xfer_i2c;
 	ec_dev->ec_name = client->name;
 	ec_dev->phys_name = client->adapter->name;
 	ec_dev->parent = &client->dev;
diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
index 0b8d328..52d4d7b 100644
--- a/drivers/mfd/cros_ec_spi.c
+++ b/drivers/mfd/cros_ec_spi.c
@@ -73,7 +73,7 @@
  *	if no record
  * @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.
- * @lock: mutex to ensure only one user of cros_ec_command_spi_xfer at a time
+ * @lock: mutex to ensure only one user of cros_ec_cmd_xfer_spi at a time
  */
 struct cros_ec_spi {
 	struct spi_device *spi;
@@ -210,13 +210,13 @@ static int cros_ec_spi_receive_response(struct cros_ec_device *ec_dev,
 }
 
 /**
- * cros_ec_command_spi_xfer - Transfer a message over SPI and receive the reply
+ * cros_ec_cmd_xfer_spi - Transfer a message over SPI and receive the reply
  *
  * @ec_dev: ChromeOS EC device
  * @ec_msg: Message to transfer
  */
-static int cros_ec_command_spi_xfer(struct cros_ec_device *ec_dev,
-				    struct cros_ec_msg *ec_msg)
+static int cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev,
+				struct cros_ec_msg *ec_msg)
 {
 	struct cros_ec_spi *ec_spi = ec_dev->priv;
 	struct spi_transfer trans;
@@ -372,7 +372,7 @@ static int cros_ec_spi_probe(struct spi_device *spi)
 	ec_dev->dev = dev;
 	ec_dev->priv = ec_spi;
 	ec_dev->irq = spi->irq;
-	ec_dev->command_xfer = cros_ec_command_spi_xfer;
+	ec_dev->cmd_xfer = cros_ec_cmd_xfer_spi;
 	ec_dev->ec_name = ec_spi->spi->modalias;
 	ec_dev->phys_name = dev_name(&ec_spi->spi->dev);
 	ec_dev->parent = &ec_spi->spi->dev;
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index 2ee3190..79a3585 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -16,7 +16,9 @@
 #ifndef __LINUX_MFD_CROS_EC_H
 #define __LINUX_MFD_CROS_EC_H
 
+#include <linux/notifier.h>
 #include <linux/mfd/cros_ec_commands.h>
+#include <linux/mutex.h>
 
 /*
  * Command interface between EC and AP, for LPC, I2C and SPI interfaces.
@@ -55,34 +57,53 @@ struct cros_ec_msg {
 /**
  * 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
+ * @was_wake_device: true if this device was set to wake the system from
+ * sleep at the last suspend
+ * @event_notifier: interrupt event notifier for transport devices
+ * @command_send: send a command
+ * @command_recv: receive a response
+ * @command_sendrecv: send a command and receive a response
+ *
  * @name: Name of this EC interface
  * @priv: Private data
  * @irq: Interrupt to use
- * @din: input buffer (from EC)
- * @dout: output buffer (to EC)
+ * @din: input buffer (for data from EC)
+ * @dout: output buffer (for data to EC)
  * \note
  * These two buffers will always be dword-aligned and include enough
  * space for up to 7 word-alignment bytes also, so we can ensure that
  * the body of the message is always dword-aligned (64-bit).
- *
  * We use this alignment to keep ARM and x86 happy. Probably word
  * alignment would be OK, there might be a small performance advantage
  * to using dword.
  * @din_size: size of din buffer to allocate (zero to use static din)
  * @dout_size: size of dout buffer to allocate (zero to use static dout)
- * @command_send: send a command
- * @command_recv: receive a command
- * @ec_name: name of EC device (e.g. 'chromeos-ec')
- * @phys_name: name of physical comms layer (e.g. 'i2c-4')
  * @parent: pointer to parent device (e.g. i2c or spi device)
- * @dev: Device pointer
- * dev_lock: Lock to prevent concurrent access
  * @wake_enabled: true if this device can wake the system from sleep
- * @was_wake_device: true if this device was set to wake the system from
- * sleep at the last suspend
- * @event_notifier: interrupt event notifier for transport devices
+ * @lock: one transaction at a time
+ * @cmd_xfer: low-level channel to the EC
  */
 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;
+	bool was_wake_device;
+	struct class *cros_class;
+	struct blocking_notifier_head event_notifier;
+	int (*command_send)(struct cros_ec_device *ec,
+			    uint16_t cmd, void *out_buf, int out_len);
+	int (*command_recv)(struct cros_ec_device *ec,
+			    uint16_t cmd, void *in_buf, int in_len);
+	int (*command_sendrecv)(struct cros_ec_device *ec,
+				uint16_t cmd, void *out_buf, int out_len,
+				void *in_buf, int in_len);
+
+	/* These are used to implement the platform-specific interface */
 	const char *name;
 	void *priv;
 	int irq;
@@ -90,26 +111,10 @@ struct cros_ec_device {
 	uint8_t *dout;
 	int din_size;
 	int dout_size;
-	int (*command_send)(struct cros_ec_device *ec,
-			uint16_t cmd, void *out_buf, int out_len);
-	int (*command_recv)(struct cros_ec_device *ec,
-			uint16_t cmd, void *in_buf, int in_len);
-	int (*command_sendrecv)(struct cros_ec_device *ec,
-			uint16_t cmd, void *out_buf, int out_len,
-			void *in_buf, int in_len);
-	int (*command_xfer)(struct cros_ec_device *ec,
-			struct cros_ec_msg *msg);
-
-	const char *ec_name;
-	const char *phys_name;
 	struct device *parent;
-
-	/* These are --private-- fields - do not assign */
-	struct device *dev;
-	struct mutex dev_lock;
 	bool wake_enabled;
-	bool was_wake_device;
-	struct blocking_notifier_head event_notifier;
+	struct mutex lock;
+	int (*cmd_xfer)(struct cros_ec_device *ec, struct cros_ec_msg *msg);
 };
 
 /**
-- 
2.0.0.526.g5318336

[PATCH v2 04/10] mdf: cros_ec: Detect in-progress commands

[Doug Anderson]

From: Simon Glass <sjg@chromium.org>

Some commands take a while to execute. Use -EAGAIN to signal this to the
caller.

Signed-off-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Doug Anderson <dianders@chromium.org>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
Changes in v2: None

 drivers/mfd/cros_ec_spi.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
index 52d4d7b..c29a2d7 100644
--- a/drivers/mfd/cros_ec_spi.c
+++ b/drivers/mfd/cros_ec_spi.c
@@ -292,6 +292,12 @@ static int cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev,
 	/* check response error code */
 	ptr = ec_dev->din;
 	if (ptr[0]) {
+		if (ptr[0] == EC_RES_IN_PROGRESS) {
+			dev_dbg(ec_dev->dev, "command 0x%02x in progress\n",
+				ec_msg->cmd);
+			ret = -EAGAIN;
+			goto exit;
+		}
 		dev_warn(ec_dev->dev, "command 0x%02x returned an error %d\n",
 			 ec_msg->cmd, ptr[0]);
 		debug_packet(ec_dev->dev, "in_err", ptr, len);
-- 
2.0.0.526.g5318336

[PATCH v2 05/10] mfd: cros_ec: Use struct cros_ec_command to communicate with the EC

[Doug Anderson]

From: Bill Richardson <wfrichar@chromium.org>

This is some internal structure reorganization / renaming to prepare
for future patches that will add a userspace API to cros_ec.  There
should be no visible changes.

Signed-off-by: Bill Richardson <wfrichar@chromium.org>
Signed-off-by: Doug Anderson <dianders@chromium.org>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
Changes in v2: None

 drivers/mfd/cros_ec.c       | 28 ++++++++++++++--------------
 drivers/mfd/cros_ec_i2c.c   | 24 ++++++++++++------------
 drivers/mfd/cros_ec_spi.c   | 16 ++++++++--------
 include/linux/mfd/cros_ec.h | 35 ++++++++++++++++++-----------------
 4 files changed, 52 insertions(+), 51 deletions(-)

diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index 04e053c..2e86c28 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -25,22 +25,22 @@
 #include <linux/mfd/cros_ec_commands.h>
 
 int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
-		       struct cros_ec_msg *msg)
+		       struct cros_ec_command *msg)
 {
 	uint8_t *out;
 	int csum, i;
 
-	BUG_ON(msg->out_len > EC_PROTO2_MAX_PARAM_SIZE);
+	BUG_ON(msg->outsize > EC_PROTO2_MAX_PARAM_SIZE);
 	out = ec_dev->dout;
 	out[0] = EC_CMD_VERSION0 + msg->version;
-	out[1] = msg->cmd;
-	out[2] = msg->out_len;
+	out[1] = msg->command;
+	out[2] = msg->outsize;
 	csum = out[0] + out[1] + out[2];
-	for (i = 0; i < msg->out_len; i++)
-		csum += out[EC_MSG_TX_HEADER_BYTES + i] = msg->out_buf[i];
-	out[EC_MSG_TX_HEADER_BYTES + msg->out_len] = (uint8_t)(csum & 0xff);
+	for (i = 0; i < msg->outsize; i++)
+		csum += out[EC_MSG_TX_HEADER_BYTES + i] = msg->outdata[i];
+	out[EC_MSG_TX_HEADER_BYTES + msg->outsize] = (uint8_t)(csum & 0xff);
 
-	return EC_MSG_TX_PROTO_BYTES + msg->out_len;
+	return EC_MSG_TX_PROTO_BYTES + msg->outsize;
 }
 EXPORT_SYMBOL(cros_ec_prepare_tx);
 
@@ -48,14 +48,14 @@ static int cros_ec_command_sendrecv(struct cros_ec_device *ec_dev,
 		uint16_t cmd, void *out_buf, int out_len,
 		void *in_buf, int in_len)
 {
-	struct cros_ec_msg msg;
+	struct cros_ec_command msg;
 
 	msg.version = cmd >> 8;
-	msg.cmd = cmd & 0xff;
-	msg.out_buf = out_buf;
-	msg.out_len = out_len;
-	msg.in_buf = in_buf;
-	msg.in_len = in_len;
+	msg.command = cmd & 0xff;
+	msg.outdata = out_buf;
+	msg.outsize = out_len;
+	msg.indata = in_buf;
+	msg.insize = in_len;
 
 	return ec_dev->cmd_xfer(ec_dev, &msg);
 }
diff --git a/drivers/mfd/cros_ec_i2c.c b/drivers/mfd/cros_ec_i2c.c
index 777e529..37ed12f 100644
--- a/drivers/mfd/cros_ec_i2c.c
+++ b/drivers/mfd/cros_ec_i2c.c
@@ -30,7 +30,7 @@ static inline struct cros_ec_device *to_ec_dev(struct device *dev)
 }
 
 static int cros_ec_cmd_xfer_i2c(struct cros_ec_device *ec_dev,
-				struct cros_ec_msg *msg)
+				struct cros_ec_command *msg)
 {
 	struct i2c_client *client = ec_dev->priv;
 	int ret = -ENOMEM;
@@ -50,7 +50,7 @@ static int cros_ec_cmd_xfer_i2c(struct cros_ec_device *ec_dev,
 	 * allocate larger packet (one byte for checksum, one byte for
 	 * length, and one for result code)
 	 */
-	packet_len = msg->in_len + 3;
+	packet_len = msg->insize + 3;
 	in_buf = kzalloc(packet_len, GFP_KERNEL);
 	if (!in_buf)
 		goto done;
@@ -61,7 +61,7 @@ static int cros_ec_cmd_xfer_i2c(struct cros_ec_device *ec_dev,
 	 * allocate larger packet (one byte for checksum, one for
 	 * command code, one for length, and one for command version)
 	 */
-	packet_len = msg->out_len + 4;
+	packet_len = msg->outsize + 4;
 	out_buf = kzalloc(packet_len, GFP_KERNEL);
 	if (!out_buf)
 		goto done;
@@ -69,16 +69,16 @@ static int cros_ec_cmd_xfer_i2c(struct cros_ec_device *ec_dev,
 	i2c_msg[0].buf = (char *)out_buf;
 
 	out_buf[0] = EC_CMD_VERSION0 + msg->version;
-	out_buf[1] = msg->cmd;
-	out_buf[2] = msg->out_len;
+	out_buf[1] = msg->command;
+	out_buf[2] = msg->outsize;
 
 	/* copy message payload and compute checksum */
 	sum = out_buf[0] + out_buf[1] + out_buf[2];
-	for (i = 0; i < msg->out_len; i++) {
-		out_buf[3 + i] = msg->out_buf[i];
+	for (i = 0; i < msg->outsize; i++) {
+		out_buf[3 + i] = msg->outdata[i];
 		sum += out_buf[3 + i];
 	}
-	out_buf[3 + msg->out_len] = sum;
+	out_buf[3 + msg->outsize] = sum;
 
 	/* send command to EC and read answer */
 	ret = i2c_transfer(client->adapter, i2c_msg, 2);
@@ -94,20 +94,20 @@ static int cros_ec_cmd_xfer_i2c(struct cros_ec_device *ec_dev,
 	/* check response error code */
 	if (i2c_msg[1].buf[0]) {
 		dev_warn(ec_dev->dev, "command 0x%02x returned an error %d\n",
-			 msg->cmd, i2c_msg[1].buf[0]);
+			 msg->command, i2c_msg[1].buf[0]);
 		ret = -EINVAL;
 		goto done;
 	}
 
 	/* copy response packet payload and compute checksum */
 	sum = in_buf[0] + in_buf[1];
-	for (i = 0; i < msg->in_len; i++) {
-		msg->in_buf[i] = in_buf[2 + i];
+	for (i = 0; i < msg->insize; i++) {
+		msg->indata[i] = in_buf[2 + i];
 		sum += in_buf[2 + i];
 	}
 	dev_dbg(ec_dev->dev, "packet: %*ph, sum = %02x\n",
 		i2c_msg[1].len, in_buf, sum);
-	if (sum != in_buf[2 + msg->in_len]) {
+	if (sum != in_buf[2 + msg->insize]) {
 		dev_err(ec_dev->dev, "bad packet checksum\n");
 		ret = -EBADMSG;
 		goto done;
diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
index c29a2d7..2d713fe 100644
--- a/drivers/mfd/cros_ec_spi.c
+++ b/drivers/mfd/cros_ec_spi.c
@@ -216,7 +216,7 @@ static int cros_ec_spi_receive_response(struct cros_ec_device *ec_dev,
  * @ec_msg: Message to transfer
  */
 static int cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev,
-				struct cros_ec_msg *ec_msg)
+				struct cros_ec_command *ec_msg)
 {
 	struct cros_ec_spi *ec_spi = ec_dev->priv;
 	struct spi_transfer trans;
@@ -261,7 +261,7 @@ 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->in_len + EC_MSG_TX_PROTO_BYTES);
+				ec_msg->insize + EC_MSG_TX_PROTO_BYTES);
 	} else {
 		dev_err(ec_dev->dev, "spi transfer failed: %d\n", ret);
 	}
@@ -294,21 +294,21 @@ static int cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev,
 	if (ptr[0]) {
 		if (ptr[0] == EC_RES_IN_PROGRESS) {
 			dev_dbg(ec_dev->dev, "command 0x%02x in progress\n",
-				ec_msg->cmd);
+				ec_msg->command);
 			ret = -EAGAIN;
 			goto exit;
 		}
 		dev_warn(ec_dev->dev, "command 0x%02x returned an error %d\n",
-			 ec_msg->cmd, ptr[0]);
+			 ec_msg->command, ptr[0]);
 		debug_packet(ec_dev->dev, "in_err", ptr, len);
 		ret = -EINVAL;
 		goto exit;
 	}
 	len = ptr[1];
 	sum = ptr[0] + ptr[1];
-	if (len > ec_msg->in_len) {
+	if (len > ec_msg->insize) {
 		dev_err(ec_dev->dev, "packet too long (%d bytes, expected %d)",
-			len, ec_msg->in_len);
+			len, ec_msg->insize);
 		ret = -ENOSPC;
 		goto exit;
 	}
@@ -316,8 +316,8 @@ static int cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev,
 	/* copy response packet payload and compute checksum */
 	for (i = 0; i < len; i++) {
 		sum += ptr[i + 2];
-		if (ec_msg->in_len)
-			ec_msg->in_buf[i] = ptr[i + 2];
+		if (ec_msg->insize)
+			ec_msg->indata[i] = ptr[i + 2];
 	}
 	sum &= 0xff;
 
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index 79a3585..f27c037 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -35,23 +35,23 @@ enum {
 					EC_MSG_TX_PROTO_BYTES,
 };
 
-/**
- * struct cros_ec_msg - A message sent to the EC, and its reply
- *
+/*
  * @version: Command version number (often 0)
- * @cmd: Command to send (EC_CMD_...)
- * @out_buf: Outgoing payload (to EC)
- * @outlen: Outgoing length
- * @in_buf: Incoming payload (from EC)
- * @in_len: Incoming length
+ * @command: Command to send (EC_CMD_...)
+ * @outdata: Outgoing data to EC
+ * @outsize: Outgoing length in bytes
+ * @indata: Where to put the incoming data from EC
+ * @insize: Incoming length in bytes (filled in by EC)
+ * @result: EC's response to the command (separate from communication failure)
  */
-struct cros_ec_msg {
-	u8 version;
-	u8 cmd;
-	uint8_t *out_buf;
-	int out_len;
-	uint8_t *in_buf;
-	int in_len;
+struct cros_ec_command {
+	uint32_t version;
+	uint32_t command;
+	uint8_t *outdata;
+	uint32_t outsize;
+	uint8_t *indata;
+	uint32_t insize;
+	uint32_t result;
 };
 
 /**
@@ -114,7 +114,8 @@ struct cros_ec_device {
 	struct device *parent;
 	bool wake_enabled;
 	struct mutex lock;
-	int (*cmd_xfer)(struct cros_ec_device *ec, struct cros_ec_msg *msg);
+	int (*cmd_xfer)(struct cros_ec_device *ec,
+			struct cros_ec_command *msg);
 };
 
 /**
@@ -148,7 +149,7 @@ int cros_ec_resume(struct cros_ec_device *ec_dev);
  * @msg: Message to write
  */
 int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
-		       struct cros_ec_msg *msg);
+		       struct cros_ec_command *msg);
 
 /**
  * cros_ec_remove - Remove a ChromeOS EC
-- 
2.0.0.526.g5318336

[PATCH v2 06/10] mfd: cros_ec: cleanup: remove unused fields from struct cros_ec_device

[Doug Anderson]

From: Bill Richardson <wfrichar@chromium.org>

struct cros_ec_device has a superfluous "name" field. We can get all the
debugging info we need from the existing ec_name and phys_name fields, so
let's take out the extra field.

The printout also has sufficient info in it without explicitly adding
the transport.  Before this change:
  cros-ec-spi spi2.0: Chrome EC (SPI)

After this change:
  cros-ec-spi spi2.0: Chrome EC device registered

Signed-off-by: Bill Richardson <wfrichar@chromium.org>
Signed-off-by: Doug Anderson <dianders@chromium.org>
Acked-by: Lee Jones <lee.jones@linaro.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Include example printouts before/after in commit message.

 drivers/mfd/cros_ec.c       | 2 +-
 drivers/mfd/cros_ec_i2c.c   | 1 -
 drivers/mfd/cros_ec_spi.c   | 1 -
 include/linux/mfd/cros_ec.h | 2 --
 4 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index 2e86c28..49ed8c3 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -140,7 +140,7 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
 		goto fail_mfd;
 	}
 
-	dev_info(dev, "Chrome EC (%s)\n", ec_dev->name);
+	dev_info(dev, "Chrome EC device registered\n");
 
 	return 0;
 
diff --git a/drivers/mfd/cros_ec_i2c.c b/drivers/mfd/cros_ec_i2c.c
index 37ed12f..5bb32f5 100644
--- a/drivers/mfd/cros_ec_i2c.c
+++ b/drivers/mfd/cros_ec_i2c.c
@@ -132,7 +132,6 @@ static int cros_ec_i2c_probe(struct i2c_client *client,
 		return -ENOMEM;
 
 	i2c_set_clientdata(client, ec_dev);
-	ec_dev->name = "I2C";
 	ec_dev->dev = dev;
 	ec_dev->priv = client;
 	ec_dev->irq = client->irq;
diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
index 2d713fe..09ca789 100644
--- a/drivers/mfd/cros_ec_spi.c
+++ b/drivers/mfd/cros_ec_spi.c
@@ -374,7 +374,6 @@ static int cros_ec_spi_probe(struct spi_device *spi)
 	cros_ec_spi_dt_probe(ec_spi, dev);
 
 	spi_set_drvdata(spi, ec_dev);
-	ec_dev->name = "SPI";
 	ec_dev->dev = dev;
 	ec_dev->priv = ec_spi;
 	ec_dev->irq = spi->irq;
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index f27c037..2b0c598 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -67,7 +67,6 @@ struct cros_ec_command {
  * @command_recv: receive a response
  * @command_sendrecv: send a command and receive a response
  *
- * @name: Name of this EC interface
  * @priv: Private data
  * @irq: Interrupt to use
  * @din: input buffer (for data from EC)
@@ -104,7 +103,6 @@ struct cros_ec_device {
 				void *in_buf, int in_len);
 
 	/* These are used to implement the platform-specific interface */
-	const char *name;
 	void *priv;
 	int irq;
 	uint8_t *din;
-- 
2.0.0.526.g5318336

[PATCH v2 07/10] mfd: cros_ec: cleanup: Remove EC wrapper functions

[Doug Anderson]

From: Bill Richardson <wfrichar@chromium.org>

Remove the three wrapper functions that talk to the EC without passing all
the desired arguments and just use the underlying communication function
that passes everything in a struct intead.

This is internal code refactoring only. Nothing should change.

Signed-off-by: Bill Richardson <wfrichar@chromium.org>
Signed-off-by: Doug Anderson <dianders@chromium.org>
Acked-by: Lee Jones <lee.jones@linaro.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Removed unneeded "ret" variable.

 drivers/i2c/busses/i2c-cros-ec-tunnel.c | 15 +++++++++++----
 drivers/input/keyboard/cros_ec_keyb.c   | 12 ++++++++++--
 drivers/mfd/cros_ec.c                   | 32 --------------------------------
 include/linux/mfd/cros_ec.h             | 19 ++++++-------------
 4 files changed, 27 insertions(+), 51 deletions(-)

diff --git a/drivers/i2c/busses/i2c-cros-ec-tunnel.c b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
index 8e7a714..dd07818 100644
--- a/drivers/i2c/busses/i2c-cros-ec-tunnel.c
+++ b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
@@ -183,6 +183,7 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg i2c_msgs[],
 	u8 *request = NULL;
 	u8 *response = NULL;
 	int result;
+	struct cros_ec_command msg;
 
 	request_len = ec_i2c_count_message(i2c_msgs, num);
 	if (request_len < 0) {
@@ -218,9 +219,15 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg i2c_msgs[],
 	}
 
 	ec_i2c_construct_message(request, i2c_msgs, num, bus_num);
-	result = bus->ec->command_sendrecv(bus->ec, EC_CMD_I2C_PASSTHRU,
-					   request, request_len,
-					   response, response_len);
+
+	msg.version = 0;
+	msg.command = EC_CMD_I2C_PASSTHRU;
+	msg.outdata = request;
+	msg.outsize = request_len;
+	msg.indata = response;
+	msg.insize = response_len;
+
+	result = bus->ec->cmd_xfer(bus->ec, &msg);
 	if (result)
 		goto exit;
 
@@ -258,7 +265,7 @@ static int ec_i2c_probe(struct platform_device *pdev)
 	u32 remote_bus;
 	int err;
 
-	if (!ec->command_sendrecv) {
+	if (!ec->cmd_xfer) {
 		dev_err(dev, "Missing sendrecv\n");
 		return -EINVAL;
 	}
diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
index 4083796..b8341ab 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -191,8 +191,16 @@ static void cros_ec_keyb_close(struct input_dev *dev)
 
 static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t *kb_state)
 {
-	return ckdev->ec->command_recv(ckdev->ec, EC_CMD_MKBP_STATE,
-					  kb_state, ckdev->cols);
+	struct cros_ec_command msg = {
+		.version = 0,
+		.command = EC_CMD_MKBP_STATE,
+		.outdata = NULL,
+		.outsize = 0,
+		.indata = kb_state,
+		.insize = ckdev->cols,
+	};
+
+	return ckdev->ec->cmd_xfer(ckdev->ec, &msg);
 }
 
 static int cros_ec_keyb_work(struct notifier_block *nb,
diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index 49ed8c3..4851ed2 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -44,34 +44,6 @@ int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
 }
 EXPORT_SYMBOL(cros_ec_prepare_tx);
 
-static int cros_ec_command_sendrecv(struct cros_ec_device *ec_dev,
-		uint16_t cmd, void *out_buf, int out_len,
-		void *in_buf, int in_len)
-{
-	struct cros_ec_command msg;
-
-	msg.version = cmd >> 8;
-	msg.command = cmd & 0xff;
-	msg.outdata = out_buf;
-	msg.outsize = out_len;
-	msg.indata = in_buf;
-	msg.insize = in_len;
-
-	return ec_dev->cmd_xfer(ec_dev, &msg);
-}
-
-static int cros_ec_command_recv(struct cros_ec_device *ec_dev,
-		uint16_t cmd, void *buf, int buf_len)
-{
-	return cros_ec_command_sendrecv(ec_dev, cmd, NULL, 0, buf, buf_len);
-}
-
-static int cros_ec_command_send(struct cros_ec_device *ec_dev,
-		uint16_t cmd, void *buf, int buf_len)
-{
-	return cros_ec_command_sendrecv(ec_dev, cmd, buf, buf_len, NULL, 0);
-}
-
 static irqreturn_t ec_irq_thread(int irq, void *data)
 {
 	struct cros_ec_device *ec_dev = data;
@@ -104,10 +76,6 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
 
 	BLOCKING_INIT_NOTIFIER_HEAD(&ec_dev->event_notifier);
 
-	ec_dev->command_send = cros_ec_command_send;
-	ec_dev->command_recv = cros_ec_command_recv;
-	ec_dev->command_sendrecv = cros_ec_command_sendrecv;
-
 	if (ec_dev->din_size) {
 		ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL);
 		if (!ec_dev->din)
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index 2b0c598..60c0880 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -63,9 +63,10 @@ struct cros_ec_command {
  * @was_wake_device: true if this device was set to wake the system from
  * sleep at the last suspend
  * @event_notifier: interrupt event notifier for transport devices
- * @command_send: send a command
- * @command_recv: receive a response
- * @command_sendrecv: send a command and receive a response
+ * @cmd_xfer: send command to EC and get response
+ *     Returns 0 if the communication succeeded, but that doesn't mean the EC
+ *     was happy with the command it got. Caller should check msg.result for
+ *     the EC's result code.
  *
  * @priv: Private data
  * @irq: Interrupt to use
@@ -83,7 +84,6 @@ struct cros_ec_command {
  * @parent: pointer to parent device (e.g. i2c or spi device)
  * @wake_enabled: true if this device can wake the system from sleep
  * @lock: one transaction at a time
- * @cmd_xfer: low-level channel to the EC
  */
 struct cros_ec_device {
 
@@ -94,13 +94,8 @@ struct cros_ec_device {
 	bool was_wake_device;
 	struct class *cros_class;
 	struct blocking_notifier_head event_notifier;
-	int (*command_send)(struct cros_ec_device *ec,
-			    uint16_t cmd, void *out_buf, int out_len);
-	int (*command_recv)(struct cros_ec_device *ec,
-			    uint16_t cmd, void *in_buf, int in_len);
-	int (*command_sendrecv)(struct cros_ec_device *ec,
-				uint16_t cmd, void *out_buf, int out_len,
-				void *in_buf, int in_len);
+	int (*cmd_xfer)(struct cros_ec_device *ec,
+			struct cros_ec_command *msg);
 
 	/* These are used to implement the platform-specific interface */
 	void *priv;
@@ -112,8 +107,6 @@ struct cros_ec_device {
 	struct device *parent;
 	bool wake_enabled;
 	struct mutex lock;
-	int (*cmd_xfer)(struct cros_ec_device *ec,
-			struct cros_ec_command *msg);
 };
 
 /**
-- 
2.0.0.526.g5318336

[PATCH v2 07/10] mfd: cros_ec: cleanup: Remove EC wrapper functions

[Doug Anderson]

From: Bill Richardson <wfrichar-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

Remove the three wrapper functions that talk to the EC without passing all
the desired arguments and just use the underlying communication function
that passes everything in a struct intead.

This is internal code refactoring only. Nothing should change.

Signed-off-by: Bill Richardson <wfrichar-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Signed-off-by: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Acked-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Reviewed-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
Changes in v2:
- Removed unneeded "ret" variable.

 drivers/i2c/busses/i2c-cros-ec-tunnel.c | 15 +++++++++++----
 drivers/input/keyboard/cros_ec_keyb.c   | 12 ++++++++++--
 drivers/mfd/cros_ec.c                   | 32 --------------------------------
 include/linux/mfd/cros_ec.h             | 19 ++++++-------------
 4 files changed, 27 insertions(+), 51 deletions(-)

diff --git a/drivers/i2c/busses/i2c-cros-ec-tunnel.c b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
index 8e7a714..dd07818 100644
--- a/drivers/i2c/busses/i2c-cros-ec-tunnel.c
+++ b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
@@ -183,6 +183,7 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg i2c_msgs[],
 	u8 *request = NULL;
 	u8 *response = NULL;
 	int result;
+	struct cros_ec_command msg;
 
 	request_len = ec_i2c_count_message(i2c_msgs, num);
 	if (request_len < 0) {
@@ -218,9 +219,15 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg i2c_msgs[],
 	}
 
 	ec_i2c_construct_message(request, i2c_msgs, num, bus_num);
-	result = bus->ec->command_sendrecv(bus->ec, EC_CMD_I2C_PASSTHRU,
-					   request, request_len,
-					   response, response_len);
+
+	msg.version = 0;
+	msg.command = EC_CMD_I2C_PASSTHRU;
+	msg.outdata = request;
+	msg.outsize = request_len;
+	msg.indata = response;
+	msg.insize = response_len;
+
+	result = bus->ec->cmd_xfer(bus->ec, &msg);
 	if (result)
 		goto exit;
 
@@ -258,7 +265,7 @@ static int ec_i2c_probe(struct platform_device *pdev)
 	u32 remote_bus;
 	int err;
 
-	if (!ec->command_sendrecv) {
+	if (!ec->cmd_xfer) {
 		dev_err(dev, "Missing sendrecv\n");
 		return -EINVAL;
 	}
diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
index 4083796..b8341ab 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -191,8 +191,16 @@ static void cros_ec_keyb_close(struct input_dev *dev)
 
 static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t *kb_state)
 {
-	return ckdev->ec->command_recv(ckdev->ec, EC_CMD_MKBP_STATE,
-					  kb_state, ckdev->cols);
+	struct cros_ec_command msg = {
+		.version = 0,
+		.command = EC_CMD_MKBP_STATE,
+		.outdata = NULL,
+		.outsize = 0,
+		.indata = kb_state,
+		.insize = ckdev->cols,
+	};
+
+	return ckdev->ec->cmd_xfer(ckdev->ec, &msg);
 }
 
 static int cros_ec_keyb_work(struct notifier_block *nb,
diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index 49ed8c3..4851ed2 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -44,34 +44,6 @@ int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
 }
 EXPORT_SYMBOL(cros_ec_prepare_tx);
 
-static int cros_ec_command_sendrecv(struct cros_ec_device *ec_dev,
-		uint16_t cmd, void *out_buf, int out_len,
-		void *in_buf, int in_len)
-{
-	struct cros_ec_command msg;
-
-	msg.version = cmd >> 8;
-	msg.command = cmd & 0xff;
-	msg.outdata = out_buf;
-	msg.outsize = out_len;
-	msg.indata = in_buf;
-	msg.insize = in_len;
-
-	return ec_dev->cmd_xfer(ec_dev, &msg);
-}
-
-static int cros_ec_command_recv(struct cros_ec_device *ec_dev,
-		uint16_t cmd, void *buf, int buf_len)
-{
-	return cros_ec_command_sendrecv(ec_dev, cmd, NULL, 0, buf, buf_len);
-}
-
-static int cros_ec_command_send(struct cros_ec_device *ec_dev,
-		uint16_t cmd, void *buf, int buf_len)
-{
-	return cros_ec_command_sendrecv(ec_dev, cmd, buf, buf_len, NULL, 0);
-}
-
 static irqreturn_t ec_irq_thread(int irq, void *data)
 {
 	struct cros_ec_device *ec_dev = data;
@@ -104,10 +76,6 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
 
 	BLOCKING_INIT_NOTIFIER_HEAD(&ec_dev->event_notifier);
 
-	ec_dev->command_send = cros_ec_command_send;
-	ec_dev->command_recv = cros_ec_command_recv;
-	ec_dev->command_sendrecv = cros_ec_command_sendrecv;
-
 	if (ec_dev->din_size) {
 		ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL);
 		if (!ec_dev->din)
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index 2b0c598..60c0880 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -63,9 +63,10 @@ struct cros_ec_command {
  * @was_wake_device: true if this device was set to wake the system from
  * sleep at the last suspend
  * @event_notifier: interrupt event notifier for transport devices
- * @command_send: send a command
- * @command_recv: receive a response
- * @command_sendrecv: send a command and receive a response
+ * @cmd_xfer: send command to EC and get response
+ *     Returns 0 if the communication succeeded, but that doesn't mean the EC
+ *     was happy with the command it got. Caller should check msg.result for
+ *     the EC's result code.
  *
  * @priv: Private data
  * @irq: Interrupt to use
@@ -83,7 +84,6 @@ struct cros_ec_command {
  * @parent: pointer to parent device (e.g. i2c or spi device)
  * @wake_enabled: true if this device can wake the system from sleep
  * @lock: one transaction at a time
- * @cmd_xfer: low-level channel to the EC
  */
 struct cros_ec_device {
 
@@ -94,13 +94,8 @@ struct cros_ec_device {
 	bool was_wake_device;
 	struct class *cros_class;
 	struct blocking_notifier_head event_notifier;
-	int (*command_send)(struct cros_ec_device *ec,
-			    uint16_t cmd, void *out_buf, int out_len);
-	int (*command_recv)(struct cros_ec_device *ec,
-			    uint16_t cmd, void *in_buf, int in_len);
-	int (*command_sendrecv)(struct cros_ec_device *ec,
-				uint16_t cmd, void *out_buf, int out_len,
-				void *in_buf, int in_len);
+	int (*cmd_xfer)(struct cros_ec_device *ec,
+			struct cros_ec_command *msg);
 
 	/* These are used to implement the platform-specific interface */
 	void *priv;
@@ -112,8 +107,6 @@ struct cros_ec_device {
 	struct device *parent;
 	bool wake_enabled;
 	struct mutex lock;
-	int (*cmd_xfer)(struct cros_ec_device *ec,
-			struct cros_ec_command *msg);
 };
 
 /**
-- 
2.0.0.526.g5318336

[PATCH v2 08/10] mfd: cros_ec: Check result code from EC messages

[Doug Anderson]

From: Bill Richardson <wfrichar@chromium.org>

Just because the host was able to talk to the EC doesn't mean that the EC
was happy with what it was told. Errors in communincation are not the same
as error messages from the EC itself.

This change lets the EC report its errors separately.

[dianders: Added common function to cros_ec.c]

Signed-off-by: Bill Richardson <wfrichar@chromium.org>
Signed-off-by: Doug Anderson <dianders@chromium.org>
---
Changes in v2:
- Added common function to cros_ec.c
- Changed to dev_dbg() as per http://crosreview.com/66726

 drivers/mfd/cros_ec.c       | 18 ++++++++++++++++++
 drivers/mfd/cros_ec_i2c.c   |  8 +++-----
 drivers/mfd/cros_ec_spi.c   | 19 ++++++-------------
 include/linux/mfd/cros_ec.h | 12 ++++++++++++
 4 files changed, 39 insertions(+), 18 deletions(-)

diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index 4851ed2..83e30c6 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -44,6 +44,24 @@ int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
 }
 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);
+
 static irqreturn_t ec_irq_thread(int irq, void *data)
 {
 	struct cros_ec_device *ec_dev = data;
diff --git a/drivers/mfd/cros_ec_i2c.c b/drivers/mfd/cros_ec_i2c.c
index 5bb32f5..189e7d1 100644
--- a/drivers/mfd/cros_ec_i2c.c
+++ b/drivers/mfd/cros_ec_i2c.c
@@ -92,12 +92,10 @@ static int cros_ec_cmd_xfer_i2c(struct cros_ec_device *ec_dev,
 	}
 
 	/* check response error code */
-	if (i2c_msg[1].buf[0]) {
-		dev_warn(ec_dev->dev, "command 0x%02x returned an error %d\n",
-			 msg->command, i2c_msg[1].buf[0]);
-		ret = -EINVAL;
+	msg->result = i2c_msg[1].buf[0];
+	ret = cros_ec_check_result(ec_dev, msg);
+	if (ret)
 		goto done;
-	}
 
 	/* copy response packet payload and compute checksum */
 	sum = in_buf[0] + in_buf[1];
diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
index 09ca789..c975087 100644
--- a/drivers/mfd/cros_ec_spi.c
+++ b/drivers/mfd/cros_ec_spi.c
@@ -289,21 +289,14 @@ static int cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev,
 		goto exit;
 	}
 
-	/* check response error code */
 	ptr = ec_dev->din;
-	if (ptr[0]) {
-		if (ptr[0] == EC_RES_IN_PROGRESS) {
-			dev_dbg(ec_dev->dev, "command 0x%02x in progress\n",
-				ec_msg->command);
-			ret = -EAGAIN;
-			goto exit;
-		}
-		dev_warn(ec_dev->dev, "command 0x%02x returned an error %d\n",
-			 ec_msg->command, ptr[0]);
-		debug_packet(ec_dev->dev, "in_err", ptr, len);
-		ret = -EINVAL;
+
+	/* check response error code */
+	ec_msg->result = ptr[0];
+	ret = cros_ec_check_result(ec_dev, ec_msg);
+	if (ret)
 		goto exit;
-	}
+
 	len = ptr[1];
 	sum = ptr[0] + ptr[1];
 	if (len > ec_msg->insize) {
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index 60c0880..1f79f16 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -143,6 +143,18 @@ int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
 		       struct cros_ec_command *msg);
 
 /**
+ * cros_ec_check_result - Check ec_msg->result
+ *
+ * This is used by ChromeOS EC drivers to check the ec_msg->result for
+ * errors and to warn about them.
+ *
+ * @ec_dev: EC device
+ * @msg: Message to check
+ */
+int cros_ec_check_result(struct cros_ec_device *ec_dev,
+			 struct cros_ec_command *msg);
+
+/**
  * cros_ec_remove - Remove a ChromeOS EC
  *
  * Call this to deregister a ChromeOS EC, then clean up any private data.
-- 
2.0.0.526.g5318336

[PATCH v2 09/10] mfd: cros_ec: ec_dev->cmd_xfer() returns number of bytes received from EC

[Doug Anderson]

From: Bill Richardson <wfrichar@chromium.org>

When communicating with the EC, the cmd_xfer() function should return the
number of bytes it received from the EC, or negative on error.

Signed-off-by: Bill Richardson <wfrichar@chromium.org>
Signed-off-by: Doug Anderson <dianders@chromium.org>
Acked-by: Lee Jones <lee.jones@linaro.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
Changes in v2: None

 drivers/i2c/busses/i2c-cros-ec-tunnel.c | 2 +-
 drivers/mfd/cros_ec_i2c.c               | 2 +-
 drivers/mfd/cros_ec_spi.c               | 2 +-
 include/linux/mfd/cros_ec.h             | 8 ++++----
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-cros-ec-tunnel.c b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
index dd07818..05e033c 100644
--- a/drivers/i2c/busses/i2c-cros-ec-tunnel.c
+++ b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
@@ -228,7 +228,7 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg i2c_msgs[],
 	msg.insize = response_len;
 
 	result = bus->ec->cmd_xfer(bus->ec, &msg);
-	if (result)
+	if (result < 0)
 		goto exit;
 
 	result = ec_i2c_parse_response(response, i2c_msgs, &num);
diff --git a/drivers/mfd/cros_ec_i2c.c b/drivers/mfd/cros_ec_i2c.c
index 189e7d1..fd7a546 100644
--- a/drivers/mfd/cros_ec_i2c.c
+++ b/drivers/mfd/cros_ec_i2c.c
@@ -111,7 +111,7 @@ static int cros_ec_cmd_xfer_i2c(struct cros_ec_device *ec_dev,
 		goto done;
 	}
 
-	ret = 0;
+	ret = i2c_msg[1].buf[1];
  done:
 	kfree(in_buf);
 	kfree(out_buf);
diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
index c975087..2ad6815 100644
--- a/drivers/mfd/cros_ec_spi.c
+++ b/drivers/mfd/cros_ec_spi.c
@@ -324,7 +324,7 @@ static int cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev,
 		goto exit;
 	}
 
-	ret = 0;
+	ret = len;
 exit:
 	mutex_unlock(&ec_spi->lock);
 	return ret;
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index 1f79f16..0ebf26f 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -41,7 +41,7 @@ enum {
  * @outdata: Outgoing data to EC
  * @outsize: Outgoing length in bytes
  * @indata: Where to put the incoming data from EC
- * @insize: Incoming length in bytes (filled in by EC)
+ * @insize: Max number of bytes to accept from EC
  * @result: EC's response to the command (separate from communication failure)
  */
 struct cros_ec_command {
@@ -64,9 +64,9 @@ struct cros_ec_command {
  * sleep at the last suspend
  * @event_notifier: interrupt event notifier for transport devices
  * @cmd_xfer: send command to EC and get response
- *     Returns 0 if the communication succeeded, but that doesn't mean the EC
- *     was happy with the command it got. Caller should check msg.result for
- *     the EC's result code.
+ *     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.
  *
  * @priv: Private data
  * @irq: Interrupt to use
-- 
2.0.0.526.g5318336

[PATCH v2 10/10] mfd: cros_ec: move EC interrupt to cros_ec_keyb

[Doug Anderson]

From: Andrew Bresticker <abrestic@chromium.org>

If we receive EC interrupts after the cros_ec driver has probed, but
before the cros_ec_keyb driver has probed, the cros_ec IRQ handler
will not run the cros_ec_keyb notifier and the EC will leave the IRQ
line asserted.  The cros_ec IRQ handler then returns IRQ_HANDLED and
the resulting flood of interrupts causes the machine to hang.

Since the EC interrupt is currently only used for the keyboard, move
the setup and handling of the EC interrupt to the cros_ec_keyb driver.

Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
Signed-off-by: Doug Anderson <dianders@chromium.org>
---
Changes in v2:
- IRQs should be optional => move EC interrupt to keyboard.

 drivers/input/keyboard/cros_ec_keyb.c | 58 ++++++++++++++++++++---------------
 drivers/mfd/cros_ec.c                 | 35 +--------------------
 include/linux/mfd/cros_ec.h           |  2 --
 3 files changed, 34 insertions(+), 61 deletions(-)

diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
index b8341ab..791781a 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -24,8 +24,8 @@
 #include <linux/module.h>
 #include <linux/i2c.h>
 #include <linux/input.h>
+#include <linux/interrupt.h>
 #include <linux/kernel.h>
-#include <linux/notifier.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/input/matrix_keypad.h>
@@ -42,7 +42,6 @@
  * @dev: Device pointer
  * @idev: Input device
  * @ec: Top level ChromeOS device to use to talk to EC
- * @event_notifier: interrupt event notifier for transport devices
  */
 struct cros_ec_keyb {
 	unsigned int rows;
@@ -55,7 +54,6 @@ struct cros_ec_keyb {
 	struct device *dev;
 	struct input_dev *idev;
 	struct cros_ec_device *ec;
-	struct notifier_block notifier;
 };
 
 
@@ -173,22 +171,6 @@ static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev,
 	input_sync(ckdev->idev);
 }
 
-static int cros_ec_keyb_open(struct input_dev *dev)
-{
-	struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
-
-	return blocking_notifier_chain_register(&ckdev->ec->event_notifier,
-						&ckdev->notifier);
-}
-
-static void cros_ec_keyb_close(struct input_dev *dev)
-{
-	struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
-
-	blocking_notifier_chain_unregister(&ckdev->ec->event_notifier,
-					   &ckdev->notifier);
-}
-
 static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t *kb_state)
 {
 	struct cros_ec_command msg = {
@@ -203,19 +185,41 @@ static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t *kb_state)
 	return ckdev->ec->cmd_xfer(ckdev->ec, &msg);
 }
 
-static int cros_ec_keyb_work(struct notifier_block *nb,
-		     unsigned long state, void *_notify)
+static irqreturn_t cros_ec_keyb_irq(int irq, void *data)
 {
+	struct cros_ec_keyb *ckdev = data;
+	struct cros_ec_device *ec = ckdev->ec;
 	int ret;
-	struct cros_ec_keyb *ckdev = container_of(nb, struct cros_ec_keyb,
-						    notifier);
 	uint8_t kb_state[ckdev->cols];
 
+	if (device_may_wakeup(ec->dev))
+		pm_wakeup_event(ec->dev, 0);
+
 	ret = cros_ec_keyb_get_state(ckdev, kb_state);
 	if (ret >= 0)
 		cros_ec_keyb_process(ckdev, kb_state, ret);
+	else
+		dev_err(ec->dev, "failed to get keyboard state: %d\n", ret);
 
-	return NOTIFY_DONE;
+	return IRQ_HANDLED;
+}
+
+static int cros_ec_keyb_open(struct input_dev *dev)
+{
+	struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
+	struct cros_ec_device *ec = ckdev->ec;
+
+	return request_threaded_irq(ec->irq, NULL, cros_ec_keyb_irq,
+					IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+					"cros_ec_keyb", ckdev);
+}
+
+static void cros_ec_keyb_close(struct input_dev *dev)
+{
+	struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
+	struct cros_ec_device *ec = ckdev->ec;
+
+	free_irq(ec->irq, ckdev);
 }
 
 static int cros_ec_keyb_probe(struct platform_device *pdev)
@@ -246,8 +250,12 @@ static int cros_ec_keyb_probe(struct platform_device *pdev)
 	if (!idev)
 		return -ENOMEM;
 
+	if (!ec->irq) {
+		dev_err(dev, "no EC IRQ specified\n");
+		return -EINVAL;
+	}
+
 	ckdev->ec = ec;
-	ckdev->notifier.notifier_call = cros_ec_keyb_work;
 	ckdev->dev = dev;
 	dev_set_drvdata(&pdev->dev, ckdev);
 
diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index 83e30c6..4873f9c 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -62,18 +62,6 @@ int cros_ec_check_result(struct cros_ec_device *ec_dev,
 }
 EXPORT_SYMBOL(cros_ec_check_result);
 
-static irqreturn_t ec_irq_thread(int irq, void *data)
-{
-	struct cros_ec_device *ec_dev = data;
-
-	if (device_may_wakeup(ec_dev->dev))
-		pm_wakeup_event(ec_dev->dev, 0);
-
-	blocking_notifier_call_chain(&ec_dev->event_notifier, 1, ec_dev);
-
-	return IRQ_HANDLED;
-}
-
 static const struct mfd_cell cros_devs[] = {
 	{
 		.name = "cros-ec-keyb",
@@ -92,8 +80,6 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
 	struct device *dev = ec_dev->dev;
 	int err = 0;
 
-	BLOCKING_INIT_NOTIFIER_HEAD(&ec_dev->event_notifier);
-
 	if (ec_dev->din_size) {
 		ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL);
 		if (!ec_dev->din)
@@ -105,42 +91,23 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
 			return -ENOMEM;
 	}
 
-	if (!ec_dev->irq) {
-		dev_dbg(dev, "no valid IRQ: %d\n", ec_dev->irq);
-		return err;
-	}
-
-	err = request_threaded_irq(ec_dev->irq, NULL, ec_irq_thread,
-				   IRQF_TRIGGER_LOW | IRQF_ONESHOT,
-				   "chromeos-ec", ec_dev);
-	if (err) {
-		dev_err(dev, "request irq %d: error %d\n", ec_dev->irq, err);
-		return err;
-	}
-
 	err = mfd_add_devices(dev, 0, cros_devs,
 			      ARRAY_SIZE(cros_devs),
 			      NULL, ec_dev->irq, NULL);
 	if (err) {
 		dev_err(dev, "failed to add mfd devices\n");
-		goto fail_mfd;
+		return err;
 	}
 
 	dev_info(dev, "Chrome EC device registered\n");
 
 	return 0;
-
-fail_mfd:
-	free_irq(ec_dev->irq, ec_dev);
-
-	return err;
 }
 EXPORT_SYMBOL(cros_ec_register);
 
 int cros_ec_remove(struct cros_ec_device *ec_dev)
 {
 	mfd_remove_devices(ec_dev->dev);
-	free_irq(ec_dev->irq, ec_dev);
 
 	return 0;
 }
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index 0ebf26f..fcbe9d1 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -62,7 +62,6 @@ struct cros_ec_command {
  * @dev: Device pointer
  * @was_wake_device: true if this device was set to wake the system from
  * sleep at the last suspend
- * @event_notifier: interrupt event notifier for transport devices
  * @cmd_xfer: send command to EC and get response
  *     Returns the number of bytes received if the communication succeeded, but
  *     that doesn't mean the EC was happy with the command. The caller
@@ -93,7 +92,6 @@ struct cros_ec_device {
 	struct device *dev;
 	bool was_wake_device;
 	struct class *cros_class;
-	struct blocking_notifier_head event_notifier;
 	int (*cmd_xfer)(struct cros_ec_device *ec,
 			struct cros_ec_command *msg);
 
-- 
2.0.0.526.g5318336

Re: [PATCH v2 05/10] mfd: cros_ec: Use struct cros_ec_command to communicate with the EC

[Simon Glass]

On 18 June 2014 12:14, Doug Anderson <dianders@chromium.org> wrote:
> From: Bill Richardson <wfrichar@chromium.org>
>
> This is some internal structure reorganization / renaming to prepare
> for future patches that will add a userspace API to cros_ec.  There
> should be no visible changes.
>
> Signed-off-by: Bill Richardson <wfrichar@chromium.org>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Acked-by: Lee Jones <lee.jones@linaro.org>

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH v2 08/10] mfd: cros_ec: Check result code from EC messages

[Simon Glass]

On 18 June 2014 12:14, Doug Anderson <dianders@chromium.org> wrote:
> From: Bill Richardson <wfrichar@chromium.org>
>
> Just because the host was able to talk to the EC doesn't mean that the EC
> was happy with what it was told. Errors in communincation are not the same
> as error messages from the EC itself.
>
> This change lets the EC report its errors separately.
>
> [dianders: Added common function to cros_ec.c]
>
> Signed-off-by: Bill Richardson <wfrichar@chromium.org>
> Signed-off-by: Doug Anderson <dianders@chromium.org>

This is better.

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH v2 10/10] mfd: cros_ec: move EC interrupt to cros_ec_keyb

[Simon Glass]

On 18 June 2014 12:14, Doug Anderson <dianders@chromium.org> wrote:
> From: Andrew Bresticker <abrestic@chromium.org>
>
> If we receive EC interrupts after the cros_ec driver has probed, but
> before the cros_ec_keyb driver has probed, the cros_ec IRQ handler
> will not run the cros_ec_keyb notifier and the EC will leave the IRQ
> line asserted.  The cros_ec IRQ handler then returns IRQ_HANDLED and
> the resulting flood of interrupts causes the machine to hang.
>
> Since the EC interrupt is currently only used for the keyboard, move
> the setup and handling of the EC interrupt to the cros_ec_keyb driver.
>
> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
> Signed-off-by: Doug Anderson <dianders@chromium.org>

Reviewed-by: Simon Glass <sjg@chromium.org>

We never needed an EC-level interrupt, and have shipped at least three
products now that use this code, so I think it is safe enough to
declare that we won't need it.

Regards,
Simon

Re: [PATCH v2 02/10] mfd: cros_ec: Allow static din/dout buffers with cros_ec_register()

[Lee Jones]

On Wed, 18 Jun 2014, Doug Anderson wrote:

> From: Bill Richardson <wfrichar@chromium.org>
> 
> The lower-level driver may want to provide its own buffers. If so,
> there's no need to allocate new ones.  This already happens to work
> just fine (since we check for size of 0 and use devm allocation), but
> it's good to document it.
> 
> [dianders: Resolved conflicts; documented that no code changes needed
> on mainline]
> 
> Signed-off-by: Bill Richardson <wfrichar@chromium.org>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
> Changes in v2: None
> 
>  include/linux/mfd/cros_ec.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

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

> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> index 7e9fe6e..2ee3190 100644
> --- a/include/linux/mfd/cros_ec.h
> +++ b/include/linux/mfd/cros_ec.h
> @@ -68,8 +68,8 @@ struct cros_ec_msg {
>   * We use this alignment to keep ARM and x86 happy. Probably word
>   * alignment would be OK, there might be a small performance advantage
>   * to using dword.
> - * @din_size: size of din buffer
> - * @dout_size: size of dout buffer
> + * @din_size: size of din buffer to allocate (zero to use static din)
> + * @dout_size: size of dout buffer to allocate (zero to use static dout)
>   * @command_send: send a command
>   * @command_recv: receive a command
>   * @ec_name: name of EC device (e.g. 'chromeos-ec')

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

Re: [PATCH v2 08/10] mfd: cros_ec: Check result code from EC messages

[Lee Jones]

On Wed, 18 Jun 2014, Doug Anderson wrote:

> From: Bill Richardson <wfrichar@chromium.org>
> 
> Just because the host was able to talk to the EC doesn't mean that the EC
> was happy with what it was told. Errors in communincation are not the same
> as error messages from the EC itself.
> 
> This change lets the EC report its errors separately.
> 
> [dianders: Added common function to cros_ec.c]
> 
> Signed-off-by: Bill Richardson <wfrichar@chromium.org>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> ---
> Changes in v2:
> - Added common function to cros_ec.c
> - Changed to dev_dbg() as per http://crosreview.com/66726
> 
>  drivers/mfd/cros_ec.c       | 18 ++++++++++++++++++
>  drivers/mfd/cros_ec_i2c.c   |  8 +++-----
>  drivers/mfd/cros_ec_spi.c   | 19 ++++++-------------
>  include/linux/mfd/cros_ec.h | 12 ++++++++++++
>  4 files changed, 39 insertions(+), 18 deletions(-)

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

> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
> index 4851ed2..83e30c6 100644
> --- a/drivers/mfd/cros_ec.c
> +++ b/drivers/mfd/cros_ec.c
> @@ -44,6 +44,24 @@ int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
>  }
>  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);
> +
>  static irqreturn_t ec_irq_thread(int irq, void *data)
>  {
>  	struct cros_ec_device *ec_dev = data;
> diff --git a/drivers/mfd/cros_ec_i2c.c b/drivers/mfd/cros_ec_i2c.c
> index 5bb32f5..189e7d1 100644
> --- a/drivers/mfd/cros_ec_i2c.c
> +++ b/drivers/mfd/cros_ec_i2c.c
> @@ -92,12 +92,10 @@ static int cros_ec_cmd_xfer_i2c(struct cros_ec_device *ec_dev,
>  	}
>  
>  	/* check response error code */
> -	if (i2c_msg[1].buf[0]) {
> -		dev_warn(ec_dev->dev, "command 0x%02x returned an error %d\n",
> -			 msg->command, i2c_msg[1].buf[0]);
> -		ret = -EINVAL;
> +	msg->result = i2c_msg[1].buf[0];
> +	ret = cros_ec_check_result(ec_dev, msg);
> +	if (ret)
>  		goto done;
> -	}
>  
>  	/* copy response packet payload and compute checksum */
>  	sum = in_buf[0] + in_buf[1];
> diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
> index 09ca789..c975087 100644
> --- a/drivers/mfd/cros_ec_spi.c
> +++ b/drivers/mfd/cros_ec_spi.c
> @@ -289,21 +289,14 @@ static int cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev,
>  		goto exit;
>  	}
>  
> -	/* check response error code */
>  	ptr = ec_dev->din;
> -	if (ptr[0]) {
> -		if (ptr[0] == EC_RES_IN_PROGRESS) {
> -			dev_dbg(ec_dev->dev, "command 0x%02x in progress\n",
> -				ec_msg->command);
> -			ret = -EAGAIN;
> -			goto exit;
> -		}
> -		dev_warn(ec_dev->dev, "command 0x%02x returned an error %d\n",
> -			 ec_msg->command, ptr[0]);
> -		debug_packet(ec_dev->dev, "in_err", ptr, len);
> -		ret = -EINVAL;
> +
> +	/* check response error code */
> +	ec_msg->result = ptr[0];
> +	ret = cros_ec_check_result(ec_dev, ec_msg);
> +	if (ret)
>  		goto exit;
> -	}
> +
>  	len = ptr[1];
>  	sum = ptr[0] + ptr[1];
>  	if (len > ec_msg->insize) {
> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> index 60c0880..1f79f16 100644
> --- a/include/linux/mfd/cros_ec.h
> +++ b/include/linux/mfd/cros_ec.h
> @@ -143,6 +143,18 @@ int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
>  		       struct cros_ec_command *msg);
>  
>  /**
> + * cros_ec_check_result - Check ec_msg->result
> + *
> + * This is used by ChromeOS EC drivers to check the ec_msg->result for
> + * errors and to warn about them.
> + *
> + * @ec_dev: EC device
> + * @msg: Message to check
> + */
> +int cros_ec_check_result(struct cros_ec_device *ec_dev,
> +			 struct cros_ec_command *msg);
> +
> +/**
>   * cros_ec_remove - Remove a ChromeOS EC
>   *
>   * Call this to deregister a ChromeOS EC, then clean up any private data.

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

Re: [PATCH v2 10/10] mfd: cros_ec: move EC interrupt to cros_ec_keyb

[Lee Jones]

On Wed, 18 Jun 2014, Doug Anderson wrote:

> From: Andrew Bresticker <abrestic@chromium.org>
> 
> If we receive EC interrupts after the cros_ec driver has probed, but
> before the cros_ec_keyb driver has probed, the cros_ec IRQ handler
> will not run the cros_ec_keyb notifier and the EC will leave the IRQ
> line asserted.  The cros_ec IRQ handler then returns IRQ_HANDLED and
> the resulting flood of interrupts causes the machine to hang.
> 
> Since the EC interrupt is currently only used for the keyboard, move
> the setup and handling of the EC interrupt to the cros_ec_keyb driver.
> 
> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> ---
> Changes in v2:
> - IRQs should be optional => move EC interrupt to keyboard.
> 
>  drivers/input/keyboard/cros_ec_keyb.c | 58 ++++++++++++++++++++---------------
>  drivers/mfd/cros_ec.c                 | 35 +--------------------
>  include/linux/mfd/cros_ec.h           |  2 --
>  3 files changed, 34 insertions(+), 61 deletions(-)

For the MFD changes:
  Acked-by: Lee Jones <lee.jones@linaro.org>
 
> diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
> index b8341ab..791781a 100644
> --- a/drivers/input/keyboard/cros_ec_keyb.c
> +++ b/drivers/input/keyboard/cros_ec_keyb.c
> @@ -24,8 +24,8 @@
>  #include <linux/module.h>
>  #include <linux/i2c.h>
>  #include <linux/input.h>
> +#include <linux/interrupt.h>
>  #include <linux/kernel.h>
> -#include <linux/notifier.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  #include <linux/input/matrix_keypad.h>
> @@ -42,7 +42,6 @@
>   * @dev: Device pointer
>   * @idev: Input device
>   * @ec: Top level ChromeOS device to use to talk to EC
> - * @event_notifier: interrupt event notifier for transport devices
>   */
>  struct cros_ec_keyb {
>  	unsigned int rows;
> @@ -55,7 +54,6 @@ struct cros_ec_keyb {
>  	struct device *dev;
>  	struct input_dev *idev;
>  	struct cros_ec_device *ec;
> -	struct notifier_block notifier;
>  };
>  
>  
> @@ -173,22 +171,6 @@ static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev,
>  	input_sync(ckdev->idev);
>  }
>  
> -static int cros_ec_keyb_open(struct input_dev *dev)
> -{
> -	struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
> -
> -	return blocking_notifier_chain_register(&ckdev->ec->event_notifier,
> -						&ckdev->notifier);
> -}
> -
> -static void cros_ec_keyb_close(struct input_dev *dev)
> -{
> -	struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
> -
> -	blocking_notifier_chain_unregister(&ckdev->ec->event_notifier,
> -					   &ckdev->notifier);
> -}
> -
>  static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t *kb_state)
>  {
>  	struct cros_ec_command msg = {
> @@ -203,19 +185,41 @@ static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t *kb_state)
>  	return ckdev->ec->cmd_xfer(ckdev->ec, &msg);
>  }
>  
> -static int cros_ec_keyb_work(struct notifier_block *nb,
> -		     unsigned long state, void *_notify)
> +static irqreturn_t cros_ec_keyb_irq(int irq, void *data)
>  {
> +	struct cros_ec_keyb *ckdev = data;
> +	struct cros_ec_device *ec = ckdev->ec;
>  	int ret;
> -	struct cros_ec_keyb *ckdev = container_of(nb, struct cros_ec_keyb,
> -						    notifier);
>  	uint8_t kb_state[ckdev->cols];
>  
> +	if (device_may_wakeup(ec->dev))
> +		pm_wakeup_event(ec->dev, 0);
> +
>  	ret = cros_ec_keyb_get_state(ckdev, kb_state);
>  	if (ret >= 0)
>  		cros_ec_keyb_process(ckdev, kb_state, ret);
> +	else
> +		dev_err(ec->dev, "failed to get keyboard state: %d\n", ret);
>  
> -	return NOTIFY_DONE;
> +	return IRQ_HANDLED;
> +}
> +
> +static int cros_ec_keyb_open(struct input_dev *dev)
> +{
> +	struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
> +	struct cros_ec_device *ec = ckdev->ec;
> +
> +	return request_threaded_irq(ec->irq, NULL, cros_ec_keyb_irq,
> +					IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> +					"cros_ec_keyb", ckdev);
> +}
> +
> +static void cros_ec_keyb_close(struct input_dev *dev)
> +{
> +	struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
> +	struct cros_ec_device *ec = ckdev->ec;
> +
> +	free_irq(ec->irq, ckdev);
>  }
>  
>  static int cros_ec_keyb_probe(struct platform_device *pdev)
> @@ -246,8 +250,12 @@ static int cros_ec_keyb_probe(struct platform_device *pdev)
>  	if (!idev)
>  		return -ENOMEM;
>  
> +	if (!ec->irq) {
> +		dev_err(dev, "no EC IRQ specified\n");
> +		return -EINVAL;
> +	}
> +
>  	ckdev->ec = ec;
> -	ckdev->notifier.notifier_call = cros_ec_keyb_work;
>  	ckdev->dev = dev;
>  	dev_set_drvdata(&pdev->dev, ckdev);
>  
> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
> index 83e30c6..4873f9c 100644
> --- a/drivers/mfd/cros_ec.c
> +++ b/drivers/mfd/cros_ec.c
> @@ -62,18 +62,6 @@ int cros_ec_check_result(struct cros_ec_device *ec_dev,
>  }
>  EXPORT_SYMBOL(cros_ec_check_result);
>  
> -static irqreturn_t ec_irq_thread(int irq, void *data)
> -{
> -	struct cros_ec_device *ec_dev = data;
> -
> -	if (device_may_wakeup(ec_dev->dev))
> -		pm_wakeup_event(ec_dev->dev, 0);
> -
> -	blocking_notifier_call_chain(&ec_dev->event_notifier, 1, ec_dev);
> -
> -	return IRQ_HANDLED;
> -}
> -
>  static const struct mfd_cell cros_devs[] = {
>  	{
>  		.name = "cros-ec-keyb",
> @@ -92,8 +80,6 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
>  	struct device *dev = ec_dev->dev;
>  	int err = 0;
>  
> -	BLOCKING_INIT_NOTIFIER_HEAD(&ec_dev->event_notifier);
> -
>  	if (ec_dev->din_size) {
>  		ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL);
>  		if (!ec_dev->din)
> @@ -105,42 +91,23 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
>  			return -ENOMEM;
>  	}
>  
> -	if (!ec_dev->irq) {
> -		dev_dbg(dev, "no valid IRQ: %d\n", ec_dev->irq);
> -		return err;
> -	}
> -
> -	err = request_threaded_irq(ec_dev->irq, NULL, ec_irq_thread,
> -				   IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> -				   "chromeos-ec", ec_dev);
> -	if (err) {
> -		dev_err(dev, "request irq %d: error %d\n", ec_dev->irq, err);
> -		return err;
> -	}
> -
>  	err = mfd_add_devices(dev, 0, cros_devs,
>  			      ARRAY_SIZE(cros_devs),
>  			      NULL, ec_dev->irq, NULL);
>  	if (err) {
>  		dev_err(dev, "failed to add mfd devices\n");
> -		goto fail_mfd;
> +		return err;
>  	}
>  
>  	dev_info(dev, "Chrome EC device registered\n");
>  
>  	return 0;
> -
> -fail_mfd:
> -	free_irq(ec_dev->irq, ec_dev);
> -
> -	return err;
>  }
>  EXPORT_SYMBOL(cros_ec_register);
>  
>  int cros_ec_remove(struct cros_ec_device *ec_dev)
>  {
>  	mfd_remove_devices(ec_dev->dev);
> -	free_irq(ec_dev->irq, ec_dev);
>  
>  	return 0;
>  }
> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> index 0ebf26f..fcbe9d1 100644
> --- a/include/linux/mfd/cros_ec.h
> +++ b/include/linux/mfd/cros_ec.h
> @@ -62,7 +62,6 @@ struct cros_ec_command {
>   * @dev: Device pointer
>   * @was_wake_device: true if this device was set to wake the system from
>   * sleep at the last suspend
> - * @event_notifier: interrupt event notifier for transport devices
>   * @cmd_xfer: send command to EC and get response
>   *     Returns the number of bytes received if the communication succeeded, but
>   *     that doesn't mean the EC was happy with the command. The caller
> @@ -93,7 +92,6 @@ struct cros_ec_device {
>  	struct device *dev;
>  	bool was_wake_device;
>  	struct class *cros_class;
> -	struct blocking_notifier_head event_notifier;
>  	int (*cmd_xfer)(struct cros_ec_device *ec,
>  			struct cros_ec_command *msg);
>  

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

Re: [PATCH v2 10/10] mfd: cros_ec: move EC interrupt to cros_ec_keyb

[Lee Jones]

On Wed, 18 Jun 2014, Doug Anderson wrote:

> From: Andrew Bresticker <abrestic@chromium.org>
> 
> If we receive EC interrupts after the cros_ec driver has probed, but
> before the cros_ec_keyb driver has probed, the cros_ec IRQ handler
> will not run the cros_ec_keyb notifier and the EC will leave the IRQ
> line asserted.  The cros_ec IRQ handler then returns IRQ_HANDLED and
> the resulting flood of interrupts causes the machine to hang.
> 
> Since the EC interrupt is currently only used for the keyboard, move
> the setup and handling of the EC interrupt to the cros_ec_keyb driver.
> 
> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> ---
> Changes in v2:
> - IRQs should be optional => move EC interrupt to keyboard.
> 
>  drivers/input/keyboard/cros_ec_keyb.c | 58 ++++++++++++++++++++---------------
>  drivers/mfd/cros_ec.c                 | 35 +--------------------
>  include/linux/mfd/cros_ec.h           |  2 --
>  3 files changed, 34 insertions(+), 61 deletions(-)

For the MFD changes:
  Acked-by: Lee Jones <lee.jones@linaro.org>
 
> diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
> index b8341ab..791781a 100644
> --- a/drivers/input/keyboard/cros_ec_keyb.c
> +++ b/drivers/input/keyboard/cros_ec_keyb.c
> @@ -24,8 +24,8 @@
>  #include <linux/module.h>
>  #include <linux/i2c.h>
>  #include <linux/input.h>
> +#include <linux/interrupt.h>
>  #include <linux/kernel.h>
> -#include <linux/notifier.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  #include <linux/input/matrix_keypad.h>
> @@ -42,7 +42,6 @@
>   * @dev: Device pointer
>   * @idev: Input device
>   * @ec: Top level ChromeOS device to use to talk to EC
> - * @event_notifier: interrupt event notifier for transport devices
>   */
>  struct cros_ec_keyb {
>  	unsigned int rows;
> @@ -55,7 +54,6 @@ struct cros_ec_keyb {
>  	struct device *dev;
>  	struct input_dev *idev;
>  	struct cros_ec_device *ec;
> -	struct notifier_block notifier;
>  };
>  
>  
> @@ -173,22 +171,6 @@ static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev,
>  	input_sync(ckdev->idev);
>  }
>  
> -static int cros_ec_keyb_open(struct input_dev *dev)
> -{
> -	struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
> -
> -	return blocking_notifier_chain_register(&ckdev->ec->event_notifier,
> -						&ckdev->notifier);
> -}
> -
> -static void cros_ec_keyb_close(struct input_dev *dev)
> -{
> -	struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
> -
> -	blocking_notifier_chain_unregister(&ckdev->ec->event_notifier,
> -					   &ckdev->notifier);
> -}
> -
>  static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t *kb_state)
>  {
>  	struct cros_ec_command msg = {
> @@ -203,19 +185,41 @@ static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t *kb_state)
>  	return ckdev->ec->cmd_xfer(ckdev->ec, &msg);
>  }
>  
> -static int cros_ec_keyb_work(struct notifier_block *nb,
> -		     unsigned long state, void *_notify)
> +static irqreturn_t cros_ec_keyb_irq(int irq, void *data)
>  {
> +	struct cros_ec_keyb *ckdev = data;
> +	struct cros_ec_device *ec = ckdev->ec;
>  	int ret;
> -	struct cros_ec_keyb *ckdev = container_of(nb, struct cros_ec_keyb,
> -						    notifier);
>  	uint8_t kb_state[ckdev->cols];
>  
> +	if (device_may_wakeup(ec->dev))
> +		pm_wakeup_event(ec->dev, 0);
> +
>  	ret = cros_ec_keyb_get_state(ckdev, kb_state);
>  	if (ret >= 0)
>  		cros_ec_keyb_process(ckdev, kb_state, ret);
> +	else
> +		dev_err(ec->dev, "failed to get keyboard state: %d\n", ret);
>  
> -	return NOTIFY_DONE;
> +	return IRQ_HANDLED;
> +}
> +
> +static int cros_ec_keyb_open(struct input_dev *dev)
> +{
> +	struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
> +	struct cros_ec_device *ec = ckdev->ec;
> +
> +	return request_threaded_irq(ec->irq, NULL, cros_ec_keyb_irq,
> +					IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> +					"cros_ec_keyb", ckdev);
> +}
> +
> +static void cros_ec_keyb_close(struct input_dev *dev)
> +{
> +	struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
> +	struct cros_ec_device *ec = ckdev->ec;
> +
> +	free_irq(ec->irq, ckdev);
>  }
>  
>  static int cros_ec_keyb_probe(struct platform_device *pdev)
> @@ -246,8 +250,12 @@ static int cros_ec_keyb_probe(struct platform_device *pdev)
>  	if (!idev)
>  		return -ENOMEM;
>  
> +	if (!ec->irq) {
> +		dev_err(dev, "no EC IRQ specified\n");
> +		return -EINVAL;
> +	}
> +
>  	ckdev->ec = ec;
> -	ckdev->notifier.notifier_call = cros_ec_keyb_work;
>  	ckdev->dev = dev;
>  	dev_set_drvdata(&pdev->dev, ckdev);
>  
> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
> index 83e30c6..4873f9c 100644
> --- a/drivers/mfd/cros_ec.c
> +++ b/drivers/mfd/cros_ec.c
> @@ -62,18 +62,6 @@ int cros_ec_check_result(struct cros_ec_device *ec_dev,
>  }
>  EXPORT_SYMBOL(cros_ec_check_result);
>  
> -static irqreturn_t ec_irq_thread(int irq, void *data)
> -{
> -	struct cros_ec_device *ec_dev = data;
> -
> -	if (device_may_wakeup(ec_dev->dev))
> -		pm_wakeup_event(ec_dev->dev, 0);
> -
> -	blocking_notifier_call_chain(&ec_dev->event_notifier, 1, ec_dev);
> -
> -	return IRQ_HANDLED;
> -}
> -
>  static const struct mfd_cell cros_devs[] = {
>  	{
>  		.name = "cros-ec-keyb",
> @@ -92,8 +80,6 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
>  	struct device *dev = ec_dev->dev;
>  	int err = 0;
>  
> -	BLOCKING_INIT_NOTIFIER_HEAD(&ec_dev->event_notifier);
> -
>  	if (ec_dev->din_size) {
>  		ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL);
>  		if (!ec_dev->din)
> @@ -105,42 +91,23 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
>  			return -ENOMEM;
>  	}
>  
> -	if (!ec_dev->irq) {
> -		dev_dbg(dev, "no valid IRQ: %d\n", ec_dev->irq);
> -		return err;
> -	}
> -
> -	err = request_threaded_irq(ec_dev->irq, NULL, ec_irq_thread,
> -				   IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> -				   "chromeos-ec", ec_dev);
> -	if (err) {
> -		dev_err(dev, "request irq %d: error %d\n", ec_dev->irq, err);
> -		return err;
> -	}
> -
>  	err = mfd_add_devices(dev, 0, cros_devs,
>  			      ARRAY_SIZE(cros_devs),
>  			      NULL, ec_dev->irq, NULL);
>  	if (err) {
>  		dev_err(dev, "failed to add mfd devices\n");
> -		goto fail_mfd;
> +		return err;
>  	}
>  
>  	dev_info(dev, "Chrome EC device registered\n");
>  
>  	return 0;
> -
> -fail_mfd:
> -	free_irq(ec_dev->irq, ec_dev);
> -
> -	return err;
>  }
>  EXPORT_SYMBOL(cros_ec_register);
>  
>  int cros_ec_remove(struct cros_ec_device *ec_dev)
>  {
>  	mfd_remove_devices(ec_dev->dev);
> -	free_irq(ec_dev->irq, ec_dev);
>  
>  	return 0;
>  }
> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> index 0ebf26f..fcbe9d1 100644
> --- a/include/linux/mfd/cros_ec.h
> +++ b/include/linux/mfd/cros_ec.h
> @@ -62,7 +62,6 @@ struct cros_ec_command {
>   * @dev: Device pointer
>   * @was_wake_device: true if this device was set to wake the system from
>   * sleep at the last suspend
> - * @event_notifier: interrupt event notifier for transport devices
>   * @cmd_xfer: send command to EC and get response
>   *     Returns the number of bytes received if the communication succeeded, but
>   *     that doesn't mean the EC was happy with the command. The caller
> @@ -93,7 +92,6 @@ struct cros_ec_device {
>  	struct device *dev;
>  	bool was_wake_device;
>  	struct class *cros_class;
> -	struct blocking_notifier_head event_notifier;
>  	int (*cmd_xfer)(struct cros_ec_device *ec,
>  			struct cros_ec_command *msg);
>  

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 07/10] mfd: cros_ec: cleanup: Remove EC wrapper functions

[Wolfram Sang]

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

On Wed, Jun 18, 2014 at 11:14:04AM -0700, Doug Anderson wrote:
> From: Bill Richardson <wfrichar@chromium.org>
> 
> Remove the three wrapper functions that talk to the EC without passing all
> the desired arguments and just use the underlying communication function
> that passes everything in a struct intead.
> 
> This is internal code refactoring only. Nothing should change.
> 
> Signed-off-by: Bill Richardson <wfrichar@chromium.org>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Acked-by: Lee Jones <lee.jones@linaro.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>

For the I2C part:

Acked-by: Wolfram Sang <wsa@the-dreams.de>


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 09/10] mfd: cros_ec: ec_dev->cmd_xfer() returns number of bytes received from EC

[Wolfram Sang]

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

On Wed, Jun 18, 2014 at 11:14:06AM -0700, Doug Anderson wrote:
> From: Bill Richardson <wfrichar@chromium.org>
> 
> When communicating with the EC, the cmd_xfer() function should return the
> number of bytes it received from the EC, or negative on error.
> 
> Signed-off-by: Bill Richardson <wfrichar@chromium.org>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Acked-by: Lee Jones <lee.jones@linaro.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>

For the I2C part:

Acked-by: Wolfram Sang <wsa@the-dreams.de>


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 09/10] mfd: cros_ec: ec_dev->cmd_xfer() returns number of bytes received from EC

[Wolfram Sang]

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

On Wed, Jun 18, 2014 at 11:14:06AM -0700, Doug Anderson wrote:
> From: Bill Richardson <wfrichar-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> 
> When communicating with the EC, the cmd_xfer() function should return the
> number of bytes it received from the EC, or negative on error.
> 
> Signed-off-by: Bill Richardson <wfrichar-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> Signed-off-by: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> Acked-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Reviewed-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

For the I2C part:

Acked-by: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 07/10] mfd: cros_ec: cleanup: Remove EC wrapper functions

[Dmitry Torokhov]

On Fri, Jun 27, 2014 at 5:31 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
> On Wed, Jun 18, 2014 at 11:14:04AM -0700, Doug Anderson wrote:
>> From: Bill Richardson <wfrichar@chromium.org>
>>
>> Remove the three wrapper functions that talk to the EC without passing all
>> the desired arguments and just use the underlying communication function
>> that passes everything in a struct intead.
>>
>> This is internal code refactoring only. Nothing should change.
>>
>> Signed-off-by: Bill Richardson <wfrichar@chromium.org>
>> Signed-off-by: Doug Anderson <dianders@chromium.org>
>> Acked-by: Lee Jones <lee.jones@linaro.org>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> For the I2C part:
>
> Acked-by: Wolfram Sang <wsa@the-dreams.de>
>

I'm good with input bits as well.

Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Thanks.

-- 
Dmitry

Re: [PATCH v2 07/10] mfd: cros_ec: cleanup: Remove EC wrapper functions

[Dmitry Torokhov]

On Fri, Jun 27, 2014 at 5:31 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
> On Wed, Jun 18, 2014 at 11:14:04AM -0700, Doug Anderson wrote:
>> From: Bill Richardson <wfrichar@chromium.org>
>>
>> Remove the three wrapper functions that talk to the EC without passing all
>> the desired arguments and just use the underlying communication function
>> that passes everything in a struct intead.
>>
>> This is internal code refactoring only. Nothing should change.
>>
>> Signed-off-by: Bill Richardson <wfrichar@chromium.org>
>> Signed-off-by: Doug Anderson <dianders@chromium.org>
>> Acked-by: Lee Jones <lee.jones@linaro.org>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> For the I2C part:
>
> Acked-by: Wolfram Sang <wsa@the-dreams.de>
>

I'm good with input bits as well.

Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Thanks.

-- 
Dmitry

Re: [PATCH v2 01/10] mfd: cros_ec: Fix the comment on cros_ec_remove()

[Lee Jones]

On Wed, 18 Jun 2014, Doug Anderson wrote:

> From: Bill Richardson <wfrichar@chromium.org>
> 
> This comment was incorrect, so update it.
> 
> Signed-off-by: Bill Richardson <wfrichar@chromium.org>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Acked-by: Lee Jones <lee.jones@linaro.org>
> ---
> Changes in v2: None
> 
>  include/linux/mfd/cros_ec.h | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Very well, patch applied than.

Clause: There is a chance that this patch might not be seen in -next
for ~24-48hrs.  If it's not there by 72hrs, feel free to poke.

> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> index 887ef4f..7e9fe6e 100644
> --- a/include/linux/mfd/cros_ec.h
> +++ b/include/linux/mfd/cros_ec.h
> @@ -148,8 +148,7 @@ int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
>  /**
>   * cros_ec_remove - Remove a ChromeOS EC
>   *
> - * Call this to deregister a ChromeOS EC. After this you should call
> - * cros_ec_free().
> + * Call this to deregister a ChromeOS EC, then clean up any private data.
>   *
>   * @ec_dev: Device to register
>   * @return 0 if ok, -ve on error

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

Re: [PATCH v2 02/10] mfd: cros_ec: Allow static din/dout buffers with cros_ec_register()

[Lee Jones]

On Wed, 18 Jun 2014, Doug Anderson wrote:

> From: Bill Richardson <wfrichar@chromium.org>
> 
> The lower-level driver may want to provide its own buffers. If so,
> there's no need to allocate new ones.  This already happens to work
> just fine (since we check for size of 0 and use devm allocation), but
> it's good to document it.
> 
> [dianders: Resolved conflicts; documented that no code changes needed
> on mainline]
> 
> Signed-off-by: Bill Richardson <wfrichar@chromium.org>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
> Changes in v2: None
> 
>  include/linux/mfd/cros_ec.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Patch applied.

Clause: There is a chance that this patch might not be seen in -next
for ~24-48hrs.  If it's not there by 72hrs, feel free to poke.

> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> index 7e9fe6e..2ee3190 100644
> --- a/include/linux/mfd/cros_ec.h
> +++ b/include/linux/mfd/cros_ec.h
> @@ -68,8 +68,8 @@ struct cros_ec_msg {
>   * We use this alignment to keep ARM and x86 happy. Probably word
>   * alignment would be OK, there might be a small performance advantage
>   * to using dword.
> - * @din_size: size of din buffer
> - * @dout_size: size of dout buffer
> + * @din_size: size of din buffer to allocate (zero to use static din)
> + * @dout_size: size of dout buffer to allocate (zero to use static dout)
>   * @command_send: send a command
>   * @command_recv: receive a command
>   * @ec_name: name of EC device (e.g. 'chromeos-ec')

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

Re: [PATCH v2 03/10] mfd: cros_ec: Tweak struct cros_ec_device for clarity

[Lee Jones]

On Wed, 18 Jun 2014, Doug Anderson wrote:

> From: Bill Richardson <wfrichar@chromium.org>
> 
> The members of struct cros_ec_device were improperly commented, and
> intermixed the private and public sections. This is just cleanup to make it
> more obvious what goes with what.
> 
> [dianders: left lock in the structure but gave it the name that will
> eventually be used.]
> 
> Signed-off-by: Bill Richardson <wfrichar@chromium.org>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Acked-by: Lee Jones <lee.jones@linaro.org>
> ---
> Changes in v2: None
> 
>  drivers/mfd/cros_ec.c       |  2 +-
>  drivers/mfd/cros_ec_i2c.c   |  4 +--
>  drivers/mfd/cros_ec_spi.c   | 10 +++----
>  include/linux/mfd/cros_ec.h | 65 ++++++++++++++++++++++++---------------------
>  4 files changed, 43 insertions(+), 38 deletions(-)

Patch applied.

Clause: There is a chance that this patch might not be seen in -next
for ~24-48hrs.  If it's not there by 72hrs, feel free to poke.

> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
> index 38fe9bf..04e053c 100644
> --- a/drivers/mfd/cros_ec.c
> +++ b/drivers/mfd/cros_ec.c
> @@ -57,7 +57,7 @@ static int cros_ec_command_sendrecv(struct cros_ec_device *ec_dev,
>  	msg.in_buf = in_buf;
>  	msg.in_len = in_len;
>  
> -	return ec_dev->command_xfer(ec_dev, &msg);
> +	return ec_dev->cmd_xfer(ec_dev, &msg);
>  }
>  
>  static int cros_ec_command_recv(struct cros_ec_device *ec_dev,
> diff --git a/drivers/mfd/cros_ec_i2c.c b/drivers/mfd/cros_ec_i2c.c
> index 4f71be9..777e529 100644
> --- a/drivers/mfd/cros_ec_i2c.c
> +++ b/drivers/mfd/cros_ec_i2c.c
> @@ -29,7 +29,7 @@ static inline struct cros_ec_device *to_ec_dev(struct device *dev)
>  	return i2c_get_clientdata(client);
>  }
>  
> -static int cros_ec_command_xfer(struct cros_ec_device *ec_dev,
> +static int cros_ec_cmd_xfer_i2c(struct cros_ec_device *ec_dev,
>  				struct cros_ec_msg *msg)
>  {
>  	struct i2c_client *client = ec_dev->priv;
> @@ -136,7 +136,7 @@ static int cros_ec_i2c_probe(struct i2c_client *client,
>  	ec_dev->dev = dev;
>  	ec_dev->priv = client;
>  	ec_dev->irq = client->irq;
> -	ec_dev->command_xfer = cros_ec_command_xfer;
> +	ec_dev->cmd_xfer = cros_ec_cmd_xfer_i2c;
>  	ec_dev->ec_name = client->name;
>  	ec_dev->phys_name = client->adapter->name;
>  	ec_dev->parent = &client->dev;
> diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
> index 0b8d328..52d4d7b 100644
> --- a/drivers/mfd/cros_ec_spi.c
> +++ b/drivers/mfd/cros_ec_spi.c
> @@ -73,7 +73,7 @@
>   *	if no record
>   * @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.
> - * @lock: mutex to ensure only one user of cros_ec_command_spi_xfer at a time
> + * @lock: mutex to ensure only one user of cros_ec_cmd_xfer_spi at a time
>   */
>  struct cros_ec_spi {
>  	struct spi_device *spi;
> @@ -210,13 +210,13 @@ static int cros_ec_spi_receive_response(struct cros_ec_device *ec_dev,
>  }
>  
>  /**
> - * cros_ec_command_spi_xfer - Transfer a message over SPI and receive the reply
> + * cros_ec_cmd_xfer_spi - Transfer a message over SPI and receive the reply
>   *
>   * @ec_dev: ChromeOS EC device
>   * @ec_msg: Message to transfer
>   */
> -static int cros_ec_command_spi_xfer(struct cros_ec_device *ec_dev,
> -				    struct cros_ec_msg *ec_msg)
> +static int cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev,
> +				struct cros_ec_msg *ec_msg)
>  {
>  	struct cros_ec_spi *ec_spi = ec_dev->priv;
>  	struct spi_transfer trans;
> @@ -372,7 +372,7 @@ static int cros_ec_spi_probe(struct spi_device *spi)
>  	ec_dev->dev = dev;
>  	ec_dev->priv = ec_spi;
>  	ec_dev->irq = spi->irq;
> -	ec_dev->command_xfer = cros_ec_command_spi_xfer;
> +	ec_dev->cmd_xfer = cros_ec_cmd_xfer_spi;
>  	ec_dev->ec_name = ec_spi->spi->modalias;
>  	ec_dev->phys_name = dev_name(&ec_spi->spi->dev);
>  	ec_dev->parent = &ec_spi->spi->dev;
> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> index 2ee3190..79a3585 100644
> --- a/include/linux/mfd/cros_ec.h
> +++ b/include/linux/mfd/cros_ec.h
> @@ -16,7 +16,9 @@
>  #ifndef __LINUX_MFD_CROS_EC_H
>  #define __LINUX_MFD_CROS_EC_H
>  
> +#include <linux/notifier.h>
>  #include <linux/mfd/cros_ec_commands.h>
> +#include <linux/mutex.h>
>  
>  /*
>   * Command interface between EC and AP, for LPC, I2C and SPI interfaces.
> @@ -55,34 +57,53 @@ struct cros_ec_msg {
>  /**
>   * 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
> + * @was_wake_device: true if this device was set to wake the system from
> + * sleep at the last suspend
> + * @event_notifier: interrupt event notifier for transport devices
> + * @command_send: send a command
> + * @command_recv: receive a response
> + * @command_sendrecv: send a command and receive a response
> + *
>   * @name: Name of this EC interface
>   * @priv: Private data
>   * @irq: Interrupt to use
> - * @din: input buffer (from EC)
> - * @dout: output buffer (to EC)
> + * @din: input buffer (for data from EC)
> + * @dout: output buffer (for data to EC)
>   * \note
>   * These two buffers will always be dword-aligned and include enough
>   * space for up to 7 word-alignment bytes also, so we can ensure that
>   * the body of the message is always dword-aligned (64-bit).
> - *
>   * We use this alignment to keep ARM and x86 happy. Probably word
>   * alignment would be OK, there might be a small performance advantage
>   * to using dword.
>   * @din_size: size of din buffer to allocate (zero to use static din)
>   * @dout_size: size of dout buffer to allocate (zero to use static dout)
> - * @command_send: send a command
> - * @command_recv: receive a command
> - * @ec_name: name of EC device (e.g. 'chromeos-ec')
> - * @phys_name: name of physical comms layer (e.g. 'i2c-4')
>   * @parent: pointer to parent device (e.g. i2c or spi device)
> - * @dev: Device pointer
> - * dev_lock: Lock to prevent concurrent access
>   * @wake_enabled: true if this device can wake the system from sleep
> - * @was_wake_device: true if this device was set to wake the system from
> - * sleep at the last suspend
> - * @event_notifier: interrupt event notifier for transport devices
> + * @lock: one transaction at a time
> + * @cmd_xfer: low-level channel to the EC
>   */
>  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;
> +	bool was_wake_device;
> +	struct class *cros_class;
> +	struct blocking_notifier_head event_notifier;
> +	int (*command_send)(struct cros_ec_device *ec,
> +			    uint16_t cmd, void *out_buf, int out_len);
> +	int (*command_recv)(struct cros_ec_device *ec,
> +			    uint16_t cmd, void *in_buf, int in_len);
> +	int (*command_sendrecv)(struct cros_ec_device *ec,
> +				uint16_t cmd, void *out_buf, int out_len,
> +				void *in_buf, int in_len);
> +
> +	/* These are used to implement the platform-specific interface */
>  	const char *name;
>  	void *priv;
>  	int irq;
> @@ -90,26 +111,10 @@ struct cros_ec_device {
>  	uint8_t *dout;
>  	int din_size;
>  	int dout_size;
> -	int (*command_send)(struct cros_ec_device *ec,
> -			uint16_t cmd, void *out_buf, int out_len);
> -	int (*command_recv)(struct cros_ec_device *ec,
> -			uint16_t cmd, void *in_buf, int in_len);
> -	int (*command_sendrecv)(struct cros_ec_device *ec,
> -			uint16_t cmd, void *out_buf, int out_len,
> -			void *in_buf, int in_len);
> -	int (*command_xfer)(struct cros_ec_device *ec,
> -			struct cros_ec_msg *msg);
> -
> -	const char *ec_name;
> -	const char *phys_name;
>  	struct device *parent;
> -
> -	/* These are --private-- fields - do not assign */
> -	struct device *dev;
> -	struct mutex dev_lock;
>  	bool wake_enabled;
> -	bool was_wake_device;
> -	struct blocking_notifier_head event_notifier;
> +	struct mutex lock;
> +	int (*cmd_xfer)(struct cros_ec_device *ec, struct cros_ec_msg *msg);
>  };
>  
>  /**

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

Re: [PATCH v2 04/10] mdf: cros_ec: Detect in-progress commands

[Lee Jones]

On Wed, 18 Jun 2014, Doug Anderson wrote:

> From: Simon Glass <sjg@chromium.org>
> 
> Some commands take a while to execute. Use -EAGAIN to signal this to the
> caller.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Acked-by: Lee Jones <lee.jones@linaro.org>
> ---
> Changes in v2: None
> 
>  drivers/mfd/cros_ec_spi.c | 6 ++++++
>  1 file changed, 6 insertions(+)

Patch applied.

Clause: There is a chance that this patch might not be seen in -next
for ~24-48hrs.  If it's not there by 72hrs, feel free to poke.

> diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
> index 52d4d7b..c29a2d7 100644
> --- a/drivers/mfd/cros_ec_spi.c
> +++ b/drivers/mfd/cros_ec_spi.c
> @@ -292,6 +292,12 @@ static int cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev,
>  	/* check response error code */
>  	ptr = ec_dev->din;
>  	if (ptr[0]) {
> +		if (ptr[0] == EC_RES_IN_PROGRESS) {
> +			dev_dbg(ec_dev->dev, "command 0x%02x in progress\n",
> +				ec_msg->cmd);
> +			ret = -EAGAIN;
> +			goto exit;
> +		}
>  		dev_warn(ec_dev->dev, "command 0x%02x returned an error %d\n",
>  			 ec_msg->cmd, ptr[0]);
>  		debug_packet(ec_dev->dev, "in_err", ptr, len);

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

Re: [PATCH v2 05/10] mfd: cros_ec: Use struct cros_ec_command to communicate with the EC

[Lee Jones]

On Wed, 18 Jun 2014, Doug Anderson wrote:

> From: Bill Richardson <wfrichar@chromium.org>
> 
> This is some internal structure reorganization / renaming to prepare
> for future patches that will add a userspace API to cros_ec.  There
> should be no visible changes.
> 
> Signed-off-by: Bill Richardson <wfrichar@chromium.org>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Acked-by: Lee Jones <lee.jones@linaro.org>
> ---
> Changes in v2: None
> 
>  drivers/mfd/cros_ec.c       | 28 ++++++++++++++--------------
>  drivers/mfd/cros_ec_i2c.c   | 24 ++++++++++++------------
>  drivers/mfd/cros_ec_spi.c   | 16 ++++++++--------
>  include/linux/mfd/cros_ec.h | 35 ++++++++++++++++++-----------------
>  4 files changed, 52 insertions(+), 51 deletions(-)

Patch applied with Simon's Reviewed-by.

Clause: There is a chance that this patch might not be seen in -next
for ~24-48hrs.  If it's not there by 72hrs, feel free to poke.

> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
> index 04e053c..2e86c28 100644
> --- a/drivers/mfd/cros_ec.c
> +++ b/drivers/mfd/cros_ec.c
> @@ -25,22 +25,22 @@
>  #include <linux/mfd/cros_ec_commands.h>
>  
>  int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
> -		       struct cros_ec_msg *msg)
> +		       struct cros_ec_command *msg)
>  {
>  	uint8_t *out;
>  	int csum, i;
>  
> -	BUG_ON(msg->out_len > EC_PROTO2_MAX_PARAM_SIZE);
> +	BUG_ON(msg->outsize > EC_PROTO2_MAX_PARAM_SIZE);
>  	out = ec_dev->dout;
>  	out[0] = EC_CMD_VERSION0 + msg->version;
> -	out[1] = msg->cmd;
> -	out[2] = msg->out_len;
> +	out[1] = msg->command;
> +	out[2] = msg->outsize;
>  	csum = out[0] + out[1] + out[2];
> -	for (i = 0; i < msg->out_len; i++)
> -		csum += out[EC_MSG_TX_HEADER_BYTES + i] = msg->out_buf[i];
> -	out[EC_MSG_TX_HEADER_BYTES + msg->out_len] = (uint8_t)(csum & 0xff);
> +	for (i = 0; i < msg->outsize; i++)
> +		csum += out[EC_MSG_TX_HEADER_BYTES + i] = msg->outdata[i];
> +	out[EC_MSG_TX_HEADER_BYTES + msg->outsize] = (uint8_t)(csum & 0xff);
>  
> -	return EC_MSG_TX_PROTO_BYTES + msg->out_len;
> +	return EC_MSG_TX_PROTO_BYTES + msg->outsize;
>  }
>  EXPORT_SYMBOL(cros_ec_prepare_tx);
>  
> @@ -48,14 +48,14 @@ static int cros_ec_command_sendrecv(struct cros_ec_device *ec_dev,
>  		uint16_t cmd, void *out_buf, int out_len,
>  		void *in_buf, int in_len)
>  {
> -	struct cros_ec_msg msg;
> +	struct cros_ec_command msg;
>  
>  	msg.version = cmd >> 8;
> -	msg.cmd = cmd & 0xff;
> -	msg.out_buf = out_buf;
> -	msg.out_len = out_len;
> -	msg.in_buf = in_buf;
> -	msg.in_len = in_len;
> +	msg.command = cmd & 0xff;
> +	msg.outdata = out_buf;
> +	msg.outsize = out_len;
> +	msg.indata = in_buf;
> +	msg.insize = in_len;
>  
>  	return ec_dev->cmd_xfer(ec_dev, &msg);
>  }
> diff --git a/drivers/mfd/cros_ec_i2c.c b/drivers/mfd/cros_ec_i2c.c
> index 777e529..37ed12f 100644
> --- a/drivers/mfd/cros_ec_i2c.c
> +++ b/drivers/mfd/cros_ec_i2c.c
> @@ -30,7 +30,7 @@ static inline struct cros_ec_device *to_ec_dev(struct device *dev)
>  }
>  
>  static int cros_ec_cmd_xfer_i2c(struct cros_ec_device *ec_dev,
> -				struct cros_ec_msg *msg)
> +				struct cros_ec_command *msg)
>  {
>  	struct i2c_client *client = ec_dev->priv;
>  	int ret = -ENOMEM;
> @@ -50,7 +50,7 @@ static int cros_ec_cmd_xfer_i2c(struct cros_ec_device *ec_dev,
>  	 * allocate larger packet (one byte for checksum, one byte for
>  	 * length, and one for result code)
>  	 */
> -	packet_len = msg->in_len + 3;
> +	packet_len = msg->insize + 3;
>  	in_buf = kzalloc(packet_len, GFP_KERNEL);
>  	if (!in_buf)
>  		goto done;
> @@ -61,7 +61,7 @@ static int cros_ec_cmd_xfer_i2c(struct cros_ec_device *ec_dev,
>  	 * allocate larger packet (one byte for checksum, one for
>  	 * command code, one for length, and one for command version)
>  	 */
> -	packet_len = msg->out_len + 4;
> +	packet_len = msg->outsize + 4;
>  	out_buf = kzalloc(packet_len, GFP_KERNEL);
>  	if (!out_buf)
>  		goto done;
> @@ -69,16 +69,16 @@ static int cros_ec_cmd_xfer_i2c(struct cros_ec_device *ec_dev,
>  	i2c_msg[0].buf = (char *)out_buf;
>  
>  	out_buf[0] = EC_CMD_VERSION0 + msg->version;
> -	out_buf[1] = msg->cmd;
> -	out_buf[2] = msg->out_len;
> +	out_buf[1] = msg->command;
> +	out_buf[2] = msg->outsize;
>  
>  	/* copy message payload and compute checksum */
>  	sum = out_buf[0] + out_buf[1] + out_buf[2];
> -	for (i = 0; i < msg->out_len; i++) {
> -		out_buf[3 + i] = msg->out_buf[i];
> +	for (i = 0; i < msg->outsize; i++) {
> +		out_buf[3 + i] = msg->outdata[i];
>  		sum += out_buf[3 + i];
>  	}
> -	out_buf[3 + msg->out_len] = sum;
> +	out_buf[3 + msg->outsize] = sum;
>  
>  	/* send command to EC and read answer */
>  	ret = i2c_transfer(client->adapter, i2c_msg, 2);
> @@ -94,20 +94,20 @@ static int cros_ec_cmd_xfer_i2c(struct cros_ec_device *ec_dev,
>  	/* check response error code */
>  	if (i2c_msg[1].buf[0]) {
>  		dev_warn(ec_dev->dev, "command 0x%02x returned an error %d\n",
> -			 msg->cmd, i2c_msg[1].buf[0]);
> +			 msg->command, i2c_msg[1].buf[0]);
>  		ret = -EINVAL;
>  		goto done;
>  	}
>  
>  	/* copy response packet payload and compute checksum */
>  	sum = in_buf[0] + in_buf[1];
> -	for (i = 0; i < msg->in_len; i++) {
> -		msg->in_buf[i] = in_buf[2 + i];
> +	for (i = 0; i < msg->insize; i++) {
> +		msg->indata[i] = in_buf[2 + i];
>  		sum += in_buf[2 + i];
>  	}
>  	dev_dbg(ec_dev->dev, "packet: %*ph, sum = %02x\n",
>  		i2c_msg[1].len, in_buf, sum);
> -	if (sum != in_buf[2 + msg->in_len]) {
> +	if (sum != in_buf[2 + msg->insize]) {
>  		dev_err(ec_dev->dev, "bad packet checksum\n");
>  		ret = -EBADMSG;
>  		goto done;
> diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
> index c29a2d7..2d713fe 100644
> --- a/drivers/mfd/cros_ec_spi.c
> +++ b/drivers/mfd/cros_ec_spi.c
> @@ -216,7 +216,7 @@ static int cros_ec_spi_receive_response(struct cros_ec_device *ec_dev,
>   * @ec_msg: Message to transfer
>   */
>  static int cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev,
> -				struct cros_ec_msg *ec_msg)
> +				struct cros_ec_command *ec_msg)
>  {
>  	struct cros_ec_spi *ec_spi = ec_dev->priv;
>  	struct spi_transfer trans;
> @@ -261,7 +261,7 @@ 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->in_len + EC_MSG_TX_PROTO_BYTES);
> +				ec_msg->insize + EC_MSG_TX_PROTO_BYTES);
>  	} else {
>  		dev_err(ec_dev->dev, "spi transfer failed: %d\n", ret);
>  	}
> @@ -294,21 +294,21 @@ static int cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev,
>  	if (ptr[0]) {
>  		if (ptr[0] == EC_RES_IN_PROGRESS) {
>  			dev_dbg(ec_dev->dev, "command 0x%02x in progress\n",
> -				ec_msg->cmd);
> +				ec_msg->command);
>  			ret = -EAGAIN;
>  			goto exit;
>  		}
>  		dev_warn(ec_dev->dev, "command 0x%02x returned an error %d\n",
> -			 ec_msg->cmd, ptr[0]);
> +			 ec_msg->command, ptr[0]);
>  		debug_packet(ec_dev->dev, "in_err", ptr, len);
>  		ret = -EINVAL;
>  		goto exit;
>  	}
>  	len = ptr[1];
>  	sum = ptr[0] + ptr[1];
> -	if (len > ec_msg->in_len) {
> +	if (len > ec_msg->insize) {
>  		dev_err(ec_dev->dev, "packet too long (%d bytes, expected %d)",
> -			len, ec_msg->in_len);
> +			len, ec_msg->insize);
>  		ret = -ENOSPC;
>  		goto exit;
>  	}
> @@ -316,8 +316,8 @@ static int cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev,
>  	/* copy response packet payload and compute checksum */
>  	for (i = 0; i < len; i++) {
>  		sum += ptr[i + 2];
> -		if (ec_msg->in_len)
> -			ec_msg->in_buf[i] = ptr[i + 2];
> +		if (ec_msg->insize)
> +			ec_msg->indata[i] = ptr[i + 2];
>  	}
>  	sum &= 0xff;
>  
> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> index 79a3585..f27c037 100644
> --- a/include/linux/mfd/cros_ec.h
> +++ b/include/linux/mfd/cros_ec.h
> @@ -35,23 +35,23 @@ enum {
>  					EC_MSG_TX_PROTO_BYTES,
>  };
>  
> -/**
> - * struct cros_ec_msg - A message sent to the EC, and its reply
> - *
> +/*
>   * @version: Command version number (often 0)
> - * @cmd: Command to send (EC_CMD_...)
> - * @out_buf: Outgoing payload (to EC)
> - * @outlen: Outgoing length
> - * @in_buf: Incoming payload (from EC)
> - * @in_len: Incoming length
> + * @command: Command to send (EC_CMD_...)
> + * @outdata: Outgoing data to EC
> + * @outsize: Outgoing length in bytes
> + * @indata: Where to put the incoming data from EC
> + * @insize: Incoming length in bytes (filled in by EC)
> + * @result: EC's response to the command (separate from communication failure)
>   */
> -struct cros_ec_msg {
> -	u8 version;
> -	u8 cmd;
> -	uint8_t *out_buf;
> -	int out_len;
> -	uint8_t *in_buf;
> -	int in_len;
> +struct cros_ec_command {
> +	uint32_t version;
> +	uint32_t command;
> +	uint8_t *outdata;
> +	uint32_t outsize;
> +	uint8_t *indata;
> +	uint32_t insize;
> +	uint32_t result;
>  };
>  
>  /**
> @@ -114,7 +114,8 @@ struct cros_ec_device {
>  	struct device *parent;
>  	bool wake_enabled;
>  	struct mutex lock;
> -	int (*cmd_xfer)(struct cros_ec_device *ec, struct cros_ec_msg *msg);
> +	int (*cmd_xfer)(struct cros_ec_device *ec,
> +			struct cros_ec_command *msg);
>  };
>  
>  /**
> @@ -148,7 +149,7 @@ int cros_ec_resume(struct cros_ec_device *ec_dev);
>   * @msg: Message to write
>   */
>  int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
> -		       struct cros_ec_msg *msg);
> +		       struct cros_ec_command *msg);
>  
>  /**
>   * cros_ec_remove - Remove a ChromeOS EC

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

Re: [PATCH v2 06/10] mfd: cros_ec: cleanup: remove unused fields from struct cros_ec_device

[Lee Jones]

On Wed, 18 Jun 2014, Doug Anderson wrote:

> From: Bill Richardson <wfrichar@chromium.org>
> 
> struct cros_ec_device has a superfluous "name" field. We can get all the
> debugging info we need from the existing ec_name and phys_name fields, so
> let's take out the extra field.
> 
> The printout also has sufficient info in it without explicitly adding
> the transport.  Before this change:
>   cros-ec-spi spi2.0: Chrome EC (SPI)
> 
> After this change:
>   cros-ec-spi spi2.0: Chrome EC device registered
> 
> Signed-off-by: Bill Richardson <wfrichar@chromium.org>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Acked-by: Lee Jones <lee.jones@linaro.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
> Changes in v2:
> - Include example printouts before/after in commit message.

Patch applied.

Clause: There is a chance that this patch might not be seen in -next
for ~24-48hrs.  If it's not there by 72hrs, feel free to poke.

>  drivers/mfd/cros_ec.c       | 2 +-
>  drivers/mfd/cros_ec_i2c.c   | 1 -
>  drivers/mfd/cros_ec_spi.c   | 1 -
>  include/linux/mfd/cros_ec.h | 2 --
>  4 files changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
> index 2e86c28..49ed8c3 100644
> --- a/drivers/mfd/cros_ec.c
> +++ b/drivers/mfd/cros_ec.c
> @@ -140,7 +140,7 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
>  		goto fail_mfd;
>  	}
>  
> -	dev_info(dev, "Chrome EC (%s)\n", ec_dev->name);
> +	dev_info(dev, "Chrome EC device registered\n");
>  
>  	return 0;
>  
> diff --git a/drivers/mfd/cros_ec_i2c.c b/drivers/mfd/cros_ec_i2c.c
> index 37ed12f..5bb32f5 100644
> --- a/drivers/mfd/cros_ec_i2c.c
> +++ b/drivers/mfd/cros_ec_i2c.c
> @@ -132,7 +132,6 @@ static int cros_ec_i2c_probe(struct i2c_client *client,
>  		return -ENOMEM;
>  
>  	i2c_set_clientdata(client, ec_dev);
> -	ec_dev->name = "I2C";
>  	ec_dev->dev = dev;
>  	ec_dev->priv = client;
>  	ec_dev->irq = client->irq;
> diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
> index 2d713fe..09ca789 100644
> --- a/drivers/mfd/cros_ec_spi.c
> +++ b/drivers/mfd/cros_ec_spi.c
> @@ -374,7 +374,6 @@ static int cros_ec_spi_probe(struct spi_device *spi)
>  	cros_ec_spi_dt_probe(ec_spi, dev);
>  
>  	spi_set_drvdata(spi, ec_dev);
> -	ec_dev->name = "SPI";
>  	ec_dev->dev = dev;
>  	ec_dev->priv = ec_spi;
>  	ec_dev->irq = spi->irq;
> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> index f27c037..2b0c598 100644
> --- a/include/linux/mfd/cros_ec.h
> +++ b/include/linux/mfd/cros_ec.h
> @@ -67,7 +67,6 @@ struct cros_ec_command {
>   * @command_recv: receive a response
>   * @command_sendrecv: send a command and receive a response
>   *
> - * @name: Name of this EC interface
>   * @priv: Private data
>   * @irq: Interrupt to use
>   * @din: input buffer (for data from EC)
> @@ -104,7 +103,6 @@ struct cros_ec_device {
>  				void *in_buf, int in_len);
>  
>  	/* These are used to implement the platform-specific interface */
> -	const char *name;
>  	void *priv;
>  	int irq;
>  	uint8_t *din;

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

Re: [PATCH v2 07/10] mfd: cros_ec: cleanup: Remove EC wrapper functions

[Lee Jones]

On Wed, 18 Jun 2014, Doug Anderson wrote:

> From: Bill Richardson <wfrichar@chromium.org>
> 
> Remove the three wrapper functions that talk to the EC without passing all
> the desired arguments and just use the underlying communication function
> that passes everything in a struct intead.
> 
> This is internal code refactoring only. Nothing should change.
> 
> Signed-off-by: Bill Richardson <wfrichar@chromium.org>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Acked-by: Lee Jones <lee.jones@linaro.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
> Changes in v2:
> - Removed unneeded "ret" variable.
> 
>  drivers/i2c/busses/i2c-cros-ec-tunnel.c | 15 +++++++++++----
>  drivers/input/keyboard/cros_ec_keyb.c   | 12 ++++++++++--
>  drivers/mfd/cros_ec.c                   | 32 --------------------------------
>  include/linux/mfd/cros_ec.h             | 19 ++++++-------------
>  4 files changed, 27 insertions(+), 51 deletions(-)

Patch applied with Wolfram and Dmitry's Acks.

Clause: There is a chance that this patch might not be seen in -next
for ~24-48hrs.  If it's not there by 72hrs, feel free to poke.

> diff --git a/drivers/i2c/busses/i2c-cros-ec-tunnel.c b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
> index 8e7a714..dd07818 100644
> --- a/drivers/i2c/busses/i2c-cros-ec-tunnel.c
> +++ b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
> @@ -183,6 +183,7 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg i2c_msgs[],
>  	u8 *request = NULL;
>  	u8 *response = NULL;
>  	int result;
> +	struct cros_ec_command msg;
>  
>  	request_len = ec_i2c_count_message(i2c_msgs, num);
>  	if (request_len < 0) {
> @@ -218,9 +219,15 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg i2c_msgs[],
>  	}
>  
>  	ec_i2c_construct_message(request, i2c_msgs, num, bus_num);
> -	result = bus->ec->command_sendrecv(bus->ec, EC_CMD_I2C_PASSTHRU,
> -					   request, request_len,
> -					   response, response_len);
> +
> +	msg.version = 0;
> +	msg.command = EC_CMD_I2C_PASSTHRU;
> +	msg.outdata = request;
> +	msg.outsize = request_len;
> +	msg.indata = response;
> +	msg.insize = response_len;
> +
> +	result = bus->ec->cmd_xfer(bus->ec, &msg);
>  	if (result)
>  		goto exit;
>  
> @@ -258,7 +265,7 @@ static int ec_i2c_probe(struct platform_device *pdev)
>  	u32 remote_bus;
>  	int err;
>  
> -	if (!ec->command_sendrecv) {
> +	if (!ec->cmd_xfer) {
>  		dev_err(dev, "Missing sendrecv\n");
>  		return -EINVAL;
>  	}
> diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
> index 4083796..b8341ab 100644
> --- a/drivers/input/keyboard/cros_ec_keyb.c
> +++ b/drivers/input/keyboard/cros_ec_keyb.c
> @@ -191,8 +191,16 @@ static void cros_ec_keyb_close(struct input_dev *dev)
>  
>  static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t *kb_state)
>  {
> -	return ckdev->ec->command_recv(ckdev->ec, EC_CMD_MKBP_STATE,
> -					  kb_state, ckdev->cols);
> +	struct cros_ec_command msg = {
> +		.version = 0,
> +		.command = EC_CMD_MKBP_STATE,
> +		.outdata = NULL,
> +		.outsize = 0,
> +		.indata = kb_state,
> +		.insize = ckdev->cols,
> +	};
> +
> +	return ckdev->ec->cmd_xfer(ckdev->ec, &msg);
>  }
>  
>  static int cros_ec_keyb_work(struct notifier_block *nb,
> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
> index 49ed8c3..4851ed2 100644
> --- a/drivers/mfd/cros_ec.c
> +++ b/drivers/mfd/cros_ec.c
> @@ -44,34 +44,6 @@ int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
>  }
>  EXPORT_SYMBOL(cros_ec_prepare_tx);
>  
> -static int cros_ec_command_sendrecv(struct cros_ec_device *ec_dev,
> -		uint16_t cmd, void *out_buf, int out_len,
> -		void *in_buf, int in_len)
> -{
> -	struct cros_ec_command msg;
> -
> -	msg.version = cmd >> 8;
> -	msg.command = cmd & 0xff;
> -	msg.outdata = out_buf;
> -	msg.outsize = out_len;
> -	msg.indata = in_buf;
> -	msg.insize = in_len;
> -
> -	return ec_dev->cmd_xfer(ec_dev, &msg);
> -}
> -
> -static int cros_ec_command_recv(struct cros_ec_device *ec_dev,
> -		uint16_t cmd, void *buf, int buf_len)
> -{
> -	return cros_ec_command_sendrecv(ec_dev, cmd, NULL, 0, buf, buf_len);
> -}
> -
> -static int cros_ec_command_send(struct cros_ec_device *ec_dev,
> -		uint16_t cmd, void *buf, int buf_len)
> -{
> -	return cros_ec_command_sendrecv(ec_dev, cmd, buf, buf_len, NULL, 0);
> -}
> -
>  static irqreturn_t ec_irq_thread(int irq, void *data)
>  {
>  	struct cros_ec_device *ec_dev = data;
> @@ -104,10 +76,6 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
>  
>  	BLOCKING_INIT_NOTIFIER_HEAD(&ec_dev->event_notifier);
>  
> -	ec_dev->command_send = cros_ec_command_send;
> -	ec_dev->command_recv = cros_ec_command_recv;
> -	ec_dev->command_sendrecv = cros_ec_command_sendrecv;
> -
>  	if (ec_dev->din_size) {
>  		ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL);
>  		if (!ec_dev->din)
> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> index 2b0c598..60c0880 100644
> --- a/include/linux/mfd/cros_ec.h
> +++ b/include/linux/mfd/cros_ec.h
> @@ -63,9 +63,10 @@ struct cros_ec_command {
>   * @was_wake_device: true if this device was set to wake the system from
>   * sleep at the last suspend
>   * @event_notifier: interrupt event notifier for transport devices
> - * @command_send: send a command
> - * @command_recv: receive a response
> - * @command_sendrecv: send a command and receive a response
> + * @cmd_xfer: send command to EC and get response
> + *     Returns 0 if the communication succeeded, but that doesn't mean the EC
> + *     was happy with the command it got. Caller should check msg.result for
> + *     the EC's result code.
>   *
>   * @priv: Private data
>   * @irq: Interrupt to use
> @@ -83,7 +84,6 @@ struct cros_ec_command {
>   * @parent: pointer to parent device (e.g. i2c or spi device)
>   * @wake_enabled: true if this device can wake the system from sleep
>   * @lock: one transaction at a time
> - * @cmd_xfer: low-level channel to the EC
>   */
>  struct cros_ec_device {
>  
> @@ -94,13 +94,8 @@ struct cros_ec_device {
>  	bool was_wake_device;
>  	struct class *cros_class;
>  	struct blocking_notifier_head event_notifier;
> -	int (*command_send)(struct cros_ec_device *ec,
> -			    uint16_t cmd, void *out_buf, int out_len);
> -	int (*command_recv)(struct cros_ec_device *ec,
> -			    uint16_t cmd, void *in_buf, int in_len);
> -	int (*command_sendrecv)(struct cros_ec_device *ec,
> -				uint16_t cmd, void *out_buf, int out_len,
> -				void *in_buf, int in_len);
> +	int (*cmd_xfer)(struct cros_ec_device *ec,
> +			struct cros_ec_command *msg);
>  
>  	/* These are used to implement the platform-specific interface */
>  	void *priv;
> @@ -112,8 +107,6 @@ struct cros_ec_device {
>  	struct device *parent;
>  	bool wake_enabled;
>  	struct mutex lock;
> -	int (*cmd_xfer)(struct cros_ec_device *ec,
> -			struct cros_ec_command *msg);
>  };
>  
>  /**

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

Re: [PATCH v2 07/10] mfd: cros_ec: cleanup: Remove EC wrapper functions

[Lee Jones]

On Wed, 18 Jun 2014, Doug Anderson wrote:

> From: Bill Richardson <wfrichar-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> 
> Remove the three wrapper functions that talk to the EC without passing all
> the desired arguments and just use the underlying communication function
> that passes everything in a struct intead.
> 
> This is internal code refactoring only. Nothing should change.
> 
> Signed-off-by: Bill Richardson <wfrichar-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> Signed-off-by: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> Acked-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Reviewed-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> ---
> Changes in v2:
> - Removed unneeded "ret" variable.
> 
>  drivers/i2c/busses/i2c-cros-ec-tunnel.c | 15 +++++++++++----
>  drivers/input/keyboard/cros_ec_keyb.c   | 12 ++++++++++--
>  drivers/mfd/cros_ec.c                   | 32 --------------------------------
>  include/linux/mfd/cros_ec.h             | 19 ++++++-------------
>  4 files changed, 27 insertions(+), 51 deletions(-)

Patch applied with Wolfram and Dmitry's Acks.

Clause: There is a chance that this patch might not be seen in -next
for ~24-48hrs.  If it's not there by 72hrs, feel free to poke.

> diff --git a/drivers/i2c/busses/i2c-cros-ec-tunnel.c b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
> index 8e7a714..dd07818 100644
> --- a/drivers/i2c/busses/i2c-cros-ec-tunnel.c
> +++ b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
> @@ -183,6 +183,7 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg i2c_msgs[],
>  	u8 *request = NULL;
>  	u8 *response = NULL;
>  	int result;
> +	struct cros_ec_command msg;
>  
>  	request_len = ec_i2c_count_message(i2c_msgs, num);
>  	if (request_len < 0) {
> @@ -218,9 +219,15 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg i2c_msgs[],
>  	}
>  
>  	ec_i2c_construct_message(request, i2c_msgs, num, bus_num);
> -	result = bus->ec->command_sendrecv(bus->ec, EC_CMD_I2C_PASSTHRU,
> -					   request, request_len,
> -					   response, response_len);
> +
> +	msg.version = 0;
> +	msg.command = EC_CMD_I2C_PASSTHRU;
> +	msg.outdata = request;
> +	msg.outsize = request_len;
> +	msg.indata = response;
> +	msg.insize = response_len;
> +
> +	result = bus->ec->cmd_xfer(bus->ec, &msg);
>  	if (result)
>  		goto exit;
>  
> @@ -258,7 +265,7 @@ static int ec_i2c_probe(struct platform_device *pdev)
>  	u32 remote_bus;
>  	int err;
>  
> -	if (!ec->command_sendrecv) {
> +	if (!ec->cmd_xfer) {
>  		dev_err(dev, "Missing sendrecv\n");
>  		return -EINVAL;
>  	}
> diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
> index 4083796..b8341ab 100644
> --- a/drivers/input/keyboard/cros_ec_keyb.c
> +++ b/drivers/input/keyboard/cros_ec_keyb.c
> @@ -191,8 +191,16 @@ static void cros_ec_keyb_close(struct input_dev *dev)
>  
>  static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t *kb_state)
>  {
> -	return ckdev->ec->command_recv(ckdev->ec, EC_CMD_MKBP_STATE,
> -					  kb_state, ckdev->cols);
> +	struct cros_ec_command msg = {
> +		.version = 0,
> +		.command = EC_CMD_MKBP_STATE,
> +		.outdata = NULL,
> +		.outsize = 0,
> +		.indata = kb_state,
> +		.insize = ckdev->cols,
> +	};
> +
> +	return ckdev->ec->cmd_xfer(ckdev->ec, &msg);
>  }
>  
>  static int cros_ec_keyb_work(struct notifier_block *nb,
> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
> index 49ed8c3..4851ed2 100644
> --- a/drivers/mfd/cros_ec.c
> +++ b/drivers/mfd/cros_ec.c
> @@ -44,34 +44,6 @@ int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
>  }
>  EXPORT_SYMBOL(cros_ec_prepare_tx);
>  
> -static int cros_ec_command_sendrecv(struct cros_ec_device *ec_dev,
> -		uint16_t cmd, void *out_buf, int out_len,
> -		void *in_buf, int in_len)
> -{
> -	struct cros_ec_command msg;
> -
> -	msg.version = cmd >> 8;
> -	msg.command = cmd & 0xff;
> -	msg.outdata = out_buf;
> -	msg.outsize = out_len;
> -	msg.indata = in_buf;
> -	msg.insize = in_len;
> -
> -	return ec_dev->cmd_xfer(ec_dev, &msg);
> -}
> -
> -static int cros_ec_command_recv(struct cros_ec_device *ec_dev,
> -		uint16_t cmd, void *buf, int buf_len)
> -{
> -	return cros_ec_command_sendrecv(ec_dev, cmd, NULL, 0, buf, buf_len);
> -}
> -
> -static int cros_ec_command_send(struct cros_ec_device *ec_dev,
> -		uint16_t cmd, void *buf, int buf_len)
> -{
> -	return cros_ec_command_sendrecv(ec_dev, cmd, buf, buf_len, NULL, 0);
> -}
> -
>  static irqreturn_t ec_irq_thread(int irq, void *data)
>  {
>  	struct cros_ec_device *ec_dev = data;
> @@ -104,10 +76,6 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
>  
>  	BLOCKING_INIT_NOTIFIER_HEAD(&ec_dev->event_notifier);
>  
> -	ec_dev->command_send = cros_ec_command_send;
> -	ec_dev->command_recv = cros_ec_command_recv;
> -	ec_dev->command_sendrecv = cros_ec_command_sendrecv;
> -
>  	if (ec_dev->din_size) {
>  		ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL);
>  		if (!ec_dev->din)
> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> index 2b0c598..60c0880 100644
> --- a/include/linux/mfd/cros_ec.h
> +++ b/include/linux/mfd/cros_ec.h
> @@ -63,9 +63,10 @@ struct cros_ec_command {
>   * @was_wake_device: true if this device was set to wake the system from
>   * sleep at the last suspend
>   * @event_notifier: interrupt event notifier for transport devices
> - * @command_send: send a command
> - * @command_recv: receive a response
> - * @command_sendrecv: send a command and receive a response
> + * @cmd_xfer: send command to EC and get response
> + *     Returns 0 if the communication succeeded, but that doesn't mean the EC
> + *     was happy with the command it got. Caller should check msg.result for
> + *     the EC's result code.
>   *
>   * @priv: Private data
>   * @irq: Interrupt to use
> @@ -83,7 +84,6 @@ struct cros_ec_command {
>   * @parent: pointer to parent device (e.g. i2c or spi device)
>   * @wake_enabled: true if this device can wake the system from sleep
>   * @lock: one transaction at a time
> - * @cmd_xfer: low-level channel to the EC
>   */
>  struct cros_ec_device {
>  
> @@ -94,13 +94,8 @@ struct cros_ec_device {
>  	bool was_wake_device;
>  	struct class *cros_class;
>  	struct blocking_notifier_head event_notifier;
> -	int (*command_send)(struct cros_ec_device *ec,
> -			    uint16_t cmd, void *out_buf, int out_len);
> -	int (*command_recv)(struct cros_ec_device *ec,
> -			    uint16_t cmd, void *in_buf, int in_len);
> -	int (*command_sendrecv)(struct cros_ec_device *ec,
> -				uint16_t cmd, void *out_buf, int out_len,
> -				void *in_buf, int in_len);
> +	int (*cmd_xfer)(struct cros_ec_device *ec,
> +			struct cros_ec_command *msg);
>  
>  	/* These are used to implement the platform-specific interface */
>  	void *priv;
> @@ -112,8 +107,6 @@ struct cros_ec_device {
>  	struct device *parent;
>  	bool wake_enabled;
>  	struct mutex lock;
> -	int (*cmd_xfer)(struct cros_ec_device *ec,
> -			struct cros_ec_command *msg);
>  };
>  
>  /**

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

Re: [PATCH v2 08/10] mfd: cros_ec: Check result code from EC messages

[Lee Jones]

On Wed, 18 Jun 2014, Doug Anderson wrote:

> From: Bill Richardson <wfrichar@chromium.org>
> 
> Just because the host was able to talk to the EC doesn't mean that the EC
> was happy with what it was told. Errors in communincation are not the same
> as error messages from the EC itself.
> 
> This change lets the EC report its errors separately.
> 
> [dianders: Added common function to cros_ec.c]
> 
> Signed-off-by: Bill Richardson <wfrichar@chromium.org>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> ---
> Changes in v2:
> - Added common function to cros_ec.c
> - Changed to dev_dbg() as per http://crosreview.com/66726
> 
>  drivers/mfd/cros_ec.c       | 18 ++++++++++++++++++
>  drivers/mfd/cros_ec_i2c.c   |  8 +++-----
>  drivers/mfd/cros_ec_spi.c   | 19 ++++++-------------
>  include/linux/mfd/cros_ec.h | 12 ++++++++++++
>  4 files changed, 39 insertions(+), 18 deletions(-)

Patch applied with Simon's Reviewed-by.

Clause: There is a chance that this patch might not be seen in -next
for ~24-48hrs.  If it's not there by 72hrs, feel free to poke.

> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
> index 4851ed2..83e30c6 100644
> --- a/drivers/mfd/cros_ec.c
> +++ b/drivers/mfd/cros_ec.c
> @@ -44,6 +44,24 @@ int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
>  }
>  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);
> +
>  static irqreturn_t ec_irq_thread(int irq, void *data)
>  {
>  	struct cros_ec_device *ec_dev = data;
> diff --git a/drivers/mfd/cros_ec_i2c.c b/drivers/mfd/cros_ec_i2c.c
> index 5bb32f5..189e7d1 100644
> --- a/drivers/mfd/cros_ec_i2c.c
> +++ b/drivers/mfd/cros_ec_i2c.c
> @@ -92,12 +92,10 @@ static int cros_ec_cmd_xfer_i2c(struct cros_ec_device *ec_dev,
>  	}
>  
>  	/* check response error code */
> -	if (i2c_msg[1].buf[0]) {
> -		dev_warn(ec_dev->dev, "command 0x%02x returned an error %d\n",
> -			 msg->command, i2c_msg[1].buf[0]);
> -		ret = -EINVAL;
> +	msg->result = i2c_msg[1].buf[0];
> +	ret = cros_ec_check_result(ec_dev, msg);
> +	if (ret)
>  		goto done;
> -	}
>  
>  	/* copy response packet payload and compute checksum */
>  	sum = in_buf[0] + in_buf[1];
> diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
> index 09ca789..c975087 100644
> --- a/drivers/mfd/cros_ec_spi.c
> +++ b/drivers/mfd/cros_ec_spi.c
> @@ -289,21 +289,14 @@ static int cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev,
>  		goto exit;
>  	}
>  
> -	/* check response error code */
>  	ptr = ec_dev->din;
> -	if (ptr[0]) {
> -		if (ptr[0] == EC_RES_IN_PROGRESS) {
> -			dev_dbg(ec_dev->dev, "command 0x%02x in progress\n",
> -				ec_msg->command);
> -			ret = -EAGAIN;
> -			goto exit;
> -		}
> -		dev_warn(ec_dev->dev, "command 0x%02x returned an error %d\n",
> -			 ec_msg->command, ptr[0]);
> -		debug_packet(ec_dev->dev, "in_err", ptr, len);
> -		ret = -EINVAL;
> +
> +	/* check response error code */
> +	ec_msg->result = ptr[0];
> +	ret = cros_ec_check_result(ec_dev, ec_msg);
> +	if (ret)
>  		goto exit;
> -	}
> +
>  	len = ptr[1];
>  	sum = ptr[0] + ptr[1];
>  	if (len > ec_msg->insize) {
> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> index 60c0880..1f79f16 100644
> --- a/include/linux/mfd/cros_ec.h
> +++ b/include/linux/mfd/cros_ec.h
> @@ -143,6 +143,18 @@ int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
>  		       struct cros_ec_command *msg);
>  
>  /**
> + * cros_ec_check_result - Check ec_msg->result
> + *
> + * This is used by ChromeOS EC drivers to check the ec_msg->result for
> + * errors and to warn about them.
> + *
> + * @ec_dev: EC device
> + * @msg: Message to check
> + */
> +int cros_ec_check_result(struct cros_ec_device *ec_dev,
> +			 struct cros_ec_command *msg);
> +
> +/**
>   * cros_ec_remove - Remove a ChromeOS EC
>   *
>   * Call this to deregister a ChromeOS EC, then clean up any private data.

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

Re: [PATCH v2 09/10] mfd: cros_ec: ec_dev->cmd_xfer() returns number of bytes received from EC

[Lee Jones]

On Wed, 18 Jun 2014, Doug Anderson wrote:

> From: Bill Richardson <wfrichar@chromium.org>
> 
> When communicating with the EC, the cmd_xfer() function should return the
> number of bytes it received from the EC, or negative on error.
> 
> Signed-off-by: Bill Richardson <wfrichar@chromium.org>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Acked-by: Lee Jones <lee.jones@linaro.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
> Changes in v2: None
> 
>  drivers/i2c/busses/i2c-cros-ec-tunnel.c | 2 +-
>  drivers/mfd/cros_ec_i2c.c               | 2 +-
>  drivers/mfd/cros_ec_spi.c               | 2 +-
>  include/linux/mfd/cros_ec.h             | 8 ++++----
>  4 files changed, 7 insertions(+), 7 deletions(-)

Patch applied with Wolfram's Ack.

Clause: There is a chance that this patch might not be seen in -next
for ~24-48hrs.  If it's not there by 72hrs, feel free to poke.

> diff --git a/drivers/i2c/busses/i2c-cros-ec-tunnel.c b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
> index dd07818..05e033c 100644
> --- a/drivers/i2c/busses/i2c-cros-ec-tunnel.c
> +++ b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
> @@ -228,7 +228,7 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg i2c_msgs[],
>  	msg.insize = response_len;
>  
>  	result = bus->ec->cmd_xfer(bus->ec, &msg);
> -	if (result)
> +	if (result < 0)
>  		goto exit;
>  
>  	result = ec_i2c_parse_response(response, i2c_msgs, &num);
> diff --git a/drivers/mfd/cros_ec_i2c.c b/drivers/mfd/cros_ec_i2c.c
> index 189e7d1..fd7a546 100644
> --- a/drivers/mfd/cros_ec_i2c.c
> +++ b/drivers/mfd/cros_ec_i2c.c
> @@ -111,7 +111,7 @@ static int cros_ec_cmd_xfer_i2c(struct cros_ec_device *ec_dev,
>  		goto done;
>  	}
>  
> -	ret = 0;
> +	ret = i2c_msg[1].buf[1];
>   done:
>  	kfree(in_buf);
>  	kfree(out_buf);
> diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
> index c975087..2ad6815 100644
> --- a/drivers/mfd/cros_ec_spi.c
> +++ b/drivers/mfd/cros_ec_spi.c
> @@ -324,7 +324,7 @@ static int cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev,
>  		goto exit;
>  	}
>  
> -	ret = 0;
> +	ret = len;
>  exit:
>  	mutex_unlock(&ec_spi->lock);
>  	return ret;
> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> index 1f79f16..0ebf26f 100644
> --- a/include/linux/mfd/cros_ec.h
> +++ b/include/linux/mfd/cros_ec.h
> @@ -41,7 +41,7 @@ enum {
>   * @outdata: Outgoing data to EC
>   * @outsize: Outgoing length in bytes
>   * @indata: Where to put the incoming data from EC
> - * @insize: Incoming length in bytes (filled in by EC)
> + * @insize: Max number of bytes to accept from EC
>   * @result: EC's response to the command (separate from communication failure)
>   */
>  struct cros_ec_command {
> @@ -64,9 +64,9 @@ struct cros_ec_command {
>   * sleep at the last suspend
>   * @event_notifier: interrupt event notifier for transport devices
>   * @cmd_xfer: send command to EC and get response
> - *     Returns 0 if the communication succeeded, but that doesn't mean the EC
> - *     was happy with the command it got. Caller should check msg.result for
> - *     the EC's result code.
> + *     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.
>   *
>   * @priv: Private data
>   * @irq: Interrupt to use

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

Re: [PATCH v2 09/10] mfd: cros_ec: ec_dev->cmd_xfer() returns number of bytes received from EC

[Lee Jones]

On Wed, 18 Jun 2014, Doug Anderson wrote:

> From: Bill Richardson <wfrichar-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> 
> When communicating with the EC, the cmd_xfer() function should return the
> number of bytes it received from the EC, or negative on error.
> 
> Signed-off-by: Bill Richardson <wfrichar-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> Signed-off-by: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> Acked-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Reviewed-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> ---
> Changes in v2: None
> 
>  drivers/i2c/busses/i2c-cros-ec-tunnel.c | 2 +-
>  drivers/mfd/cros_ec_i2c.c               | 2 +-
>  drivers/mfd/cros_ec_spi.c               | 2 +-
>  include/linux/mfd/cros_ec.h             | 8 ++++----
>  4 files changed, 7 insertions(+), 7 deletions(-)

Patch applied with Wolfram's Ack.

Clause: There is a chance that this patch might not be seen in -next
for ~24-48hrs.  If it's not there by 72hrs, feel free to poke.

> diff --git a/drivers/i2c/busses/i2c-cros-ec-tunnel.c b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
> index dd07818..05e033c 100644
> --- a/drivers/i2c/busses/i2c-cros-ec-tunnel.c
> +++ b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
> @@ -228,7 +228,7 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg i2c_msgs[],
>  	msg.insize = response_len;
>  
>  	result = bus->ec->cmd_xfer(bus->ec, &msg);
> -	if (result)
> +	if (result < 0)
>  		goto exit;
>  
>  	result = ec_i2c_parse_response(response, i2c_msgs, &num);
> diff --git a/drivers/mfd/cros_ec_i2c.c b/drivers/mfd/cros_ec_i2c.c
> index 189e7d1..fd7a546 100644
> --- a/drivers/mfd/cros_ec_i2c.c
> +++ b/drivers/mfd/cros_ec_i2c.c
> @@ -111,7 +111,7 @@ static int cros_ec_cmd_xfer_i2c(struct cros_ec_device *ec_dev,
>  		goto done;
>  	}
>  
> -	ret = 0;
> +	ret = i2c_msg[1].buf[1];
>   done:
>  	kfree(in_buf);
>  	kfree(out_buf);
> diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
> index c975087..2ad6815 100644
> --- a/drivers/mfd/cros_ec_spi.c
> +++ b/drivers/mfd/cros_ec_spi.c
> @@ -324,7 +324,7 @@ static int cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev,
>  		goto exit;
>  	}
>  
> -	ret = 0;
> +	ret = len;
>  exit:
>  	mutex_unlock(&ec_spi->lock);
>  	return ret;
> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> index 1f79f16..0ebf26f 100644
> --- a/include/linux/mfd/cros_ec.h
> +++ b/include/linux/mfd/cros_ec.h
> @@ -41,7 +41,7 @@ enum {
>   * @outdata: Outgoing data to EC
>   * @outsize: Outgoing length in bytes
>   * @indata: Where to put the incoming data from EC
> - * @insize: Incoming length in bytes (filled in by EC)
> + * @insize: Max number of bytes to accept from EC
>   * @result: EC's response to the command (separate from communication failure)
>   */
>  struct cros_ec_command {
> @@ -64,9 +64,9 @@ struct cros_ec_command {
>   * sleep at the last suspend
>   * @event_notifier: interrupt event notifier for transport devices
>   * @cmd_xfer: send command to EC and get response
> - *     Returns 0 if the communication succeeded, but that doesn't mean the EC
> - *     was happy with the command it got. Caller should check msg.result for
> - *     the EC's result code.
> + *     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.
>   *
>   * @priv: Private data
>   * @irq: Interrupt to use

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

Re: [PATCH v2 10/10] mfd: cros_ec: move EC interrupt to cros_ec_keyb

[Lee Jones]

On Wed, 18 Jun 2014, Doug Anderson wrote:

> From: Andrew Bresticker <abrestic@chromium.org>
> 
> If we receive EC interrupts after the cros_ec driver has probed, but
> before the cros_ec_keyb driver has probed, the cros_ec IRQ handler
> will not run the cros_ec_keyb notifier and the EC will leave the IRQ
> line asserted.  The cros_ec IRQ handler then returns IRQ_HANDLED and
> the resulting flood of interrupts causes the machine to hang.
> 
> Since the EC interrupt is currently only used for the keyboard, move
> the setup and handling of the EC interrupt to the cros_ec_keyb driver.
> 
> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> ---
> Changes in v2:
> - IRQs should be optional => move EC interrupt to keyboard.
> 
>  drivers/input/keyboard/cros_ec_keyb.c | 58 ++++++++++++++++++++---------------
>  drivers/mfd/cros_ec.c                 | 35 +--------------------
>  include/linux/mfd/cros_ec.h           |  2 --
>  3 files changed, 34 insertions(+), 61 deletions(-)

Patch applied with Simon's Reviewed-by.

Clause: There is a chance that this patch might not be seen in -next
for ~24-48hrs.  If it's not there by 72hrs, feel free to poke.

> diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
> index b8341ab..791781a 100644
> --- a/drivers/input/keyboard/cros_ec_keyb.c
> +++ b/drivers/input/keyboard/cros_ec_keyb.c
> @@ -24,8 +24,8 @@
>  #include <linux/module.h>
>  #include <linux/i2c.h>
>  #include <linux/input.h>
> +#include <linux/interrupt.h>
>  #include <linux/kernel.h>
> -#include <linux/notifier.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  #include <linux/input/matrix_keypad.h>
> @@ -42,7 +42,6 @@
>   * @dev: Device pointer
>   * @idev: Input device
>   * @ec: Top level ChromeOS device to use to talk to EC
> - * @event_notifier: interrupt event notifier for transport devices
>   */
>  struct cros_ec_keyb {
>  	unsigned int rows;
> @@ -55,7 +54,6 @@ struct cros_ec_keyb {
>  	struct device *dev;
>  	struct input_dev *idev;
>  	struct cros_ec_device *ec;
> -	struct notifier_block notifier;
>  };
>  
>  
> @@ -173,22 +171,6 @@ static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev,
>  	input_sync(ckdev->idev);
>  }
>  
> -static int cros_ec_keyb_open(struct input_dev *dev)
> -{
> -	struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
> -
> -	return blocking_notifier_chain_register(&ckdev->ec->event_notifier,
> -						&ckdev->notifier);
> -}
> -
> -static void cros_ec_keyb_close(struct input_dev *dev)
> -{
> -	struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
> -
> -	blocking_notifier_chain_unregister(&ckdev->ec->event_notifier,
> -					   &ckdev->notifier);
> -}
> -
>  static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t *kb_state)
>  {
>  	struct cros_ec_command msg = {
> @@ -203,19 +185,41 @@ static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t *kb_state)
>  	return ckdev->ec->cmd_xfer(ckdev->ec, &msg);
>  }
>  
> -static int cros_ec_keyb_work(struct notifier_block *nb,
> -		     unsigned long state, void *_notify)
> +static irqreturn_t cros_ec_keyb_irq(int irq, void *data)
>  {
> +	struct cros_ec_keyb *ckdev = data;
> +	struct cros_ec_device *ec = ckdev->ec;
>  	int ret;
> -	struct cros_ec_keyb *ckdev = container_of(nb, struct cros_ec_keyb,
> -						    notifier);
>  	uint8_t kb_state[ckdev->cols];
>  
> +	if (device_may_wakeup(ec->dev))
> +		pm_wakeup_event(ec->dev, 0);
> +
>  	ret = cros_ec_keyb_get_state(ckdev, kb_state);
>  	if (ret >= 0)
>  		cros_ec_keyb_process(ckdev, kb_state, ret);
> +	else
> +		dev_err(ec->dev, "failed to get keyboard state: %d\n", ret);
>  
> -	return NOTIFY_DONE;
> +	return IRQ_HANDLED;
> +}
> +
> +static int cros_ec_keyb_open(struct input_dev *dev)
> +{
> +	struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
> +	struct cros_ec_device *ec = ckdev->ec;
> +
> +	return request_threaded_irq(ec->irq, NULL, cros_ec_keyb_irq,
> +					IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> +					"cros_ec_keyb", ckdev);
> +}
> +
> +static void cros_ec_keyb_close(struct input_dev *dev)
> +{
> +	struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
> +	struct cros_ec_device *ec = ckdev->ec;
> +
> +	free_irq(ec->irq, ckdev);
>  }
>  
>  static int cros_ec_keyb_probe(struct platform_device *pdev)
> @@ -246,8 +250,12 @@ static int cros_ec_keyb_probe(struct platform_device *pdev)
>  	if (!idev)
>  		return -ENOMEM;
>  
> +	if (!ec->irq) {
> +		dev_err(dev, "no EC IRQ specified\n");
> +		return -EINVAL;
> +	}
> +
>  	ckdev->ec = ec;
> -	ckdev->notifier.notifier_call = cros_ec_keyb_work;
>  	ckdev->dev = dev;
>  	dev_set_drvdata(&pdev->dev, ckdev);
>  
> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
> index 83e30c6..4873f9c 100644
> --- a/drivers/mfd/cros_ec.c
> +++ b/drivers/mfd/cros_ec.c
> @@ -62,18 +62,6 @@ int cros_ec_check_result(struct cros_ec_device *ec_dev,
>  }
>  EXPORT_SYMBOL(cros_ec_check_result);
>  
> -static irqreturn_t ec_irq_thread(int irq, void *data)
> -{
> -	struct cros_ec_device *ec_dev = data;
> -
> -	if (device_may_wakeup(ec_dev->dev))
> -		pm_wakeup_event(ec_dev->dev, 0);
> -
> -	blocking_notifier_call_chain(&ec_dev->event_notifier, 1, ec_dev);
> -
> -	return IRQ_HANDLED;
> -}
> -
>  static const struct mfd_cell cros_devs[] = {
>  	{
>  		.name = "cros-ec-keyb",
> @@ -92,8 +80,6 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
>  	struct device *dev = ec_dev->dev;
>  	int err = 0;
>  
> -	BLOCKING_INIT_NOTIFIER_HEAD(&ec_dev->event_notifier);
> -
>  	if (ec_dev->din_size) {
>  		ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL);
>  		if (!ec_dev->din)
> @@ -105,42 +91,23 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
>  			return -ENOMEM;
>  	}
>  
> -	if (!ec_dev->irq) {
> -		dev_dbg(dev, "no valid IRQ: %d\n", ec_dev->irq);
> -		return err;
> -	}
> -
> -	err = request_threaded_irq(ec_dev->irq, NULL, ec_irq_thread,
> -				   IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> -				   "chromeos-ec", ec_dev);
> -	if (err) {
> -		dev_err(dev, "request irq %d: error %d\n", ec_dev->irq, err);
> -		return err;
> -	}
> -
>  	err = mfd_add_devices(dev, 0, cros_devs,
>  			      ARRAY_SIZE(cros_devs),
>  			      NULL, ec_dev->irq, NULL);
>  	if (err) {
>  		dev_err(dev, "failed to add mfd devices\n");
> -		goto fail_mfd;
> +		return err;
>  	}
>  
>  	dev_info(dev, "Chrome EC device registered\n");
>  
>  	return 0;
> -
> -fail_mfd:
> -	free_irq(ec_dev->irq, ec_dev);
> -
> -	return err;
>  }
>  EXPORT_SYMBOL(cros_ec_register);
>  
>  int cros_ec_remove(struct cros_ec_device *ec_dev)
>  {
>  	mfd_remove_devices(ec_dev->dev);
> -	free_irq(ec_dev->irq, ec_dev);
>  
>  	return 0;
>  }
> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> index 0ebf26f..fcbe9d1 100644
> --- a/include/linux/mfd/cros_ec.h
> +++ b/include/linux/mfd/cros_ec.h
> @@ -62,7 +62,6 @@ struct cros_ec_command {
>   * @dev: Device pointer
>   * @was_wake_device: true if this device was set to wake the system from
>   * sleep at the last suspend
> - * @event_notifier: interrupt event notifier for transport devices
>   * @cmd_xfer: send command to EC and get response
>   *     Returns the number of bytes received if the communication succeeded, but
>   *     that doesn't mean the EC was happy with the command. The caller
> @@ -93,7 +92,6 @@ struct cros_ec_device {
>  	struct device *dev;
>  	bool was_wake_device;
>  	struct class *cros_class;
> -	struct blocking_notifier_head event_notifier;
>  	int (*cmd_xfer)(struct cros_ec_device *ec,
>  			struct cros_ec_command *msg);
>  

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