[PATCH v5 1/2] mfd: cros ec: spi: Add delay for raising CS

[PATCH v5 1/2] mfd: cros ec: spi: Add delay for raising CS

[Thierry Reding]

From: Rhyland Klein <rklein@nvidia.com>

The EC has specific timing it requires. Add support for an optional delay
after raising CS to fix timing issues. This is configurable based on
a DT property "google,cros-ec-spi-msg-delay".

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

Signed-off-by: Rhyland Klein <rklein@nvidia.com>
Reviewed-by: Bernie Thompson <bhthompson@chromium.org>
Reviewed-by: Andrew Bresticker <abrestic@chromium.org>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v5:
- modify correct hunk to use 0 instead of '\0'
- add Mark's Acked-by

Changes in v4:
- unconditionally call cros_ec_spi_dt_probe() since the driver now
  depends on OF

Changes in v3:
- rename cros_ec_probe_spi_dt() to cros_ec_spi_dt_probe()
- use 0 instead of '\0'

Changes in v2:
- make property description more verbose

 Documentation/devicetree/bindings/mfd/cros-ec.txt |  9 +++++++
 drivers/mfd/cros_ec_spi.c                         | 29 +++++++++++++++++++++++
 2 files changed, 38 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/cros-ec.txt b/Documentation/devicetree/bindings/mfd/cros-ec.txt
index 5f229c5f6da9..8009c3d87f33 100644
--- a/Documentation/devicetree/bindings/mfd/cros-ec.txt
+++ b/Documentation/devicetree/bindings/mfd/cros-ec.txt
@@ -17,6 +17,15 @@ Required properties (SPI):
 - compatible: "google,cros-ec-spi"
 - reg: SPI chip select
 
+Optional properties (SPI):
+- google,cros-ec-spi-msg-delay: Some implementations of the EC require some
+  additional processing time in order to accept new transactions. If the delay
+  between transactions is not long enough the EC may not be able to respond
+  properly to subsequent transactions and cause them to hang. This property
+  specifies the delay, in usecs, introduced between transactions to account
+  for the time required by the EC to get back into a state in which new data
+  can be accepted.
+
 Required properties (LPC):
 - compatible: "google,cros-ec-lpc"
 - reg: List of (IO address, size) pairs defining the interface uses
diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
index d64d88a9e247..8e262bfb603e 100644
--- a/drivers/mfd/cros_ec_spi.c
+++ b/drivers/mfd/cros_ec_spi.c
@@ -18,6 +18,7 @@
 #include <linux/module.h>
 #include <linux/mfd/cros_ec.h>
 #include <linux/mfd/cros_ec_commands.h>
+#include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/spi/spi.h>
@@ -62,10 +63,13 @@
  * @spi: SPI device we are connected to
  * @last_transfer_ns: time that we last finished a transfer, or 0 if there
  *	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.
  */
 struct cros_ec_spi {
 	struct spi_device *spi;
 	s64 last_transfer_ns;
+	unsigned int end_of_msg_delay;
 };
 
 static void debug_packet(struct device *dev, const char *name, u8 *ptr,
@@ -238,6 +242,17 @@ static int cros_ec_command_spi_xfer(struct cros_ec_device *ec_dev,
 
 	/* turn off CS */
 	spi_message_init(&msg);
+
+	if (ec_spi->end_of_msg_delay) {
+		/*
+		 * Add delay for last transaction, to ensure the rising edge
+		 * doesn't come too soon after the end of the data.
+		 */
+		memset(&trans, 0, sizeof(trans));
+		trans.delay_usecs = ec_spi->end_of_msg_delay;
+		spi_message_add_tail(&trans, &msg);
+	}
+
 	final_ret = spi_sync(ec_spi->spi, &msg);
 	ktime_get_ts(&ts);
 	ec_spi->last_transfer_ns = timespec_to_ns(&ts);
@@ -284,6 +299,17 @@ static int cros_ec_command_spi_xfer(struct cros_ec_device *ec_dev,
 	return 0;
 }
 
+static void cros_ec_spi_dt_probe(struct cros_ec_spi *ec_spi, struct device *dev)
+{
+	struct device_node *np = dev->of_node;
+	u32 val;
+	int ret;
+
+	ret = of_property_read_u32(np, "google,cros-ec-spi-msg-delay", &val);
+	if (!ret)
+		ec_spi->end_of_msg_delay = val;
+}
+
 static int cros_ec_spi_probe(struct spi_device *spi)
 {
 	struct device *dev = &spi->dev;
@@ -305,6 +331,9 @@ static int cros_ec_spi_probe(struct spi_device *spi)
 	if (!ec_dev)
 		return -ENOMEM;
 
+	/* Check for any DT properties */
+	cros_ec_spi_dt_probe(ec_spi, dev);
+
 	spi_set_drvdata(spi, ec_dev);
 	ec_dev->name = "SPI";
 	ec_dev->dev = dev;
-- 
1.8.4.2

[PATCH v5 2/2] mfd: cros ec: spi: Use 0 instead of '\0' consistently

[Thierry Reding]

memset() was being called with the second parameter set to '\0', which
is equivalent but longer than the more canonical 0. Update the code to
use the latter variant consistently across the driver.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v5:
- new patch

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

diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
index 8e262bfb603e..84af8d7a4295 100644
--- a/drivers/mfd/cros_ec_spi.c
+++ b/drivers/mfd/cros_ec_spi.c
@@ -112,7 +112,7 @@ 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 {
-		memset(&trans, '\0', sizeof(trans));
+		memset(&trans, 0, sizeof(trans));
 		trans.cs_change = 1;
 		trans.rx_buf = ptr = ec_dev->din;
 		trans.len = EC_MSG_PREAMBLE_COUNT;
@@ -164,7 +164,7 @@ static int cros_ec_spi_receive_response(struct cros_ec_device *ec_dev,
 		dev_dbg(ec_dev->dev, "loop, todo=%d, need_len=%d, ptr=%zd\n",
 			todo, need_len, ptr - ec_dev->din);
 
-		memset(&trans, '\0', sizeof(trans));
+		memset(&trans, 0, sizeof(trans));
 		trans.cs_change = 1;
 		trans.rx_buf = ptr;
 		trans.len = todo;
@@ -224,7 +224,7 @@ static int cros_ec_command_spi_xfer(struct cros_ec_device *ec_dev,
 
 	/* Transmit phase - send our message */
 	debug_packet(ec_dev->dev, "out", ec_dev->dout, len);
-	memset(&trans, '\0', sizeof(trans));
+	memset(&trans, 0, sizeof(trans));
 	trans.tx_buf = ec_dev->dout;
 	trans.len = len;
 	trans.cs_change = 1;
-- 
1.8.4.2

Re: [PATCH v5 1/2] mfd: cros ec: spi: Add delay for raising CS

[Lee Jones]

> From: Rhyland Klein <rklein@nvidia.com>
> 
> The EC has specific timing it requires. Add support for an optional delay
> after raising CS to fix timing issues. This is configurable based on
> a DT property "google,cros-ec-spi-msg-delay".
> 
> If this property isn't set, then no delay will be added. However, if set
> it will cause a delay equal to the value passed to it to be inserted at
> the end of a transaction.
> 
> Signed-off-by: Rhyland Klein <rklein@nvidia.com>
> Reviewed-by: Bernie Thompson <bhthompson@chromium.org>
> Reviewed-by: Andrew Bresticker <abrestic@chromium.org>
> Acked-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> Changes in v5:
> - modify correct hunk to use 0 instead of '\0'
> - add Mark's Acked-by
> 
> Changes in v4:
> - unconditionally call cros_ec_spi_dt_probe() since the driver now
>   depends on OF
> 
> Changes in v3:
> - rename cros_ec_probe_spi_dt() to cros_ec_spi_dt_probe()
> - use 0 instead of '\0'
> 
> Changes in v2:
> - make property description more verbose
> 
>  Documentation/devicetree/bindings/mfd/cros-ec.txt |  9 +++++++
>  drivers/mfd/cros_ec_spi.c                         | 29 +++++++++++++++++++++++
>  2 files changed, 38 insertions(+)

Applied, thanks.

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

Re: [PATCH v5 2/2] mfd: cros ec: spi: Use 0 instead of '\0' consistently

[Lee Jones]

On Mon, 16 Dec 2013, Thierry Reding wrote:

> memset() was being called with the second parameter set to '\0', which
> is equivalent but longer than the more canonical 0. Update the code to
> use the latter variant consistently across the driver.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> Changes in v5:
> - new patch
> 
>  drivers/mfd/cros_ec_spi.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Applied, thanks.

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