All of lore.kernel.org
 help / color / mirror / Atom feed
* [RESEND PATCH 0/7] Second batch of cleanups for cros_ec
@ 2014-08-20 12:13 Javier Martinez Canillas
  2014-08-20 12:13 ` [RESEND PATCH 1/7] mfd: cros_ec: Delay for 50ms when we see EC_CMD_REBOOT_EC Javier Martinez Canillas
                   ` (3 more replies)
  0 siblings, 4 replies; 19+ messages in thread
From: Javier Martinez Canillas @ 2014-08-20 12:13 UTC (permalink / raw)
  To: Lee Jones
  Cc: Wolfram Sang, Dmitry Torokhov, Doug Anderson, Simon Glass,
	Bill Richardson, Andrew Bresticker, Derek Basehore, Todd Broch,
	Olof Johansson, Andreas Färber,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
	Javier Martinez Canillas

Hello Lee,

This is a resend of a patch series originally sent [0] almost a
month ago (July, 28). The series add a batch of cleanups patches
for the mfd cros_ec driver and its subdevices drivers. The first
batch of cleanups was posted by Doug Anderson [1] and have already
been merged. The patches were taken from the ChromeOS 3.8 kernel
and after this series, no cleanups patches for cros_ec are left.
The remaining commits add support not yet available in mainline.

There is almost no functionality added on this series but the
idea is to reduce the delta between the mainline drivers and
the ones in the downstream Chrome OS 3.8 kernel so the missing
functionality can be added on top once these cleanups patches
are merged. The missing functionlity currently in mainline is:

- Chrome OS Embedded Controller userspace device interface
- Chrome OS Embedded Controller Low Pin Count (LPC) inteface
- Access to vboot context stored on a block device
- Access to vboot context stored on EC's nvram

The patches in this series are authored by different people
(all on cc) and consist of the following:

Andrew Bresticker (3):
  mfd: cros_ec: stop calling ->cmd_xfer() directly
  mfd: cros_ec: move locking into cros_ec_cmd_xfer
  mfd: cros_ec: wait for completion of commands that return IN_PROGRESS

Derek Basehore (1):
  i2c: i2c-cros-ec-tunnel: Set retries to 3

Doug Anderson (1):
  mfd: cros_ec: Delay for 50ms when we see EC_CMD_REBOOT_EC

Todd Broch (2):
  mfd: cros_ec: Instantiate sub-devices from device tree
  Input: cros_ec_keyb: Optimize ghosting algorithm.

 drivers/i2c/busses/i2c-cros-ec-tunnel.c |  5 +-
 drivers/input/keyboard/cros_ec_keyb.c   | 89 +++++++++++++++++----------------
 drivers/mfd/cros_ec.c                   | 88 ++++++++++++++++++++++++++++----
 drivers/mfd/cros_ec_spi.c               | 20 ++++----
 include/linux/mfd/cros_ec.h             | 24 ++++++---
 5 files changed, 154 insertions(+), 72 deletions(-)

There were no changes on this resend, just picked Acked-by and
Tested-by tags and also added my own Signed-off-by tag to all
the patches as suggested by Andreas Färber even when I just
picked them from downstream and rebased on top of linux-next.

The patches should be merged together which means that they
should go through your mfd tree once the relevant acks are
obtained.

Best regards,
Javier

[0]: https://www.mail-archive.com/linux-input-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg11385.html
[1]: https://lkml.org/lkml/2014/6/16/681

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [RESEND PATCH 1/7] mfd: cros_ec: Delay for 50ms when we see EC_CMD_REBOOT_EC
  2014-08-20 12:13 [RESEND PATCH 0/7] Second batch of cleanups for cros_ec Javier Martinez Canillas
@ 2014-08-20 12:13 ` Javier Martinez Canillas
  2014-08-21 13:37   ` Lee Jones
       [not found] ` <1408536812-7836-1-git-send-email-javier.martinez-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 19+ messages in thread
From: Javier Martinez Canillas @ 2014-08-20 12:13 UTC (permalink / raw)
  To: Lee Jones
  Cc: Wolfram Sang, Dmitry Torokhov, Doug Anderson, Simon Glass,
	Bill Richardson, Andrew Bresticker, Derek Basehore, Todd Broch,
	Olof Johansson, Andreas Färber, linux-i2c, linux-input,
	linux-samsung-soc, Javier Martinez Canillas

From: Doug Anderson <dianders@chromium.org>

If someone sends a EC_CMD_REBOOT_EC to the EC, the EC will likely be
unresponsive for quite a while.  Add a delay to the end of the command
to prevent random failures of future commands.

NOTES:
* This could be optimized a bit by simply delaying the next command
  sent, but EC_CMD_REBOOT_EC is such a rare command that the extra
  complexity doesn't seem worth it.
* This is a bit of an "ugly hack" since the SPI driver is effectively
  snooping on the communication and making a lot of assumptions.  It
  would be nice to architect in some better solution long term.
* This same logic probably needs to be applied to the i2c driver.

Signed-off-by: Doug Anderson <dianders@chromium.org>
Reviewed-by: Randall Spangler <rspangler@chromium.org>
Reviewed-by: Vadim Bendebury <vbendeb@chromium.org>
Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 drivers/mfd/cros_ec_spi.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
index 588c700..b396705 100644
--- a/drivers/mfd/cros_ec_spi.c
+++ b/drivers/mfd/cros_ec_spi.c
@@ -65,6 +65,12 @@
   */
 #define EC_SPI_RECOVERY_TIME_NS	(200 * 1000)
 
+/*
+ * The EC is unresponsive for a time after a reboot command.  Add a
+ * simple delay to make sure that the bus stays locked.
+ */
+#define EC_REBOOT_DELAY_MS	50
+
 /**
  * struct cros_ec_spi - information about a SPI-connected EC
  *
@@ -318,6 +324,9 @@ static int cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev,
 
 	ret = len;
 exit:
+	if (ec_msg->command == EC_CMD_REBOOT_EC)
+		msleep(EC_REBOOT_DELAY_MS);
+
 	mutex_unlock(&ec_spi->lock);
 	return ret;
 }
-- 
2.0.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RESEND PATCH 2/7] i2c: i2c-cros-ec-tunnel: Set retries to 3
       [not found] ` <1408536812-7836-1-git-send-email-javier.martinez-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
@ 2014-08-20 12:13   ` Javier Martinez Canillas
  2014-08-20 12:13   ` [RESEND PATCH 3/7] mfd: cros_ec: stop calling ->cmd_xfer() directly Javier Martinez Canillas
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 19+ messages in thread
From: Javier Martinez Canillas @ 2014-08-20 12:13 UTC (permalink / raw)
  To: Lee Jones
  Cc: Wolfram Sang, Dmitry Torokhov, Doug Anderson, Simon Glass,
	Bill Richardson, Andrew Bresticker, Derek Basehore, Todd Broch,
	Olof Johansson, Andreas Färber,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
	Javier Martinez Canillas

From: Derek Basehore <dbasehore-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

Since the i2c bus can get wedged on the EC sometimes, set the number of retries
to 3. Since we un-wedge the bus immediately after the wedge happens, this is the
correct fix since only one transfer will fail.

Signed-off-by: Derek Basehore <dbasehore-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Reviewed-by: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Acked-by: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
Signed-off-by: Javier Martinez Canillas <javier.martinez-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
---
 drivers/i2c/busses/i2c-cros-ec-tunnel.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/i2c/busses/i2c-cros-ec-tunnel.c b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
index 05e033c..a4411da 100644
--- a/drivers/i2c/busses/i2c-cros-ec-tunnel.c
+++ b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
@@ -16,6 +16,8 @@
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 
+#define I2C_MAX_RETRIES 3
+
 /**
  * struct ec_i2c_device - Driver data for I2C tunnel
  *
@@ -290,6 +292,7 @@ static int ec_i2c_probe(struct platform_device *pdev)
 	bus->adap.algo_data = bus;
 	bus->adap.dev.parent = &pdev->dev;
 	bus->adap.dev.of_node = np;
+	bus->adap.retries = I2C_MAX_RETRIES;
 
 	err = i2c_add_adapter(&bus->adap);
 	if (err) {
-- 
2.0.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RESEND PATCH 3/7] mfd: cros_ec: stop calling ->cmd_xfer() directly
       [not found] ` <1408536812-7836-1-git-send-email-javier.martinez-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
  2014-08-20 12:13   ` [RESEND PATCH 2/7] i2c: i2c-cros-ec-tunnel: Set retries to 3 Javier Martinez Canillas
@ 2014-08-20 12:13   ` Javier Martinez Canillas
  2014-08-20 22:33     ` Doug Anderson
  2014-08-21 14:08     ` Lee Jones
  2014-08-20 12:13   ` [RESEND PATCH 6/7] mfd: cros_ec: Instantiate sub-devices from device tree Javier Martinez Canillas
  2014-08-20 12:13   ` [RESEND PATCH 7/7] Input: cros_ec_keyb: Optimize ghosting algorithm Javier Martinez Canillas
  3 siblings, 2 replies; 19+ messages in thread
From: Javier Martinez Canillas @ 2014-08-20 12:13 UTC (permalink / raw)
  To: Lee Jones
  Cc: Wolfram Sang, Dmitry Torokhov, Doug Anderson, Simon Glass,
	Bill Richardson, Andrew Bresticker, Derek Basehore, Todd Broch,
	Olof Johansson, Andreas Färber,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
	Javier Martinez Canillas

From: Andrew Bresticker <abrestic-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

Instead of having users of the ChromeOS EC call the interface-specific
cmd_xfer() callback directly, introduce a central cros_ec_cmd_xfer()
to use instead.  This will allow us to put all the locking and retry
logic in one place instead of duplicating it across the different
drivers.

Signed-off-by: Andrew Bresticker <abrestic-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Reviewed-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Signed-off-by: Javier Martinez Canillas <javier.martinez-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
---
 drivers/i2c/busses/i2c-cros-ec-tunnel.c |  2 +-
 drivers/input/keyboard/cros_ec_keyb.c   |  2 +-
 drivers/mfd/cros_ec.c                   |  7 +++++++
 include/linux/mfd/cros_ec.h             | 24 ++++++++++++++++++------
 4 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/busses/i2c-cros-ec-tunnel.c b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
index a4411da..8ca5cbb 100644
--- a/drivers/i2c/busses/i2c-cros-ec-tunnel.c
+++ b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
@@ -229,7 +229,7 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg i2c_msgs[],
 	msg.indata = response;
 	msg.insize = response_len;
 
-	result = bus->ec->cmd_xfer(bus->ec, &msg);
+	result = cros_ec_cmd_xfer(bus->ec, &msg);
 	if (result < 0)
 		goto exit;
 
diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
index 791781a..93111d1 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -182,7 +182,7 @@ static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t *kb_state)
 		.insize = ckdev->cols,
 	};
 
-	return ckdev->ec->cmd_xfer(ckdev->ec, &msg);
+	return cros_ec_cmd_xfer(ckdev->ec, &msg);
 }
 
 static irqreturn_t cros_ec_keyb_irq(int irq, void *data)
diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index 4873f9c..a9faebd 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -62,6 +62,13 @@ int cros_ec_check_result(struct cros_ec_device *ec_dev,
 }
 EXPORT_SYMBOL(cros_ec_check_result);
 
+int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
+		     struct cros_ec_command *msg)
+{
+	return ec_dev->cmd_xfer(ec_dev, msg);
+}
+EXPORT_SYMBOL(cros_ec_cmd_xfer);
+
 static const struct mfd_cell cros_devs[] = {
 	{
 		.name = "cros-ec-keyb",
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index fcbe9d1..0e166b9 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -62,10 +62,6 @@ struct cros_ec_command {
  * @dev: Device pointer
  * @was_wake_device: true if this device was set to wake the system from
  * sleep at the last suspend
- * @cmd_xfer: send command to EC and get response
- *     Returns the number of bytes received if the communication succeeded, but
- *     that doesn't mean the EC was happy with the command. The caller
- *     should check msg.result for the EC's result code.
  *
  * @priv: Private data
  * @irq: Interrupt to use
@@ -82,6 +78,10 @@ struct cros_ec_command {
  * @dout_size: size of dout buffer to allocate (zero to use static dout)
  * @parent: pointer to parent device (e.g. i2c or spi device)
  * @wake_enabled: true if this device can wake the system from sleep
+ * @cmd_xfer: send command to EC and get response
+ *     Returns the number of bytes received if the communication succeeded, but
+ *     that doesn't mean the EC was happy with the command. The caller
+ *     should check msg.result for the EC's result code.
  * @lock: one transaction at a time
  */
 struct cros_ec_device {
@@ -92,8 +92,6 @@ struct cros_ec_device {
 	struct device *dev;
 	bool was_wake_device;
 	struct class *cros_class;
-	int (*cmd_xfer)(struct cros_ec_device *ec,
-			struct cros_ec_command *msg);
 
 	/* These are used to implement the platform-specific interface */
 	void *priv;
@@ -104,6 +102,8 @@ struct cros_ec_device {
 	int dout_size;
 	struct device *parent;
 	bool wake_enabled;
+	int (*cmd_xfer)(struct cros_ec_device *ec,
+			struct cros_ec_command *msg);
 	struct mutex lock;
 };
 
@@ -153,6 +153,18 @@ int cros_ec_check_result(struct cros_ec_device *ec_dev,
 			 struct cros_ec_command *msg);
 
 /**
+ * cros_ec_cmd_xfer - Send a command to the ChromeOS EC
+ *
+ * Call this to send a command to the ChromeOS EC.  This should be used
+ * instead of calling the EC's cmd_xfer() callback directly.
+ *
+ * @ec_dev: EC device
+ * @msg: Message to write
+ */
+int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
+		     struct cros_ec_command *msg);
+
+/**
  * cros_ec_remove - Remove a ChromeOS EC
  *
  * Call this to deregister a ChromeOS EC, then clean up any private data.
-- 
2.0.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RESEND PATCH 4/7] mfd: cros_ec: move locking into cros_ec_cmd_xfer
  2014-08-20 12:13 [RESEND PATCH 0/7] Second batch of cleanups for cros_ec Javier Martinez Canillas
  2014-08-20 12:13 ` [RESEND PATCH 1/7] mfd: cros_ec: Delay for 50ms when we see EC_CMD_REBOOT_EC Javier Martinez Canillas
       [not found] ` <1408536812-7836-1-git-send-email-javier.martinez-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
@ 2014-08-20 12:13 ` Javier Martinez Canillas
  2014-08-20 22:36   ` Doug Anderson
  2014-08-21 14:09   ` Lee Jones
  2014-08-20 12:13 ` [RESEND PATCH 5/7] mfd: cros_ec: wait for completion of commands that return IN_PROGRESS Javier Martinez Canillas
  3 siblings, 2 replies; 19+ messages in thread
From: Javier Martinez Canillas @ 2014-08-20 12:13 UTC (permalink / raw)
  To: Lee Jones
  Cc: Wolfram Sang, Dmitry Torokhov, Doug Anderson, Simon Glass,
	Bill Richardson, Andrew Bresticker, Derek Basehore, Todd Broch,
	Olof Johansson, Andreas Färber, linux-i2c, linux-input,
	linux-samsung-soc, Javier Martinez Canillas

From: Andrew Bresticker <abrestic@chromium.org>

Now that there's a central cros_ec_cmd_xfer(), move the locking
out of the SPI and LPC drivers.

Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 drivers/mfd/cros_ec.c     | 10 +++++++++-
 drivers/mfd/cros_ec_spi.c | 11 -----------
 2 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index a9faebd..c53804a 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -65,7 +65,13 @@ EXPORT_SYMBOL(cros_ec_check_result);
 int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
 		     struct cros_ec_command *msg)
 {
-	return ec_dev->cmd_xfer(ec_dev, msg);
+	int ret;
+
+	mutex_lock(&ec_dev->lock);
+	ret = ec_dev->cmd_xfer(ec_dev, msg);
+	mutex_unlock(&ec_dev->lock);
+
+	return ret;
 }
 EXPORT_SYMBOL(cros_ec_cmd_xfer);
 
@@ -98,6 +104,8 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
 			return -ENOMEM;
 	}
 
+	mutex_init(&ec_dev->lock);
+
 	err = mfd_add_devices(dev, 0, cros_devs,
 			      ARRAY_SIZE(cros_devs),
 			      NULL, ec_dev->irq, NULL);
diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
index b396705..bf6e08e 100644
--- a/drivers/mfd/cros_ec_spi.c
+++ b/drivers/mfd/cros_ec_spi.c
@@ -79,13 +79,11 @@
  *	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_cmd_xfer_spi 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,
@@ -232,13 +230,6 @@ static int cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev,
 	int sum;
 	int ret = 0, final_ret;
 
-	/*
-	 * 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);
 
@@ -327,7 +318,6 @@ exit:
 	if (ec_msg->command == EC_CMD_REBOOT_EC)
 		msleep(EC_REBOOT_DELAY_MS);
 
-	mutex_unlock(&ec_spi->lock);
 	return ret;
 }
 
@@ -359,7 +349,6 @@ 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;
-- 
2.0.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RESEND PATCH 5/7] mfd: cros_ec: wait for completion of commands that return IN_PROGRESS
  2014-08-20 12:13 [RESEND PATCH 0/7] Second batch of cleanups for cros_ec Javier Martinez Canillas
                   ` (2 preceding siblings ...)
  2014-08-20 12:13 ` [RESEND PATCH 4/7] mfd: cros_ec: move locking into cros_ec_cmd_xfer Javier Martinez Canillas
@ 2014-08-20 12:13 ` Javier Martinez Canillas
       [not found]   ` <1408536812-7836-6-git-send-email-javier.martinez-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
  3 siblings, 1 reply; 19+ messages in thread
From: Javier Martinez Canillas @ 2014-08-20 12:13 UTC (permalink / raw)
  To: Lee Jones
  Cc: Wolfram Sang, Dmitry Torokhov, Doug Anderson, Simon Glass,
	Bill Richardson, Andrew Bresticker, Derek Basehore, Todd Broch,
	Olof Johansson, Andreas Färber, linux-i2c, linux-input,
	linux-samsung-soc, Javier Martinez Canillas

From: Andrew Bresticker <abrestic@chromium.org>

When an EC command returns EC_RES_IN_PROGRESS, we need to query
the state of the EC until it indicates that it is no longer busy.
Do this in cros_ec_cmd_xfer() under the EC's mutex so that other
commands (e.g. keyboard, I2C passtru) aren't issued to the EC while
it is working on the in-progress command.

Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 drivers/mfd/cros_ec.c | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index c53804a..634c434 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -23,6 +23,10 @@
 #include <linux/mfd/core.h>
 #include <linux/mfd/cros_ec.h>
 #include <linux/mfd/cros_ec_commands.h>
+#include <linux/delay.h>
+
+#define EC_COMMAND_RETRIES	50
+#define EC_RETRY_DELAY_MS	10
 
 int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
 		       struct cros_ec_command *msg)
@@ -65,10 +69,39 @@ EXPORT_SYMBOL(cros_ec_check_result);
 int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
 		     struct cros_ec_command *msg)
 {
-	int ret;
+	int ret, i;
 
 	mutex_lock(&ec_dev->lock);
 	ret = ec_dev->cmd_xfer(ec_dev, msg);
+	if (ret == -EAGAIN && msg->result == EC_RES_IN_PROGRESS) {
+		/*
+		 * Query the EC's status until it's no longer busy or
+		 * we encounter an error.
+		 */
+		for (i = 0; i < EC_COMMAND_RETRIES; i++) {
+			struct cros_ec_command status_msg;
+			struct ec_response_get_comms_status status;
+
+			msleep(EC_RETRY_DELAY_MS);
+
+			status_msg.version = 0;
+			status_msg.command = EC_CMD_GET_COMMS_STATUS;
+			status_msg.outdata = NULL;
+			status_msg.outsize = 0;
+			status_msg.indata = (uint8_t *)&status;
+			status_msg.insize = sizeof(status);
+
+			ret = ec_dev->cmd_xfer(ec_dev, &status_msg);
+			if (ret < 0)
+				break;
+
+			msg->result = status_msg.result;
+			if (status_msg.result != EC_RES_SUCCESS)
+				break;
+			if (!(status.flags & EC_COMMS_STATUS_PROCESSING))
+				break;
+		}
+	}
 	mutex_unlock(&ec_dev->lock);
 
 	return ret;
-- 
2.0.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RESEND PATCH 6/7] mfd: cros_ec: Instantiate sub-devices from device tree
       [not found] ` <1408536812-7836-1-git-send-email-javier.martinez-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
  2014-08-20 12:13   ` [RESEND PATCH 2/7] i2c: i2c-cros-ec-tunnel: Set retries to 3 Javier Martinez Canillas
  2014-08-20 12:13   ` [RESEND PATCH 3/7] mfd: cros_ec: stop calling ->cmd_xfer() directly Javier Martinez Canillas
@ 2014-08-20 12:13   ` Javier Martinez Canillas
       [not found]     ` <1408536812-7836-7-git-send-email-javier.martinez-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
  2014-08-20 12:13   ` [RESEND PATCH 7/7] Input: cros_ec_keyb: Optimize ghosting algorithm Javier Martinez Canillas
  3 siblings, 1 reply; 19+ messages in thread
From: Javier Martinez Canillas @ 2014-08-20 12:13 UTC (permalink / raw)
  To: Lee Jones
  Cc: Wolfram Sang, Dmitry Torokhov, Doug Anderson, Simon Glass,
	Bill Richardson, Andrew Bresticker, Derek Basehore, Todd Broch,
	Olof Johansson, Andreas Färber,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
	Javier Martinez Canillas

From: Todd Broch <tbroch-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

If the EC device tree node has sub-nodes, try to instantiate them as
MFD sub-devices.  We can configure the EC features provided by the board.

Signed-off-by: Todd Broch <tbroch-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Signed-off-by: Javier Martinez Canillas <javier.martinez-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
---
 drivers/mfd/cros_ec.c | 40 ++++++++++++++++++++++++++++++----------
 1 file changed, 30 insertions(+), 10 deletions(-)

diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index 634c434..96c926c 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -21,6 +21,7 @@
 #include <linux/slab.h>
 #include <linux/module.h>
 #include <linux/mfd/core.h>
+#include <linux/of_platform.h>
 #include <linux/mfd/cros_ec.h>
 #include <linux/mfd/cros_ec_commands.h>
 #include <linux/delay.h>
@@ -109,22 +110,16 @@ int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
 EXPORT_SYMBOL(cros_ec_cmd_xfer);
 
 static const struct mfd_cell cros_devs[] = {
-	{
-		.name = "cros-ec-keyb",
-		.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)
 {
 	struct device *dev = ec_dev->dev;
 	int err = 0;
+#ifdef CONFIG_OF
+	struct device_node *node;
+	int id = ARRAY_SIZE(cros_devs);
+#endif
 
 	if (ec_dev->din_size) {
 		ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL);
@@ -146,6 +141,31 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
 		dev_err(dev, "failed to add mfd devices\n");
 		return err;
 	}
+#ifdef CONFIG_OF
+	/*
+	 * Add sub-devices declared in the device tree.  NOTE they should NOT be
+	 * declared in cros_devs
+	 */
+	for_each_child_of_node(dev->of_node, node) {
+		char name[128];
+		struct mfd_cell cell = {
+			.id = 0,
+			.name = name,
+		};
+
+		if (of_modalias_node(node, name, sizeof(name)) < 0) {
+			dev_err(dev, "modalias failure on %s\n",
+				node->full_name);
+			continue;
+		}
+		dev_dbg(dev, "adding MFD sub-device %s\n", node->name);
+		cell.of_compatible = of_get_property(node, "compatible", NULL);
+		err = mfd_add_devices(dev, ++id, &cell, 1, NULL, ec_dev->irq,
+				      NULL);
+		if (err)
+			dev_err(dev, "fail to add %s\n", node->full_name);
+	}
+#endif
 
 	dev_info(dev, "Chrome EC device registered\n");
 
-- 
2.0.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RESEND PATCH 7/7] Input: cros_ec_keyb: Optimize ghosting algorithm.
       [not found] ` <1408536812-7836-1-git-send-email-javier.martinez-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
                     ` (2 preceding siblings ...)
  2014-08-20 12:13   ` [RESEND PATCH 6/7] mfd: cros_ec: Instantiate sub-devices from device tree Javier Martinez Canillas
@ 2014-08-20 12:13   ` Javier Martinez Canillas
  3 siblings, 0 replies; 19+ messages in thread
From: Javier Martinez Canillas @ 2014-08-20 12:13 UTC (permalink / raw)
  To: Lee Jones
  Cc: Wolfram Sang, Dmitry Torokhov, Doug Anderson, Simon Glass,
	Bill Richardson, Andrew Bresticker, Derek Basehore, Todd Broch,
	Olof Johansson, Andreas Färber,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
	Javier Martinez Canillas

From: Todd Broch <tbroch-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

Previous algorithm was a bit conservative and complicating with
respect to identifying key ghosting.  This CL uses the bitops hamming
weight function (hweight8) to count the number of matching rows for
colM & colN.  If that number is > 1 ghosting is present.

Additionally it removes NULL keys and our one virtual keypress
KEY_BATTERY from consideration as these inputs are never physical
keypresses.

Signed-off-by: Todd Broch <tbroch-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Reviewed-by: Vincent Palatin <vpalatin-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Reviewed-by: Luigi Semenzato <semenzato-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Tested-by: Andreas Färber <afaerber-l3A5Bk7waGM@public.gmane.org>
Signed-off-by: Javier Martinez Canillas <javier.martinez-ZGY8ohtN/8oXa1LcwEujcA@public.gmane.orgk>
---
 drivers/input/keyboard/cros_ec_keyb.c | 92 +++++++++++++++++++----------------
 1 file changed, 49 insertions(+), 43 deletions(-)

diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
index 93111d1..5d773d2 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -22,6 +22,7 @@
  */
 
 #include <linux/module.h>
+#include <linux/bitops.h>
 #include <linux/i2c.h>
 #include <linux/input.h>
 #include <linux/interrupt.h>
@@ -38,6 +39,7 @@
  * @row_shift: log2 or number of rows, rounded up
  * @keymap_data: Matrix keymap data used to convert to keyscan values
  * @ghost_filter: true to enable the matrix key-ghosting filter
+ * @valid_keys: bitmap of existing keys for each matrix column
  * @old_kb_state: bitmap of keys pressed last scan
  * @dev: Device pointer
  * @idev: Input device
@@ -49,6 +51,7 @@ struct cros_ec_keyb {
 	int row_shift;
 	const struct matrix_keymap_data *keymap_data;
 	bool ghost_filter;
+	uint8_t *valid_keys;
 	uint8_t *old_kb_state;
 
 	struct device *dev;
@@ -57,39 +60,15 @@ struct cros_ec_keyb {
 };
 
 
-static bool cros_ec_keyb_row_has_ghosting(struct cros_ec_keyb *ckdev,
-					  uint8_t *buf, int row)
-{
-	int pressed_in_row = 0;
-	int row_has_teeth = 0;
-	int col, mask;
-
-	mask = 1 << row;
-	for (col = 0; col < ckdev->cols; col++) {
-		if (buf[col] & mask) {
-			pressed_in_row++;
-			row_has_teeth |= buf[col] & ~mask;
-			if (pressed_in_row > 1 && row_has_teeth) {
-				/* ghosting */
-				dev_dbg(ckdev->dev,
-					"ghost found at: r%d c%d, pressed %d, teeth 0x%x\n",
-					row, col, pressed_in_row,
-					row_has_teeth);
-				return true;
-			}
-		}
-	}
-
-	return false;
-}
-
 /*
  * Returns true when there is at least one combination of pressed keys that
  * results in ghosting.
  */
 static bool cros_ec_keyb_has_ghosting(struct cros_ec_keyb *ckdev, uint8_t *buf)
 {
-	int row;
+	int col1, col2, buf1, buf2;
+	struct device *dev = ckdev->dev;
+	uint8_t *valid_keys = ckdev->valid_keys;
 
 	/*
 	 * Ghosting happens if for any pressed key X there are other keys
@@ -103,27 +82,23 @@ static bool cros_ec_keyb_has_ghosting(struct cros_ec_keyb *ckdev, uint8_t *buf)
 	 *
 	 * In this case only X, Y, and Z are pressed, but g appears to be
 	 * pressed too (see Wikipedia).
-	 *
-	 * We can detect ghosting in a single pass (*) over the keyboard state
-	 * by maintaining two arrays.  pressed_in_row counts how many pressed
-	 * keys we have found in a row.  row_has_teeth is true if any of the
-	 * pressed keys for this row has other pressed keys in its column.  If
-	 * at any point of the scan we find that a row has multiple pressed
-	 * keys, and at least one of them is at the intersection with a column
-	 * with multiple pressed keys, we're sure there is ghosting.
-	 * Conversely, if there is ghosting, we will detect such situation for
-	 * at least one key during the pass.
-	 *
-	 * (*) This looks linear in the number of keys, but it's not.  We can
-	 * cheat because the number of rows is small.
 	 */
-	for (row = 0; row < ckdev->rows; row++)
-		if (cros_ec_keyb_row_has_ghosting(ckdev, buf, row))
-			return true;
+	for (col1 = 0; col1 < ckdev->cols; col1++) {
+		buf1 = buf[col1] & valid_keys[col1];
+		for (col2 = col1 + 1; col2 < ckdev->cols; col2++) {
+			buf2 = buf[col2] & valid_keys[col2];
+			if (hweight8(buf1 & buf2) > 1) {
+				dev_dbg(dev, "ghost found at: B[%02d]:0x%02x & B[%02d]:0x%02x",
+					col1, buf1, col2, buf2);
+				return true;
+			}
+		}
+	}
 
 	return false;
 }
 
+
 /*
  * Compares the new keyboard state to the old one and produces key
  * press/release events accordingly.  The keyboard state is 13 bytes (one byte
@@ -222,6 +197,30 @@ static void cros_ec_keyb_close(struct input_dev *dev)
 	free_irq(ec->irq, ckdev);
 }
 
+/*
+ * Walks keycodes flipping bit in buffer COLUMNS deep where bit is ROW.  Used by
+ * ghosting logic to ignore NULL or virtual keys.
+ */
+static void cros_ec_keyb_compute_valid_keys(struct cros_ec_keyb *ckdev)
+{
+	int row, col;
+	int row_shift = ckdev->row_shift;
+	unsigned short *keymap = ckdev->idev->keycode;
+	unsigned short code;
+
+	BUG_ON(ckdev->idev->keycodesize != sizeof(*keymap));
+
+	for (col = 0; col < ckdev->cols; col++) {
+		for (row = 0; row < ckdev->rows; row++) {
+			code = keymap[MATRIX_SCAN_CODE(row, col, row_shift)];
+			if (code && (code != KEY_BATTERY))
+				ckdev->valid_keys[col] |= 1 << row;
+		}
+		dev_dbg(ckdev->dev, "valid_keys[%02d] = 0x%02x\n",
+			col, ckdev->valid_keys[col]);
+	}
+}
+
 static int cros_ec_keyb_probe(struct platform_device *pdev)
 {
 	struct cros_ec_device *ec = dev_get_drvdata(pdev->dev.parent);
@@ -242,6 +241,11 @@ static int cros_ec_keyb_probe(struct platform_device *pdev)
 					    &ckdev->cols);
 	if (err)
 		return err;
+
+	ckdev->valid_keys = devm_kzalloc(&pdev->dev, ckdev->cols, GFP_KERNEL);
+	if (!ckdev->valid_keys)
+		return -ENOMEM;
+
 	ckdev->old_kb_state = devm_kzalloc(&pdev->dev, ckdev->cols, GFP_KERNEL);
 	if (!ckdev->old_kb_state)
 		return -ENOMEM;
@@ -285,6 +289,8 @@ static int cros_ec_keyb_probe(struct platform_device *pdev)
 	input_set_capability(idev, EV_MSC, MSC_SCAN);
 	input_set_drvdata(idev, ckdev);
 	ckdev->idev = idev;
+	cros_ec_keyb_compute_valid_keys(ckdev);
+
 	err = input_register_device(ckdev->idev);
 	if (err) {
 		dev_err(dev, "cannot register input device\n");
-- 
2.0.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [RESEND PATCH 3/7] mfd: cros_ec: stop calling ->cmd_xfer() directly
  2014-08-20 12:13   ` [RESEND PATCH 3/7] mfd: cros_ec: stop calling ->cmd_xfer() directly Javier Martinez Canillas
@ 2014-08-20 22:33     ` Doug Anderson
  2014-08-21 14:08     ` Lee Jones
  1 sibling, 0 replies; 19+ messages in thread
From: Doug Anderson @ 2014-08-20 22:33 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: Lee Jones, Wolfram Sang, Dmitry Torokhov, Simon Glass,
	Bill Richardson, Andrew Bresticker, Derek Basehore, Todd Broch,
	Olof Johansson, Andreas Färber, linux-i2c, linux-input,
	linux-samsung-soc

Javier,

On Wed, Aug 20, 2014 at 5:13 AM, Javier Martinez Canillas
<javier.martinez@collabora.co.uk> wrote:
> From: Andrew Bresticker <abrestic@chromium.org>
>
> Instead of having users of the ChromeOS EC call the interface-specific
> cmd_xfer() callback directly, introduce a central cros_ec_cmd_xfer()
> to use instead.  This will allow us to put all the locking and retry
> logic in one place instead of duplicating it across the different
> drivers.
>
> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> ---
>  drivers/i2c/busses/i2c-cros-ec-tunnel.c |  2 +-
>  drivers/input/keyboard/cros_ec_keyb.c   |  2 +-
>  drivers/mfd/cros_ec.c                   |  7 +++++++
>  include/linux/mfd/cros_ec.h             | 24 ++++++++++++++++++------
>  4 files changed, 27 insertions(+), 8 deletions(-)

Reviewed-by: Doug Anderson <dianders@chromium.org>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RESEND PATCH 4/7] mfd: cros_ec: move locking into cros_ec_cmd_xfer
  2014-08-20 12:13 ` [RESEND PATCH 4/7] mfd: cros_ec: move locking into cros_ec_cmd_xfer Javier Martinez Canillas
@ 2014-08-20 22:36   ` Doug Anderson
  2014-08-21 10:24     ` Javier Martinez Canillas
  2014-08-21 14:09   ` Lee Jones
  1 sibling, 1 reply; 19+ messages in thread
From: Doug Anderson @ 2014-08-20 22:36 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: Lee Jones, Wolfram Sang, Dmitry Torokhov, Simon Glass,
	Bill Richardson, Andrew Bresticker, Derek Basehore, Todd Broch,
	Olof Johansson, Andreas Färber, linux-i2c, linux-input,
	linux-samsung-soc

Javier,

On Wed, Aug 20, 2014 at 5:13 AM, Javier Martinez Canillas
<javier.martinez@collabora.co.uk> wrote:
> From: Andrew Bresticker <abrestic@chromium.org>
>
> Now that there's a central cros_ec_cmd_xfer(), move the locking
> out of the SPI and LPC drivers.

Slight nit that the LPC driver doesn't exist upstream.  This is in
prep for adding the LPC driver, though.


> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> ---
>  drivers/mfd/cros_ec.c     | 10 +++++++++-
>  drivers/mfd/cros_ec_spi.c | 11 -----------
>  2 files changed, 9 insertions(+), 12 deletions(-)

After comment nitfix:

Reviewed-by: Doug Anderson <dianders@chromium.org>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RESEND PATCH 4/7] mfd: cros_ec: move locking into cros_ec_cmd_xfer
  2014-08-20 22:36   ` Doug Anderson
@ 2014-08-21 10:24     ` Javier Martinez Canillas
  0 siblings, 0 replies; 19+ messages in thread
From: Javier Martinez Canillas @ 2014-08-21 10:24 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Lee Jones, Wolfram Sang, Dmitry Torokhov, Simon Glass,
	Bill Richardson, Andrew Bresticker, Derek Basehore, Todd Broch,
	Olof Johansson, Andreas Färber, linux-i2c, linux-input,
	linux-samsung-soc

Hello Doug,

On 08/21/2014 12:36 AM, Doug Anderson wrote:
> Javier,
> 
> On Wed, Aug 20, 2014 at 5:13 AM, Javier Martinez Canillas
> <javier.martinez@collabora.co.uk> wrote:
>> From: Andrew Bresticker <abrestic@chromium.org>
>>
>> Now that there's a central cros_ec_cmd_xfer(), move the locking
>> out of the SPI and LPC drivers.
> 
> Slight nit that the LPC driver doesn't exist upstream.  This is in
> prep for adding the LPC driver, though.
> 

Right, the downstream commit was also touching the LPC driver and I
stripped that part but forget to update the commit message, sorry about
that. I'll fix it and do a re-spin.

> 
>> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
>> ---
>>  drivers/mfd/cros_ec.c     | 10 +++++++++-
>>  drivers/mfd/cros_ec_spi.c | 11 -----------
>>  2 files changed, 9 insertions(+), 12 deletions(-)
> 
> After comment nitfix:
> 
> Reviewed-by: Doug Anderson <dianders@chromium.org>
> 

Thanks and best regards,
Javier

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RESEND PATCH 1/7] mfd: cros_ec: Delay for 50ms when we see EC_CMD_REBOOT_EC
  2014-08-20 12:13 ` [RESEND PATCH 1/7] mfd: cros_ec: Delay for 50ms when we see EC_CMD_REBOOT_EC Javier Martinez Canillas
@ 2014-08-21 13:37   ` Lee Jones
  2014-08-21 13:49     ` Javier Martinez Canillas
  0 siblings, 1 reply; 19+ messages in thread
From: Lee Jones @ 2014-08-21 13:37 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: Wolfram Sang, Dmitry Torokhov, Doug Anderson, Simon Glass,
	Bill Richardson, Andrew Bresticker, Derek Basehore, Todd Broch,
	Olof Johansson, Andreas Färber, linux-i2c, linux-input,
	linux-samsung-soc

On Wed, 20 Aug 2014, Javier Martinez Canillas wrote:
> From: Doug Anderson <dianders@chromium.org>
> 
> If someone sends a EC_CMD_REBOOT_EC to the EC, the EC will likely be
> unresponsive for quite a while.  Add a delay to the end of the command
> to prevent random failures of future commands.
> 
> NOTES:
> * This could be optimized a bit by simply delaying the next command
>   sent, but EC_CMD_REBOOT_EC is such a rare command that the extra
>   complexity doesn't seem worth it.
> * This is a bit of an "ugly hack" since the SPI driver is effectively
>   snooping on the communication and making a lot of assumptions.  It
>   would be nice to architect in some better solution long term.

Are you planning on doing that?

> * This same logic probably needs to be applied to the i2c driver.
> 
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Reviewed-by: Randall Spangler <rspangler@chromium.org>
> Reviewed-by: Vadim Bendebury <vbendeb@chromium.org>
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> ---
>  drivers/mfd/cros_ec_spi.c | 9 +++++++++
>  1 file changed, 9 insertions(+)

I'm willing to accept this as a stand-in.

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RESEND PATCH 1/7] mfd: cros_ec: Delay for 50ms when we see EC_CMD_REBOOT_EC
  2014-08-21 13:37   ` Lee Jones
@ 2014-08-21 13:49     ` Javier Martinez Canillas
  0 siblings, 0 replies; 19+ messages in thread
From: Javier Martinez Canillas @ 2014-08-21 13:49 UTC (permalink / raw)
  To: Lee Jones
  Cc: Wolfram Sang, Dmitry Torokhov, Doug Anderson, Simon Glass,
	Bill Richardson, Andrew Bresticker, Derek Basehore, Todd Broch,
	Olof Johansson, Andreas Färber, linux-i2c, linux-input,
	linux-samsung-soc

Hello Lee,

On 08/21/2014 03:37 PM, Lee Jones wrote:
> On Wed, 20 Aug 2014, Javier Martinez Canillas wrote:
>> From: Doug Anderson <dianders@chromium.org>
>> 
>> If someone sends a EC_CMD_REBOOT_EC to the EC, the EC will likely be
>> unresponsive for quite a while.  Add a delay to the end of the command
>> to prevent random failures of future commands.
>> 
>> NOTES:
>> * This could be optimized a bit by simply delaying the next command
>>   sent, but EC_CMD_REBOOT_EC is such a rare command that the extra
>>   complexity doesn't seem worth it.
>> * This is a bit of an "ugly hack" since the SPI driver is effectively
>>   snooping on the communication and making a lot of assumptions.  It
>>   would be nice to architect in some better solution long term.
> 
> Are you planning on doing that?
> 

Yes, I'll add to my TO-DO list to look how better solve this after the
remaining functionality that is present in downstream but is still not in
mainline gets merged.

>> * This same logic probably needs to be applied to the i2c driver.
>> 
>> Signed-off-by: Doug Anderson <dianders@chromium.org>
>> Reviewed-by: Randall Spangler <rspangler@chromium.org>
>> Reviewed-by: Vadim Bendebury <vbendeb@chromium.org>
>> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
>> ---
>>  drivers/mfd/cros_ec_spi.c | 9 +++++++++
>>  1 file changed, 9 insertions(+)
> 
> I'm willing to accept this as a stand-in.
> 

Thanks.

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

Best regards,
Javier

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RESEND PATCH 3/7] mfd: cros_ec: stop calling ->cmd_xfer() directly
  2014-08-20 12:13   ` [RESEND PATCH 3/7] mfd: cros_ec: stop calling ->cmd_xfer() directly Javier Martinez Canillas
  2014-08-20 22:33     ` Doug Anderson
@ 2014-08-21 14:08     ` Lee Jones
  1 sibling, 0 replies; 19+ messages in thread
From: Lee Jones @ 2014-08-21 14:08 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: Wolfram Sang, Dmitry Torokhov, Doug Anderson, Simon Glass,
	Bill Richardson, Andrew Bresticker, Derek Basehore, Todd Broch,
	Olof Johansson, Andreas Färber, linux-i2c, linux-input,
	linux-samsung-soc

On Wed, 20 Aug 2014, Javier Martinez Canillas wrote:

> From: Andrew Bresticker <abrestic@chromium.org>
> 
> Instead of having users of the ChromeOS EC call the interface-specific
> cmd_xfer() callback directly, introduce a central cros_ec_cmd_xfer()
> to use instead.  This will allow us to put all the locking and retry
> logic in one place instead of duplicating it across the different
> drivers.
> 
> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> ---
>  drivers/i2c/busses/i2c-cros-ec-tunnel.c |  2 +-
>  drivers/input/keyboard/cros_ec_keyb.c   |  2 +-
>  drivers/mfd/cros_ec.c                   |  7 +++++++
>  include/linux/mfd/cros_ec.h             | 24 ++++++++++++++++++------
>  4 files changed, 27 insertions(+), 8 deletions(-)

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
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RESEND PATCH 4/7] mfd: cros_ec: move locking into cros_ec_cmd_xfer
  2014-08-20 12:13 ` [RESEND PATCH 4/7] mfd: cros_ec: move locking into cros_ec_cmd_xfer Javier Martinez Canillas
  2014-08-20 22:36   ` Doug Anderson
@ 2014-08-21 14:09   ` Lee Jones
  1 sibling, 0 replies; 19+ messages in thread
From: Lee Jones @ 2014-08-21 14:09 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: Wolfram Sang, Dmitry Torokhov, Doug Anderson, Simon Glass,
	Bill Richardson, Andrew Bresticker, Derek Basehore, Todd Broch,
	Olof Johansson, Andreas Färber, linux-i2c, linux-input,
	linux-samsung-soc

On Wed, 20 Aug 2014, Javier Martinez Canillas wrote:

> From: Andrew Bresticker <abrestic@chromium.org>
> 
> Now that there's a central cros_ec_cmd_xfer(), move the locking
> out of the SPI and LPC drivers.
> 
> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> ---
>  drivers/mfd/cros_ec.c     | 10 +++++++++-
>  drivers/mfd/cros_ec_spi.c | 11 -----------
>  2 files changed, 9 insertions(+), 12 deletions(-)

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
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RESEND PATCH 5/7] mfd: cros_ec: wait for completion of commands that return IN_PROGRESS
       [not found]   ` <1408536812-7836-6-git-send-email-javier.martinez-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
@ 2014-08-21 14:21     ` Lee Jones
  2014-08-22 11:24       ` Javier Martinez Canillas
  0 siblings, 1 reply; 19+ messages in thread
From: Lee Jones @ 2014-08-21 14:21 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: Wolfram Sang, Dmitry Torokhov, Doug Anderson, Simon Glass,
	Bill Richardson, Andrew Bresticker, Derek Basehore, Todd Broch,
	Olof Johansson, Andreas Färber,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA

On Wed, 20 Aug 2014, Javier Martinez Canillas wrote:

> From: Andrew Bresticker <abrestic-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> 
> When an EC command returns EC_RES_IN_PROGRESS, we need to query
> the state of the EC until it indicates that it is no longer busy.
> Do this in cros_ec_cmd_xfer() under the EC's mutex so that other
> commands (e.g. keyboard, I2C passtru) aren't issued to the EC while
> it is working on the in-progress command.
> 
> Signed-off-by: Andrew Bresticker <abrestic-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> Reviewed-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> Signed-off-by: Javier Martinez Canillas <javier.martinez-ZGY8ohtN/8rSCDK34cm6iQ@public.gmane.org.uk>
> ---
>  drivers/mfd/cros_ec.c | 35 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
> index c53804a..634c434 100644
> --- a/drivers/mfd/cros_ec.c
> +++ b/drivers/mfd/cros_ec.c
> @@ -23,6 +23,10 @@
>  #include <linux/mfd/core.h>
>  #include <linux/mfd/cros_ec.h>
>  #include <linux/mfd/cros_ec_commands.h>
> +#include <linux/delay.h>
> +
> +#define EC_COMMAND_RETRIES	50
> +#define EC_RETRY_DELAY_MS	10
>  
>  int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
>  		       struct cros_ec_command *msg)
> @@ -65,10 +69,39 @@ EXPORT_SYMBOL(cros_ec_check_result);
>  int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
>  		     struct cros_ec_command *msg)
>  {
> -	int ret;
> +	int ret, i;
>  
>  	mutex_lock(&ec_dev->lock);
>  	ret = ec_dev->cmd_xfer(ec_dev, msg);
> +	if (ret == -EAGAIN && msg->result == EC_RES_IN_PROGRESS) {
> +		/*
> +		 * Query the EC's status until it's no longer busy or
> +		 * we encounter an error.
> +		 */
> +		for (i = 0; i < EC_COMMAND_RETRIES; i++) {
> +			struct cros_ec_command status_msg;
> +			struct ec_response_get_comms_status status;
> +
> +			msleep(EC_RETRY_DELAY_MS);
> +
> +			status_msg.version = 0;
> +			status_msg.command = EC_CMD_GET_COMMS_STATUS;
> +			status_msg.outdata = NULL;
> +			status_msg.outsize = 0;
> +			status_msg.indata = (uint8_t *)&status;
> +			status_msg.insize = sizeof(status);
> +
> +			ret = ec_dev->cmd_xfer(ec_dev, &status_msg);
> +			if (ret < 0)
> +				break;
> +
> +			msg->result = status_msg.result;
> +			if (status_msg.result != EC_RES_SUCCESS)
> +				break;
> +			if (!(status.flags & EC_COMMS_STATUS_PROCESSING))
> +				break;
> +		}
> +	}

Wow!  Things just got ugly real fast.

Do the *xfer() calls fiddle with msg passed into cros_ec_cmd_xfer()?
If not, why is it necessary to keep populating it?

If all this stuff is necessary (and I really hope that it's not) I
think it would be better to have the for() loop as the outer layer.
Then we only have one instance of cmd_xfer() invocation and we save a
layer of tabbing. 

>  	mutex_unlock(&ec_dev->lock);
>  
>  	return ret;

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RESEND PATCH 6/7] mfd: cros_ec: Instantiate sub-devices from device tree
       [not found]     ` <1408536812-7836-7-git-send-email-javier.martinez-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
@ 2014-08-21 14:25       ` Lee Jones
  2014-08-22 11:27         ` Javier Martinez Canillas
  0 siblings, 1 reply; 19+ messages in thread
From: Lee Jones @ 2014-08-21 14:25 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: Wolfram Sang, Dmitry Torokhov, Doug Anderson, Simon Glass,
	Bill Richardson, Andrew Bresticker, Derek Basehore, Todd Broch,
	Olof Johansson, Andreas Färber,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA

On Wed, 20 Aug 2014, Javier Martinez Canillas wrote:

> From: Todd Broch <tbroch-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> 
> If the EC device tree node has sub-nodes, try to instantiate them as
> MFD sub-devices.  We can configure the EC features provided by the board.
> 
> Signed-off-by: Todd Broch <tbroch-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> Signed-off-by: Javier Martinez Canillas <javier.martinez-ZGY8ohtN/8rSCDK34cm6iQ@public.gmane.org.uk>
> ---
>  drivers/mfd/cros_ec.c | 40 ++++++++++++++++++++++++++++++----------
>  1 file changed, 30 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
> index 634c434..96c926c 100644
> --- a/drivers/mfd/cros_ec.c
> +++ b/drivers/mfd/cros_ec.c
> @@ -21,6 +21,7 @@
>  #include <linux/slab.h>
>  #include <linux/module.h>
>  #include <linux/mfd/core.h>
> +#include <linux/of_platform.h>
>  #include <linux/mfd/cros_ec.h>
>  #include <linux/mfd/cros_ec_commands.h>
>  #include <linux/delay.h>
> @@ -109,22 +110,16 @@ int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
>  EXPORT_SYMBOL(cros_ec_cmd_xfer);
>  
>  static const struct mfd_cell cros_devs[] = {
> -	{
> -		.name = "cros-ec-keyb",
> -		.id = 1,
> -		.of_compatible = "google,cros-ec-keyb",
> -	},
> -	{
> -		.name = "cros-ec-i2c-tunnel",
> -		.id = 2,
> -		.of_compatible = "google,cros-ec-i2c-tunnel",
> -	},
>  };

Why are you keeping this round if it's empty?

>  int cros_ec_register(struct cros_ec_device *ec_dev)
>  {
>  	struct device *dev = ec_dev->dev;
>  	int err = 0;
> +#ifdef CONFIG_OF
> +	struct device_node *node;
> +	int id = ARRAY_SIZE(cros_devs);
> +#endif
>  
>  	if (ec_dev->din_size) {
>  		ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL);
> @@ -146,6 +141,31 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
>  		dev_err(dev, "failed to add mfd devices\n");
>  		return err;
>  	}
> +#ifdef CONFIG_OF
> +	/*
> +	 * Add sub-devices declared in the device tree.  NOTE they should NOT be
> +	 * declared in cros_devs
> +	 */
> +	for_each_child_of_node(dev->of_node, node) {
> +		char name[128];
> +		struct mfd_cell cell = {
> +			.id = 0,
> +			.name = name,
> +		};
> +
> +		if (of_modalias_node(node, name, sizeof(name)) < 0) {
> +			dev_err(dev, "modalias failure on %s\n",
> +				node->full_name);
> +			continue;
> +		}
> +		dev_dbg(dev, "adding MFD sub-device %s\n", node->name);
> +		cell.of_compatible = of_get_property(node, "compatible", NULL);
> +		err = mfd_add_devices(dev, ++id, &cell, 1, NULL, ec_dev->irq,
> +				      NULL);
> +		if (err)
> +			dev_err(dev, "fail to add %s\n", node->full_name);
> +	}
> +#endif

This is grim!

Why don't you use of_platform_populate()?

>  	dev_info(dev, "Chrome EC device registered\n");
>  

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RESEND PATCH 5/7] mfd: cros_ec: wait for completion of commands that return IN_PROGRESS
  2014-08-21 14:21     ` Lee Jones
@ 2014-08-22 11:24       ` Javier Martinez Canillas
  0 siblings, 0 replies; 19+ messages in thread
From: Javier Martinez Canillas @ 2014-08-22 11:24 UTC (permalink / raw)
  To: Lee Jones
  Cc: Wolfram Sang, Dmitry Torokhov, Doug Anderson, Simon Glass,
	Bill Richardson, Andrew Bresticker, Derek Basehore, Todd Broch,
	Olof Johansson, Andreas Färber,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA

Hello Lee,

Thanks a lot for your feedback.

On 08/21/2014 04:21 PM, Lee Jones wrote:
> On Wed, 20 Aug 2014, Javier Martinez Canillas wrote:
> 
>> From: Andrew Bresticker <abrestic-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>> 
>> When an EC command returns EC_RES_IN_PROGRESS, we need to query
>> the state of the EC until it indicates that it is no longer busy.
>> Do this in cros_ec_cmd_xfer() under the EC's mutex so that other
>> commands (e.g. keyboard, I2C passtru) aren't issued to the EC while
>> it is working on the in-progress command.
>> 
>> Signed-off-by: Andrew Bresticker <abrestic-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>> Reviewed-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>> Signed-off-by: Javier Martinez Canillas <javier.martinez-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
>> ---
>>  drivers/mfd/cros_ec.c | 35 ++++++++++++++++++++++++++++++++++-
>>  1 file changed, 34 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
>> index c53804a..634c434 100644
>> --- a/drivers/mfd/cros_ec.c
>> +++ b/drivers/mfd/cros_ec.c
>> @@ -23,6 +23,10 @@
>>  #include <linux/mfd/core.h>
>>  #include <linux/mfd/cros_ec.h>
>>  #include <linux/mfd/cros_ec_commands.h>
>> +#include <linux/delay.h>
>> +
>> +#define EC_COMMAND_RETRIES	50
>> +#define EC_RETRY_DELAY_MS	10
>>  
>>  int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
>>  		       struct cros_ec_command *msg)
>> @@ -65,10 +69,39 @@ EXPORT_SYMBOL(cros_ec_check_result);
>>  int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
>>  		     struct cros_ec_command *msg)
>>  {
>> -	int ret;
>> +	int ret, i;
>>  
>>  	mutex_lock(&ec_dev->lock);
>>  	ret = ec_dev->cmd_xfer(ec_dev, msg);
>> +	if (ret == -EAGAIN && msg->result == EC_RES_IN_PROGRESS) {
>> +		/*
>> +		 * Query the EC's status until it's no longer busy or
>> +		 * we encounter an error.
>> +		 */
>> +		for (i = 0; i < EC_COMMAND_RETRIES; i++) {
>> +			struct cros_ec_command status_msg;
>> +			struct ec_response_get_comms_status status;
>> +
>> +			msleep(EC_RETRY_DELAY_MS);
>> +
>> +			status_msg.version = 0;
>> +			status_msg.command = EC_CMD_GET_COMMS_STATUS;
>> +			status_msg.outdata = NULL;
>> +			status_msg.outsize = 0;
>> +			status_msg.indata = (uint8_t *)&status;
>> +			status_msg.insize = sizeof(status);
>> +
>> +			ret = ec_dev->cmd_xfer(ec_dev, &status_msg);
>> +			if (ret < 0)
>> +				break;
>> +
>> +			msg->result = status_msg.result;
>> +			if (status_msg.result != EC_RES_SUCCESS)
>> +				break;
>> +			if (!(status.flags & EC_COMMS_STATUS_PROCESSING))
>> +				break;
>> +		}
>> +	}
> 
> Wow!  Things just got ugly real fast.
> 
> Do the *xfer() calls fiddle with msg passed into cros_ec_cmd_xfer()?
> If not, why is it necessary to keep populating it?
> 

Not really, I see that only struct cros_ec_command .result and .indata
fields are modified by the cmd_xfer() function handlers so I agree with
you that these variables can be defined outside of the for loop and
reused. I think that even zero'ing indata is not needed between calls
since on success the full sizeof(status) is copied and on failure it
doesn't matter what is there.

Will change this when doing the re-spin.

> If all this stuff is necessary (and I really hope that it's not) I
> think it would be better to have the for() loop as the outer layer.
> Then we only have one instance of cmd_xfer() invocation and we save a
> layer of tabbing. 
> 

Most of this doesn't need to be inside the for loop as you said but there
is a need for two struct cros_ec_command *msg though, one for the actual
command made by the caller and other to query the EC if it already
finished processing the first command.

>>  	mutex_unlock(&ec_dev->lock);
>>  
>>  	return ret;
> 

Best regards,
Javier

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RESEND PATCH 6/7] mfd: cros_ec: Instantiate sub-devices from device tree
  2014-08-21 14:25       ` Lee Jones
@ 2014-08-22 11:27         ` Javier Martinez Canillas
  0 siblings, 0 replies; 19+ messages in thread
From: Javier Martinez Canillas @ 2014-08-22 11:27 UTC (permalink / raw)
  To: Lee Jones
  Cc: Wolfram Sang, Dmitry Torokhov, Doug Anderson, Simon Glass,
	Bill Richardson, Andrew Bresticker, Derek Basehore, Todd Broch,
	Olof Johansson, Andreas Färber, linux-i2c, linux-input,
	linux-samsung-soc

Hello Lee,

On 08/21/2014 04:25 PM, Lee Jones wrote:
>>  
>>  static const struct mfd_cell cros_devs[] = {
>> -	{
>> -		.name = "cros-ec-keyb",
>> -		.id = 1,
>> -		.of_compatible = "google,cros-ec-keyb",
>> -	},
>> -	{
>> -		.name = "cros-ec-i2c-tunnel",
>> -		.id = 2,
>> -		.of_compatible = "google,cros-ec-i2c-tunnel",
>> -	},
>>  };
> 
> Why are you keeping this round if it's empty?
>

Right, I'll just remove it.

>>  	}
>> +#ifdef CONFIG_OF
>> +	/*
>> +	 * Add sub-devices declared in the device tree.  NOTE they should NOT be
>> +	 * declared in cros_devs
>> +	 */
>> +	for_each_child_of_node(dev->of_node, node) {
>> +		char name[128];
>> +		struct mfd_cell cell = {
>> +			.id = 0,
>> +			.name = name,
>> +		};
>> +
>> +		if (of_modalias_node(node, name, sizeof(name)) < 0) {
>> +			dev_err(dev, "modalias failure on %s\n",
>> +				node->full_name);
>> +			continue;
>> +		}
>> +		dev_dbg(dev, "adding MFD sub-device %s\n", node->name);
>> +		cell.of_compatible = of_get_property(node, "compatible", NULL);
>> +		err = mfd_add_devices(dev, ++id, &cell, 1, NULL, ec_dev->irq,
>> +				      NULL);
>> +		if (err)
>> +			dev_err(dev, "fail to add %s\n", node->full_name);
>> +	}
>> +#endif
> 
> This is grim!
> 
> Why don't you use of_platform_populate()?
>

Indeed, I think it may just work and all these is not needed.

Thanks for your feedback and best regards,
Javier

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2014-08-22 11:27 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-20 12:13 [RESEND PATCH 0/7] Second batch of cleanups for cros_ec Javier Martinez Canillas
2014-08-20 12:13 ` [RESEND PATCH 1/7] mfd: cros_ec: Delay for 50ms when we see EC_CMD_REBOOT_EC Javier Martinez Canillas
2014-08-21 13:37   ` Lee Jones
2014-08-21 13:49     ` Javier Martinez Canillas
     [not found] ` <1408536812-7836-1-git-send-email-javier.martinez-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
2014-08-20 12:13   ` [RESEND PATCH 2/7] i2c: i2c-cros-ec-tunnel: Set retries to 3 Javier Martinez Canillas
2014-08-20 12:13   ` [RESEND PATCH 3/7] mfd: cros_ec: stop calling ->cmd_xfer() directly Javier Martinez Canillas
2014-08-20 22:33     ` Doug Anderson
2014-08-21 14:08     ` Lee Jones
2014-08-20 12:13   ` [RESEND PATCH 6/7] mfd: cros_ec: Instantiate sub-devices from device tree Javier Martinez Canillas
     [not found]     ` <1408536812-7836-7-git-send-email-javier.martinez-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
2014-08-21 14:25       ` Lee Jones
2014-08-22 11:27         ` Javier Martinez Canillas
2014-08-20 12:13   ` [RESEND PATCH 7/7] Input: cros_ec_keyb: Optimize ghosting algorithm Javier Martinez Canillas
2014-08-20 12:13 ` [RESEND PATCH 4/7] mfd: cros_ec: move locking into cros_ec_cmd_xfer Javier Martinez Canillas
2014-08-20 22:36   ` Doug Anderson
2014-08-21 10:24     ` Javier Martinez Canillas
2014-08-21 14:09   ` Lee Jones
2014-08-20 12:13 ` [RESEND PATCH 5/7] mfd: cros_ec: wait for completion of commands that return IN_PROGRESS Javier Martinez Canillas
     [not found]   ` <1408536812-7836-6-git-send-email-javier.martinez-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
2014-08-21 14:21     ` Lee Jones
2014-08-22 11:24       ` Javier Martinez Canillas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.