[PATCH v2 0/7] Add cros_ec changes for newer boards

[PATCH v2 0/7] Add cros_ec changes for newer boards

[Doug Anderson]

This series adds the most critical cros_ec changes for newer boards
using cros_ec.  Specifically:
* Fixes timing/locking issues with the previously upstreamed (but
  never used upstream) cros_ec_spi driver.
* Updates the cros_ec header file to the latest version which allows
  us to use newer EC features like i2c tunneling.
* Adds an i2c tunnel driver to allow communication to the EC's i2c
  devices.

This _doesn't_ get the EC driver fully up to speed with what's in the
current Chromium OS trees.  There are a whole slew of cleanup patches
there, an addition of an LPC transport mode, and exports of functions
to userspace.  Once these patches land and we have functionality we
can continue to pick more cleanup patches.

Changes in v2:
- Update tunnel binding as per swarren
- Removed i2c20 alias for i2c tunnel

Bill Richardson (1):
  mfd: cros_ec: Sync to the latest cros_ec_commands.h from EC sources

David Hendricks (1):
  mfd: cros_ec: spi: calculate delay between transfers correctly

Doug Anderson (5):
  mfd: cros_ec: spi: Add mutex to cros_ec_spi
  mfd: cros_ec: spi: Make the cros_ec_spi timeout more reliable
  mfd: cros_ec: spi: Increase cros_ec_spi deadline from 5ms to 100ms
  i2c: ChromeOS EC tunnel driver
  ARM: tegra: Add the EC i2c tunnel to tegra124-venice2

 .../devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt |   39 +
 arch/arm/boot/dts/tegra124-venice2.dts             |   26 +
 drivers/i2c/busses/Kconfig                         |    9 +
 drivers/i2c/busses/Makefile                        |    1 +
 drivers/i2c/busses/i2c-cros-ec-tunnel.c            |  304 ++++++
 drivers/mfd/cros_ec.c                              |    7 +-
 drivers/mfd/cros_ec_spi.c                          |   67 +-
 include/linux/mfd/cros_ec.h                        |    4 +-
 include/linux/mfd/cros_ec_commands.h               | 1128 ++++++++++++++++++--
 9 files changed, 1493 insertions(+), 92 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt
 create mode 100644 drivers/i2c/busses/i2c-cros-ec-tunnel.c

-- 
1.9.1.423.g4596e3a

[PATCH v2 0/7] Add cros_ec changes for newer boards

[Doug Anderson]

This series adds the most critical cros_ec changes for newer boards
using cros_ec.  Specifically:
* Fixes timing/locking issues with the previously upstreamed (but
  never used upstream) cros_ec_spi driver.
* Updates the cros_ec header file to the latest version which allows
  us to use newer EC features like i2c tunneling.
* Adds an i2c tunnel driver to allow communication to the EC's i2c
  devices.

This _doesn't_ get the EC driver fully up to speed with what's in the
current Chromium OS trees.  There are a whole slew of cleanup patches
there, an addition of an LPC transport mode, and exports of functions
to userspace.  Once these patches land and we have functionality we
can continue to pick more cleanup patches.

Changes in v2:
- Update tunnel binding as per swarren
- Removed i2c20 alias for i2c tunnel

Bill Richardson (1):
  mfd: cros_ec: Sync to the latest cros_ec_commands.h from EC sources

David Hendricks (1):
  mfd: cros_ec: spi: calculate delay between transfers correctly

Doug Anderson (5):
  mfd: cros_ec: spi: Add mutex to cros_ec_spi
  mfd: cros_ec: spi: Make the cros_ec_spi timeout more reliable
  mfd: cros_ec: spi: Increase cros_ec_spi deadline from 5ms to 100ms
  i2c: ChromeOS EC tunnel driver
  ARM: tegra: Add the EC i2c tunnel to tegra124-venice2

 .../devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt |   39 +
 arch/arm/boot/dts/tegra124-venice2.dts             |   26 +
 drivers/i2c/busses/Kconfig                         |    9 +
 drivers/i2c/busses/Makefile                        |    1 +
 drivers/i2c/busses/i2c-cros-ec-tunnel.c            |  304 ++++++
 drivers/mfd/cros_ec.c                              |    7 +-
 drivers/mfd/cros_ec_spi.c                          |   67 +-
 include/linux/mfd/cros_ec.h                        |    4 +-
 include/linux/mfd/cros_ec_commands.h               | 1128 ++++++++++++++++++--
 9 files changed, 1493 insertions(+), 92 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt
 create mode 100644 drivers/i2c/busses/i2c-cros-ec-tunnel.c

-- 
1.9.1.423.g4596e3a

[PATCH v2 0/7] Add cros_ec changes for newer boards

[Doug Anderson]

This series adds the most critical cros_ec changes for newer boards
using cros_ec.  Specifically:
* Fixes timing/locking issues with the previously upstreamed (but
  never used upstream) cros_ec_spi driver.
* Updates the cros_ec header file to the latest version which allows
  us to use newer EC features like i2c tunneling.
* Adds an i2c tunnel driver to allow communication to the EC's i2c
  devices.

This _doesn't_ get the EC driver fully up to speed with what's in the
current Chromium OS trees.  There are a whole slew of cleanup patches
there, an addition of an LPC transport mode, and exports of functions
to userspace.  Once these patches land and we have functionality we
can continue to pick more cleanup patches.

Changes in v2:
- Update tunnel binding as per swarren
- Removed i2c20 alias for i2c tunnel

Bill Richardson (1):
  mfd: cros_ec: Sync to the latest cros_ec_commands.h from EC sources

David Hendricks (1):
  mfd: cros_ec: spi: calculate delay between transfers correctly

Doug Anderson (5):
  mfd: cros_ec: spi: Add mutex to cros_ec_spi
  mfd: cros_ec: spi: Make the cros_ec_spi timeout more reliable
  mfd: cros_ec: spi: Increase cros_ec_spi deadline from 5ms to 100ms
  i2c: ChromeOS EC tunnel driver
  ARM: tegra: Add the EC i2c tunnel to tegra124-venice2

 .../devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt |   39 +
 arch/arm/boot/dts/tegra124-venice2.dts             |   26 +
 drivers/i2c/busses/Kconfig                         |    9 +
 drivers/i2c/busses/Makefile                        |    1 +
 drivers/i2c/busses/i2c-cros-ec-tunnel.c            |  304 ++++++
 drivers/mfd/cros_ec.c                              |    7 +-
 drivers/mfd/cros_ec_spi.c                          |   67 +-
 include/linux/mfd/cros_ec.h                        |    4 +-
 include/linux/mfd/cros_ec_commands.h               | 1128 ++++++++++++++++++--
 9 files changed, 1493 insertions(+), 92 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt
 create mode 100644 drivers/i2c/busses/i2c-cros-ec-tunnel.c

-- 
1.9.1.423.g4596e3a

[PATCH v2 1/7] mfd: cros_ec: spi: calculate delay between transfers correctly

[Doug Anderson]

From: David Hendricks <dhendrix-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

To avoid spamming the EC we calculate the time between the previous
transfer and the current transfer and force a delay if the time delta
is too small.

However, a small miscalculation causes the delay period to be
far too short. Most noticably this impacts commands with a long
turnaround time such as EC firmware reads and writes.

Signed-off-by: David Hendricks <dhendrix-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Signed-off-by: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Reviewed-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Tested-by: Andrew Bresticker <abrestic-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Tested-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
Changes in v2: None

 drivers/mfd/cros_ec_spi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
index 84af8d7..c185eb6 100644
--- a/drivers/mfd/cros_ec_spi.c
+++ b/drivers/mfd/cros_ec_spi.c
@@ -219,7 +219,7 @@ static int cros_ec_command_spi_xfer(struct cros_ec_device *ec_dev,
 		ktime_get_ts(&ts);
 		delay = timespec_to_ns(&ts) - ec_spi->last_transfer_ns;
 		if (delay < EC_SPI_RECOVERY_TIME_NS)
-			ndelay(delay);
+			ndelay(EC_SPI_RECOVERY_TIME_NS - delay);
 	}
 
 	/* Transmit phase - send our message */
-- 
1.9.1.423.g4596e3a

[PATCH v2 1/7] mfd: cros_ec: spi: calculate delay between transfers correctly

[Doug Anderson]

From: David Hendricks <dhendrix@chromium.org>

To avoid spamming the EC we calculate the time between the previous
transfer and the current transfer and force a delay if the time delta
is too small.

However, a small miscalculation causes the delay period to be
far too short. Most noticably this impacts commands with a long
turnaround time such as EC firmware reads and writes.

Signed-off-by: David Hendricks <dhendrix@chromium.org>
Signed-off-by: Doug Anderson <dianders@chromium.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
Tested-by: Andrew Bresticker <abrestic@chromium.org>
Tested-by: Stephen Warren <swarren@nvidia.com>
---
Changes in v2: None

 drivers/mfd/cros_ec_spi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
index 84af8d7..c185eb6 100644
--- a/drivers/mfd/cros_ec_spi.c
+++ b/drivers/mfd/cros_ec_spi.c
@@ -219,7 +219,7 @@ static int cros_ec_command_spi_xfer(struct cros_ec_device *ec_dev,
 		ktime_get_ts(&ts);
 		delay = timespec_to_ns(&ts) - ec_spi->last_transfer_ns;
 		if (delay < EC_SPI_RECOVERY_TIME_NS)
-			ndelay(delay);
+			ndelay(EC_SPI_RECOVERY_TIME_NS - delay);
 	}
 
 	/* Transmit phase - send our message */
-- 
1.9.1.423.g4596e3a

[PATCH v2 2/7] mfd: cros_ec: spi: Add mutex to cros_ec_spi

[Doug Anderson]

The main transfer function for cros_ec_spi can be called by more than
one client at a time.  Make sure that those clients don't stomp on
each other by locking the bus for the duration of the transfer
function.

Signed-off-by: Doug Anderson <dianders@chromium.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
Tested-by: Andrew Bresticker <abrestic@chromium.org>
Tested-by: Stephen Warren <swarren@nvidia.com>
---
Changes in v2: None

 drivers/mfd/cros_ec_spi.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
index c185eb6..a2a605d 100644
--- a/drivers/mfd/cros_ec_spi.c
+++ b/drivers/mfd/cros_ec_spi.c
@@ -65,11 +65,13 @@
  *	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
  */
 struct cros_ec_spi {
 	struct spi_device *spi;
 	s64 last_transfer_ns;
 	unsigned int end_of_msg_delay;
+	struct mutex lock;
 };
 
 static void debug_packet(struct device *dev, const char *name, u8 *ptr,
@@ -208,6 +210,13 @@ static int cros_ec_command_spi_xfer(struct cros_ec_device *ec_dev,
 	int ret = 0, final_ret;
 	struct timespec ts;
 
+	/*
+	 * We have the shared ec_dev buffer plus we do lots of separate spi_sync
+	 * calls, so we need to make sure only one person is using this at a
+	 * time.
+	 */
+	mutex_lock(&ec_spi->lock);
+
 	len = cros_ec_prepare_tx(ec_dev, ec_msg);
 	dev_dbg(ec_dev->dev, "prepared, len=%d\n", len);
 
@@ -260,7 +269,7 @@ static int cros_ec_command_spi_xfer(struct cros_ec_device *ec_dev,
 		ret = final_ret;
 	if (ret < 0) {
 		dev_err(ec_dev->dev, "spi transfer failed: %d\n", ret);
-		return ret;
+		goto exit;
 	}
 
 	/* check response error code */
@@ -269,14 +278,16 @@ static int cros_ec_command_spi_xfer(struct cros_ec_device *ec_dev,
 		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);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto exit;
 	}
 	len = ptr[1];
 	sum = ptr[0] + ptr[1];
 	if (len > ec_msg->in_len) {
 		dev_err(ec_dev->dev, "packet too long (%d bytes, expected %d)",
 			len, ec_msg->in_len);
-		return -ENOSPC;
+		ret = -ENOSPC;
+		goto exit;
 	}
 
 	/* copy response packet payload and compute checksum */
@@ -293,10 +304,14 @@ static int cros_ec_command_spi_xfer(struct cros_ec_device *ec_dev,
 		dev_err(ec_dev->dev,
 			"bad packet checksum, expected %02x, got %02x\n",
 			sum, ptr[len + 2]);
-		return -EBADMSG;
+		ret = -EBADMSG;
+		goto exit;
 	}
 
-	return 0;
+	ret = 0;
+exit:
+	mutex_unlock(&ec_spi->lock);
+	return ret;
 }
 
 static void cros_ec_spi_dt_probe(struct cros_ec_spi *ec_spi, struct device *dev)
@@ -327,6 +342,7 @@ static int cros_ec_spi_probe(struct spi_device *spi)
 	if (ec_spi == NULL)
 		return -ENOMEM;
 	ec_spi->spi = spi;
+	mutex_init(&ec_spi->lock);
 	ec_dev = devm_kzalloc(dev, sizeof(*ec_dev), GFP_KERNEL);
 	if (!ec_dev)
 		return -ENOMEM;
-- 
1.9.1.423.g4596e3a

[PATCH v2 3/7] mfd: cros_ec: spi: Make the cros_ec_spi timeout more reliable

[Doug Anderson]

The cros_ec_spi transfer had two problems with its timeout code:

1. It looked at the timeout even in the case that it found valid data.
2. If the cros_ec_spi code got switched out for a while, it's possible
   it could get a timeout after a single loop.  Let's be paranoid and
   make sure we do one last transfer after the timeout expires.

Signed-off-by: Doug Anderson <dianders@chromium.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
Tested-by: Andrew Bresticker <abrestic@chromium.org>
Tested-by: Stephen Warren <swarren@nvidia.com>
---
Changes in v2: None

 drivers/mfd/cros_ec_spi.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
index a2a605d..4f863c3 100644
--- a/drivers/mfd/cros_ec_spi.c
+++ b/drivers/mfd/cros_ec_spi.c
@@ -113,7 +113,9 @@ static int cros_ec_spi_receive_response(struct cros_ec_device *ec_dev,
 
 	/* Receive data until we see the header byte */
 	deadline = jiffies + msecs_to_jiffies(EC_MSG_DEADLINE_MS);
-	do {
+	while (true) {
+		unsigned long start_jiffies = jiffies;
+
 		memset(&trans, 0, sizeof(trans));
 		trans.cs_change = 1;
 		trans.rx_buf = ptr = ec_dev->din;
@@ -134,12 +136,19 @@ static int cros_ec_spi_receive_response(struct cros_ec_device *ec_dev,
 				break;
 			}
 		}
+		if (ptr != end)
+			break;
 
-		if (time_after(jiffies, deadline)) {
+		/*
+		 * Use the time at the start of the loop as a timeout.  This
+		 * gives us one last shot at getting the transfer and is useful
+		 * in case we got context switched out for a while.
+		 */
+		if (time_after(start_jiffies, deadline)) {
 			dev_warn(ec_dev->dev, "EC failed to respond in time\n");
 			return -ETIMEDOUT;
 		}
-	} while (ptr == end);
+	}
 
 	/*
 	 * ptr now points to the header byte. Copy any valid data to the
-- 
1.9.1.423.g4596e3a

[PATCH v2 4/7] mfd: cros_ec: spi: Increase cros_ec_spi deadline from 5ms to 100ms

[Doug Anderson]

We're adding i2c tunneling to the list of things that goes over
cros_ec.  i2c tunneling can be slooooooow, so increase our deadline to
100ms to account for that.

Signed-off-by: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Reviewed-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Tested-by: Andrew Bresticker <abrestic-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Tested-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
Changes in v2: None

 drivers/mfd/cros_ec_spi.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
index 4f863c3..0b8d328 100644
--- a/drivers/mfd/cros_ec_spi.c
+++ b/drivers/mfd/cros_ec_spi.c
@@ -39,14 +39,22 @@
 #define EC_MSG_PREAMBLE_COUNT		32
 
 /*
-  * We must get a response from the EC in 5ms. This is a very long
-  * time, but the flash write command can take 2-3ms. The EC command
-  * processing is currently not very fast (about 500us). We could
-  * look at speeding this up and making the flash write command a
-  * 'slow' command, requiring a GET_STATUS wait loop, like flash
-  * erase.
-  */
-#define EC_MSG_DEADLINE_MS		5
+ * Allow for a long time for the EC to respond.  We support i2c
+ * tunneling and support fairly long messages for the tunnel (249
+ * bytes long at the moment).  If we're talking to a 100 kHz device
+ * on the other end and need to transfer ~256 bytes, then we need:
+ *  10 us/bit * ~10 bits/byte * ~256 bytes = ~25ms
+ *
+ * We'll wait 4 times that to handle clock stretching and other
+ * paranoia.
+ *
+ * It's pretty unlikely that we'll really see a 249 byte tunnel in
+ * anything other than testing.  If this was more common we might
+ * consider having slow commands like this require a GET_STATUS
+ * wait loop.  The 'flash write' command would be another candidate
+ * for this, clocking in at 2-3ms.
+ */
+#define EC_MSG_DEADLINE_MS		100
 
 /*
   * Time between raising the SPI chip select (for the end of a
-- 
1.9.1.423.g4596e3a

[PATCH v2 4/7] mfd: cros_ec: spi: Increase cros_ec_spi deadline from 5ms to 100ms

[Doug Anderson]

We're adding i2c tunneling to the list of things that goes over
cros_ec.  i2c tunneling can be slooooooow, so increase our deadline to
100ms to account for that.

Signed-off-by: Doug Anderson <dianders@chromium.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
Tested-by: Andrew Bresticker <abrestic@chromium.org>
Tested-by: Stephen Warren <swarren@nvidia.com>
---
Changes in v2: None

 drivers/mfd/cros_ec_spi.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
index 4f863c3..0b8d328 100644
--- a/drivers/mfd/cros_ec_spi.c
+++ b/drivers/mfd/cros_ec_spi.c
@@ -39,14 +39,22 @@
 #define EC_MSG_PREAMBLE_COUNT		32
 
 /*
-  * We must get a response from the EC in 5ms. This is a very long
-  * time, but the flash write command can take 2-3ms. The EC command
-  * processing is currently not very fast (about 500us). We could
-  * look at speeding this up and making the flash write command a
-  * 'slow' command, requiring a GET_STATUS wait loop, like flash
-  * erase.
-  */
-#define EC_MSG_DEADLINE_MS		5
+ * Allow for a long time for the EC to respond.  We support i2c
+ * tunneling and support fairly long messages for the tunnel (249
+ * bytes long at the moment).  If we're talking to a 100 kHz device
+ * on the other end and need to transfer ~256 bytes, then we need:
+ *  10 us/bit * ~10 bits/byte * ~256 bytes = ~25ms
+ *
+ * We'll wait 4 times that to handle clock stretching and other
+ * paranoia.
+ *
+ * It's pretty unlikely that we'll really see a 249 byte tunnel in
+ * anything other than testing.  If this was more common we might
+ * consider having slow commands like this require a GET_STATUS
+ * wait loop.  The 'flash write' command would be another candidate
+ * for this, clocking in at 2-3ms.
+ */
+#define EC_MSG_DEADLINE_MS		100
 
 /*
   * Time between raising the SPI chip select (for the end of a
-- 
1.9.1.423.g4596e3a

[PATCH v2 5/7] mfd: cros_ec: Sync to the latest cros_ec_commands.h from EC sources

[Doug Anderson]

From: Bill Richardson <wfrichar@chromium.org>

This just updates include/linux/mfd/cros_ec_commands.h to match the
latest EC version (which is the One True Source for such things).  See
<https://chromium.googlesource.com/chromiumos/platform/ec>

[dianders: took today's ToT version from the Chromium OS EC; deleted
references to cros_ec_dev and cros_ec_lpc since those aren't upstream
yet]

Signed-off-by: Bill Richardson <wfrichar@chromium.org>
Signed-off-by: Doug Anderson <dianders@chromium.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
Tested-by: Andrew Bresticker <abrestic@chromium.org>
Tested-by: Stephen Warren <swarren@nvidia.com>
---
Changes in v2: None

 drivers/mfd/cros_ec.c                |    2 +-
 include/linux/mfd/cros_ec.h          |    4 +-
 include/linux/mfd/cros_ec_commands.h | 1128 +++++++++++++++++++++++++++++++---
 3 files changed, 1059 insertions(+), 75 deletions(-)

diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index 783fe2e..c58ab96 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -30,7 +30,7 @@ int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
 	uint8_t *out;
 	int csum, i;
 
-	BUG_ON(msg->out_len > EC_HOST_PARAM_SIZE);
+	BUG_ON(msg->out_len > EC_PROTO2_MAX_PARAM_SIZE);
 	out = ec_dev->dout;
 	out[0] = EC_CMD_VERSION0 + msg->version;
 	out[1] = msg->cmd;
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index 032af7f..887ef4f 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -29,8 +29,8 @@ enum {
 	EC_MSG_RX_PROTO_BYTES	= 3,
 
 	/* Max length of messages */
-	EC_MSG_BYTES		= EC_HOST_PARAM_SIZE + EC_MSG_TX_PROTO_BYTES,
-
+	EC_MSG_BYTES		= EC_PROTO2_MAX_PARAM_SIZE +
+					EC_MSG_TX_PROTO_BYTES,
 };
 
 /**
diff --git a/include/linux/mfd/cros_ec_commands.h b/include/linux/mfd/cros_ec_commands.h
index 86fd069..7853a64 100644
--- a/include/linux/mfd/cros_ec_commands.h
+++ b/include/linux/mfd/cros_ec_commands.h
@@ -24,25 +24,12 @@
 #define __CROS_EC_COMMANDS_H
 
 /*
- * Protocol overview
+ * Current version of this protocol
  *
- * request:  CMD [ P0 P1 P2 ... Pn S ]
- * response: ERR [ P0 P1 P2 ... Pn S ]
- *
- * where the bytes are defined as follow :
- *      - CMD is the command code. (defined by EC_CMD_ constants)
- *      - ERR is the error code. (defined by EC_RES_ constants)
- *      - Px is the optional payload.
- *        it is not sent if the error code is not success.
- *        (defined by ec_params_ and ec_response_ structures)
- *      - S is the checksum which is the sum of all payload bytes.
- *
- * On LPC, CMD and ERR are sent/received at EC_LPC_ADDR_KERNEL|USER_CMD
- * and the payloads are sent/received at EC_LPC_ADDR_KERNEL|USER_PARAM.
- * On I2C, all bytes are sent serially in the same message.
+ * TODO(crosbug.com/p/11223): This is effectively useless; protocol is
+ * determined in other ways.  Remove this once the kernel code no longer
+ * depends on it.
  */
-
-/* Current version of this protocol */
 #define EC_PROTO_VERSION          0x00000002
 
 /* Command version mask */
@@ -57,13 +44,19 @@
 #define EC_LPC_ADDR_HOST_CMD   0x204
 
 /* I/O addresses for host command args and params */
-#define EC_LPC_ADDR_HOST_ARGS  0x800
-#define EC_LPC_ADDR_HOST_PARAM 0x804
-#define EC_HOST_PARAM_SIZE     0x0fc  /* Size of param area in bytes */
-
-/* I/O addresses for host command params, old interface */
-#define EC_LPC_ADDR_OLD_PARAM  0x880
-#define EC_OLD_PARAM_SIZE      0x080  /* Size of param area in bytes */
+/* Protocol version 2 */
+#define EC_LPC_ADDR_HOST_ARGS    0x800  /* And 0x801, 0x802, 0x803 */
+#define EC_LPC_ADDR_HOST_PARAM   0x804  /* For version 2 params; size is
+					 * EC_PROTO2_MAX_PARAM_SIZE */
+/* Protocol version 3 */
+#define EC_LPC_ADDR_HOST_PACKET  0x800  /* Offset of version 3 packet */
+#define EC_LPC_HOST_PACKET_SIZE  0x100  /* Max size of version 3 packet */
+
+/* The actual block is 0x800-0x8ff, but some BIOSes think it's 0x880-0x8ff
+ * and they tell the kernel that so we have to think of it as two parts. */
+#define EC_HOST_CMD_REGION0    0x800
+#define EC_HOST_CMD_REGION1    0x880
+#define EC_HOST_CMD_REGION_SIZE 0x80
 
 /* EC command register bit functions */
 #define EC_LPC_CMDR_DATA	(1 << 0)  /* Data ready for host to read */
@@ -79,18 +72,22 @@
 #define EC_MEMMAP_TEXT_MAX     8   /* Size of a string in the memory map */
 
 /* The offset address of each type of data in mapped memory. */
-#define EC_MEMMAP_TEMP_SENSOR      0x00 /* Temp sensors */
-#define EC_MEMMAP_FAN              0x10 /* Fan speeds */
-#define EC_MEMMAP_TEMP_SENSOR_B    0x18 /* Temp sensors (second set) */
-#define EC_MEMMAP_ID               0x20 /* 'E' 'C' */
+#define EC_MEMMAP_TEMP_SENSOR      0x00 /* Temp sensors 0x00 - 0x0f */
+#define EC_MEMMAP_FAN              0x10 /* Fan speeds 0x10 - 0x17 */
+#define EC_MEMMAP_TEMP_SENSOR_B    0x18 /* More temp sensors 0x18 - 0x1f */
+#define EC_MEMMAP_ID               0x20 /* 0x20 == 'E', 0x21 == 'C' */
 #define EC_MEMMAP_ID_VERSION       0x22 /* Version of data in 0x20 - 0x2f */
 #define EC_MEMMAP_THERMAL_VERSION  0x23 /* Version of data in 0x00 - 0x1f */
 #define EC_MEMMAP_BATTERY_VERSION  0x24 /* Version of data in 0x40 - 0x7f */
 #define EC_MEMMAP_SWITCHES_VERSION 0x25 /* Version of data in 0x30 - 0x33 */
 #define EC_MEMMAP_EVENTS_VERSION   0x26 /* Version of data in 0x34 - 0x3f */
-#define EC_MEMMAP_HOST_CMD_FLAGS   0x27 /* Host command interface flags */
-#define EC_MEMMAP_SWITCHES         0x30
-#define EC_MEMMAP_HOST_EVENTS      0x34
+#define EC_MEMMAP_HOST_CMD_FLAGS   0x27 /* Host cmd interface flags (8 bits) */
+/* Unused 0x28 - 0x2f */
+#define EC_MEMMAP_SWITCHES         0x30	/* 8 bits */
+/* Unused 0x31 - 0x33 */
+#define EC_MEMMAP_HOST_EVENTS      0x34 /* 32 bits */
+/* Reserve 0x38 - 0x3f for additional host event-related stuff */
+/* Battery values are all 32 bits */
 #define EC_MEMMAP_BATT_VOLT        0x40 /* Battery Present Voltage */
 #define EC_MEMMAP_BATT_RATE        0x44 /* Battery Present Rate */
 #define EC_MEMMAP_BATT_CAP         0x48 /* Battery Remaining Capacity */
@@ -99,10 +96,24 @@
 #define EC_MEMMAP_BATT_DVLT        0x54 /* Battery Design Voltage */
 #define EC_MEMMAP_BATT_LFCC        0x58 /* Battery Last Full Charge Capacity */
 #define EC_MEMMAP_BATT_CCNT        0x5c /* Battery Cycle Count */
+/* Strings are all 8 bytes (EC_MEMMAP_TEXT_MAX) */
 #define EC_MEMMAP_BATT_MFGR        0x60 /* Battery Manufacturer String */
 #define EC_MEMMAP_BATT_MODEL       0x68 /* Battery Model Number String */
 #define EC_MEMMAP_BATT_SERIAL      0x70 /* Battery Serial Number String */
 #define EC_MEMMAP_BATT_TYPE        0x78 /* Battery Type String */
+#define EC_MEMMAP_ALS              0x80 /* ALS readings in lux (2 X 16 bits) */
+/* Unused 0x84 - 0x8f */
+#define EC_MEMMAP_ACC_STATUS       0x90 /* Accelerometer status (8 bits )*/
+/* Unused 0x91 */
+#define EC_MEMMAP_ACC_DATA         0x92 /* Accelerometer data 0x92 - 0x9f */
+#define EC_MEMMAP_GYRO_DATA        0xa0 /* Gyroscope data 0xa0 - 0xa5 */
+/* Unused 0xa6 - 0xfe (remember, 0xff is NOT part of the memmap region) */
+
+
+/* Define the format of the accelerometer mapped memory status byte. */
+#define EC_MEMMAP_ACC_STATUS_SAMPLE_ID_MASK  0x0f
+#define EC_MEMMAP_ACC_STATUS_BUSY_BIT        (1 << 4)
+#define EC_MEMMAP_ACC_STATUS_PRESENCE_BIT    (1 << 7)
 
 /* Number of temp sensors at EC_MEMMAP_TEMP_SENSOR */
 #define EC_TEMP_SENSOR_ENTRIES     16
@@ -112,6 +123,8 @@
  * Valid only if EC_MEMMAP_THERMAL_VERSION returns >= 2.
  */
 #define EC_TEMP_SENSOR_B_ENTRIES      8
+
+/* Special values for mapped temperature sensors */
 #define EC_TEMP_SENSOR_NOT_PRESENT    0xff
 #define EC_TEMP_SENSOR_ERROR          0xfe
 #define EC_TEMP_SENSOR_NOT_POWERED    0xfd
@@ -122,6 +135,18 @@
  */
 #define EC_TEMP_SENSOR_OFFSET      200
 
+/*
+ * Number of ALS readings at EC_MEMMAP_ALS
+ */
+#define EC_ALS_ENTRIES             2
+
+/*
+ * The default value a temperature sensor will return when it is present but
+ * has not been read this boot.  This is a reasonable number to avoid
+ * triggering alarms on the host.
+ */
+#define EC_TEMP_SENSOR_DEFAULT     (296 - EC_TEMP_SENSOR_OFFSET)
+
 #define EC_FAN_SPEED_ENTRIES       4       /* Number of fans at EC_MEMMAP_FAN */
 #define EC_FAN_SPEED_NOT_PRESENT   0xffff  /* Entry not present */
 #define EC_FAN_SPEED_STALLED       0xfffe  /* Fan stalled */
@@ -137,8 +162,8 @@
 #define EC_SWITCH_LID_OPEN               0x01
 #define EC_SWITCH_POWER_BUTTON_PRESSED   0x02
 #define EC_SWITCH_WRITE_PROTECT_DISABLED 0x04
-/* Recovery requested via keyboard */
-#define EC_SWITCH_KEYBOARD_RECOVERY      0x08
+/* Was recovery requested via keyboard; now unused. */
+#define EC_SWITCH_IGNORE1		 0x08
 /* Recovery requested via dedicated signal (from servo board) */
 #define EC_SWITCH_DEDICATED_RECOVERY     0x10
 /* Was fake developer mode switch; now unused.  Remove in next refactor. */
@@ -147,10 +172,15 @@
 /* Host command interface flags */
 /* Host command interface supports LPC args (LPC interface only) */
 #define EC_HOST_CMD_FLAG_LPC_ARGS_SUPPORTED  0x01
+/* Host command interface supports version 3 protocol */
+#define EC_HOST_CMD_FLAG_VERSION_3   0x02
 
 /* Wireless switch flags */
-#define EC_WIRELESS_SWITCH_WLAN      0x01
-#define EC_WIRELESS_SWITCH_BLUETOOTH 0x02
+#define EC_WIRELESS_SWITCH_ALL       ~0x00  /* All flags */
+#define EC_WIRELESS_SWITCH_WLAN       0x01  /* WLAN radio */
+#define EC_WIRELESS_SWITCH_BLUETOOTH  0x02  /* Bluetooth radio */
+#define EC_WIRELESS_SWITCH_WWAN       0x04  /* WWAN power */
+#define EC_WIRELESS_SWITCH_WLAN_POWER 0x08  /* WLAN power */
 
 /*
  * This header file is used in coreboot both in C and ACPI code.  The ACPI code
@@ -159,6 +189,14 @@
  */
 #ifndef __ACPI__
 
+/*
+ * Define __packed if someone hasn't beat us to it.  Linux kernel style
+ * checking prefers __packed over __attribute__((packed)).
+ */
+#ifndef __packed
+#define __packed __attribute__((packed))
+#endif
+
 /* LPC command status byte masks */
 /* EC has written a byte in the data register and host hasn't read it yet */
 #define EC_LPC_STATUS_TO_HOST     0x01
@@ -198,6 +236,9 @@ enum ec_status {
 	EC_RES_UNAVAILABLE = 9,		/* No response available */
 	EC_RES_TIMEOUT = 10,		/* We got a timeout */
 	EC_RES_OVERFLOW = 11,		/* Table / data overflow */
+	EC_RES_INVALID_HEADER = 12,     /* Header contains invalid data */
+	EC_RES_REQUEST_TRUNCATED = 13,  /* Didn't get the entire request */
+	EC_RES_RESPONSE_TOO_BIG = 14    /* Response was too big to handle */
 };
 
 /*
@@ -235,6 +276,16 @@ enum host_event_code {
 	/* Shutdown due to battery level too low */
 	EC_HOST_EVENT_BATTERY_SHUTDOWN = 17,
 
+	/* Suggest that the AP throttle itself */
+	EC_HOST_EVENT_THROTTLE_START = 18,
+	/* Suggest that the AP resume normal speed */
+	EC_HOST_EVENT_THROTTLE_STOP = 19,
+
+	/* Hang detect logic detected a hang and host event timeout expired */
+	EC_HOST_EVENT_HANG_DETECT = 20,
+	/* Hang detect logic detected a hang and warm rebooted the AP */
+	EC_HOST_EVENT_HANG_REBOOT = 21,
+
 	/*
 	 * The high bit of the event mask is not used as a host event code.  If
 	 * it reads back as set, then the entire event mask should be
@@ -279,6 +330,188 @@ struct ec_lpc_host_args {
  */
 #define EC_HOST_ARGS_FLAG_TO_HOST   0x02
 
+/*****************************************************************************/
+/*
+ * Byte codes returned by EC over SPI interface.
+ *
+ * These can be used by the AP to debug the EC interface, and to determine
+ * when the EC is not in a state where it will ever get around to responding
+ * to the AP.
+ *
+ * Example of sequence of bytes read from EC for a current good transfer:
+ *   1. -                  - AP asserts chip select (CS#)
+ *   2. EC_SPI_OLD_READY   - AP sends first byte(s) of request
+ *   3. -                  - EC starts handling CS# interrupt
+ *   4. EC_SPI_RECEIVING   - AP sends remaining byte(s) of request
+ *   5. EC_SPI_PROCESSING  - EC starts processing request; AP is clocking in
+ *                           bytes looking for EC_SPI_FRAME_START
+ *   6. -                  - EC finishes processing and sets up response
+ *   7. EC_SPI_FRAME_START - AP reads frame byte
+ *   8. (response packet)  - AP reads response packet
+ *   9. EC_SPI_PAST_END    - Any additional bytes read by AP
+ *   10 -                  - AP deasserts chip select
+ *   11 -                  - EC processes CS# interrupt and sets up DMA for
+ *                           next request
+ *
+ * If the AP is waiting for EC_SPI_FRAME_START and sees any value other than
+ * the following byte values:
+ *   EC_SPI_OLD_READY
+ *   EC_SPI_RX_READY
+ *   EC_SPI_RECEIVING
+ *   EC_SPI_PROCESSING
+ *
+ * Then the EC found an error in the request, or was not ready for the request
+ * and lost data.  The AP should give up waiting for EC_SPI_FRAME_START,
+ * because the EC is unable to tell when the AP is done sending its request.
+ */
+
+/*
+ * Framing byte which precedes a response packet from the EC.  After sending a
+ * request, the AP will clock in bytes until it sees the framing byte, then
+ * clock in the response packet.
+ */
+#define EC_SPI_FRAME_START    0xec
+
+/*
+ * Padding bytes which are clocked out after the end of a response packet.
+ */
+#define EC_SPI_PAST_END       0xed
+
+/*
+ * EC is ready to receive, and has ignored the byte sent by the AP.  EC expects
+ * that the AP will send a valid packet header (starting with
+ * EC_COMMAND_PROTOCOL_3) in the next 32 bytes.
+ */
+#define EC_SPI_RX_READY       0xf8
+
+/*
+ * EC has started receiving the request from the AP, but hasn't started
+ * processing it yet.
+ */
+#define EC_SPI_RECEIVING      0xf9
+
+/* EC has received the entire request from the AP and is processing it. */
+#define EC_SPI_PROCESSING     0xfa
+
+/*
+ * EC received bad data from the AP, such as a packet header with an invalid
+ * length.  EC will ignore all data until chip select deasserts.
+ */
+#define EC_SPI_RX_BAD_DATA    0xfb
+
+/*
+ * EC received data from the AP before it was ready.  That is, the AP asserted
+ * chip select and started clocking data before the EC was ready to receive it.
+ * EC will ignore all data until chip select deasserts.
+ */
+#define EC_SPI_NOT_READY      0xfc
+
+/*
+ * EC was ready to receive a request from the AP.  EC has treated the byte sent
+ * by the AP as part of a request packet, or (for old-style ECs) is processing
+ * a fully received packet but is not ready to respond yet.
+ */
+#define EC_SPI_OLD_READY      0xfd
+
+/*****************************************************************************/
+
+/*
+ * Protocol version 2 for I2C and SPI send a request this way:
+ *
+ *	0	EC_CMD_VERSION0 + (command version)
+ *	1	Command number
+ *	2	Length of params = N
+ *	3..N+2	Params, if any
+ *	N+3	8-bit checksum of bytes 0..N+2
+ *
+ * The corresponding response is:
+ *
+ *	0	Result code (EC_RES_*)
+ *	1	Length of params = M
+ *	2..M+1	Params, if any
+ *	M+2	8-bit checksum of bytes 0..M+1
+ */
+#define EC_PROTO2_REQUEST_HEADER_BYTES 3
+#define EC_PROTO2_REQUEST_TRAILER_BYTES 1
+#define EC_PROTO2_REQUEST_OVERHEAD (EC_PROTO2_REQUEST_HEADER_BYTES +	\
+				    EC_PROTO2_REQUEST_TRAILER_BYTES)
+
+#define EC_PROTO2_RESPONSE_HEADER_BYTES 2
+#define EC_PROTO2_RESPONSE_TRAILER_BYTES 1
+#define EC_PROTO2_RESPONSE_OVERHEAD (EC_PROTO2_RESPONSE_HEADER_BYTES +	\
+				     EC_PROTO2_RESPONSE_TRAILER_BYTES)
+
+/* Parameter length was limited by the LPC interface */
+#define EC_PROTO2_MAX_PARAM_SIZE 0xfc
+
+/* Maximum request and response packet sizes for protocol version 2 */
+#define EC_PROTO2_MAX_REQUEST_SIZE (EC_PROTO2_REQUEST_OVERHEAD +	\
+				    EC_PROTO2_MAX_PARAM_SIZE)
+#define EC_PROTO2_MAX_RESPONSE_SIZE (EC_PROTO2_RESPONSE_OVERHEAD +	\
+				     EC_PROTO2_MAX_PARAM_SIZE)
+
+/*****************************************************************************/
+
+/*
+ * Value written to legacy command port / prefix byte to indicate protocol
+ * 3+ structs are being used.  Usage is bus-dependent.
+ */
+#define EC_COMMAND_PROTOCOL_3 0xda
+
+#define EC_HOST_REQUEST_VERSION 3
+
+/* Version 3 request from host */
+struct ec_host_request {
+	/* Struct version (=3)
+	 *
+	 * EC will return EC_RES_INVALID_HEADER if it receives a header with a
+	 * version it doesn't know how to parse.
+	 */
+	uint8_t struct_version;
+
+	/*
+	 * Checksum of request and data; sum of all bytes including checksum
+	 * should total to 0.
+	 */
+	uint8_t checksum;
+
+	/* Command code */
+	uint16_t command;
+
+	/* Command version */
+	uint8_t command_version;
+
+	/* Unused byte in current protocol version; set to 0 */
+	uint8_t reserved;
+
+	/* Length of data which follows this header */
+	uint16_t data_len;
+} __packed;
+
+#define EC_HOST_RESPONSE_VERSION 3
+
+/* Version 3 response from EC */
+struct ec_host_response {
+	/* Struct version (=3) */
+	uint8_t struct_version;
+
+	/*
+	 * Checksum of response and data; sum of all bytes including checksum
+	 * should total to 0.
+	 */
+	uint8_t checksum;
+
+	/* Result code (EC_RES_*) */
+	uint16_t result;
+
+	/* Length of data which follows this header */
+	uint16_t data_len;
+
+	/* Unused bytes in current protocol version; set to 0 */
+	uint16_t reserved;
+} __packed;
+
+/*****************************************************************************/
 /*
  * Notes on commands:
  *
@@ -418,6 +651,68 @@ struct ec_response_get_comms_status {
 	uint32_t flags;		/* Mask of enum ec_comms_status */
 } __packed;
 
+/* Fake a variety of responses, purely for testing purposes. */
+#define EC_CMD_TEST_PROTOCOL		0x0a
+
+/* Tell the EC what to send back to us. */
+struct ec_params_test_protocol {
+	uint32_t ec_result;
+	uint32_t ret_len;
+	uint8_t buf[32];
+} __packed;
+
+/* Here it comes... */
+struct ec_response_test_protocol {
+	uint8_t buf[32];
+} __packed;
+
+/* Get prococol information */
+#define EC_CMD_GET_PROTOCOL_INFO	0x0b
+
+/* Flags for ec_response_get_protocol_info.flags */
+/* EC_RES_IN_PROGRESS may be returned if a command is slow */
+#define EC_PROTOCOL_INFO_IN_PROGRESS_SUPPORTED (1 << 0)
+
+struct ec_response_get_protocol_info {
+	/* Fields which exist if at least protocol version 3 supported */
+
+	/* Bitmask of protocol versions supported (1 << n means version n)*/
+	uint32_t protocol_versions;
+
+	/* Maximum request packet size, in bytes */
+	uint16_t max_request_packet_size;
+
+	/* Maximum response packet size, in bytes */
+	uint16_t max_response_packet_size;
+
+	/* Flags; see EC_PROTOCOL_INFO_* */
+	uint32_t flags;
+} __packed;
+
+
+/*****************************************************************************/
+/* Get/Set miscellaneous values */
+
+/* The upper byte of .flags tells what to do (nothing means "get") */
+#define EC_GSV_SET        0x80000000
+
+/* The lower three bytes of .flags identifies the parameter, if that has
+   meaning for an individual command. */
+#define EC_GSV_PARAM_MASK 0x00ffffff
+
+struct ec_params_get_set_value {
+	uint32_t flags;
+	uint32_t value;
+} __packed;
+
+struct ec_response_get_set_value {
+	uint32_t flags;
+	uint32_t value;
+} __packed;
+
+/* More than one command can use these structs to get/set paramters. */
+#define EC_CMD_GSV_PAUSE_IN_S5	0x0c
+
 
 /*****************************************************************************/
 /* Flash commands */
@@ -425,6 +720,7 @@ struct ec_response_get_comms_status {
 /* Get flash info */
 #define EC_CMD_FLASH_INFO 0x10
 
+/* Version 0 returns these fields */
 struct ec_response_flash_info {
 	/* Usable flash size, in bytes */
 	uint32_t flash_size;
@@ -445,6 +741,37 @@ struct ec_response_flash_info {
 	uint32_t protect_block_size;
 } __packed;
 
+/* Flags for version 1+ flash info command */
+/* EC flash erases bits to 0 instead of 1 */
+#define EC_FLASH_INFO_ERASE_TO_0 (1 << 0)
+
+/*
+ * Version 1 returns the same initial fields as version 0, with additional
+ * fields following.
+ *
+ * gcc anonymous structs don't seem to get along with the __packed directive;
+ * if they did we'd define the version 0 struct as a sub-struct of this one.
+ */
+struct ec_response_flash_info_1 {
+	/* Version 0 fields; see above for description */
+	uint32_t flash_size;
+	uint32_t write_block_size;
+	uint32_t erase_block_size;
+	uint32_t protect_block_size;
+
+	/* Version 1 adds these fields: */
+	/*
+	 * Ideal write size in bytes.  Writes will be fastest if size is
+	 * exactly this and offset is a multiple of this.  For example, an EC
+	 * may have a write buffer which can do half-page operations if data is
+	 * aligned, and a slower word-at-a-time write mode.
+	 */
+	uint32_t write_ideal_size;
+
+	/* Flags; see EC_FLASH_INFO_* */
+	uint32_t flags;
+} __packed;
+
 /*
  * Read flash
  *
@@ -459,15 +786,15 @@ struct ec_params_flash_read {
 
 /* Write flash */
 #define EC_CMD_FLASH_WRITE 0x12
+#define EC_VER_FLASH_WRITE 1
+
+/* Version 0 of the flash command supported only 64 bytes of data */
+#define EC_FLASH_WRITE_VER0_SIZE 64
 
 struct ec_params_flash_write {
 	uint32_t offset;   /* Byte offset to write */
 	uint32_t size;     /* Size to write in bytes */
-	/*
-	 * Data to write.  Could really use EC_PARAM_SIZE - 8, but tidiest to
-	 * use a power of 2 so writes stay aligned.
-	 */
-	uint8_t data[64];
+	/* Followed by data to write */
 } __packed;
 
 /* Erase flash */
@@ -543,7 +870,7 @@ struct ec_response_flash_protect {
 
 enum ec_flash_region {
 	/* Region which holds read-only EC image */
-	EC_FLASH_REGION_RO,
+	EC_FLASH_REGION_RO = 0,
 	/* Region which holds rewritable EC image */
 	EC_FLASH_REGION_RW,
 	/*
@@ -551,6 +878,8 @@ enum ec_flash_region {
 	 * EC_FLASH_REGION_RO)
 	 */
 	EC_FLASH_REGION_WP_RO,
+	/* Number of regions */
+	EC_FLASH_REGION_COUNT,
 };
 
 struct ec_params_flash_region_info {
@@ -639,15 +968,15 @@ struct rgb_s {
  */
 struct lightbar_params {
 	/* Timing */
-	int google_ramp_up;
-	int google_ramp_down;
-	int s3s0_ramp_up;
-	int s0_tick_delay[2];			/* AC=0/1 */
-	int s0a_tick_delay[2];			/* AC=0/1 */
-	int s0s3_ramp_down;
-	int s3_sleep_for;
-	int s3_ramp_up;
-	int s3_ramp_down;
+	int32_t google_ramp_up;
+	int32_t google_ramp_down;
+	int32_t s3s0_ramp_up;
+	int32_t s0_tick_delay[2];		/* AC=0/1 */
+	int32_t s0a_tick_delay[2];		/* AC=0/1 */
+	int32_t s0s3_ramp_down;
+	int32_t s3_sleep_for;
+	int32_t s3_ramp_up;
+	int32_t s3_ramp_down;
 
 	/* Oscillation */
 	uint8_t new_s0;
@@ -676,7 +1005,7 @@ struct ec_params_lightbar {
 	union {
 		struct {
 			/* no args */
-		} dump, off, on, init, get_seq, get_params;
+		} dump, off, on, init, get_seq, get_params, version;
 
 		struct num {
 			uint8_t num;
@@ -710,6 +1039,11 @@ struct ec_response_lightbar {
 
 		struct lightbar_params get_params;
 
+		struct version {
+			uint32_t num;
+			uint32_t flags;
+		} version;
+
 		struct {
 			/* no return params */
 		} off, on, init, brightness, seq, reg, rgb, demo, set_params;
@@ -730,10 +1064,62 @@ enum lightbar_command {
 	LIGHTBAR_CMD_DEMO = 9,
 	LIGHTBAR_CMD_GET_PARAMS = 10,
 	LIGHTBAR_CMD_SET_PARAMS = 11,
+	LIGHTBAR_CMD_VERSION = 12,
 	LIGHTBAR_NUM_CMDS
 };
 
 /*****************************************************************************/
+/* LED control commands */
+
+#define EC_CMD_LED_CONTROL 0x29
+
+enum ec_led_id {
+	/* LED to indicate battery state of charge */
+	EC_LED_ID_BATTERY_LED = 0,
+	/*
+	 * LED to indicate system power state (on or in suspend).
+	 * May be on power button or on C-panel.
+	 */
+	EC_LED_ID_POWER_LED,
+	/* LED on power adapter or its plug */
+	EC_LED_ID_ADAPTER_LED,
+
+	EC_LED_ID_COUNT
+};
+
+/* LED control flags */
+#define EC_LED_FLAGS_QUERY (1 << 0) /* Query LED capability only */
+#define EC_LED_FLAGS_AUTO  (1 << 1) /* Switch LED back to automatic control */
+
+enum ec_led_colors {
+	EC_LED_COLOR_RED = 0,
+	EC_LED_COLOR_GREEN,
+	EC_LED_COLOR_BLUE,
+	EC_LED_COLOR_YELLOW,
+	EC_LED_COLOR_WHITE,
+
+	EC_LED_COLOR_COUNT
+};
+
+struct ec_params_led_control {
+	uint8_t led_id;     /* Which LED to control */
+	uint8_t flags;      /* Control flags */
+
+	uint8_t brightness[EC_LED_COLOR_COUNT];
+} __packed;
+
+struct ec_response_led_control {
+	/*
+	 * Available brightness value range.
+	 *
+	 * Range 0 means color channel not present.
+	 * Range 1 means on/off control.
+	 * Other values means the LED is control by PWM.
+	 */
+	uint8_t brightness_range[EC_LED_COLOR_COUNT];
+} __packed;
+
+/*****************************************************************************/
 /* Verified boot commands */
 
 /*
@@ -790,6 +1176,181 @@ enum ec_vboot_hash_status {
 #define EC_VBOOT_HASH_OFFSET_RW 0xfffffffd
 
 /*****************************************************************************/
+/*
+ * Motion sense commands. We'll make separate structs for sub-commands with
+ * different input args, so that we know how much to expect.
+ */
+#define EC_CMD_MOTION_SENSE_CMD 0x2B
+
+/* Motion sense commands */
+enum motionsense_command {
+	/*
+	 * Dump command returns all motion sensor data including motion sense
+	 * module flags and individual sensor flags.
+	 */
+	MOTIONSENSE_CMD_DUMP = 0,
+
+	/*
+	 * Info command returns data describing the details of a given sensor,
+	 * including enum motionsensor_type, enum motionsensor_location, and
+	 * enum motionsensor_chip.
+	 */
+	MOTIONSENSE_CMD_INFO = 1,
+
+	/*
+	 * EC Rate command is a setter/getter command for the EC sampling rate
+	 * of all motion sensors in milliseconds.
+	 */
+	MOTIONSENSE_CMD_EC_RATE = 2,
+
+	/*
+	 * Sensor ODR command is a setter/getter command for the output data
+	 * rate of a specific motion sensor in millihertz.
+	 */
+	MOTIONSENSE_CMD_SENSOR_ODR = 3,
+
+	/*
+	 * Sensor range command is a setter/getter command for the range of
+	 * a specified motion sensor in +/-G's or +/- deg/s.
+	 */
+	MOTIONSENSE_CMD_SENSOR_RANGE = 4,
+
+	/*
+	 * Setter/getter command for the keyboard wake angle. When the lid
+	 * angle is greater than this value, keyboard wake is disabled in S3,
+	 * and when the lid angle goes less than this value, keyboard wake is
+	 * enabled. Note, the lid angle measurement is an approximate,
+	 * un-calibrated value, hence the wake angle isn't exact.
+	 */
+	MOTIONSENSE_CMD_KB_WAKE_ANGLE = 5,
+
+	/* Number of motionsense sub-commands. */
+	MOTIONSENSE_NUM_CMDS
+};
+
+enum motionsensor_id {
+	EC_MOTION_SENSOR_ACCEL_BASE = 0,
+	EC_MOTION_SENSOR_ACCEL_LID = 1,
+	EC_MOTION_SENSOR_GYRO = 2,
+
+	/*
+	 * Note, if more sensors are added and this count changes, the padding
+	 * in ec_response_motion_sense dump command must be modified.
+	 */
+	EC_MOTION_SENSOR_COUNT = 3
+};
+
+/* List of motion sensor types. */
+enum motionsensor_type {
+	MOTIONSENSE_TYPE_ACCEL = 0,
+	MOTIONSENSE_TYPE_GYRO = 1,
+};
+
+/* List of motion sensor locations. */
+enum motionsensor_location {
+	MOTIONSENSE_LOC_BASE = 0,
+	MOTIONSENSE_LOC_LID = 1,
+};
+
+/* List of motion sensor chips. */
+enum motionsensor_chip {
+	MOTIONSENSE_CHIP_KXCJ9 = 0,
+};
+
+/* Module flag masks used for the dump sub-command. */
+#define MOTIONSENSE_MODULE_FLAG_ACTIVE (1<<0)
+
+/* Sensor flag masks used for the dump sub-command. */
+#define MOTIONSENSE_SENSOR_FLAG_PRESENT (1<<0)
+
+/*
+ * Send this value for the data element to only perform a read. If you
+ * send any other value, the EC will interpret it as data to set and will
+ * return the actual value set.
+ */
+#define EC_MOTION_SENSE_NO_VALUE -1
+
+struct ec_params_motion_sense {
+	uint8_t cmd;
+	union {
+		/* Used for MOTIONSENSE_CMD_DUMP. */
+		struct {
+			/* no args */
+		} dump;
+
+		/*
+		 * Used for MOTIONSENSE_CMD_EC_RATE and
+		 * MOTIONSENSE_CMD_KB_WAKE_ANGLE.
+		 */
+		struct {
+			/* Data to set or EC_MOTION_SENSE_NO_VALUE to read. */
+			int16_t data;
+		} ec_rate, kb_wake_angle;
+
+		/* Used for MOTIONSENSE_CMD_INFO. */
+		struct {
+			/* Should be element of enum motionsensor_id. */
+			uint8_t sensor_num;
+		} info;
+
+		/*
+		 * Used for MOTIONSENSE_CMD_SENSOR_ODR and
+		 * MOTIONSENSE_CMD_SENSOR_RANGE.
+		 */
+		struct {
+			/* Should be element of enum motionsensor_id. */
+			uint8_t sensor_num;
+
+			/* Rounding flag, true for round-up, false for down. */
+			uint8_t roundup;
+
+			uint16_t reserved;
+
+			/* Data to set or EC_MOTION_SENSE_NO_VALUE to read. */
+			int32_t data;
+		} sensor_odr, sensor_range;
+	};
+} __packed;
+
+struct ec_response_motion_sense {
+	union {
+		/* Used for MOTIONSENSE_CMD_DUMP. */
+		struct {
+			/* Flags representing the motion sensor module. */
+			uint8_t module_flags;
+
+			/* Flags for each sensor in enum motionsensor_id. */
+			uint8_t sensor_flags[EC_MOTION_SENSOR_COUNT];
+
+			/* Array of all sensor data. Each sensor is 3-axis. */
+			int16_t data[3*EC_MOTION_SENSOR_COUNT];
+		} dump;
+
+		/* Used for MOTIONSENSE_CMD_INFO. */
+		struct {
+			/* Should be element of enum motionsensor_type. */
+			uint8_t type;
+
+			/* Should be element of enum motionsensor_location. */
+			uint8_t location;
+
+			/* Should be element of enum motionsensor_chip. */
+			uint8_t chip;
+		} info;
+
+		/*
+		 * Used for MOTIONSENSE_CMD_EC_RATE, MOTIONSENSE_CMD_SENSOR_ODR,
+		 * MOTIONSENSE_CMD_SENSOR_RANGE, and
+		 * MOTIONSENSE_CMD_KB_WAKE_ANGLE.
+		 */
+		struct {
+			/* Current value of the parameter queried. */
+			int32_t ret;
+		} ec_rate, sensor_odr, sensor_range, kb_wake_angle;
+	};
+} __packed;
+
+/*****************************************************************************/
 /* USB charging control commands */
 
 /* Set USB port charging mode */
@@ -868,20 +1429,27 @@ struct ec_response_port80_last_boot {
 } __packed;
 
 /*****************************************************************************/
-/* Thermal engine commands */
+/* Thermal engine commands. Note that there are two implementations. We'll
+ * reuse the command number, but the data and behavior is incompatible.
+ * Version 0 is what originally shipped on Link.
+ * Version 1 separates the CPU thermal limits from the fan control.
+ */
 
-/* Set thershold value */
 #define EC_CMD_THERMAL_SET_THRESHOLD 0x50
+#define EC_CMD_THERMAL_GET_THRESHOLD 0x51
+
+/* The version 0 structs are opaque. You have to know what they are for
+ * the get/set commands to make any sense.
+ */
 
+/* Version 0 - set */
 struct ec_params_thermal_set_threshold {
 	uint8_t sensor_type;
 	uint8_t threshold_id;
 	uint16_t value;
 } __packed;
 
-/* Get threshold value */
-#define EC_CMD_THERMAL_GET_THRESHOLD 0x51
-
+/* Version 0 - get */
 struct ec_params_thermal_get_threshold {
 	uint8_t sensor_type;
 	uint8_t threshold_id;
@@ -891,6 +1459,41 @@ struct ec_response_thermal_get_threshold {
 	uint16_t value;
 } __packed;
 
+
+/* The version 1 structs are visible. */
+enum ec_temp_thresholds {
+	EC_TEMP_THRESH_WARN = 0,
+	EC_TEMP_THRESH_HIGH,
+	EC_TEMP_THRESH_HALT,
+
+	EC_TEMP_THRESH_COUNT
+};
+
+/* Thermal configuration for one temperature sensor. Temps are in degrees K.
+ * Zero values will be silently ignored by the thermal task.
+ */
+struct ec_thermal_config {
+	uint32_t temp_host[EC_TEMP_THRESH_COUNT]; /* levels of hotness */
+	uint32_t temp_fan_off;		/* no active cooling needed */
+	uint32_t temp_fan_max;		/* max active cooling needed */
+} __packed;
+
+/* Version 1 - get config for one sensor. */
+struct ec_params_thermal_get_threshold_v1 {
+	uint32_t sensor_num;
+} __packed;
+/* This returns a struct ec_thermal_config */
+
+/* Version 1 - set config for one sensor.
+ * Use read-modify-write for best results! */
+struct ec_params_thermal_set_threshold_v1 {
+	uint32_t sensor_num;
+	struct ec_thermal_config cfg;
+} __packed;
+/* This returns no data */
+
+/****************************************************************************/
+
 /* Toggle automatic fan control */
 #define EC_CMD_THERMAL_AUTO_FAN_CTRL 0x52
 
@@ -920,6 +1523,18 @@ struct ec_params_tmp006_set_calibration {
 	float b2;
 } __packed;
 
+/* Read raw TMP006 data */
+#define EC_CMD_TMP006_GET_RAW 0x55
+
+struct ec_params_tmp006_get_raw {
+	uint8_t index;
+} __packed;
+
+struct ec_response_tmp006_get_raw {
+	int32_t t;  /* In 1/100 K */
+	int32_t v;  /* In nV */
+};
+
 /*****************************************************************************/
 /* MKBP - Matrix KeyBoard Protocol */
 
@@ -1118,11 +1733,41 @@ struct ec_params_switch_enable_backlight {
 
 /* Enable/disable WLAN/Bluetooth */
 #define EC_CMD_SWITCH_ENABLE_WIRELESS 0x91
+#define EC_VER_SWITCH_ENABLE_WIRELESS 1
 
-struct ec_params_switch_enable_wireless {
+/* Version 0 params; no response */
+struct ec_params_switch_enable_wireless_v0 {
 	uint8_t enabled;
 } __packed;
 
+/* Version 1 params */
+struct ec_params_switch_enable_wireless_v1 {
+	/* Flags to enable now */
+	uint8_t now_flags;
+
+	/* Which flags to copy from now_flags */
+	uint8_t now_mask;
+
+	/*
+	 * Flags to leave enabled in S3, if they're on at the S0->S3
+	 * transition.  (Other flags will be disabled by the S0->S3
+	 * transition.)
+	 */
+	uint8_t suspend_flags;
+
+	/* Which flags to copy from suspend_flags */
+	uint8_t suspend_mask;
+} __packed;
+
+/* Version 1 response */
+struct ec_response_switch_enable_wireless_v1 {
+	/* Flags to enable now */
+	uint8_t now_flags;
+
+	/* Flags to leave enabled in S3 */
+	uint8_t suspend_flags;
+} __packed;
+
 /*****************************************************************************/
 /* GPIO commands. Only available on EC if write protect has been disabled. */
 
@@ -1147,11 +1792,16 @@ struct ec_response_gpio_get {
 /*****************************************************************************/
 /* I2C commands. Only available when flash write protect is unlocked. */
 
+/*
+ * TODO(crosbug.com/p/23570): These commands are deprecated, and will be
+ * removed soon.  Use EC_CMD_I2C_XFER instead.
+ */
+
 /* Read I2C bus */
 #define EC_CMD_I2C_READ 0x94
 
 struct ec_params_i2c_read {
-	uint16_t addr;
+	uint16_t addr; /* 8-bit address (7-bit shifted << 1) */
 	uint8_t read_size; /* Either 8 or 16. */
 	uint8_t port;
 	uint8_t offset;
@@ -1165,7 +1815,7 @@ struct ec_response_i2c_read {
 
 struct ec_params_i2c_write {
 	uint16_t data;
-	uint16_t addr;
+	uint16_t addr; /* 8-bit address (7-bit shifted << 1) */
 	uint8_t write_size; /* Either 8 or 16. */
 	uint8_t port;
 	uint8_t offset;
@@ -1174,11 +1824,20 @@ struct ec_params_i2c_write {
 /*****************************************************************************/
 /* Charge state commands. Only available when flash write protect unlocked. */
 
-/* Force charge state machine to stop in idle mode */
-#define EC_CMD_CHARGE_FORCE_IDLE 0x96
+/* Force charge state machine to stop charging the battery or force it to
+ * discharge the battery.
+ */
+#define EC_CMD_CHARGE_CONTROL 0x96
+#define EC_VER_CHARGE_CONTROL 1
 
-struct ec_params_force_idle {
-	uint8_t enabled;
+enum ec_charge_control_mode {
+	CHARGE_CONTROL_NORMAL = 0,
+	CHARGE_CONTROL_IDLE,
+	CHARGE_CONTROL_DISCHARGE,
+};
+
+struct ec_params_charge_control {
+	uint32_t mode;  /* enum charge_control_mode */
 } __packed;
 
 /*****************************************************************************/
@@ -1206,14 +1865,231 @@ struct ec_params_force_idle {
 #define EC_CMD_BATTERY_CUT_OFF 0x99
 
 /*****************************************************************************/
-/* Temporary debug commands. TODO: remove this crosbug.com/p/13849 */
+/* USB port mux control. */
 
 /*
- * Dump charge state machine context.
- *
- * Response is a binary dump of charge state machine context.
+ * Switch USB mux or return to automatic switching.
+ */
+#define EC_CMD_USB_MUX 0x9a
+
+struct ec_params_usb_mux {
+	uint8_t mux;
+} __packed;
+
+/*****************************************************************************/
+/* LDOs / FETs control. */
+
+enum ec_ldo_state {
+	EC_LDO_STATE_OFF = 0,	/* the LDO / FET is shut down */
+	EC_LDO_STATE_ON = 1,	/* the LDO / FET is ON / providing power */
+};
+
+/*
+ * Switch on/off a LDO.
+ */
+#define EC_CMD_LDO_SET 0x9b
+
+struct ec_params_ldo_set {
+	uint8_t index;
+	uint8_t state;
+} __packed;
+
+/*
+ * Get LDO state.
+ */
+#define EC_CMD_LDO_GET 0x9c
+
+struct ec_params_ldo_get {
+	uint8_t index;
+} __packed;
+
+struct ec_response_ldo_get {
+	uint8_t state;
+} __packed;
+
+/*****************************************************************************/
+/* Power info. */
+
+/*
+ * Get power info.
+ */
+#define EC_CMD_POWER_INFO 0x9d
+
+struct ec_response_power_info {
+	uint32_t usb_dev_type;
+	uint16_t voltage_ac;
+	uint16_t voltage_system;
+	uint16_t current_system;
+	uint16_t usb_current_limit;
+} __packed;
+
+/*****************************************************************************/
+/* I2C passthru command */
+
+#define EC_CMD_I2C_PASSTHRU 0x9e
+
+/* Slave address is 10 (not 7) bit */
+#define EC_I2C_FLAG_10BIT	(1 << 16)
+
+/* Read data; if not present, message is a write */
+#define EC_I2C_FLAG_READ	(1 << 15)
+
+/* Mask for address */
+#define EC_I2C_ADDR_MASK	0x3ff
+
+#define EC_I2C_STATUS_NAK	(1 << 0) /* Transfer was not acknowledged */
+#define EC_I2C_STATUS_TIMEOUT	(1 << 1) /* Timeout during transfer */
+
+/* Any error */
+#define EC_I2C_STATUS_ERROR	(EC_I2C_STATUS_NAK | EC_I2C_STATUS_TIMEOUT)
+
+struct ec_params_i2c_passthru_msg {
+	uint16_t addr_flags;	/* I2C slave address (7 or 10 bits) and flags */
+	uint16_t len;		/* Number of bytes to read or write */
+} __packed;
+
+struct ec_params_i2c_passthru {
+	uint8_t port;		/* I2C port number */
+	uint8_t num_msgs;	/* Number of messages */
+	struct ec_params_i2c_passthru_msg msg[];
+	/* Data to write for all messages is concatenated here */
+} __packed;
+
+struct ec_response_i2c_passthru {
+	uint8_t i2c_status;	/* Status flags (EC_I2C_STATUS_...) */
+	uint8_t num_msgs;	/* Number of messages processed */
+	uint8_t data[];		/* Data read by messages concatenated here */
+} __packed;
+
+/*****************************************************************************/
+/* Power button hang detect */
+
+#define EC_CMD_HANG_DETECT 0x9f
+
+/* Reasons to start hang detection timer */
+/* Power button pressed */
+#define EC_HANG_START_ON_POWER_PRESS  (1 << 0)
+
+/* Lid closed */
+#define EC_HANG_START_ON_LID_CLOSE    (1 << 1)
+
+ /* Lid opened */
+#define EC_HANG_START_ON_LID_OPEN     (1 << 2)
+
+/* Start of AP S3->S0 transition (booting or resuming from suspend) */
+#define EC_HANG_START_ON_RESUME       (1 << 3)
+
+/* Reasons to cancel hang detection */
+
+/* Power button released */
+#define EC_HANG_STOP_ON_POWER_RELEASE (1 << 8)
+
+/* Any host command from AP received */
+#define EC_HANG_STOP_ON_HOST_COMMAND  (1 << 9)
+
+/* Stop on end of AP S0->S3 transition (suspending or shutting down) */
+#define EC_HANG_STOP_ON_SUSPEND       (1 << 10)
+
+/*
+ * If this flag is set, all the other fields are ignored, and the hang detect
+ * timer is started.  This provides the AP a way to start the hang timer
+ * without reconfiguring any of the other hang detect settings.  Note that
+ * you must previously have configured the timeouts.
+ */
+#define EC_HANG_START_NOW             (1 << 30)
+
+/*
+ * If this flag is set, all the other fields are ignored (including
+ * EC_HANG_START_NOW).  This provides the AP a way to stop the hang timer
+ * without reconfiguring any of the other hang detect settings.
  */
-#define EC_CMD_CHARGE_DUMP 0xa0
+#define EC_HANG_STOP_NOW              (1 << 31)
+
+struct ec_params_hang_detect {
+	/* Flags; see EC_HANG_* */
+	uint32_t flags;
+
+	/* Timeout in msec before generating host event, if enabled */
+	uint16_t host_event_timeout_msec;
+
+	/* Timeout in msec before generating warm reboot, if enabled */
+	uint16_t warm_reboot_timeout_msec;
+} __packed;
+
+/*****************************************************************************/
+/* Commands for battery charging */
+
+/*
+ * This is the single catch-all host command to exchange data regarding the
+ * charge state machine (v2 and up).
+ */
+#define EC_CMD_CHARGE_STATE 0xa0
+
+/* Subcommands for this host command */
+enum charge_state_command {
+	CHARGE_STATE_CMD_GET_STATE,
+	CHARGE_STATE_CMD_GET_PARAM,
+	CHARGE_STATE_CMD_SET_PARAM,
+	CHARGE_STATE_NUM_CMDS
+};
+
+/*
+ * Known param numbers are defined here. Ranges are reserved for board-specific
+ * params, which are handled by the particular implementations.
+ */
+enum charge_state_params {
+	CS_PARAM_CHG_VOLTAGE,	      /* charger voltage limit */
+	CS_PARAM_CHG_CURRENT,	      /* charger current limit */
+	CS_PARAM_CHG_INPUT_CURRENT,   /* charger input current limit */
+	CS_PARAM_CHG_STATUS,	      /* charger-specific status */
+	CS_PARAM_CHG_OPTION,	      /* charger-specific options */
+	/* How many so far? */
+	CS_NUM_BASE_PARAMS,
+
+	/* Range for CONFIG_CHARGER_PROFILE_OVERRIDE params */
+	CS_PARAM_CUSTOM_PROFILE_MIN = 0x10000,
+	CS_PARAM_CUSTOM_PROFILE_MAX = 0x1ffff,
+
+	/* Other custom param ranges go here... */
+};
+
+struct ec_params_charge_state {
+	uint8_t cmd;				/* enum charge_state_command */
+	union {
+		struct {
+			/* no args */
+		} get_state;
+
+		struct {
+			uint32_t param;		/* enum charge_state_param */
+		} get_param;
+
+		struct {
+			uint32_t param;		/* param to set */
+			uint32_t value;		/* value to set */
+		} set_param;
+	};
+} __packed;
+
+struct ec_response_charge_state {
+	union {
+		struct {
+			int ac;
+			int chg_voltage;
+			int chg_current;
+			int chg_input_current;
+			int batt_state_of_charge;
+		} get_state;
+
+		struct {
+			uint32_t value;
+		} get_param;
+		struct {
+			/* no return values */
+		} set_param;
+	};
+} __packed;
+
 
 /*
  * Set maximum battery charging current.
@@ -1221,15 +2097,59 @@ struct ec_params_force_idle {
 #define EC_CMD_CHARGE_CURRENT_LIMIT 0xa1
 
 struct ec_params_current_limit {
-	uint32_t limit;
+	uint32_t limit; /* in mA */
+} __packed;
+
+/*
+ * Set maximum external power current.
+ */
+#define EC_CMD_EXT_POWER_CURRENT_LIMIT 0xa2
+
+struct ec_params_ext_power_current_limit {
+	uint32_t limit; /* in mA */
+} __packed;
+
+/*****************************************************************************/
+/* Smart battery pass-through */
+
+/* Get / Set 16-bit smart battery registers */
+#define EC_CMD_SB_READ_WORD   0xb0
+#define EC_CMD_SB_WRITE_WORD  0xb1
+
+/* Get / Set string smart battery parameters
+ * formatted as SMBUS "block".
+ */
+#define EC_CMD_SB_READ_BLOCK  0xb2
+#define EC_CMD_SB_WRITE_BLOCK 0xb3
+
+struct ec_params_sb_rd {
+	uint8_t reg;
+} __packed;
+
+struct ec_response_sb_rd_word {
+	uint16_t value;
+} __packed;
+
+struct ec_params_sb_wr_word {
+	uint8_t reg;
+	uint16_t value;
+} __packed;
+
+struct ec_response_sb_rd_block {
+	uint8_t data[32];
+} __packed;
+
+struct ec_params_sb_wr_block {
+	uint8_t reg;
+	uint16_t data[32];
 } __packed;
 
 /*****************************************************************************/
 /* System commands */
 
 /*
- * TODO: this is a confusing name, since it doesn't necessarily reboot the EC.
- * Rename to "set image" or something similar.
+ * TODO(crosbug.com/p/23747): This is a confusing name, since it doesn't
+ * necessarily reboot the EC.  Rename to "image" or something similar?
  */
 #define EC_CMD_REBOOT_EC 0xd2
 
@@ -1308,6 +2228,7 @@ struct ec_params_reboot_ec {
 #define EC_CMD_ACPI_QUERY_EVENT 0x84
 
 /* Valid addresses in ACPI memory space, for read/write commands */
+
 /* Memory space version; set to EC_ACPI_MEM_VERSION_CURRENT */
 #define EC_ACPI_MEM_VERSION            0x00
 /*
@@ -1317,8 +2238,60 @@ struct ec_params_reboot_ec {
 #define EC_ACPI_MEM_TEST               0x01
 /* Test compliment; writes here are ignored. */
 #define EC_ACPI_MEM_TEST_COMPLIMENT    0x02
+
 /* Keyboard backlight brightness percent (0 - 100) */
 #define EC_ACPI_MEM_KEYBOARD_BACKLIGHT 0x03
+/* DPTF Target Fan Duty (0-100, 0xff for auto/none) */
+#define EC_ACPI_MEM_FAN_DUTY           0x04
+
+/*
+ * DPTF temp thresholds. Any of the EC's temp sensors can have up to two
+ * independent thresholds attached to them. The current value of the ID
+ * register determines which sensor is affected by the THRESHOLD and COMMIT
+ * registers. The THRESHOLD register uses the same EC_TEMP_SENSOR_OFFSET scheme
+ * as the memory-mapped sensors. The COMMIT register applies those settings.
+ *
+ * The spec does not mandate any way to read back the threshold settings
+ * themselves, but when a threshold is crossed the AP needs a way to determine
+ * which sensor(s) are responsible. Each reading of the ID register clears and
+ * returns one sensor ID that has crossed one of its threshold (in either
+ * direction) since the last read. A value of 0xFF means "no new thresholds
+ * have tripped". Setting or enabling the thresholds for a sensor will clear
+ * the unread event count for that sensor.
+ */
+#define EC_ACPI_MEM_TEMP_ID            0x05
+#define EC_ACPI_MEM_TEMP_THRESHOLD     0x06
+#define EC_ACPI_MEM_TEMP_COMMIT        0x07
+/*
+ * Here are the bits for the COMMIT register:
+ *   bit 0 selects the threshold index for the chosen sensor (0/1)
+ *   bit 1 enables/disables the selected threshold (0 = off, 1 = on)
+ * Each write to the commit register affects one threshold.
+ */
+#define EC_ACPI_MEM_TEMP_COMMIT_SELECT_MASK (1 << 0)
+#define EC_ACPI_MEM_TEMP_COMMIT_ENABLE_MASK (1 << 1)
+/*
+ * Example:
+ *
+ * Set the thresholds for sensor 2 to 50 C and 60 C:
+ *   write 2 to [0x05]      --  select temp sensor 2
+ *   write 0x7b to [0x06]   --  C_TO_K(50) - EC_TEMP_SENSOR_OFFSET
+ *   write 0x2 to [0x07]    --  enable threshold 0 with this value
+ *   write 0x85 to [0x06]   --  C_TO_K(60) - EC_TEMP_SENSOR_OFFSET
+ *   write 0x3 to [0x07]    --  enable threshold 1 with this value
+ *
+ * Disable the 60 C threshold, leaving the 50 C threshold unchanged:
+ *   write 2 to [0x05]      --  select temp sensor 2
+ *   write 0x1 to [0x07]    --  disable threshold 1
+ */
+
+/* DPTF battery charging current limit */
+#define EC_ACPI_MEM_CHARGING_LIMIT     0x08
+
+/* Charging limit is specified in 64 mA steps */
+#define EC_ACPI_MEM_CHARGING_LIMIT_STEP_MA   64
+/* Value to disable DPTF battery charging limit */
+#define EC_ACPI_MEM_CHARGING_LIMIT_DISABLED  0xff
 
 /* Current version of ACPI memory address space */
 #define EC_ACPI_MEM_VERSION_CURRENT 1
@@ -1360,10 +2333,21 @@ struct ec_params_reboot_ec {
  * Header bytes greater than this indicate a later version. For example,
  * EC_CMD_VERSION0 + 1 means we are using version 1.
  *
- * The old EC interface must not use commands 0dc or higher.
+ * The old EC interface must not use commands 0xdc or higher.
  */
 #define EC_CMD_VERSION0 0xdc
 
 #endif  /* !__ACPI__ */
 
+/*****************************************************************************/
+/*
+ * Deprecated constants. These constants have been renamed for clarity. The
+ * meaning and size has not changed. Programs that use the old names should
+ * switch to the new names soon, as the old names may not be carried forward
+ * forever.
+ */
+#define EC_HOST_PARAM_SIZE      EC_PROTO2_MAX_PARAM_SIZE
+#define EC_LPC_ADDR_OLD_PARAM   EC_HOST_CMD_REGION1
+#define EC_OLD_PARAM_SIZE       EC_HOST_CMD_REGION_SIZE
+
 #endif  /* __CROS_EC_COMMANDS_H */
-- 
1.9.1.423.g4596e3a

[PATCH v2 6/7] i2c: ChromeOS EC tunnel driver

[Doug Anderson]

On ARM Chromebooks we have a few devices that are accessed by both the
AP (the main "Application Processor") and the EC (the Embedded
Controller).  These are:
* The battery (sbs-battery).
* The power management unit tps65090.

On the original Samsung ARM Chromebook these devices were on an I2C
bus that was shared between the AP and the EC and arbitrated using
some extranal GPIOs (see i2c-arb-gpio-challenge).

The original arbitration scheme worked well enough but had some
downsides:
* It was nonstandard (not using standard I2C multimaster)
* It only worked if the EC-AP communication was I2C
* It was relatively hard to debug problems (hard to tell if i2c issues
  were caused by the EC, the AP, or some device on the bus).

On the HP Chromebook 11 the design was changed to:
* The AP/EC comms were still i2c, but the battery/tps65090 were no
  longer on the bus used for AP/EC communication.  The battery was
  exposed to the AP through a limited i2c tunnel and tps65090 was
  exposed to the AP through a custom Linux driver.

On the Samsung ARM Chromebook 2 the scheme is changed yet again, now:
* The AP/EC comms are now using SPI for faster speeds.
* The EC's i2c bus is exposed to the AP through a full i2c tunnel.

The upstream "tegra124-venice2" uses the same scheme as the Samsung
ARM Chromebook 2, though it has a different set of components on the
other side of the bus.

This driver supports the scheme used by the Samsung ARM Chromebook 2.
Future patches to this driver could add support for the battery tunnel
on the HP Chromebook 11 (and perhaps could even be used to access
tps65090 on the HP Chromebook 11 instead of using a special driver,
but I haven't researched that enough).

Signed-off-by: Vincent Palatin <vpalatin-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Signed-off-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Signed-off-by: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Tested-by: Andrew Bresticker <abrestic-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Tested-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
Changes in v2:
- Update tunnel binding as per swarren

 .../devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt |  39 +++
 drivers/i2c/busses/Kconfig                         |   9 +
 drivers/i2c/busses/Makefile                        |   1 +
 drivers/i2c/busses/i2c-cros-ec-tunnel.c            | 304 +++++++++++++++++++++
 drivers/mfd/cros_ec.c                              |   5 +
 5 files changed, 358 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt
 create mode 100644 drivers/i2c/busses/i2c-cros-ec-tunnel.c

diff --git a/Documentation/devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt b/Documentation/devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt
new file mode 100644
index 0000000..898f030
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt
@@ -0,0 +1,39 @@
+I2C bus that tunnels through the ChromeOS EC (cros-ec)
+======================================================
+On some ChromeOS board designs we've got a connection to the EC (embedded
+controller) but no direct connection to some devices on the other side of
+the EC (like a battery and PMIC).  To get access to those devices we need
+to tunnel our i2c commands through the EC.
+
+The node for this device should be under a cros-ec node like google,cros-ec-spi
+or google,cros-ec-i2c.
+
+
+Required properties:
+- compatible: google,cros-ec-i2c-tunnel
+- google,remote-bus: The EC bus we'd like to talk to.
+
+Optional child nodes:
+- One node per I2C device connected to the tunnelled I2C bus.
+
+
+Example:
+	cros-ec@0 {
+		compatible = "google,cros-ec-spi";
+
+		...
+
+		i2c-tunnel {
+			compatible = "google,cros-ec-i2c-tunnel";
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			google,remote-bus = <0>;
+
+			battery: sbs-battery@b {
+				compatible = "sbs,sbs-battery";
+				reg = <0xb>;
+				sbs,poll-retry-count = <1>;
+			};
+		};
+	}
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index c94db1c..9a0a6cc 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -993,6 +993,15 @@ config I2C_SIBYTE
 	help
 	  Supports the SiByte SOC on-chip I2C interfaces (2 channels).
 
+config I2C_CROS_EC_TUNNEL
+	tristate "ChromeOS EC tunnel I2C bus"
+	depends on MFD_CROS_EC
+	help
+	  If you say yes here you get an I2C bus that will tunnel i2c commands
+	  through to the other side of the ChromeOS EC to the i2c bus
+	  connected there. This will work whatever the interface used to
+	  talk to the EC (SPI, I2C or LPC).
+
 config SCx200_I2C
 	tristate "NatSemi SCx200 I2C using GPIO pins (DEPRECATED)"
 	depends on SCx200_GPIO
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 18d18ff..e110ca9 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -95,6 +95,7 @@ obj-$(CONFIG_I2C_VIPERBOARD)	+= i2c-viperboard.o
 # Other I2C/SMBus bus drivers
 obj-$(CONFIG_I2C_ACORN)		+= i2c-acorn.o
 obj-$(CONFIG_I2C_BCM_KONA)	+= i2c-bcm-kona.o
+obj-$(CONFIG_I2C_CROS_EC_TUNNEL)	+= i2c-cros-ec-tunnel.o
 obj-$(CONFIG_I2C_ELEKTOR)	+= i2c-elektor.o
 obj-$(CONFIG_I2C_PCA_ISA)	+= i2c-pca-isa.o
 obj-$(CONFIG_I2C_SIBYTE)	+= i2c-sibyte.o
diff --git a/drivers/i2c/busses/i2c-cros-ec-tunnel.c b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
new file mode 100644
index 0000000..7e53fcb
--- /dev/null
+++ b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
@@ -0,0 +1,304 @@
+/*
+ *  Copyright (C) 2013 Google, Inc
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ * Expose an I2C passthrough to the ChromeOS EC.
+ */
+
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/mfd/cros_ec.h>
+#include <linux/mfd/cros_ec_commands.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+/**
+ * struct ec_i2c_device - Driver data for I2C tunnel
+ *
+ * @dev: Device node
+ * @adap: I2C adapter
+ * @ec: Pointer to EC device
+ * @remote_bus: The EC bus number we tunnel to on the other side.
+ * @request_buf: Buffer for transmitting data; we expect most transfers to fit.
+ * @response_buf: Buffer for receiving data; we expect most transfers to fit.
+ */
+
+struct ec_i2c_device {
+	struct device *dev;
+	struct i2c_adapter adap;
+	struct cros_ec_device *ec;
+
+	u16 remote_bus;
+
+	u8 request_buf[256];
+	u8 response_buf[256];
+};
+
+/**
+ * ec_i2c_construct_message - construct a message to go to the EC
+ *
+ * This function effectively stuffs the standard i2c_msg format of Linux into
+ * a format that the EC understands.
+ *
+ * @buf: The buffer to fill.  Can pass NULL to count how many bytes the message
+ *       would be.
+ * @i2c_msgs: The i2c messages to read.
+ * @num: The number of i2c messages.
+ * @bus_num: The remote bus number we want to talk to.
+ *
+ * Returns the number of bytes that the message would take up or a negative
+ * error number.
+ */
+static int ec_i2c_construct_message(u8 *buf, const struct i2c_msg i2c_msgs[],
+				    int num, u16 bus_num)
+{
+	struct ec_params_i2c_passthru *params;
+	u8 *out_data;
+	int i;
+	int size;
+
+	size = sizeof(struct ec_params_i2c_passthru);
+	size += num * sizeof(struct ec_params_i2c_passthru_msg);
+	out_data = buf + size;
+	for (i = 0; i < num; i++)
+		if (!(i2c_msgs[i].flags & I2C_M_RD))
+			size += i2c_msgs[i].len;
+
+	/* If there is no buffer, we can't build the message */
+	if (!buf)
+		return size;
+
+	params = (struct ec_params_i2c_passthru *)buf;
+	params->port = bus_num;
+	params->num_msgs = num;
+	for (i = 0; i < num; i++) {
+		const struct i2c_msg *i2c_msg = &i2c_msgs[i];
+		struct ec_params_i2c_passthru_msg *msg = &params->msg[i];
+
+		msg->len = i2c_msg->len;
+		msg->addr_flags = i2c_msg->addr;
+
+		if (i2c_msg->flags & I2C_M_TEN)
+			msg->addr_flags |= EC_I2C_FLAG_10BIT;
+
+		if (i2c_msg->flags & I2C_M_RD) {
+			msg->addr_flags |= EC_I2C_FLAG_READ;
+		} else {
+			memcpy(out_data, i2c_msg->buf, msg->len);
+			out_data += msg->len;
+		}
+	}
+
+	return size;
+}
+
+/**
+ * ec_i2c_parse_response - Parse a response from the EC
+ *
+ * We'll take the EC's response and copy it back into msgs.
+ *
+ * @buf: The buffer to parse.  Can pass NULL to count how many bytes we expect
+ *	 the response to be. Otherwise we assume that the right number of
+ *	 bytes are available.
+ * @i2c_msgs: The i2c messages to to fill up.
+ * @num: The number of i2c messages; will be modified to include the actual
+ *	 number received.
+ *
+ * Returns the number of response bytes or a negative error number.
+ */
+static int ec_i2c_parse_response(const u8 *buf, struct i2c_msg i2c_msgs[],
+				 int *num)
+{
+	const struct ec_response_i2c_passthru *resp;
+	const u8 *in_data;
+	int size;
+	int i;
+
+	size = sizeof(struct ec_response_i2c_passthru);
+	in_data = buf + size;
+	for (i = 0; i < *num; i++)
+		if (i2c_msgs[i].flags & I2C_M_RD)
+			size += i2c_msgs[i].len;
+
+	if (buf == NULL)
+		return size;
+
+	resp = (const struct ec_response_i2c_passthru *)buf;
+	if (resp->i2c_status & EC_I2C_STATUS_TIMEOUT)
+		return -ETIMEDOUT;
+	else if (resp->i2c_status & EC_I2C_STATUS_ERROR)
+		return -EREMOTEIO;
+
+	/* Other side could send us back fewer messages, but not more */
+	if (resp->num_msgs > *num)
+		return -EPROTO;
+	*num = resp->num_msgs;
+
+	for (i = 0; i < *num; i++) {
+		struct i2c_msg *i2c_msg = &i2c_msgs[i];
+
+		if (i2c_msgs[i].flags & I2C_M_RD) {
+			memcpy(i2c_msg->buf, in_data, i2c_msg->len);
+			in_data += i2c_msg->len;
+		}
+	}
+
+	return size;
+}
+
+static int ec_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg i2c_msgs[],
+		       int num)
+{
+	struct ec_i2c_device *bus = adap->algo_data;
+	struct device *dev = bus->dev;
+	const u16 bus_num = bus->remote_bus;
+	int request_len;
+	int response_len;
+	u8 *request = NULL;
+	u8 *response = NULL;
+	int result;
+
+	request_len = ec_i2c_construct_message(NULL, i2c_msgs, num, bus_num);
+	if (request_len < 0) {
+		dev_warn(dev, "Error constructing message %d\n", request_len);
+		result = request_len;
+		goto exit;
+	}
+	response_len = ec_i2c_parse_response(NULL, i2c_msgs, &num);
+	if (response_len < 0) {
+		/* Unexpected; no errors should come when NULL response */
+		dev_warn(dev, "Error preparing response %d\n", response_len);
+		result = response_len;
+		goto exit;
+	}
+
+	if (request_len <= ARRAY_SIZE(bus->request_buf)) {
+		request = bus->request_buf;
+	} else {
+		request = kzalloc(request_len, GFP_KERNEL);
+		if (request == NULL) {
+			result = -ENOMEM;
+			goto exit;
+		}
+	}
+	if (response_len <= ARRAY_SIZE(bus->response_buf)) {
+		response = bus->response_buf;
+	} else {
+		response = kzalloc(response_len, GFP_KERNEL);
+		if (response == NULL) {
+			result = -ENOMEM;
+			goto exit;
+		}
+	}
+
+	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);
+	if (result)
+		goto exit;
+
+	result = ec_i2c_parse_response(response, i2c_msgs, &num);
+	if (result < 0)
+		goto exit;
+
+	/* Indicate success by saying how many messages were sent */
+	result = num;
+exit:
+	if (request != bus->request_buf)
+		kfree(request);
+	if (response != bus->response_buf)
+		kfree(response);
+
+	return result;
+}
+
+static u32 ec_i2c_functionality(struct i2c_adapter *adap)
+{
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+}
+
+static const struct i2c_algorithm ec_i2c_algorithm = {
+	.master_xfer	= ec_i2c_xfer,
+	.functionality	= ec_i2c_functionality,
+};
+
+static int ec_i2c_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct cros_ec_device *ec = dev_get_drvdata(pdev->dev.parent);
+	struct device *dev = &pdev->dev;
+	struct ec_i2c_device *bus = NULL;
+	u32 remote_bus;
+	int err;
+
+	dev_dbg(dev, "Probing\n");
+
+	if (!np) {
+		dev_err(dev, "no device node\n");
+		return -ENOENT;
+	}
+
+	bus = devm_kzalloc(dev, sizeof(*bus), GFP_KERNEL);
+	if (bus == NULL) {
+		dev_err(dev, "cannot allocate bus device\n");
+		return -ENOMEM;
+	}
+
+	err = of_property_read_u32(np, "google,remote-bus", &remote_bus);
+	if (err) {
+		dev_err(dev, "Couldn't read remote-bus property\n");
+		return err;
+	}
+	bus->remote_bus = remote_bus;
+
+	bus->ec = ec;
+	bus->dev = dev;
+
+	bus->adap.owner = THIS_MODULE;
+	strlcpy(bus->adap.name, "cros-ec-i2c-tunnel", sizeof(bus->adap.name));
+	bus->adap.algo = &ec_i2c_algorithm;
+	bus->adap.algo_data = bus;
+	bus->adap.dev.parent = &pdev->dev;
+	bus->adap.dev.of_node = np;
+
+	err = i2c_add_adapter(&bus->adap);
+	if (err) {
+		dev_err(dev, "cannot register i2c adapter\n");
+		return err;
+	}
+	platform_set_drvdata(pdev, bus);
+
+	dev_dbg(dev, "ChromeOS EC I2C tunnel adapter\n");
+
+	return err;
+}
+
+static int ec_i2c_remove(struct platform_device *dev)
+{
+	struct ec_i2c_device *bus = platform_get_drvdata(dev);
+
+	platform_set_drvdata(dev, NULL);
+
+	i2c_del_adapter(&bus->adap);
+
+	return 0;
+}
+
+static struct platform_driver ec_i2c_tunnel_driver = {
+	.probe = ec_i2c_probe,
+	.remove = ec_i2c_remove,
+	.driver = {
+		.name = "cros-ec-i2c-tunnel",
+	},
+};
+
+module_platform_driver(ec_i2c_tunnel_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("EC I2C tunnel driver");
+MODULE_ALIAS("platform:cros-ec-i2c-tunnel");
diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index c58ab96..61bc909 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -90,6 +90,11 @@ static const struct mfd_cell cros_devs[] = {
 		.id = 1,
 		.of_compatible = "google,cros-ec-keyb",
 	},
+	{
+		.name = "cros-ec-i2c-tunnel",
+		.id = 2,
+		.of_compatible = "google,cros-ec-i2c-tunnel",
+	},
 };
 
 int cros_ec_register(struct cros_ec_device *ec_dev)
-- 
1.9.1.423.g4596e3a

[PATCH v2 6/7] i2c: ChromeOS EC tunnel driver

[Doug Anderson]

On ARM Chromebooks we have a few devices that are accessed by both the
AP (the main "Application Processor") and the EC (the Embedded
Controller).  These are:
* The battery (sbs-battery).
* The power management unit tps65090.

On the original Samsung ARM Chromebook these devices were on an I2C
bus that was shared between the AP and the EC and arbitrated using
some extranal GPIOs (see i2c-arb-gpio-challenge).

The original arbitration scheme worked well enough but had some
downsides:
* It was nonstandard (not using standard I2C multimaster)
* It only worked if the EC-AP communication was I2C
* It was relatively hard to debug problems (hard to tell if i2c issues
  were caused by the EC, the AP, or some device on the bus).

On the HP Chromebook 11 the design was changed to:
* The AP/EC comms were still i2c, but the battery/tps65090 were no
  longer on the bus used for AP/EC communication.  The battery was
  exposed to the AP through a limited i2c tunnel and tps65090 was
  exposed to the AP through a custom Linux driver.

On the Samsung ARM Chromebook 2 the scheme is changed yet again, now:
* The AP/EC comms are now using SPI for faster speeds.
* The EC's i2c bus is exposed to the AP through a full i2c tunnel.

The upstream "tegra124-venice2" uses the same scheme as the Samsung
ARM Chromebook 2, though it has a different set of components on the
other side of the bus.

This driver supports the scheme used by the Samsung ARM Chromebook 2.
Future patches to this driver could add support for the battery tunnel
on the HP Chromebook 11 (and perhaps could even be used to access
tps65090 on the HP Chromebook 11 instead of using a special driver,
but I haven't researched that enough).

Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
Signed-off-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Doug Anderson <dianders@chromium.org>
Tested-by: Andrew Bresticker <abrestic@chromium.org>
Tested-by: Stephen Warren <swarren@nvidia.com>
---
Changes in v2:
- Update tunnel binding as per swarren

 .../devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt |  39 +++
 drivers/i2c/busses/Kconfig                         |   9 +
 drivers/i2c/busses/Makefile                        |   1 +
 drivers/i2c/busses/i2c-cros-ec-tunnel.c            | 304 +++++++++++++++++++++
 drivers/mfd/cros_ec.c                              |   5 +
 5 files changed, 358 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt
 create mode 100644 drivers/i2c/busses/i2c-cros-ec-tunnel.c

diff --git a/Documentation/devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt b/Documentation/devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt
new file mode 100644
index 0000000..898f030
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt
@@ -0,0 +1,39 @@
+I2C bus that tunnels through the ChromeOS EC (cros-ec)
+======================================================
+On some ChromeOS board designs we've got a connection to the EC (embedded
+controller) but no direct connection to some devices on the other side of
+the EC (like a battery and PMIC).  To get access to those devices we need
+to tunnel our i2c commands through the EC.
+
+The node for this device should be under a cros-ec node like google,cros-ec-spi
+or google,cros-ec-i2c.
+
+
+Required properties:
+- compatible: google,cros-ec-i2c-tunnel
+- google,remote-bus: The EC bus we'd like to talk to.
+
+Optional child nodes:
+- One node per I2C device connected to the tunnelled I2C bus.
+
+
+Example:
+	cros-ec@0 {
+		compatible = "google,cros-ec-spi";
+
+		...
+
+		i2c-tunnel {
+			compatible = "google,cros-ec-i2c-tunnel";
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			google,remote-bus = <0>;
+
+			battery: sbs-battery@b {
+				compatible = "sbs,sbs-battery";
+				reg = <0xb>;
+				sbs,poll-retry-count = <1>;
+			};
+		};
+	}
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index c94db1c..9a0a6cc 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -993,6 +993,15 @@ config I2C_SIBYTE
 	help
 	  Supports the SiByte SOC on-chip I2C interfaces (2 channels).
 
+config I2C_CROS_EC_TUNNEL
+	tristate "ChromeOS EC tunnel I2C bus"
+	depends on MFD_CROS_EC
+	help
+	  If you say yes here you get an I2C bus that will tunnel i2c commands
+	  through to the other side of the ChromeOS EC to the i2c bus
+	  connected there. This will work whatever the interface used to
+	  talk to the EC (SPI, I2C or LPC).
+
 config SCx200_I2C
 	tristate "NatSemi SCx200 I2C using GPIO pins (DEPRECATED)"
 	depends on SCx200_GPIO
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 18d18ff..e110ca9 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -95,6 +95,7 @@ obj-$(CONFIG_I2C_VIPERBOARD)	+= i2c-viperboard.o
 # Other I2C/SMBus bus drivers
 obj-$(CONFIG_I2C_ACORN)		+= i2c-acorn.o
 obj-$(CONFIG_I2C_BCM_KONA)	+= i2c-bcm-kona.o
+obj-$(CONFIG_I2C_CROS_EC_TUNNEL)	+= i2c-cros-ec-tunnel.o
 obj-$(CONFIG_I2C_ELEKTOR)	+= i2c-elektor.o
 obj-$(CONFIG_I2C_PCA_ISA)	+= i2c-pca-isa.o
 obj-$(CONFIG_I2C_SIBYTE)	+= i2c-sibyte.o
diff --git a/drivers/i2c/busses/i2c-cros-ec-tunnel.c b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
new file mode 100644
index 0000000..7e53fcb
--- /dev/null
+++ b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
@@ -0,0 +1,304 @@
+/*
+ *  Copyright (C) 2013 Google, Inc
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ * Expose an I2C passthrough to the ChromeOS EC.
+ */
+
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/mfd/cros_ec.h>
+#include <linux/mfd/cros_ec_commands.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+/**
+ * struct ec_i2c_device - Driver data for I2C tunnel
+ *
+ * @dev: Device node
+ * @adap: I2C adapter
+ * @ec: Pointer to EC device
+ * @remote_bus: The EC bus number we tunnel to on the other side.
+ * @request_buf: Buffer for transmitting data; we expect most transfers to fit.
+ * @response_buf: Buffer for receiving data; we expect most transfers to fit.
+ */
+
+struct ec_i2c_device {
+	struct device *dev;
+	struct i2c_adapter adap;
+	struct cros_ec_device *ec;
+
+	u16 remote_bus;
+
+	u8 request_buf[256];
+	u8 response_buf[256];
+};
+
+/**
+ * ec_i2c_construct_message - construct a message to go to the EC
+ *
+ * This function effectively stuffs the standard i2c_msg format of Linux into
+ * a format that the EC understands.
+ *
+ * @buf: The buffer to fill.  Can pass NULL to count how many bytes the message
+ *       would be.
+ * @i2c_msgs: The i2c messages to read.
+ * @num: The number of i2c messages.
+ * @bus_num: The remote bus number we want to talk to.
+ *
+ * Returns the number of bytes that the message would take up or a negative
+ * error number.
+ */
+static int ec_i2c_construct_message(u8 *buf, const struct i2c_msg i2c_msgs[],
+				    int num, u16 bus_num)
+{
+	struct ec_params_i2c_passthru *params;
+	u8 *out_data;
+	int i;
+	int size;
+
+	size = sizeof(struct ec_params_i2c_passthru);
+	size += num * sizeof(struct ec_params_i2c_passthru_msg);
+	out_data = buf + size;
+	for (i = 0; i < num; i++)
+		if (!(i2c_msgs[i].flags & I2C_M_RD))
+			size += i2c_msgs[i].len;
+
+	/* If there is no buffer, we can't build the message */
+	if (!buf)
+		return size;
+
+	params = (struct ec_params_i2c_passthru *)buf;
+	params->port = bus_num;
+	params->num_msgs = num;
+	for (i = 0; i < num; i++) {
+		const struct i2c_msg *i2c_msg = &i2c_msgs[i];
+		struct ec_params_i2c_passthru_msg *msg = &params->msg[i];
+
+		msg->len = i2c_msg->len;
+		msg->addr_flags = i2c_msg->addr;
+
+		if (i2c_msg->flags & I2C_M_TEN)
+			msg->addr_flags |= EC_I2C_FLAG_10BIT;
+
+		if (i2c_msg->flags & I2C_M_RD) {
+			msg->addr_flags |= EC_I2C_FLAG_READ;
+		} else {
+			memcpy(out_data, i2c_msg->buf, msg->len);
+			out_data += msg->len;
+		}
+	}
+
+	return size;
+}
+
+/**
+ * ec_i2c_parse_response - Parse a response from the EC
+ *
+ * We'll take the EC's response and copy it back into msgs.
+ *
+ * @buf: The buffer to parse.  Can pass NULL to count how many bytes we expect
+ *	 the response to be. Otherwise we assume that the right number of
+ *	 bytes are available.
+ * @i2c_msgs: The i2c messages to to fill up.
+ * @num: The number of i2c messages; will be modified to include the actual
+ *	 number received.
+ *
+ * Returns the number of response bytes or a negative error number.
+ */
+static int ec_i2c_parse_response(const u8 *buf, struct i2c_msg i2c_msgs[],
+				 int *num)
+{
+	const struct ec_response_i2c_passthru *resp;
+	const u8 *in_data;
+	int size;
+	int i;
+
+	size = sizeof(struct ec_response_i2c_passthru);
+	in_data = buf + size;
+	for (i = 0; i < *num; i++)
+		if (i2c_msgs[i].flags & I2C_M_RD)
+			size += i2c_msgs[i].len;
+
+	if (buf == NULL)
+		return size;
+
+	resp = (const struct ec_response_i2c_passthru *)buf;
+	if (resp->i2c_status & EC_I2C_STATUS_TIMEOUT)
+		return -ETIMEDOUT;
+	else if (resp->i2c_status & EC_I2C_STATUS_ERROR)
+		return -EREMOTEIO;
+
+	/* Other side could send us back fewer messages, but not more */
+	if (resp->num_msgs > *num)
+		return -EPROTO;
+	*num = resp->num_msgs;
+
+	for (i = 0; i < *num; i++) {
+		struct i2c_msg *i2c_msg = &i2c_msgs[i];
+
+		if (i2c_msgs[i].flags & I2C_M_RD) {
+			memcpy(i2c_msg->buf, in_data, i2c_msg->len);
+			in_data += i2c_msg->len;
+		}
+	}
+
+	return size;
+}
+
+static int ec_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg i2c_msgs[],
+		       int num)
+{
+	struct ec_i2c_device *bus = adap->algo_data;
+	struct device *dev = bus->dev;
+	const u16 bus_num = bus->remote_bus;
+	int request_len;
+	int response_len;
+	u8 *request = NULL;
+	u8 *response = NULL;
+	int result;
+
+	request_len = ec_i2c_construct_message(NULL, i2c_msgs, num, bus_num);
+	if (request_len < 0) {
+		dev_warn(dev, "Error constructing message %d\n", request_len);
+		result = request_len;
+		goto exit;
+	}
+	response_len = ec_i2c_parse_response(NULL, i2c_msgs, &num);
+	if (response_len < 0) {
+		/* Unexpected; no errors should come when NULL response */
+		dev_warn(dev, "Error preparing response %d\n", response_len);
+		result = response_len;
+		goto exit;
+	}
+
+	if (request_len <= ARRAY_SIZE(bus->request_buf)) {
+		request = bus->request_buf;
+	} else {
+		request = kzalloc(request_len, GFP_KERNEL);
+		if (request == NULL) {
+			result = -ENOMEM;
+			goto exit;
+		}
+	}
+	if (response_len <= ARRAY_SIZE(bus->response_buf)) {
+		response = bus->response_buf;
+	} else {
+		response = kzalloc(response_len, GFP_KERNEL);
+		if (response == NULL) {
+			result = -ENOMEM;
+			goto exit;
+		}
+	}
+
+	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);
+	if (result)
+		goto exit;
+
+	result = ec_i2c_parse_response(response, i2c_msgs, &num);
+	if (result < 0)
+		goto exit;
+
+	/* Indicate success by saying how many messages were sent */
+	result = num;
+exit:
+	if (request != bus->request_buf)
+		kfree(request);
+	if (response != bus->response_buf)
+		kfree(response);
+
+	return result;
+}
+
+static u32 ec_i2c_functionality(struct i2c_adapter *adap)
+{
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+}
+
+static const struct i2c_algorithm ec_i2c_algorithm = {
+	.master_xfer	= ec_i2c_xfer,
+	.functionality	= ec_i2c_functionality,
+};
+
+static int ec_i2c_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct cros_ec_device *ec = dev_get_drvdata(pdev->dev.parent);
+	struct device *dev = &pdev->dev;
+	struct ec_i2c_device *bus = NULL;
+	u32 remote_bus;
+	int err;
+
+	dev_dbg(dev, "Probing\n");
+
+	if (!np) {
+		dev_err(dev, "no device node\n");
+		return -ENOENT;
+	}
+
+	bus = devm_kzalloc(dev, sizeof(*bus), GFP_KERNEL);
+	if (bus == NULL) {
+		dev_err(dev, "cannot allocate bus device\n");
+		return -ENOMEM;
+	}
+
+	err = of_property_read_u32(np, "google,remote-bus", &remote_bus);
+	if (err) {
+		dev_err(dev, "Couldn't read remote-bus property\n");
+		return err;
+	}
+	bus->remote_bus = remote_bus;
+
+	bus->ec = ec;
+	bus->dev = dev;
+
+	bus->adap.owner = THIS_MODULE;
+	strlcpy(bus->adap.name, "cros-ec-i2c-tunnel", sizeof(bus->adap.name));
+	bus->adap.algo = &ec_i2c_algorithm;
+	bus->adap.algo_data = bus;
+	bus->adap.dev.parent = &pdev->dev;
+	bus->adap.dev.of_node = np;
+
+	err = i2c_add_adapter(&bus->adap);
+	if (err) {
+		dev_err(dev, "cannot register i2c adapter\n");
+		return err;
+	}
+	platform_set_drvdata(pdev, bus);
+
+	dev_dbg(dev, "ChromeOS EC I2C tunnel adapter\n");
+
+	return err;
+}
+
+static int ec_i2c_remove(struct platform_device *dev)
+{
+	struct ec_i2c_device *bus = platform_get_drvdata(dev);
+
+	platform_set_drvdata(dev, NULL);
+
+	i2c_del_adapter(&bus->adap);
+
+	return 0;
+}
+
+static struct platform_driver ec_i2c_tunnel_driver = {
+	.probe = ec_i2c_probe,
+	.remove = ec_i2c_remove,
+	.driver = {
+		.name = "cros-ec-i2c-tunnel",
+	},
+};
+
+module_platform_driver(ec_i2c_tunnel_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("EC I2C tunnel driver");
+MODULE_ALIAS("platform:cros-ec-i2c-tunnel");
diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index c58ab96..61bc909 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -90,6 +90,11 @@ static const struct mfd_cell cros_devs[] = {
 		.id = 1,
 		.of_compatible = "google,cros-ec-keyb",
 	},
+	{
+		.name = "cros-ec-i2c-tunnel",
+		.id = 2,
+		.of_compatible = "google,cros-ec-i2c-tunnel",
+	},
 };
 
 int cros_ec_register(struct cros_ec_device *ec_dev)
-- 
1.9.1.423.g4596e3a

[PATCH v2 7/7] ARM: tegra: Add the EC i2c tunnel to tegra124-venice2

[Doug Anderson]

This adds the EC i2c tunnel (and devices under it) to the
tegra124-venice2 device tree.

Signed-off-by: Doug Anderson <dianders@chromium.org>
Tested-by: Andrew Bresticker <abrestic@chromium.org>
Tested-by: Stephen Warren <swarren@nvidia.com>
---
Changes in v2:
- Removed i2c20 alias for i2c tunnel

 arch/arm/boot/dts/tegra124-venice2.dts | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/arch/arm/boot/dts/tegra124-venice2.dts b/arch/arm/boot/dts/tegra124-venice2.dts
index c17283c..89cf776 100644
--- a/arch/arm/boot/dts/tegra124-venice2.dts
+++ b/arch/arm/boot/dts/tegra124-venice2.dts
@@ -813,6 +813,32 @@
 
 			google,cros-ec-spi-msg-delay = <2000>;
 
+			i2c-tunnel {
+				compatible = "google,cros-ec-i2c-tunnel";
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				google,remote-bus = <0>;
+
+				charger: bq24735@9 {
+					compatible = "ti,bq24735";
+					reg = <0x9>;
+					interrupt-parent = <&gpio>;
+					interrupts = <TEGRA_GPIO(J, 0)
+							GPIO_ACTIVE_HIGH>;
+					ti,ac-detect-gpios = <&gpio
+							TEGRA_GPIO(J, 0)
+							GPIO_ACTIVE_HIGH>;
+				};
+
+				battery: sbs-battery@b {
+					compatible = "sbs,sbs-battery";
+					reg = <0xb>;
+					sbs,i2c-retry-count = <2>;
+					sbs,poll-retry-count = <1>;
+				};
+			};
+
 			cros-ec-keyb {
 				compatible = "google,cros-ec-keyb";
 				keypad,num-rows = <8>;
-- 
1.9.1.423.g4596e3a

[PATCH v2 7/7] ARM: tegra: Add the EC i2c tunnel to tegra124-venice2

[Doug Anderson]

This adds the EC i2c tunnel (and devices under it) to the
tegra124-venice2 device tree.

Signed-off-by: Doug Anderson <dianders@chromium.org>
Tested-by: Andrew Bresticker <abrestic@chromium.org>
Tested-by: Stephen Warren <swarren@nvidia.com>
---
Changes in v2:
- Removed i2c20 alias for i2c tunnel

 arch/arm/boot/dts/tegra124-venice2.dts | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/arch/arm/boot/dts/tegra124-venice2.dts b/arch/arm/boot/dts/tegra124-venice2.dts
index c17283c..89cf776 100644
--- a/arch/arm/boot/dts/tegra124-venice2.dts
+++ b/arch/arm/boot/dts/tegra124-venice2.dts
@@ -813,6 +813,32 @@
 
 			google,cros-ec-spi-msg-delay = <2000>;
 
+			i2c-tunnel {
+				compatible = "google,cros-ec-i2c-tunnel";
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				google,remote-bus = <0>;
+
+				charger: bq24735@9 {
+					compatible = "ti,bq24735";
+					reg = <0x9>;
+					interrupt-parent = <&gpio>;
+					interrupts = <TEGRA_GPIO(J, 0)
+							GPIO_ACTIVE_HIGH>;
+					ti,ac-detect-gpios = <&gpio
+							TEGRA_GPIO(J, 0)
+							GPIO_ACTIVE_HIGH>;
+				};
+
+				battery: sbs-battery@b {
+					compatible = "sbs,sbs-battery";
+					reg = <0xb>;
+					sbs,i2c-retry-count = <2>;
+					sbs,poll-retry-count = <1>;
+				};
+			};
+
 			cros-ec-keyb {
 				compatible = "google,cros-ec-keyb";
 				keypad,num-rows = <8>;
-- 
1.9.1.423.g4596e3a

[PATCH v2 7/7] ARM: tegra: Add the EC i2c tunnel to tegra124-venice2

[Doug Anderson]

This adds the EC i2c tunnel (and devices under it) to the
tegra124-venice2 device tree.

Signed-off-by: Doug Anderson <dianders@chromium.org>
Tested-by: Andrew Bresticker <abrestic@chromium.org>
Tested-by: Stephen Warren <swarren@nvidia.com>
---
Changes in v2:
- Removed i2c20 alias for i2c tunnel

 arch/arm/boot/dts/tegra124-venice2.dts | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/arch/arm/boot/dts/tegra124-venice2.dts b/arch/arm/boot/dts/tegra124-venice2.dts
index c17283c..89cf776 100644
--- a/arch/arm/boot/dts/tegra124-venice2.dts
+++ b/arch/arm/boot/dts/tegra124-venice2.dts
@@ -813,6 +813,32 @@
 
 			google,cros-ec-spi-msg-delay = <2000>;
 
+			i2c-tunnel {
+				compatible = "google,cros-ec-i2c-tunnel";
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				google,remote-bus = <0>;
+
+				charger: bq24735 at 9 {
+					compatible = "ti,bq24735";
+					reg = <0x9>;
+					interrupt-parent = <&gpio>;
+					interrupts = <TEGRA_GPIO(J, 0)
+							GPIO_ACTIVE_HIGH>;
+					ti,ac-detect-gpios = <&gpio
+							TEGRA_GPIO(J, 0)
+							GPIO_ACTIVE_HIGH>;
+				};
+
+				battery: sbs-battery at b {
+					compatible = "sbs,sbs-battery";
+					reg = <0xb>;
+					sbs,i2c-retry-count = <2>;
+					sbs,poll-retry-count = <1>;
+				};
+			};
+
 			cros-ec-keyb {
 				compatible = "google,cros-ec-keyb";
 				keypad,num-rows = <8>;
-- 
1.9.1.423.g4596e3a

Re: [PATCH v2 0/7] Add cros_ec changes for newer boards

[Lee Jones]

On Tue, 22 Apr 2014, Doug Anderson wrote:

> This series adds the most critical cros_ec changes for newer boards
> using cros_ec.  Specifically:
> * Fixes timing/locking issues with the previously upstreamed (but
>   never used upstream) cros_ec_spi driver.
> * Updates the cros_ec header file to the latest version which allows
>   us to use newer EC features like i2c tunneling.
> * Adds an i2c tunnel driver to allow communication to the EC's i2c
>   devices.
> 
> This _doesn't_ get the EC driver fully up to speed with what's in the
> current Chromium OS trees.  There are a whole slew of cleanup patches
> there, an addition of an LPC transport mode, and exports of functions
> to userspace.  Once these patches land and we have functionality we
> can continue to pick more cleanup patches.
> 
> Changes in v2:
> - Update tunnel binding as per swarren
> - Removed i2c20 alias for i2c tunnel
> 
> Bill Richardson (1):
>   mfd: cros_ec: Sync to the latest cros_ec_commands.h from EC sources
> 
> David Hendricks (1):
>   mfd: cros_ec: spi: calculate delay between transfers correctly
> 
> Doug Anderson (5):
>   mfd: cros_ec: spi: Add mutex to cros_ec_spi
>   mfd: cros_ec: spi: Make the cros_ec_spi timeout more reliable
>   mfd: cros_ec: spi: Increase cros_ec_spi deadline from 5ms to 100ms
>   i2c: ChromeOS EC tunnel driver
>   ARM: tegra: Add the EC i2c tunnel to tegra124-venice2
> 
>  .../devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt |   39 +
>  arch/arm/boot/dts/tegra124-venice2.dts             |   26 +
>  drivers/i2c/busses/Kconfig                         |    9 +
>  drivers/i2c/busses/Makefile                        |    1 +
>  drivers/i2c/busses/i2c-cros-ec-tunnel.c            |  304 ++++++
>  drivers/mfd/cros_ec.c                              |    7 +-
>  drivers/mfd/cros_ec_spi.c                          |   67 +-
>  include/linux/mfd/cros_ec.h                        |    4 +-
>  include/linux/mfd/cros_ec_commands.h               | 1128 ++++++++++++++++++--
>  9 files changed, 1493 insertions(+), 92 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt
>  create mode 100644 drivers/i2c/busses/i2c-cros-ec-tunnel.c

Need to wait for the ARM, DT and I2C guys to review, at which point
I'll be happy to take in and supply a branch for them to pull from if
required. If there are no _true_ dependencies and the MFD changes can
be added independently without fear of build breakages, let me know
and I'll apply them separately.

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

Re: [PATCH v2 0/7] Add cros_ec changes for newer boards

[Lee Jones]

On Tue, 22 Apr 2014, Doug Anderson wrote:

> This series adds the most critical cros_ec changes for newer boards
> using cros_ec.  Specifically:
> * Fixes timing/locking issues with the previously upstreamed (but
>   never used upstream) cros_ec_spi driver.
> * Updates the cros_ec header file to the latest version which allows
>   us to use newer EC features like i2c tunneling.
> * Adds an i2c tunnel driver to allow communication to the EC's i2c
>   devices.
> 
> This _doesn't_ get the EC driver fully up to speed with what's in the
> current Chromium OS trees.  There are a whole slew of cleanup patches
> there, an addition of an LPC transport mode, and exports of functions
> to userspace.  Once these patches land and we have functionality we
> can continue to pick more cleanup patches.
> 
> Changes in v2:
> - Update tunnel binding as per swarren
> - Removed i2c20 alias for i2c tunnel
> 
> Bill Richardson (1):
>   mfd: cros_ec: Sync to the latest cros_ec_commands.h from EC sources
> 
> David Hendricks (1):
>   mfd: cros_ec: spi: calculate delay between transfers correctly
> 
> Doug Anderson (5):
>   mfd: cros_ec: spi: Add mutex to cros_ec_spi
>   mfd: cros_ec: spi: Make the cros_ec_spi timeout more reliable
>   mfd: cros_ec: spi: Increase cros_ec_spi deadline from 5ms to 100ms
>   i2c: ChromeOS EC tunnel driver
>   ARM: tegra: Add the EC i2c tunnel to tegra124-venice2
> 
>  .../devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt |   39 +
>  arch/arm/boot/dts/tegra124-venice2.dts             |   26 +
>  drivers/i2c/busses/Kconfig                         |    9 +
>  drivers/i2c/busses/Makefile                        |    1 +
>  drivers/i2c/busses/i2c-cros-ec-tunnel.c            |  304 ++++++
>  drivers/mfd/cros_ec.c                              |    7 +-
>  drivers/mfd/cros_ec_spi.c                          |   67 +-
>  include/linux/mfd/cros_ec.h                        |    4 +-
>  include/linux/mfd/cros_ec_commands.h               | 1128 ++++++++++++++++++--
>  9 files changed, 1493 insertions(+), 92 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt
>  create mode 100644 drivers/i2c/busses/i2c-cros-ec-tunnel.c

Need to wait for the ARM, DT and I2C guys to review, at which point
I'll be happy to take in and supply a branch for them to pull from if
required. If there are no _true_ dependencies and the MFD changes can
be added independently without fear of build breakages, let me know
and I'll apply them separately.

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

[PATCH v2 0/7] Add cros_ec changes for newer boards

[Lee Jones]

On Tue, 22 Apr 2014, Doug Anderson wrote:

> This series adds the most critical cros_ec changes for newer boards
> using cros_ec.  Specifically:
> * Fixes timing/locking issues with the previously upstreamed (but
>   never used upstream) cros_ec_spi driver.
> * Updates the cros_ec header file to the latest version which allows
>   us to use newer EC features like i2c tunneling.
> * Adds an i2c tunnel driver to allow communication to the EC's i2c
>   devices.
> 
> This _doesn't_ get the EC driver fully up to speed with what's in the
> current Chromium OS trees.  There are a whole slew of cleanup patches
> there, an addition of an LPC transport mode, and exports of functions
> to userspace.  Once these patches land and we have functionality we
> can continue to pick more cleanup patches.
> 
> Changes in v2:
> - Update tunnel binding as per swarren
> - Removed i2c20 alias for i2c tunnel
> 
> Bill Richardson (1):
>   mfd: cros_ec: Sync to the latest cros_ec_commands.h from EC sources
> 
> David Hendricks (1):
>   mfd: cros_ec: spi: calculate delay between transfers correctly
> 
> Doug Anderson (5):
>   mfd: cros_ec: spi: Add mutex to cros_ec_spi
>   mfd: cros_ec: spi: Make the cros_ec_spi timeout more reliable
>   mfd: cros_ec: spi: Increase cros_ec_spi deadline from 5ms to 100ms
>   i2c: ChromeOS EC tunnel driver
>   ARM: tegra: Add the EC i2c tunnel to tegra124-venice2
> 
>  .../devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt |   39 +
>  arch/arm/boot/dts/tegra124-venice2.dts             |   26 +
>  drivers/i2c/busses/Kconfig                         |    9 +
>  drivers/i2c/busses/Makefile                        |    1 +
>  drivers/i2c/busses/i2c-cros-ec-tunnel.c            |  304 ++++++
>  drivers/mfd/cros_ec.c                              |    7 +-
>  drivers/mfd/cros_ec_spi.c                          |   67 +-
>  include/linux/mfd/cros_ec.h                        |    4 +-
>  include/linux/mfd/cros_ec_commands.h               | 1128 ++++++++++++++++++--
>  9 files changed, 1493 insertions(+), 92 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt
>  create mode 100644 drivers/i2c/busses/i2c-cros-ec-tunnel.c

Need to wait for the ARM, DT and I2C guys to review, at which point
I'll be happy to take in and supply a branch for them to pull from if
required. If there are no _true_ dependencies and the MFD changes can
be added independently without fear of build breakages, let me know
and I'll apply them separately.

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

Re: [PATCH v2 1/7] mfd: cros_ec: spi: calculate delay between transfers correctly

[Lee Jones]

> From: David Hendricks <dhendrix@chromium.org>
> 
> To avoid spamming the EC we calculate the time between the previous
> transfer and the current transfer and force a delay if the time delta
> is too small.
> 
> However, a small miscalculation causes the delay period to be
> far too short. Most noticably this impacts commands with a long
> turnaround time such as EC firmware reads and writes.
> 
> Signed-off-by: David Hendricks <dhendrix@chromium.org>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Tested-by: Andrew Bresticker <abrestic@chromium.org>
> Tested-by: Stephen Warren <swarren@nvidia.com>
> ---
> Changes in v2: None
> 
>  drivers/mfd/cros_ec_spi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Waiting for other subsystems to Ack, so for now:
  Acked-by: Lee Jones <lee.jones@linaro.org>

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

Re: [PATCH v2 2/7] mfd: cros_ec: spi: Add mutex to cros_ec_spi

[Lee Jones]

On Tue, 22 Apr 2014, Doug Anderson wrote:

> The main transfer function for cros_ec_spi can be called by more than
> one client at a time.  Make sure that those clients don't stomp on
> each other by locking the bus for the duration of the transfer
> function.
> 
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Tested-by: Andrew Bresticker <abrestic@chromium.org>
> Tested-by: Stephen Warren <swarren@nvidia.com>
> ---
> Changes in v2: None
> 
>  drivers/mfd/cros_ec_spi.c | 26 +++++++++++++++++++++-----
>  1 file changed, 21 insertions(+), 5 deletions(-)

Waiting for other subsystems to Ack, so for now:
  Acked-by: Lee Jones <lee.jones@linaro.org>

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

Re: [PATCH v2 3/7] mfd: cros_ec: spi: Make the cros_ec_spi timeout more reliable

[Lee Jones]

On Tue, 22 Apr 2014, Doug Anderson wrote:

> The cros_ec_spi transfer had two problems with its timeout code:
> 
> 1. It looked at the timeout even in the case that it found valid data.
> 2. If the cros_ec_spi code got switched out for a while, it's possible
>    it could get a timeout after a single loop.  Let's be paranoid and
>    make sure we do one last transfer after the timeout expires.
> 
> Signed-off-by: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> Reviewed-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> Tested-by: Andrew Bresticker <abrestic-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> Tested-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
> Changes in v2: None
> 
>  drivers/mfd/cros_ec_spi.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)

Waiting for other subsystems to Ack, so for now:
  Acked-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

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

Re: [PATCH v2 3/7] mfd: cros_ec: spi: Make the cros_ec_spi timeout more reliable

[Lee Jones]

On Tue, 22 Apr 2014, Doug Anderson wrote:

> The cros_ec_spi transfer had two problems with its timeout code:
> 
> 1. It looked at the timeout even in the case that it found valid data.
> 2. If the cros_ec_spi code got switched out for a while, it's possible
>    it could get a timeout after a single loop.  Let's be paranoid and
>    make sure we do one last transfer after the timeout expires.
> 
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Tested-by: Andrew Bresticker <abrestic@chromium.org>
> Tested-by: Stephen Warren <swarren@nvidia.com>
> ---
> Changes in v2: None
> 
>  drivers/mfd/cros_ec_spi.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)

Waiting for other subsystems to Ack, so for now:
  Acked-by: Lee Jones <lee.jones@linaro.org>

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

Re: [PATCH v2 4/7] mfd: cros_ec: spi: Increase cros_ec_spi deadline from 5ms to 100ms

[Lee Jones]

On Tue, 22 Apr 2014, Doug Anderson wrote:

> We're adding i2c tunneling to the list of things that goes over
> cros_ec.  i2c tunneling can be slooooooow, so increase our deadline to
> 100ms to account for that.
> 
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Tested-by: Andrew Bresticker <abrestic@chromium.org>
> Tested-by: Stephen Warren <swarren@nvidia.com>
> ---
> Changes in v2: None
> 
>  drivers/mfd/cros_ec_spi.c | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)

Waiting for other subsystems to Ack, so for now:
  Acked-by: Lee Jones <lee.jones@linaro.org>

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

Re: [PATCH v2 5/7] mfd: cros_ec: Sync to the latest cros_ec_commands.h from EC sources

[Lee Jones]

On Tue, 22 Apr 2014, Doug Anderson wrote:

> From: Bill Richardson <wfrichar@chromium.org>
> 
> This just updates include/linux/mfd/cros_ec_commands.h to match the
> latest EC version (which is the One True Source for such things).  See
> <https://chromium.googlesource.com/chromiumos/platform/ec>
> 
> [dianders: took today's ToT version from the Chromium OS EC; deleted
> references to cros_ec_dev and cros_ec_lpc since those aren't upstream
> yet]
> 
> Signed-off-by: Bill Richardson <wfrichar@chromium.org>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Tested-by: Andrew Bresticker <abrestic@chromium.org>
> Tested-by: Stephen Warren <swarren@nvidia.com>
> ---
> Changes in v2: None
> 
>  drivers/mfd/cros_ec.c                |    2 +-
>  include/linux/mfd/cros_ec.h          |    4 +-
>  include/linux/mfd/cros_ec_commands.h | 1128 +++++++++++++++++++++++++++++++---
>  3 files changed, 1059 insertions(+), 75 deletions(-)

Waiting for other subsystems to Ack, so for now:
  Acked-by: Lee Jones <lee.jones@linaro.org>

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

Re: [PATCH v2 6/7] i2c: ChromeOS EC tunnel driver

[Lee Jones]

On Tue, 22 Apr 2014, Doug Anderson wrote:

> On ARM Chromebooks we have a few devices that are accessed by both the
> AP (the main "Application Processor") and the EC (the Embedded
> Controller).  These are:
> * The battery (sbs-battery).
> * The power management unit tps65090.
> 
> On the original Samsung ARM Chromebook these devices were on an I2C
> bus that was shared between the AP and the EC and arbitrated using
> some extranal GPIOs (see i2c-arb-gpio-challenge).
> 
> The original arbitration scheme worked well enough but had some
> downsides:
> * It was nonstandard (not using standard I2C multimaster)
> * It only worked if the EC-AP communication was I2C
> * It was relatively hard to debug problems (hard to tell if i2c issues
>   were caused by the EC, the AP, or some device on the bus).
> 
> On the HP Chromebook 11 the design was changed to:
> * The AP/EC comms were still i2c, but the battery/tps65090 were no
>   longer on the bus used for AP/EC communication.  The battery was
>   exposed to the AP through a limited i2c tunnel and tps65090 was
>   exposed to the AP through a custom Linux driver.
> 
> On the Samsung ARM Chromebook 2 the scheme is changed yet again, now:
> * The AP/EC comms are now using SPI for faster speeds.
> * The EC's i2c bus is exposed to the AP through a full i2c tunnel.
> 
> The upstream "tegra124-venice2" uses the same scheme as the Samsung
> ARM Chromebook 2, though it has a different set of components on the
> other side of the bus.
> 
> This driver supports the scheme used by the Samsung ARM Chromebook 2.
> Future patches to this driver could add support for the battery tunnel
> on the HP Chromebook 11 (and perhaps could even be used to access
> tps65090 on the HP Chromebook 11 instead of using a special driver,
> but I haven't researched that enough).
> 
> Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Tested-by: Andrew Bresticker <abrestic@chromium.org>
> Tested-by: Stephen Warren <swarren@nvidia.com>
> ---
> Changes in v2:
> - Update tunnel binding as per swarren
> 
>  .../devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt |  39 +++
>  drivers/i2c/busses/Kconfig                         |   9 +
>  drivers/i2c/busses/Makefile                        |   1 +
>  drivers/i2c/busses/i2c-cros-ec-tunnel.c            | 304 +++++++++++++++++++++
>  drivers/mfd/cros_ec.c                              |   5 +

For the MFD changes:
  Acked-by: Lee Jones <lee.jones@linaro.org>

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

Re: [PATCH v2 0/7] Add cros_ec changes for newer boards

[Stephen Warren]

On 04/23/2014 06:32 AM, Lee Jones wrote:
> On Tue, 22 Apr 2014, Doug Anderson wrote:
> 
>> This series adds the most critical cros_ec changes for newer boards
>> using cros_ec.  Specifically:
>> * Fixes timing/locking issues with the previously upstreamed (but
>>   never used upstream) cros_ec_spi driver.
>> * Updates the cros_ec header file to the latest version which allows
>>   us to use newer EC features like i2c tunneling.
>> * Adds an i2c tunnel driver to allow communication to the EC's i2c
>>   devices.
>>
>> This _doesn't_ get the EC driver fully up to speed with what's in the
>> current Chromium OS trees.  There are a whole slew of cleanup patches
>> there, an addition of an LPC transport mode, and exports of functions
>> to userspace.  Once these patches land and we have functionality we
>> can continue to pick more cleanup patches.
...
> Need to wait for the ARM, DT and I2C guys to review, at which point
> I'll be happy to take in and supply a branch for them to pull from if
> required. If there are no _true_ dependencies and the MFD changes can
> be added independently without fear of build breakages, let me know
> and I'll apply them separately.

I believe there aren't direct dependencies between the patches. So, the
MFD patches can be applied to the MFD tree and the DT patch applied to
the Tegra tree. I'm simply waiting for the MFD patches to be applied
before applying the DT patch so that I know the DT binding definition is
fully accepted before applying a patch that uses it.

Re: [PATCH v2 0/7] Add cros_ec changes for newer boards

[Stephen Warren]

On 04/23/2014 06:32 AM, Lee Jones wrote:
> On Tue, 22 Apr 2014, Doug Anderson wrote:
> 
>> This series adds the most critical cros_ec changes for newer boards
>> using cros_ec.  Specifically:
>> * Fixes timing/locking issues with the previously upstreamed (but
>>   never used upstream) cros_ec_spi driver.
>> * Updates the cros_ec header file to the latest version which allows
>>   us to use newer EC features like i2c tunneling.
>> * Adds an i2c tunnel driver to allow communication to the EC's i2c
>>   devices.
>>
>> This _doesn't_ get the EC driver fully up to speed with what's in the
>> current Chromium OS trees.  There are a whole slew of cleanup patches
>> there, an addition of an LPC transport mode, and exports of functions
>> to userspace.  Once these patches land and we have functionality we
>> can continue to pick more cleanup patches.
...
> Need to wait for the ARM, DT and I2C guys to review, at which point
> I'll be happy to take in and supply a branch for them to pull from if
> required. If there are no _true_ dependencies and the MFD changes can
> be added independently without fear of build breakages, let me know
> and I'll apply them separately.

I believe there aren't direct dependencies between the patches. So, the
MFD patches can be applied to the MFD tree and the DT patch applied to
the Tegra tree. I'm simply waiting for the MFD patches to be applied
before applying the DT patch so that I know the DT binding definition is
fully accepted before applying a patch that uses it.

[PATCH v2 0/7] Add cros_ec changes for newer boards

[Stephen Warren]

On 04/23/2014 06:32 AM, Lee Jones wrote:
> On Tue, 22 Apr 2014, Doug Anderson wrote:
> 
>> This series adds the most critical cros_ec changes for newer boards
>> using cros_ec.  Specifically:
>> * Fixes timing/locking issues with the previously upstreamed (but
>>   never used upstream) cros_ec_spi driver.
>> * Updates the cros_ec header file to the latest version which allows
>>   us to use newer EC features like i2c tunneling.
>> * Adds an i2c tunnel driver to allow communication to the EC's i2c
>>   devices.
>>
>> This _doesn't_ get the EC driver fully up to speed with what's in the
>> current Chromium OS trees.  There are a whole slew of cleanup patches
>> there, an addition of an LPC transport mode, and exports of functions
>> to userspace.  Once these patches land and we have functionality we
>> can continue to pick more cleanup patches.
...
> Need to wait for the ARM, DT and I2C guys to review, at which point
> I'll be happy to take in and supply a branch for them to pull from if
> required. If there are no _true_ dependencies and the MFD changes can
> be added independently without fear of build breakages, let me know
> and I'll apply them separately.

I believe there aren't direct dependencies between the patches. So, the
MFD patches can be applied to the MFD tree and the DT patch applied to
the Tegra tree. I'm simply waiting for the MFD patches to be applied
before applying the DT patch so that I know the DT binding definition is
fully accepted before applying a patch that uses it.

Re: [PATCH v2 0/7] Add cros_ec changes for newer boards

[Doug Anderson]

Hi,

On Wed, Apr 23, 2014 at 9:20 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 04/23/2014 06:32 AM, Lee Jones wrote:
>> On Tue, 22 Apr 2014, Doug Anderson wrote:
>>
>>> This series adds the most critical cros_ec changes for newer boards
>>> using cros_ec.  Specifically:
>>> * Fixes timing/locking issues with the previously upstreamed (but
>>>   never used upstream) cros_ec_spi driver.
>>> * Updates the cros_ec header file to the latest version which allows
>>>   us to use newer EC features like i2c tunneling.
>>> * Adds an i2c tunnel driver to allow communication to the EC's i2c
>>>   devices.
>>>
>>> This _doesn't_ get the EC driver fully up to speed with what's in the
>>> current Chromium OS trees.  There are a whole slew of cleanup patches
>>> there, an addition of an LPC transport mode, and exports of functions
>>> to userspace.  Once these patches land and we have functionality we
>>> can continue to pick more cleanup patches.
> ...
>> Need to wait for the ARM, DT and I2C guys to review, at which point
>> I'll be happy to take in and supply a branch for them to pull from if
>> required. If there are no _true_ dependencies and the MFD changes can
>> be added independently without fear of build breakages, let me know
>> and I'll apply them separately.
>
> I believe there aren't direct dependencies between the patches. So, the
> MFD patches can be applied to the MFD tree and the DT patch applied to
> the Tegra tree. I'm simply waiting for the MFD patches to be applied
> before applying the DT patch so that I know the DT binding definition is
> fully accepted before applying a patch that uses it.

All of the MFD patches are safe to apply and in pretty much arbitrary
order.  The strong dependencies in the chain are:

* We need patch #5 (mfd: cros_ec: Sync to the latest
cros_ec_commands.h from EC sources) before the i2c tunnel can compile.

* As Stephen says, he shouldn't apply the device tree until we're
confident that the bindings are right.  However there's no strong
dependency otherwise.

* Patches #1 #2 and #3 are simply reliability fixes.  Those could land
at any point in time and will improve other users of cros_ec_spi (like
the keyboard on tegra124-venice2).

* Patch #4 can apply any time with no issues.  Without it large i2c
tunnel transfers won't work, but that's not a terrible problem (all
normal transfers are small).

---

All that being said, I'd request that you merge patches #1-#4 as soon
as you can and make sure you can provide a way that Wolfram can pull
them (or at least patch #4) into his i2c tree to keep them applying
when he is ready to land #5.

-Doug

[PATCH v2 0/7] Add cros_ec changes for newer boards

[Doug Anderson]

Hi,

On Wed, Apr 23, 2014 at 9:20 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 04/23/2014 06:32 AM, Lee Jones wrote:
>> On Tue, 22 Apr 2014, Doug Anderson wrote:
>>
>>> This series adds the most critical cros_ec changes for newer boards
>>> using cros_ec.  Specifically:
>>> * Fixes timing/locking issues with the previously upstreamed (but
>>>   never used upstream) cros_ec_spi driver.
>>> * Updates the cros_ec header file to the latest version which allows
>>>   us to use newer EC features like i2c tunneling.
>>> * Adds an i2c tunnel driver to allow communication to the EC's i2c
>>>   devices.
>>>
>>> This _doesn't_ get the EC driver fully up to speed with what's in the
>>> current Chromium OS trees.  There are a whole slew of cleanup patches
>>> there, an addition of an LPC transport mode, and exports of functions
>>> to userspace.  Once these patches land and we have functionality we
>>> can continue to pick more cleanup patches.
> ...
>> Need to wait for the ARM, DT and I2C guys to review, at which point
>> I'll be happy to take in and supply a branch for them to pull from if
>> required. If there are no _true_ dependencies and the MFD changes can
>> be added independently without fear of build breakages, let me know
>> and I'll apply them separately.
>
> I believe there aren't direct dependencies between the patches. So, the
> MFD patches can be applied to the MFD tree and the DT patch applied to
> the Tegra tree. I'm simply waiting for the MFD patches to be applied
> before applying the DT patch so that I know the DT binding definition is
> fully accepted before applying a patch that uses it.

All of the MFD patches are safe to apply and in pretty much arbitrary
order.  The strong dependencies in the chain are:

* We need patch #5 (mfd: cros_ec: Sync to the latest
cros_ec_commands.h from EC sources) before the i2c tunnel can compile.

* As Stephen says, he shouldn't apply the device tree until we're
confident that the bindings are right.  However there's no strong
dependency otherwise.

* Patches #1 #2 and #3 are simply reliability fixes.  Those could land
at any point in time and will improve other users of cros_ec_spi (like
the keyboard on tegra124-venice2).

* Patch #4 can apply any time with no issues.  Without it large i2c
tunnel transfers won't work, but that's not a terrible problem (all
normal transfers are small).

---

All that being said, I'd request that you merge patches #1-#4 as soon
as you can and make sure you can provide a way that Wolfram can pull
them (or at least patch #4) into his i2c tree to keep them applying
when he is ready to land #5.

-Doug

Re: [PATCH v2 0/7] Add cros_ec changes for newer boards

[Doug Anderson]

Hi,

On Wed, Apr 23, 2014 at 9:32 AM, Doug Anderson <dianders@chromium.org> wrote:
> Hi,
>
> On Wed, Apr 23, 2014 at 9:20 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 04/23/2014 06:32 AM, Lee Jones wrote:
>>> On Tue, 22 Apr 2014, Doug Anderson wrote:
>>>
>>>> This series adds the most critical cros_ec changes for newer boards
>>>> using cros_ec.  Specifically:
>>>> * Fixes timing/locking issues with the previously upstreamed (but
>>>>   never used upstream) cros_ec_spi driver.
>>>> * Updates the cros_ec header file to the latest version which allows
>>>>   us to use newer EC features like i2c tunneling.
>>>> * Adds an i2c tunnel driver to allow communication to the EC's i2c
>>>>   devices.
>>>>
>>>> This _doesn't_ get the EC driver fully up to speed with what's in the
>>>> current Chromium OS trees.  There are a whole slew of cleanup patches
>>>> there, an addition of an LPC transport mode, and exports of functions
>>>> to userspace.  Once these patches land and we have functionality we
>>>> can continue to pick more cleanup patches.
>> ...
>>> Need to wait for the ARM, DT and I2C guys to review, at which point
>>> I'll be happy to take in and supply a branch for them to pull from if
>>> required. If there are no _true_ dependencies and the MFD changes can
>>> be added independently without fear of build breakages, let me know
>>> and I'll apply them separately.
>>
>> I believe there aren't direct dependencies between the patches. So, the
>> MFD patches can be applied to the MFD tree and the DT patch applied to
>> the Tegra tree. I'm simply waiting for the MFD patches to be applied
>> before applying the DT patch so that I know the DT binding definition is
>> fully accepted before applying a patch that uses it.
>
> All of the MFD patches are safe to apply and in pretty much arbitrary
> order.  The strong dependencies in the chain are:
>
> * We need patch #5 (mfd: cros_ec: Sync to the latest
> cros_ec_commands.h from EC sources) before the i2c tunnel can compile.
>
> * As Stephen says, he shouldn't apply the device tree until we're
> confident that the bindings are right.  However there's no strong
> dependency otherwise.
>
> * Patches #1 #2 and #3 are simply reliability fixes.  Those could land
> at any point in time and will improve other users of cros_ec_spi (like
> the keyboard on tegra124-venice2).
>
> * Patch #4 can apply any time with no issues.  Without it large i2c
> tunnel transfers won't work, but that's not a terrible problem (all
> normal transfers are small).
>
> ---
>
> All that being said, I'd request that you merge patches #1-#4 as soon
> as you can and make sure you can provide a way that Wolfram can pull
> them (or at least patch #4) into his i2c tree to keep them applying
> when he is ready to land #5.

Oops, I missed a patch.  Let me say that again.

Patch #5 (latest ec commands) can also apply at any time with no
issues, but it's needed for patch #6 (the tunnel) to compile.

All that being said, I'd request that you merge patches #1-#5 as soon
as you can and make sure you can provide a way that Wolfram can pull
them (or at least patch #5) into his i2c tree to keep them applying
when he is ready to land #6.

[PATCH v2 0/7] Add cros_ec changes for newer boards

[Doug Anderson]

Hi,

On Wed, Apr 23, 2014 at 9:32 AM, Doug Anderson <dianders@chromium.org> wrote:
> Hi,
>
> On Wed, Apr 23, 2014 at 9:20 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 04/23/2014 06:32 AM, Lee Jones wrote:
>>> On Tue, 22 Apr 2014, Doug Anderson wrote:
>>>
>>>> This series adds the most critical cros_ec changes for newer boards
>>>> using cros_ec.  Specifically:
>>>> * Fixes timing/locking issues with the previously upstreamed (but
>>>>   never used upstream) cros_ec_spi driver.
>>>> * Updates the cros_ec header file to the latest version which allows
>>>>   us to use newer EC features like i2c tunneling.
>>>> * Adds an i2c tunnel driver to allow communication to the EC's i2c
>>>>   devices.
>>>>
>>>> This _doesn't_ get the EC driver fully up to speed with what's in the
>>>> current Chromium OS trees.  There are a whole slew of cleanup patches
>>>> there, an addition of an LPC transport mode, and exports of functions
>>>> to userspace.  Once these patches land and we have functionality we
>>>> can continue to pick more cleanup patches.
>> ...
>>> Need to wait for the ARM, DT and I2C guys to review, at which point
>>> I'll be happy to take in and supply a branch for them to pull from if
>>> required. If there are no _true_ dependencies and the MFD changes can
>>> be added independently without fear of build breakages, let me know
>>> and I'll apply them separately.
>>
>> I believe there aren't direct dependencies between the patches. So, the
>> MFD patches can be applied to the MFD tree and the DT patch applied to
>> the Tegra tree. I'm simply waiting for the MFD patches to be applied
>> before applying the DT patch so that I know the DT binding definition is
>> fully accepted before applying a patch that uses it.
>
> All of the MFD patches are safe to apply and in pretty much arbitrary
> order.  The strong dependencies in the chain are:
>
> * We need patch #5 (mfd: cros_ec: Sync to the latest
> cros_ec_commands.h from EC sources) before the i2c tunnel can compile.
>
> * As Stephen says, he shouldn't apply the device tree until we're
> confident that the bindings are right.  However there's no strong
> dependency otherwise.
>
> * Patches #1 #2 and #3 are simply reliability fixes.  Those could land
> at any point in time and will improve other users of cros_ec_spi (like
> the keyboard on tegra124-venice2).
>
> * Patch #4 can apply any time with no issues.  Without it large i2c
> tunnel transfers won't work, but that's not a terrible problem (all
> normal transfers are small).
>
> ---
>
> All that being said, I'd request that you merge patches #1-#4 as soon
> as you can and make sure you can provide a way that Wolfram can pull
> them (or at least patch #4) into his i2c tree to keep them applying
> when he is ready to land #5.

Oops, I missed a patch.  Let me say that again.

Patch #5 (latest ec commands) can also apply at any time with no
issues, but it's needed for patch #6 (the tunnel) to compile.

All that being said, I'd request that you merge patches #1-#5 as soon
as you can and make sure you can provide a way that Wolfram can pull
them (or at least patch #5) into his i2c tree to keep them applying
when he is ready to land #6.

Re: [PATCH v2 6/7] i2c: ChromeOS EC tunnel driver

[Doug Anderson]

Wolfram,

On Tue, Apr 22, 2014 at 9:45 AM, Doug Anderson <dianders@chromium.org> wrote:
> On ARM Chromebooks we have a few devices that are accessed by both the
> AP (the main "Application Processor") and the EC (the Embedded
> Controller).  These are:
> * The battery (sbs-battery).
> * The power management unit tps65090.
>
> On the original Samsung ARM Chromebook these devices were on an I2C
> bus that was shared between the AP and the EC and arbitrated using
> some extranal GPIOs (see i2c-arb-gpio-challenge).
>
> The original arbitration scheme worked well enough but had some
> downsides:
> * It was nonstandard (not using standard I2C multimaster)
> * It only worked if the EC-AP communication was I2C
> * It was relatively hard to debug problems (hard to tell if i2c issues
>   were caused by the EC, the AP, or some device on the bus).
>
> On the HP Chromebook 11 the design was changed to:
> * The AP/EC comms were still i2c, but the battery/tps65090 were no
>   longer on the bus used for AP/EC communication.  The battery was
>   exposed to the AP through a limited i2c tunnel and tps65090 was
>   exposed to the AP through a custom Linux driver.
>
> On the Samsung ARM Chromebook 2 the scheme is changed yet again, now:
> * The AP/EC comms are now using SPI for faster speeds.
> * The EC's i2c bus is exposed to the AP through a full i2c tunnel.
>
> The upstream "tegra124-venice2" uses the same scheme as the Samsung
> ARM Chromebook 2, though it has a different set of components on the
> other side of the bus.
>
> This driver supports the scheme used by the Samsung ARM Chromebook 2.
> Future patches to this driver could add support for the battery tunnel
> on the HP Chromebook 11 (and perhaps could even be used to access
> tps65090 on the HP Chromebook 11 instead of using a special driver,
> but I haven't researched that enough).
>
> Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Tested-by: Andrew Bresticker <abrestic@chromium.org>
> Tested-by: Stephen Warren <swarren@nvidia.com>
> ---
> Changes in v2:
> - Update tunnel binding as per swarren
>
>  .../devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt |  39 +++
>  drivers/i2c/busses/Kconfig                         |   9 +
>  drivers/i2c/busses/Makefile                        |   1 +
>  drivers/i2c/busses/i2c-cros-ec-tunnel.c            | 304 +++++++++++++++++++++
>  drivers/mfd/cros_ec.c                              |   5 +
>  5 files changed, 358 insertions(+)

Did you have any thoughts on this i2c driver?  Please let me know and
I'd be happy to spin with changes.

-Doug

Re: [PATCH v2 0/7] Add cros_ec changes for newer boards

[Lee Jones]

[...]

> >>> Need to wait for the ARM, DT and I2C guys to review, at which point
> >>> I'll be happy to take in and supply a branch for them to pull from if
> >>> required. If there are no _true_ dependencies and the MFD changes can
> >>> be added independently without fear of build breakages, let me know
> >>> and I'll apply them separately.
> >>
> >> I believe there aren't direct dependencies between the patches. So, the
> >> MFD patches can be applied to the MFD tree and the DT patch applied to
> >> the Tegra tree. I'm simply waiting for the MFD patches to be applied
> >> before applying the DT patch so that I know the DT binding definition is
> >> fully accepted before applying a patch that uses it.
> >
> > All of the MFD patches are safe to apply and in pretty much arbitrary
> > order.  The strong dependencies in the chain are:
> >
> > * We need patch #5 (mfd: cros_ec: Sync to the latest
> > cros_ec_commands.h from EC sources) before the i2c tunnel can compile.
> >
> > * As Stephen says, he shouldn't apply the device tree until we're
> > confident that the bindings are right.  However there's no strong
> > dependency otherwise.
> >
> > * Patches #1 #2 and #3 are simply reliability fixes.  Those could land
> > at any point in time and will improve other users of cros_ec_spi (like
> > the keyboard on tegra124-venice2).
> >
> > * Patch #4 can apply any time with no issues.  Without it large i2c
> > tunnel transfers won't work, but that's not a terrible problem (all
> > normal transfers are small).
> 
> Patch #5 (latest ec commands) can also apply at any time with no
> issues, but it's needed for patch #6 (the tunnel) to compile.
> 
> All that being said, I'd request that you merge patches #1-#5 as soon
> as you can and make sure you can provide a way that Wolfram can pull
> them (or at least patch #5) into his i2c tree to keep them applying
> when he is ready to land #6.

Very well. So if I can obtain Wolfram's Ack, I can apply the MFD
changes along with patch #6 and supply him with a branch.

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 0/7] Add cros_ec changes for newer boards

[Lee Jones]

[...]

> >>> Need to wait for the ARM, DT and I2C guys to review, at which point
> >>> I'll be happy to take in and supply a branch for them to pull from if
> >>> required. If there are no _true_ dependencies and the MFD changes can
> >>> be added independently without fear of build breakages, let me know
> >>> and I'll apply them separately.
> >>
> >> I believe there aren't direct dependencies between the patches. So, the
> >> MFD patches can be applied to the MFD tree and the DT patch applied to
> >> the Tegra tree. I'm simply waiting for the MFD patches to be applied
> >> before applying the DT patch so that I know the DT binding definition is
> >> fully accepted before applying a patch that uses it.
> >
> > All of the MFD patches are safe to apply and in pretty much arbitrary
> > order.  The strong dependencies in the chain are:
> >
> > * We need patch #5 (mfd: cros_ec: Sync to the latest
> > cros_ec_commands.h from EC sources) before the i2c tunnel can compile.
> >
> > * As Stephen says, he shouldn't apply the device tree until we're
> > confident that the bindings are right.  However there's no strong
> > dependency otherwise.
> >
> > * Patches #1 #2 and #3 are simply reliability fixes.  Those could land
> > at any point in time and will improve other users of cros_ec_spi (like
> > the keyboard on tegra124-venice2).
> >
> > * Patch #4 can apply any time with no issues.  Without it large i2c
> > tunnel transfers won't work, but that's not a terrible problem (all
> > normal transfers are small).
> 
> Patch #5 (latest ec commands) can also apply at any time with no
> issues, but it's needed for patch #6 (the tunnel) to compile.
> 
> All that being said, I'd request that you merge patches #1-#5 as soon
> as you can and make sure you can provide a way that Wolfram can pull
> them (or at least patch #5) into his i2c tree to keep them applying
> when he is ready to land #6.

Very well. So if I can obtain Wolfram's Ack, I can apply the MFD
changes along with patch #6 and supply him with a branch.

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

Re: [PATCH v2 0/7] Add cros_ec changes for newer boards

[Doug Anderson]

Lee,

On Mon, Apr 28, 2014 at 2:19 AM, Lee Jones <lee.jones@linaro.org> wrote:
> [...]
>
>> >>> Need to wait for the ARM, DT and I2C guys to review, at which point
>> >>> I'll be happy to take in and supply a branch for them to pull from if
>> >>> required. If there are no _true_ dependencies and the MFD changes can
>> >>> be added independently without fear of build breakages, let me know
>> >>> and I'll apply them separately.
>> >>
>> >> I believe there aren't direct dependencies between the patches. So, the
>> >> MFD patches can be applied to the MFD tree and the DT patch applied to
>> >> the Tegra tree. I'm simply waiting for the MFD patches to be applied
>> >> before applying the DT patch so that I know the DT binding definition is
>> >> fully accepted before applying a patch that uses it.
>> >
>> > All of the MFD patches are safe to apply and in pretty much arbitrary
>> > order.  The strong dependencies in the chain are:
>> >
>> > * We need patch #5 (mfd: cros_ec: Sync to the latest
>> > cros_ec_commands.h from EC sources) before the i2c tunnel can compile.
>> >
>> > * As Stephen says, he shouldn't apply the device tree until we're
>> > confident that the bindings are right.  However there's no strong
>> > dependency otherwise.
>> >
>> > * Patches #1 #2 and #3 are simply reliability fixes.  Those could land
>> > at any point in time and will improve other users of cros_ec_spi (like
>> > the keyboard on tegra124-venice2).
>> >
>> > * Patch #4 can apply any time with no issues.  Without it large i2c
>> > tunnel transfers won't work, but that's not a terrible problem (all
>> > normal transfers are small).
>>
>> Patch #5 (latest ec commands) can also apply at any time with no
>> issues, but it's needed for patch #6 (the tunnel) to compile.
>>
>> All that being said, I'd request that you merge patches #1-#5 as soon
>> as you can and make sure you can provide a way that Wolfram can pull
>> them (or at least patch #5) into his i2c tree to keep them applying
>> when he is ready to land #6.
>
> Very well. So if I can obtain Wolfram's Ack, I can apply the MFD
> changes along with patch #6 and supply him with a branch.

Can you explain the reason to wait for Wolfram's Ack before applying
#1 - #5?  I would think:

1. Create a topic branch.
2. Apply patches 1-5 to the topic branch
3. Merge the topic branch to your for-next branch

When Wolfram wants to take patch #6, he can either:
A. Pull your topic branch
B. If it's been long enough, patches will already be in ToT and no extra work.

If I understand correctly, using a topic branch and doing merges /
pulls means that you can provide Wolfram with stable git hashes when
he needs them and there will be no merge conflicts.

Patches #1 - #5 are bonafide bugfixes irrespective of the i2c tunnel.

-Doug

[PATCH v2 0/7] Add cros_ec changes for newer boards

[Doug Anderson]

Lee,

On Mon, Apr 28, 2014 at 2:19 AM, Lee Jones <lee.jones@linaro.org> wrote:
> [...]
>
>> >>> Need to wait for the ARM, DT and I2C guys to review, at which point
>> >>> I'll be happy to take in and supply a branch for them to pull from if
>> >>> required. If there are no _true_ dependencies and the MFD changes can
>> >>> be added independently without fear of build breakages, let me know
>> >>> and I'll apply them separately.
>> >>
>> >> I believe there aren't direct dependencies between the patches. So, the
>> >> MFD patches can be applied to the MFD tree and the DT patch applied to
>> >> the Tegra tree. I'm simply waiting for the MFD patches to be applied
>> >> before applying the DT patch so that I know the DT binding definition is
>> >> fully accepted before applying a patch that uses it.
>> >
>> > All of the MFD patches are safe to apply and in pretty much arbitrary
>> > order.  The strong dependencies in the chain are:
>> >
>> > * We need patch #5 (mfd: cros_ec: Sync to the latest
>> > cros_ec_commands.h from EC sources) before the i2c tunnel can compile.
>> >
>> > * As Stephen says, he shouldn't apply the device tree until we're
>> > confident that the bindings are right.  However there's no strong
>> > dependency otherwise.
>> >
>> > * Patches #1 #2 and #3 are simply reliability fixes.  Those could land
>> > at any point in time and will improve other users of cros_ec_spi (like
>> > the keyboard on tegra124-venice2).
>> >
>> > * Patch #4 can apply any time with no issues.  Without it large i2c
>> > tunnel transfers won't work, but that's not a terrible problem (all
>> > normal transfers are small).
>>
>> Patch #5 (latest ec commands) can also apply at any time with no
>> issues, but it's needed for patch #6 (the tunnel) to compile.
>>
>> All that being said, I'd request that you merge patches #1-#5 as soon
>> as you can and make sure you can provide a way that Wolfram can pull
>> them (or at least patch #5) into his i2c tree to keep them applying
>> when he is ready to land #6.
>
> Very well. So if I can obtain Wolfram's Ack, I can apply the MFD
> changes along with patch #6 and supply him with a branch.

Can you explain the reason to wait for Wolfram's Ack before applying
#1 - #5?  I would think:

1. Create a topic branch.
2. Apply patches 1-5 to the topic branch
3. Merge the topic branch to your for-next branch

When Wolfram wants to take patch #6, he can either:
A. Pull your topic branch
B. If it's been long enough, patches will already be in ToT and no extra work.

If I understand correctly, using a topic branch and doing merges /
pulls means that you can provide Wolfram with stable git hashes when
he needs them and there will be no merge conflicts.

Patches #1 - #5 are bonafide bugfixes irrespective of the i2c tunnel.

-Doug

Re: [PATCH v2 0/7] Add cros_ec changes for newer boards

[Lee Jones]

> >> >>> Need to wait for the ARM, DT and I2C guys to review, at which point
> >> >>> I'll be happy to take in and supply a branch for them to pull from if
> >> >>> required. If there are no _true_ dependencies and the MFD changes can
> >> >>> be added independently without fear of build breakages, let me know
> >> >>> and I'll apply them separately.
> >> >>
> >> >> I believe there aren't direct dependencies between the patches. So, the
> >> >> MFD patches can be applied to the MFD tree and the DT patch applied to
> >> >> the Tegra tree. I'm simply waiting for the MFD patches to be applied
> >> >> before applying the DT patch so that I know the DT binding definition is
> >> >> fully accepted before applying a patch that uses it.
> >> >
> >> > All of the MFD patches are safe to apply and in pretty much arbitrary
> >> > order.  The strong dependencies in the chain are:
> >> >
> >> > * We need patch #5 (mfd: cros_ec: Sync to the latest
> >> > cros_ec_commands.h from EC sources) before the i2c tunnel can compile.
> >> >
> >> > * As Stephen says, he shouldn't apply the device tree until we're
> >> > confident that the bindings are right.  However there's no strong
> >> > dependency otherwise.
> >> >
> >> > * Patches #1 #2 and #3 are simply reliability fixes.  Those could land
> >> > at any point in time and will improve other users of cros_ec_spi (like
> >> > the keyboard on tegra124-venice2).
> >> >
> >> > * Patch #4 can apply any time with no issues.  Without it large i2c
> >> > tunnel transfers won't work, but that's not a terrible problem (all
> >> > normal transfers are small).
> >>
> >> Patch #5 (latest ec commands) can also apply at any time with no
> >> issues, but it's needed for patch #6 (the tunnel) to compile.
> >>
> >> All that being said, I'd request that you merge patches #1-#5 as soon
> >> as you can and make sure you can provide a way that Wolfram can pull
> >> them (or at least patch #5) into his i2c tree to keep them applying
> >> when he is ready to land #6.
> >
> > Very well. So if I can obtain Wolfram's Ack, I can apply the MFD
> > changes along with patch #6 and supply him with a branch.
> 
> Can you explain the reason to wait for Wolfram's Ack before applying
> #1 - #5?  I would think:
> 
> 1. Create a topic branch.
> 2. Apply patches 1-5 to the topic branch
> 3. Merge the topic branch to your for-next branch
> 
> When Wolfram wants to take patch #6, he can either:
> A. Pull your topic branch
> B. If it's been long enough, patches will already be in ToT and no extra work.
> 
> If I understand correctly, using a topic branch and doing merges /
> pulls means that you can provide Wolfram with stable git hashes when
> he needs them and there will be no merge conflicts.

I don't use TBs for MFD yet, as I've never seen the need.  The current
WoW is to only create extra branches when I have patch{es, sets} to
share.  If I start using a more TB focused methodology it will be
insinuated that the branches are stable - I like the fact that this is
_not_ the case.  Currently I am able to rebase, rework and reorder the
repo as and when I see fit, and do regularly. Except the IBs of course.

> Patches #1 - #5 are bonafide bugfixes irrespective of the i2c tunnel.

I only want to create an IB if I know it's going to be used, else I'd
prefer the patches remain transient.  Why are you so keen to rush into
having these patches applied?  They _will_ make it into v3.15, whether
they are applied immediately or after a length of time (in the case
that Wolfram does not respond).

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 0/7] Add cros_ec changes for newer boards

[Lee Jones]

> >> >>> Need to wait for the ARM, DT and I2C guys to review, at which point
> >> >>> I'll be happy to take in and supply a branch for them to pull from if
> >> >>> required. If there are no _true_ dependencies and the MFD changes can
> >> >>> be added independently without fear of build breakages, let me know
> >> >>> and I'll apply them separately.
> >> >>
> >> >> I believe there aren't direct dependencies between the patches. So, the
> >> >> MFD patches can be applied to the MFD tree and the DT patch applied to
> >> >> the Tegra tree. I'm simply waiting for the MFD patches to be applied
> >> >> before applying the DT patch so that I know the DT binding definition is
> >> >> fully accepted before applying a patch that uses it.
> >> >
> >> > All of the MFD patches are safe to apply and in pretty much arbitrary
> >> > order.  The strong dependencies in the chain are:
> >> >
> >> > * We need patch #5 (mfd: cros_ec: Sync to the latest
> >> > cros_ec_commands.h from EC sources) before the i2c tunnel can compile.
> >> >
> >> > * As Stephen says, he shouldn't apply the device tree until we're
> >> > confident that the bindings are right.  However there's no strong
> >> > dependency otherwise.
> >> >
> >> > * Patches #1 #2 and #3 are simply reliability fixes.  Those could land
> >> > at any point in time and will improve other users of cros_ec_spi (like
> >> > the keyboard on tegra124-venice2).
> >> >
> >> > * Patch #4 can apply any time with no issues.  Without it large i2c
> >> > tunnel transfers won't work, but that's not a terrible problem (all
> >> > normal transfers are small).
> >>
> >> Patch #5 (latest ec commands) can also apply at any time with no
> >> issues, but it's needed for patch #6 (the tunnel) to compile.
> >>
> >> All that being said, I'd request that you merge patches #1-#5 as soon
> >> as you can and make sure you can provide a way that Wolfram can pull
> >> them (or at least patch #5) into his i2c tree to keep them applying
> >> when he is ready to land #6.
> >
> > Very well. So if I can obtain Wolfram's Ack, I can apply the MFD
> > changes along with patch #6 and supply him with a branch.
> 
> Can you explain the reason to wait for Wolfram's Ack before applying
> #1 - #5?  I would think:
> 
> 1. Create a topic branch.
> 2. Apply patches 1-5 to the topic branch
> 3. Merge the topic branch to your for-next branch
> 
> When Wolfram wants to take patch #6, he can either:
> A. Pull your topic branch
> B. If it's been long enough, patches will already be in ToT and no extra work.
> 
> If I understand correctly, using a topic branch and doing merges /
> pulls means that you can provide Wolfram with stable git hashes when
> he needs them and there will be no merge conflicts.

I don't use TBs for MFD yet, as I've never seen the need.  The current
WoW is to only create extra branches when I have patch{es, sets} to
share.  If I start using a more TB focused methodology it will be
insinuated that the branches are stable - I like the fact that this is
_not_ the case.  Currently I am able to rebase, rework and reorder the
repo as and when I see fit, and do regularly. Except the IBs of course.

> Patches #1 - #5 are bonafide bugfixes irrespective of the i2c tunnel.

I only want to create an IB if I know it's going to be used, else I'd
prefer the patches remain transient.  Why are you so keen to rush into
having these patches applied?  They _will_ make it into v3.15, whether
they are applied immediately or after a length of time (in the case
that Wolfram does not respond).

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

Re: [PATCH v2 6/7] i2c: ChromeOS EC tunnel driver

[Wolfram Sang]

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

Hi,

just a basic review to keep things rolling...

> On the original Samsung ARM Chromebook these devices were on an I2C
> bus that was shared between the AP and the EC and arbitrated using
> some extranal GPIOs (see i2c-arb-gpio-challenge).
> 
> The original arbitration scheme worked well enough but had some
> downsides:
> * It was nonstandard (not using standard I2C multimaster)
> * It only worked if the EC-AP communication was I2C
> * It was relatively hard to debug problems (hard to tell if i2c issues
>   were caused by the EC, the AP, or some device on the bus).
> 
> On the HP Chromebook 11 the design was changed to:

This paragraph would be a nice update for the gpio-arbitration docs.

> diff --git a/Documentation/devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt b/Documentation/devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt

The bindings should independently be sent to the devicetree list.

> new file mode 100644
> index 0000000..898f030
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt
> @@ -0,0 +1,39 @@
> +I2C bus that tunnels through the ChromeOS EC (cros-ec)
> +======================================================
> +On some ChromeOS board designs we've got a connection to the EC (embedded
> +controller) but no direct connection to some devices on the other side of
> +the EC (like a battery and PMIC).  To get access to those devices we need
> +to tunnel our i2c commands through the EC.
> +
> +The node for this device should be under a cros-ec node like google,cros-ec-spi
> +or google,cros-ec-i2c.
> +
> +
> +Required properties:
> +- compatible: google,cros-ec-i2c-tunnel
> +- google,remote-bus: The EC bus we'd like to talk to.
> +
> +Optional child nodes:
> +- One node per I2C device connected to the tunnelled I2C bus.
> +
> +
> +Example:
> +	cros-ec@0 {
> +		compatible = "google,cros-ec-spi";
> +
> +		...
> +
> +		i2c-tunnel {
> +			compatible = "google,cros-ec-i2c-tunnel";
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			google,remote-bus = <0>;
> +
> +			battery: sbs-battery@b {
> +				compatible = "sbs,sbs-battery";
> +				reg = <0xb>;
> +				sbs,poll-retry-count = <1>;
> +			};
> +		};
> +	}

Can the tunnel have n busses? How to represent them then? I think the
remote-bus property should go in favor of proper sub-nodes? Wouldn't it
also be more generic to have the tunnel node seperate and reference the
tunnel-mechanism (spi here) as a phandle?

> +/**
> + * ec_i2c_construct_message - construct a message to go to the EC
> + *
> + * This function effectively stuffs the standard i2c_msg format of Linux into
> + * a format that the EC understands.
> + *
> + * @buf: The buffer to fill.  Can pass NULL to count how many bytes the message
> + *       would be.

I wonder about this NULL thing. That means the size is calculated twice.
Why not make two functions instead, one fir size calc, one for setting
up?

> +/**
> + * ec_i2c_parse_response - Parse a response from the EC
> + *
> + * We'll take the EC's response and copy it back into msgs.
> + *
> + * @buf: The buffer to parse.  Can pass NULL to count how many bytes we expect
> + *	 the response to be. Otherwise we assume that the right number of
> + *	 bytes are available.

Ditto.

> +	result = bus->ec->command_sendrecv(bus->ec, EC_CMD_I2C_PASSTHRU,
> +					   request, request_len,
> +					   response, response_len);

This function pointer should be checked against NULL in probe, I
think.

> +static int ec_i2c_probe(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct cros_ec_device *ec = dev_get_drvdata(pdev->dev.parent);
> +	struct device *dev = &pdev->dev;
> +	struct ec_i2c_device *bus = NULL;
> +	u32 remote_bus;
> +	int err;
> +
> +	dev_dbg(dev, "Probing\n");

Drop. Device core has it already.

> +
> +	if (!np) {
> +		dev_err(dev, "no device node\n");
> +		return -ENOENT;
> +	}

Can this happen?

> +
> +	bus = devm_kzalloc(dev, sizeof(*bus), GFP_KERNEL);
> +	if (bus == NULL) {
> +		dev_err(dev, "cannot allocate bus device\n");

No need for error strings when allocating.

> +		return -ENOMEM;
> +	}
> +
> +	dev_dbg(dev, "ChromeOS EC I2C tunnel adapter\n");

Drop. Device core debug has it, too.

> +
> +	return err;
> +}
> +
> +static int ec_i2c_remove(struct platform_device *dev)
> +{
> +	struct ec_i2c_device *bus = platform_get_drvdata(dev);
> +
> +	platform_set_drvdata(dev, NULL);

Not needed.

> +
> +	i2c_del_adapter(&bus->adap);
> +
> +	return 0;
> +}
> +

Regards,

   Wolfram


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

Re: [PATCH v2 6/7] i2c: ChromeOS EC tunnel driver

[Wolfram Sang]

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

Hi,

just a basic review to keep things rolling...

> On the original Samsung ARM Chromebook these devices were on an I2C
> bus that was shared between the AP and the EC and arbitrated using
> some extranal GPIOs (see i2c-arb-gpio-challenge).
> 
> The original arbitration scheme worked well enough but had some
> downsides:
> * It was nonstandard (not using standard I2C multimaster)
> * It only worked if the EC-AP communication was I2C
> * It was relatively hard to debug problems (hard to tell if i2c issues
>   were caused by the EC, the AP, or some device on the bus).
> 
> On the HP Chromebook 11 the design was changed to:

This paragraph would be a nice update for the gpio-arbitration docs.

> diff --git a/Documentation/devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt b/Documentation/devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt

The bindings should independently be sent to the devicetree list.

> new file mode 100644
> index 0000000..898f030
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt
> @@ -0,0 +1,39 @@
> +I2C bus that tunnels through the ChromeOS EC (cros-ec)
> +======================================================
> +On some ChromeOS board designs we've got a connection to the EC (embedded
> +controller) but no direct connection to some devices on the other side of
> +the EC (like a battery and PMIC).  To get access to those devices we need
> +to tunnel our i2c commands through the EC.
> +
> +The node for this device should be under a cros-ec node like google,cros-ec-spi
> +or google,cros-ec-i2c.
> +
> +
> +Required properties:
> +- compatible: google,cros-ec-i2c-tunnel
> +- google,remote-bus: The EC bus we'd like to talk to.
> +
> +Optional child nodes:
> +- One node per I2C device connected to the tunnelled I2C bus.
> +
> +
> +Example:
> +	cros-ec@0 {
> +		compatible = "google,cros-ec-spi";
> +
> +		...
> +
> +		i2c-tunnel {
> +			compatible = "google,cros-ec-i2c-tunnel";
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			google,remote-bus = <0>;
> +
> +			battery: sbs-battery@b {
> +				compatible = "sbs,sbs-battery";
> +				reg = <0xb>;
> +				sbs,poll-retry-count = <1>;
> +			};
> +		};
> +	}

Can the tunnel have n busses? How to represent them then? I think the
remote-bus property should go in favor of proper sub-nodes? Wouldn't it
also be more generic to have the tunnel node seperate and reference the
tunnel-mechanism (spi here) as a phandle?

> +/**
> + * ec_i2c_construct_message - construct a message to go to the EC
> + *
> + * This function effectively stuffs the standard i2c_msg format of Linux into
> + * a format that the EC understands.
> + *
> + * @buf: The buffer to fill.  Can pass NULL to count how many bytes the message
> + *       would be.

I wonder about this NULL thing. That means the size is calculated twice.
Why not make two functions instead, one fir size calc, one for setting
up?

> +/**
> + * ec_i2c_parse_response - Parse a response from the EC
> + *
> + * We'll take the EC's response and copy it back into msgs.
> + *
> + * @buf: The buffer to parse.  Can pass NULL to count how many bytes we expect
> + *	 the response to be. Otherwise we assume that the right number of
> + *	 bytes are available.

Ditto.

> +	result = bus->ec->command_sendrecv(bus->ec, EC_CMD_I2C_PASSTHRU,
> +					   request, request_len,
> +					   response, response_len);

This function pointer should be checked against NULL in probe, I
think.

> +static int ec_i2c_probe(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct cros_ec_device *ec = dev_get_drvdata(pdev->dev.parent);
> +	struct device *dev = &pdev->dev;
> +	struct ec_i2c_device *bus = NULL;
> +	u32 remote_bus;
> +	int err;
> +
> +	dev_dbg(dev, "Probing\n");

Drop. Device core has it already.

> +
> +	if (!np) {
> +		dev_err(dev, "no device node\n");
> +		return -ENOENT;
> +	}

Can this happen?

> +
> +	bus = devm_kzalloc(dev, sizeof(*bus), GFP_KERNEL);
> +	if (bus == NULL) {
> +		dev_err(dev, "cannot allocate bus device\n");

No need for error strings when allocating.

> +		return -ENOMEM;
> +	}
> +
> +	dev_dbg(dev, "ChromeOS EC I2C tunnel adapter\n");

Drop. Device core debug has it, too.

> +
> +	return err;
> +}
> +
> +static int ec_i2c_remove(struct platform_device *dev)
> +{
> +	struct ec_i2c_device *bus = platform_get_drvdata(dev);
> +
> +	platform_set_drvdata(dev, NULL);

Not needed.

> +
> +	i2c_del_adapter(&bus->adap);
> +
> +	return 0;
> +}
> +

Regards,

   Wolfram


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

Re: [PATCH v2 0/7] Add cros_ec changes for newer boards

[Doug Anderson]

Lee,

On Tue, Apr 29, 2014 at 1:21 AM, Lee Jones <lee.jones@linaro.org> wrote:
> I don't use TBs for MFD yet, as I've never seen the need.  The current
> WoW is to only create extra branches when I have patch{es, sets} to
> share.  If I start using a more TB focused methodology it will be
> insinuated that the branches are stable - I like the fact that this is
> _not_ the case.  Currently I am able to rebase, rework and reorder the
> repo as and when I see fit, and do regularly. Except the IBs of course.

OK.

>> Patches #1 - #5 are bonafide bugfixes irrespective of the i2c tunnel.
>
> I only want to create an IB if I know it's going to be used, else I'd
> prefer the patches remain transient.  Why are you so keen to rush into
> having these patches applied?  They _will_ make it into v3.15, whether
> they are applied immediately or after a length of time (in the case
> that Wolfram does not respond).

No strong reason, and it's actually not even a huge deal if they make
it to 3.15 or in 3.16.  Having outstanding patches simply increases
the number of things that I need to keep track of / check up on.  If
patches are good to go and reviewed I like to get them landed.

Another reason I'd love to see patches landed sooner is that it will
unblock me sending the next set of patches up.  I collected all of the
most important patches in this series, but there are a bunch of other
patches in our tree that would be nice to eventually send up.  At the
moment I'm in a position where I can dedicate a reasonable amount of
time to upstreaming.  It's likely that before long I will get sucked
into tight deadlines and will have to squeeze upstreaming in among
other priorities.

I see a response from Wolfram now, so I'll spin a V2 in the next day
or two with changes to the tunnel driver.  I'm at ELC so my hacking
time may be limited.

-Doug

[PATCH v2 0/7] Add cros_ec changes for newer boards

[Doug Anderson]

Lee,

On Tue, Apr 29, 2014 at 1:21 AM, Lee Jones <lee.jones@linaro.org> wrote:
> I don't use TBs for MFD yet, as I've never seen the need.  The current
> WoW is to only create extra branches when I have patch{es, sets} to
> share.  If I start using a more TB focused methodology it will be
> insinuated that the branches are stable - I like the fact that this is
> _not_ the case.  Currently I am able to rebase, rework and reorder the
> repo as and when I see fit, and do regularly. Except the IBs of course.

OK.

>> Patches #1 - #5 are bonafide bugfixes irrespective of the i2c tunnel.
>
> I only want to create an IB if I know it's going to be used, else I'd
> prefer the patches remain transient.  Why are you so keen to rush into
> having these patches applied?  They _will_ make it into v3.15, whether
> they are applied immediately or after a length of time (in the case
> that Wolfram does not respond).

No strong reason, and it's actually not even a huge deal if they make
it to 3.15 or in 3.16.  Having outstanding patches simply increases
the number of things that I need to keep track of / check up on.  If
patches are good to go and reviewed I like to get them landed.

Another reason I'd love to see patches landed sooner is that it will
unblock me sending the next set of patches up.  I collected all of the
most important patches in this series, but there are a bunch of other
patches in our tree that would be nice to eventually send up.  At the
moment I'm in a position where I can dedicate a reasonable amount of
time to upstreaming.  It's likely that before long I will get sucked
into tight deadlines and will have to squeeze upstreaming in among
other priorities.

I see a response from Wolfram now, so I'll spin a V2 in the next day
or two with changes to the tunnel driver.  I'm at ELC so my hacking
time may be limited.

-Doug

Re: [PATCH v2 6/7] i2c: ChromeOS EC tunnel driver

[Doug Anderson]

Wolfram,

On Tue, Apr 29, 2014 at 5:33 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
> Hi,
>
> just a basic review to keep things rolling...

Thanks for the review!  It sounds like other changes are blocked on
this one landing, so hopefully we can make some good progress!  :)

>> On the original Samsung ARM Chromebook these devices were on an I2C
>> bus that was shared between the AP and the EC and arbitrated using
>> some extranal GPIOs (see i2c-arb-gpio-challenge).
>>
>> The original arbitration scheme worked well enough but had some
>> downsides:
>> * It was nonstandard (not using standard I2C multimaster)
>> * It only worked if the EC-AP communication was I2C
>> * It was relatively hard to debug problems (hard to tell if i2c issues
>>   were caused by the EC, the AP, or some device on the bus).
>>
>> On the HP Chromebook 11 the design was changed to:
>
> This paragraph would be a nice update for the gpio-arbitration docs.

Done.  <https://patchwork.kernel.org/patch/4088551/>


>> diff --git a/Documentation/devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt b/Documentation/devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt
>
> The bindings should independently be sent to the devicetree list.
>
>> new file mode 100644
>> index 0000000..898f030
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/i2c/i2c-cros-ec-tunnel.txt
>> @@ -0,0 +1,39 @@
>> +I2C bus that tunnels through the ChromeOS EC (cros-ec)
>> +======================================================
>> +On some ChromeOS board designs we've got a connection to the EC (embedded
>> +controller) but no direct connection to some devices on the other side of
>> +the EC (like a battery and PMIC).  To get access to those devices we need
>> +to tunnel our i2c commands through the EC.
>> +
>> +The node for this device should be under a cros-ec node like google,cros-ec-spi
>> +or google,cros-ec-i2c.
>> +
>> +
>> +Required properties:
>> +- compatible: google,cros-ec-i2c-tunnel
>> +- google,remote-bus: The EC bus we'd like to talk to.
>> +
>> +Optional child nodes:
>> +- One node per I2C device connected to the tunnelled I2C bus.
>> +
>> +
>> +Example:
>> +     cros-ec@0 {
>> +             compatible = "google,cros-ec-spi";
>> +
>> +             ...
>> +
>> +             i2c-tunnel {
>> +                     compatible = "google,cros-ec-i2c-tunnel";
>> +                     #address-cells = <1>;
>> +                     #size-cells = <0>;
>> +
>> +                     google,remote-bus = <0>;
>> +
>> +                     battery: sbs-battery@b {
>> +                             compatible = "sbs,sbs-battery";
>> +                             reg = <0xb>;
>> +                             sbs,poll-retry-count = <1>;
>> +                     };
>> +             };
>> +     }
>
> Can the tunnel have n busses? How to represent them then? I think the
> remote-bus property should go in favor of proper sub-nodes?

I suppose it could have n busses, though today's ECs don't.

I think that the binding could later be extended to support this with:

        cros-ec@0 {
                compatible = "google,cros-ec-spi";

                ...

                i2c-tunnel0 {
                        compatible = "google,cros-ec-i2c-tunnel";
                        #address-cells = <1>;
                        #size-cells = <0>;

                        google,remote-bus = <0>;

                        battery: sbs-battery@b {
                                compatible = "sbs,sbs-battery";
                                reg = <0xb>;
                                sbs,poll-retry-count = <1>;
                        };
                };

                i2c-tunnel1 {
                        compatible = "google,cros-ec-i2c-tunnel";
                        #address-cells = <1>;
                        #size-cells = <0>;

                        google,remote-bus = <1>;

                        battery: sbs-battery@b {
                                compatible = "sbs,sbs-battery";
                                reg = <0xb>;
                                sbs,poll-retry-count = <1>;
                        };
                };
       }

...but since there's no need now, it seems like we could just leave it
with supporting 1 for now.  If we extend it like above then old device
trees will still work on newer kernels.

Note about "google,remote-bus" instead of "reg".   The main
"google,cros-ec-spi" generally has non-addressed sub-nodes.  It has no
"#address-cells" or "#size-cells".  The keyboard component, for
instance, doesn't have an "address".  You talk to it using the
keyboard commands.

...I don't think it's generally possible to have some sub nodes that
have addresses and other sub nodes that don't.  I'm happy to be
corrected, though.


Note: according to the device tree stable bindings theorem, I don't
think I'm allowed to retroactively go back and say that the keyboard
node needs a dummy "reg = <0>;" nor can I require that
"google,cros-ec-spi" add #address-cells and #size-cells.  Both device
tree bindings have been in kernel since ~3.9.


Can you give me an example of what you mean by "proper sub nodes"?


> Wouldn't it
> also be more generic to have the tunnel node seperate and reference the
> tunnel-mechanism (spi here) as a phandle?

What makes it more generic?  It's still a MFD and still only works
atop the cros_ec, so it should be under the cros_ec device, shouldn't
it?

I know use a phandle for mux, but from
<https://lkml.org/lkml/2013/2/13/517> it sounds like we were forced
into it.


>> +/**
>> + * ec_i2c_construct_message - construct a message to go to the EC
>> + *
>> + * This function effectively stuffs the standard i2c_msg format of Linux into
>> + * a format that the EC understands.
>> + *
>> + * @buf: The buffer to fill.  Can pass NULL to count how many bytes the message
>> + *       would be.
>
> I wonder about this NULL thing. That means the size is calculated twice.
> Why not make two functions instead, one fir size calc, one for setting
> up?

Done.


>> +/**
>> + * ec_i2c_parse_response - Parse a response from the EC
>> + *
>> + * We'll take the EC's response and copy it back into msgs.
>> + *
>> + * @buf: The buffer to parse.  Can pass NULL to count how many bytes we expect
>> + *    the response to be. Otherwise we assume that the right number of
>> + *    bytes are available.
>
> Ditto.

Done.


>> +     result = bus->ec->command_sendrecv(bus->ec, EC_CMD_I2C_PASSTHRU,
>> +                                        request, request_len,
>> +                                        response, response_len);
>
> This function pointer should be checked against NULL in probe, I
> think.

Done.


>> +static int ec_i2c_probe(struct platform_device *pdev)
>> +{
>> +     struct device_node *np = pdev->dev.of_node;
>> +     struct cros_ec_device *ec = dev_get_drvdata(pdev->dev.parent);
>> +     struct device *dev = &pdev->dev;
>> +     struct ec_i2c_device *bus = NULL;
>> +     u32 remote_bus;
>> +     int err;
>> +
>> +     dev_dbg(dev, "Probing\n");
>
> Drop. Device core has it already.

Done.


>> +
>> +     if (!np) {
>> +             dev_err(dev, "no device node\n");
>> +             return -ENOENT;
>> +     }
>
> Can this happen?

Not that I know of.  I assume you want me to remove this, so done.


>> +     bus = devm_kzalloc(dev, sizeof(*bus), GFP_KERNEL);
>> +     if (bus == NULL) {
>> +             dev_err(dev, "cannot allocate bus device\n");
>
> No need for error strings when allocating.

Done.


>> +             return -ENOMEM;
>> +     }
>> +
>> +     dev_dbg(dev, "ChromeOS EC I2C tunnel adapter\n");
>
> Drop. Device core debug has it, too.

Done.


>> +
>> +     return err;
>> +}
>> +
>> +static int ec_i2c_remove(struct platform_device *dev)
>> +{
>> +     struct ec_i2c_device *bus = platform_get_drvdata(dev);
>> +
>> +     platform_set_drvdata(dev, NULL);
>
> Not needed.

Duh, thanks.  Done.


>> +     i2c_del_adapter(&bus->adap);
>> +
>> +     return 0;
>> +}
>> +

Sending up a v3 with fixes...

-Doug