* [PATCH v2 1/7] mfd: cros_ec: Delay for 50ms when we see EC_CMD_REBOOT_EC
2014-08-25 13:40 [PATCH v2 0/7] Second batch of cleanups for cros_ec Javier Martinez Canillas
@ 2014-08-25 13:40 ` Javier Martinez Canillas
2014-08-25 13:40 ` [PATCH v2 2/7] i2c: i2c-cros-ec-tunnel: Set retries to 3 Javier Martinez Canillas
` (5 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Javier Martinez Canillas @ 2014-08-25 13:40 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, 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>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
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] 17+ messages in thread
* [PATCH v2 2/7] i2c: i2c-cros-ec-tunnel: Set retries to 3
2014-08-25 13:40 [PATCH v2 0/7] Second batch of cleanups for cros_ec Javier Martinez Canillas
2014-08-25 13:40 ` [PATCH v2 1/7] mfd: cros_ec: Delay for 50ms when we see EC_CMD_REBOOT_EC Javier Martinez Canillas
@ 2014-08-25 13:40 ` Javier Martinez Canillas
[not found] ` <1408974008-17184-1-git-send-email-javier.martinez-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
` (4 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Javier Martinez Canillas @ 2014-08-25 13:40 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, linux-i2c, linux-input, linux-samsung-soc,
Javier Martinez Canillas
From: Derek Basehore <dbasehore@chromium.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@chromium.org>
Reviewed-by: Doug Anderson <dianders@chromium.org>
Acked-by: Wolfram Sang <wsa@the-dreams.de>
Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
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 3c15dcc..97e6369 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] 17+ messages in thread
[parent not found: <1408974008-17184-1-git-send-email-javier.martinez-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>]
* [PATCH v2 3/7] mfd: cros_ec: stop calling ->cmd_xfer() directly
[not found] ` <1408974008-17184-1-git-send-email-javier.martinez-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
@ 2014-08-25 13:40 ` Javier Martinez Canillas
2014-08-25 13:40 ` [PATCH v2 7/7] Input: cros_ec_keyb: Optimize ghosting algorithm Javier Martinez Canillas
1 sibling, 0 replies; 17+ messages in thread
From: Javier Martinez Canillas @ 2014-08-25 13:40 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, 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>
Reviewed-by: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Acked-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@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 97e6369..ec5c38d 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 0bdbf2d..f8d4a8b 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] 17+ messages in thread
* [PATCH v2 7/7] Input: cros_ec_keyb: Optimize ghosting algorithm.
[not found] ` <1408974008-17184-1-git-send-email-javier.martinez-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
2014-08-25 13:40 ` [PATCH v2 3/7] mfd: cros_ec: stop calling ->cmd_xfer() directly Javier Martinez Canillas
@ 2014-08-25 13:40 ` Javier Martinez Canillas
2014-09-04 0:06 ` Dmitry Torokhov
1 sibling, 1 reply; 17+ messages in thread
From: Javier Martinez Canillas @ 2014-08-25 13:40 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, 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 f8d4a8b..462bfcb 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] 17+ messages in thread
* Re: [PATCH v2 7/7] Input: cros_ec_keyb: Optimize ghosting algorithm.
2014-08-25 13:40 ` [PATCH v2 7/7] Input: cros_ec_keyb: Optimize ghosting algorithm Javier Martinez Canillas
@ 2014-09-04 0:06 ` Dmitry Torokhov
0 siblings, 0 replies; 17+ messages in thread
From: Dmitry Torokhov @ 2014-09-04 0:06 UTC (permalink / raw)
To: Javier Martinez Canillas
Cc: Lee Jones, Wolfram Sang, Doug Anderson, Simon Glass,
Bill Richardson, Andrew Bresticker, Derek Basehore, Todd Broch,
Olof Johansson, linux-i2c, linux-input, linux-samsung-soc
On Mon, Aug 25, 2014 at 03:40:08PM +0200, Javier Martinez Canillas wrote:
> From: Todd Broch <tbroch@chromium.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@chromium.org>
> Reviewed-by: Vincent Palatin <vpalatin@chromium.org>
> Reviewed-by: Luigi Semenzato <semenzato@chromium.org>
> Tested-by: Andreas Färber <afaerber@suse.de>
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
Applied, thank you.
--
Dmitry
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 4/7] mfd: cros_ec: move locking into cros_ec_cmd_xfer
2014-08-25 13:40 [PATCH v2 0/7] Second batch of cleanups for cros_ec Javier Martinez Canillas
` (2 preceding siblings ...)
[not found] ` <1408974008-17184-1-git-send-email-javier.martinez-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
@ 2014-08-25 13:40 ` Javier Martinez Canillas
2014-08-25 13:40 ` [PATCH v2 5/7] mfd: cros_ec: wait for completion of commands that return IN_PROGRESS Javier Martinez Canillas
` (2 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Javier Martinez Canillas @ 2014-08-25 13:40 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, 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 driver.
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>
Reviewed-by: Doug Anderson <dianders@chromium.org>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
Changes since v1:
- Remove mention of LPC driver in the commit message since it does not
exist in mainline yet. Suggested by Doug Anderson.
---
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] 17+ messages in thread
* [PATCH v2 5/7] mfd: cros_ec: wait for completion of commands that return IN_PROGRESS
2014-08-25 13:40 [PATCH v2 0/7] Second batch of cleanups for cros_ec Javier Martinez Canillas
` (3 preceding siblings ...)
2014-08-25 13:40 ` [PATCH v2 4/7] mfd: cros_ec: move locking into cros_ec_cmd_xfer Javier Martinez Canillas
@ 2014-08-25 13:40 ` Javier Martinez Canillas
[not found] ` <20140904083426.GD8257@lee--X1>
2014-08-25 13:40 ` [PATCH v2 6/7] mfd: cros_ec: Instantiate sub-devices from device tree Javier Martinez Canillas
2014-08-25 17:05 ` [PATCH v2 0/7] Second batch of cleanups for cros_ec Dmitry Torokhov
6 siblings, 1 reply; 17+ messages in thread
From: Javier Martinez Canillas @ 2014-08-25 13:40 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, 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>
---
Changes since v1:
- The *xfer() calls don't modify the passed cros_ec_command so there is
no need to populate it inside the for loop. Suggested by Lee Jones.
---
drivers/mfd/cros_ec.c | 34 +++++++++++++++++++++++++++++++++-
1 file changed, 33 insertions(+), 1 deletion(-)
diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index c53804a..cd0c93c 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,38 @@ 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;
+ struct cros_ec_command status_msg;
+ struct ec_response_get_comms_status status;
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.
+ */
+ 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);
+
+ for (i = 0; i < EC_COMMAND_RETRIES; i++) {
+ msleep(EC_RETRY_DELAY_MS);
+
+ 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] 17+ messages in thread
* [PATCH v2 6/7] mfd: cros_ec: Instantiate sub-devices from device tree
2014-08-25 13:40 [PATCH v2 0/7] Second batch of cleanups for cros_ec Javier Martinez Canillas
` (4 preceding siblings ...)
2014-08-25 13:40 ` [PATCH v2 5/7] mfd: cros_ec: wait for completion of commands that return IN_PROGRESS Javier Martinez Canillas
@ 2014-08-25 13:40 ` Javier Martinez Canillas
[not found] ` <20140904082513.GC8257@lee--X1>
2014-08-25 17:05 ` [PATCH v2 0/7] Second batch of cleanups for cros_ec Dmitry Torokhov
6 siblings, 1 reply; 17+ messages in thread
From: Javier Martinez Canillas @ 2014-08-25 13:40 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, linux-i2c, linux-input, linux-samsung-soc,
Javier Martinez Canillas
From: Todd Broch <tbroch@chromium.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@chromium.org>
Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
Changes since v1:
- Don't leave an empty struct mfd_cell array. Suggested by Lee Jones.
- Just use of_platform_populate() instead of manually iterating through
sub-devices and calling mfd_add_devices. Suggested by Lee Jones.
---
drivers/mfd/cros_ec.c | 35 ++++++++++++++++-------------------
1 file changed, 16 insertions(+), 19 deletions(-)
diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index cd0c93c..ab70791 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>
@@ -107,22 +108,12 @@ 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;
+#ifdef CONFIG_OF
+ struct device_node *node = dev->of_node;
+#endif
int err = 0;
if (ec_dev->din_size) {
@@ -138,13 +129,19 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
mutex_init(&ec_dev->lock);
- err = mfd_add_devices(dev, 0, cros_devs,
- ARRAY_SIZE(cros_devs),
- NULL, ec_dev->irq, NULL);
- if (err) {
- dev_err(dev, "failed to add mfd devices\n");
- return err;
+#ifdef CONFIG_OF
+ /*
+ * Add sub-devices declared in the device tree. NOTE they should NOT be
+ * declared in cros_devs
+ */
+ if (node) {
+ err = of_platform_populate(node, NULL, NULL, dev);
+ if (err) {
+ dev_err(dev, "fail to add %s\n", node->full_name);
+ return err;
+ }
}
+#endif
dev_info(dev, "Chrome EC device registered\n");
--
2.0.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 0/7] Second batch of cleanups for cros_ec
2014-08-25 13:40 [PATCH v2 0/7] Second batch of cleanups for cros_ec Javier Martinez Canillas
` (5 preceding siblings ...)
2014-08-25 13:40 ` [PATCH v2 6/7] mfd: cros_ec: Instantiate sub-devices from device tree Javier Martinez Canillas
@ 2014-08-25 17:05 ` Dmitry Torokhov
[not found] ` <20140825170533.GA29350-WlK9ik9hQGAhIp7JRqBPierSzoNAToWh@public.gmane.org>
6 siblings, 1 reply; 17+ messages in thread
From: Dmitry Torokhov @ 2014-08-25 17:05 UTC (permalink / raw)
To: Javier Martinez Canillas
Cc: Lee Jones, Wolfram Sang, Doug Anderson, Simon Glass,
Bill Richardson, Andrew Bresticker, Derek Basehore, Todd Broch,
Olof Johansson, linux-i2c, linux-input, linux-samsung-soc
On Mon, Aug 25, 2014 at 03:40:01PM +0200, Javier Martinez Canillas wrote:
> This is a second batch of cleanups patches for the mfd cros_ec
> driver and its subdevices drivers. The first batch of cleanups
> was posted by Doug Anderson [0] and have already been merged.
> The patches were picked from the ChromeOS 3.8 kernel and after
> these no cleanups patches for cros_ec are left, only commits
> that add cros ec support not yet available in mainline.
>
> This is a second version of the patch series that addresses
> issues pointed out by Doug Anderson and Lee Jones on v1 [1].
>
> 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 | 94 ++++++++++++++++++---------------
> drivers/mfd/cros_ec.c | 78 ++++++++++++++++++++-------
> drivers/mfd/cros_ec_spi.c | 20 ++++---
> include/linux/mfd/cros_ec.h | 24 ++++++---
> 5 files changed, 141 insertions(+), 80 deletions(-)
>
> Patches #1, #2, #6 and #7 do not depend of others so they can be
> merged independently but patches #3, #4 and #5 have to be merged
> in that specific order since they depend on the previous one.
#7 does not apply to my tree (I guess it depends on the 1st batch which I
expect will go through MFD tree?). Maybe you could rebase it on top of my next
so it can be applied sooner? Or it really needs parts of patchset #1?
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 17+ messages in thread