[PATCH v2] cros_ec_i2c: prevent i2c timeout for EC_CMD_FLASH_ERASE

[PATCH v2] cros_ec_i2c: prevent i2c timeout for EC_CMD_FLASH_ERASE

[Wei-Ning Huang]

From: Wei-Ning Huang <wnhuang@chromium.org>

Some EC chip has larger flash sector size which requires longer erase
time. During erase the CPU is usually stalled and can't even respond to
interrupts. We sleep a while to block any EC command from executing
during the flash erase period.

Signed-off-by: Wei-Ning Huang <wnhuang@chromium.org>
---
 drivers/mfd/cros_ec_i2c.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/mfd/cros_ec_i2c.c b/drivers/mfd/cros_ec_i2c.c
index 9f70de1e4c70..8f23d5a8cc5b 100644
--- a/drivers/mfd/cros_ec_i2c.c
+++ b/drivers/mfd/cros_ec_i2c.c
@@ -23,6 +23,14 @@
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 
+/*
+ * Some EC chip has larger flash sector size which requires longer erase time.
+ * During erase the CPU is usually stalled and can't even respond to
+ * interrupts. We sleep for a while to block any EC command from executing
+ * during the flash erase period to prevent i2c timeout.
+ */
+#define EC_FLASH_ERASE_DELAY_MS	5000
+
 /**
  * Request format for protocol v3
  * byte 0	0xda (EC_COMMAND_PROTOCOL_3)
@@ -177,6 +185,16 @@ static int cros_ec_pkt_xfer_i2c(struct cros_ec_device *ec_dev,
 
 	ret = ec_response->data_len;
 
+	/*
+	 * If we get EC_RES_IN_PROGRESS for EC_CMD_FLASH_ERASE this means EC
+	 * need a long time to erase flash, during flash erase CPU is stalled
+	 * and can't respond to interrupts, so we sleep for a while to stop new
+	 * EC commands from communicating with EC.
+	 */
+	if (msg->command == EC_CMD_FLASH_ERASE &&
+	    msg->result == EC_RES_IN_PROGRESS)
+		msleep(EC_FLASH_ERASE_DELAY_MS);
+
 done:
 	if (msg->command == EC_CMD_REBOOT_EC)
 		msleep(EC_REBOOT_DELAY_MS);
-- 
2.12.2

Re: [PATCH v2] cros_ec_i2c: prevent i2c timeout for EC_CMD_FLASH_ERASE

[Doug Anderson]

Hi,

On Mon, May 15, 2017 at 9:22 AM, Wei-Ning Huang <wnhuang@chromium.org> wrote:
> From: Wei-Ning Huang <wnhuang@chromium.org>
>
> Some EC chip has larger flash sector size which requires longer erase
> time. During erase the CPU is usually stalled and can't even respond to
> interrupts. We sleep a while to block any EC command from executing
> during the flash erase period.
>
> Signed-off-by: Wei-Ning Huang <wnhuang@chromium.org>
> ---
>  drivers/mfd/cros_ec_i2c.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)

A few notes:

* I added Randall to the v1 thread, but you dropped him here.  When
someone gets CCed to a patch, it's nice to add them to future
versions.

* I added my Reviewed-by to v1.  When sending a v2, it's nice to carry
that forward since there were no significant changes from v1 to v2.

* I just talked to Randall, and he has an alternate proposal that
avoids the hardcoded delay.  It looks like discussion will carry
forward on the gerrit review.  Once Randall is happy then it'd be good
to post a v3.


-Doug