pd: Return immediately to host from erase RW.

With CL:
 5ef45ad pd: Add timeout for HC flash commands.

I thought I'd licked the timeout related issues with flashing dingdong
& hoho.

With further testing however I found I was occassionally hitting the
failure where I couldn't return from 'flash erase' on PD peripheral
prior to the 1 second limit for host command timeout.

That could be remedied by retrying the erase on the host side which
then succeeds quickly.  That solution seems non-optimal however.
Additionally, even when erase does succeed in <1sec we have the shared
i2c bus pending.  That too is non-optimal.

For those reasons I've decide to return immediately from flash erase
command and instead put the burden of waiting the necessary time on
the host which at least does have some wider perspective on what the
system is attached to and doing.

CL also adds following related changes:
  1. corresponding ectool edit to delay 3sec after RW erase.
  2. fixup to error returns from hc_remote_flash for timeouts.

Signed-off-by: Todd Broch <tbroch@chromium.org>

BRANCH=samus
BUG=chrome-os-partner:33947
TEST=flashing dingdong/hoho via ectool works reliably.

Change-Id: I8fbfb592f760273b26bcb16b67210d569454eee2
Reviewed-on: https://chromium-review.googlesource.com/239253
Trybot-Ready: Todd Broch <tbroch@chromium.org>
Tested-by: Todd Broch <tbroch@chromium.org>
Reviewed-by: Alec Berg <alecaberg@chromium.org>
Commit-Queue: Todd Broch <tbroch@chromium.org>

3 files changed, 27 insertions, 16 deletions

diff --git a/common/usb_pd_protocol.c b/common/usb_pd_protocol.c
index 586afdcebd..fdacae05f9 100644
--- a/common/usb_pd_protocol.c
+++ b/common/usb_pd_protocol.c
@@ -3078,7 +3078,7 @@ static int hc_remote_flash(struct host_cmd_handler_args *args)
 	const struct ec_params_usb_pd_fw_update *p = args->params;
 	int port = p->port;
 	const uint32_t *data = &(p->size) + 1;
-	int i, size;
+	int i, size, rv = EC_RES_SUCCESS;
 	timestamp_t timeout;
 
 	if (p->size + sizeof(*p) > args->params_size)
@@ -3095,25 +3095,31 @@ static int hc_remote_flash(struct host_cmd_handler_args *args)
 		return EC_RES_UNAVAILABLE;
 #endif
 
+	/*
+	 * Busy still with a VDM that host likely generated.  1 deep VDM queue
+	 * so just return for retry logic on host side to deal with.
+	 */
+	if (pd[port].vdm_state > 0)
+		return EC_RES_BUSY;
+
 	switch (p->cmd) {
 	case USB_PD_FW_REBOOT:
 		pd_send_vdm(port, USB_VID_GOOGLE, VDO_CMD_REBOOT, NULL, 0);
 
-		/* Delay to give time for device to reboot */
-		usleep(PD_HOST_COMMAND_TIMEOUT_US - 100*MSEC);
+		/*
+		 * Return immediately to free pending i2c bus.	Host needs to
+		 * manage this delay.
+		 */
 		return EC_RES_SUCCESS;
 
 	case USB_PD_FW_FLASH_ERASE:
 		pd_send_vdm(port, USB_VID_GOOGLE, VDO_CMD_FLASH_ERASE, NULL, 0);
+
 		/*
-		 * TODO: Note, page erase for 2K is 20-40msec while host command
-		 * timeout is 1sec so this can't be longer.  In practice,
-		 * for 32 pages, its passed.  Better way to implement may be to
-		 * push burden to HOST to manage latency.
+		 * Return immediately.	Host needs to manage delays here which
+		 * can be as long as 1.2 seconds on 64KB RW flash.
 		 */
-		timeout.val = get_time().val +
-			PD_HOST_COMMAND_TIMEOUT_US - 100*MSEC;
-		break;
+		return EC_RES_SUCCESS;
 
 	case USB_PD_FW_ERASE_SIG:
 		pd_send_vdm(port, USB_VID_GOOGLE, VDO_CMD_ERASE_SIG, NULL, 0);
@@ -3150,12 +3156,13 @@ static int hc_remote_flash(struct host_cmd_handler_args *args)
 	while ((pd[port].vdm_state > 0) && (get_time().val < timeout.val))
 		task_wait_event(50*MSEC);
 
-	if (pd[port].vdm_state < 0)
-		return EC_RES_ERROR;
-	if (pd[port].vdm_state > 0)
-		return EC_RES_TIMEOUT;
+	if ((pd[port].vdm_state > 0) ||
+	    (pd[port].vdm_state == VDM_STATE_ERR_TMOUT))
+		rv = EC_RES_TIMEOUT;
+	else if (pd[port].vdm_state < 0)
+		rv = EC_RES_ERROR;
 
-	return EC_RES_SUCCESS;
+	return rv;
 }
 DECLARE_HOST_COMMAND(EC_CMD_USB_PD_FW_UPDATE,
 		     hc_remote_flash,
diff --git a/include/ec_commands.h b/include/ec_commands.h
index 0f98f70a79..55aef77a7a 100644
--- a/include/ec_commands.h
+++ b/include/ec_commands.h
@@ -224,7 +224,8 @@ enum ec_status {
 	EC_RES_INVALID_HEADER = 12,     /* Header contains invalid data */
 	EC_RES_REQUEST_TRUNCATED = 13,  /* Didn't get the entire request */
 	EC_RES_RESPONSE_TOO_BIG = 14,   /* Response was too big to handle */
-	EC_RES_BUS_ERROR = 15           /* Communications bus error */
+	EC_RES_BUS_ERROR = 15,          /* Communications bus error */
+	EC_RES_BUSY = 16                /* Up but too busy.  Should retry */
 };
 
 /*
diff --git a/util/ectool.c b/util/ectool.c
index 8a5e63a9f4..4509951dbd 100644
--- a/util/ectool.c
+++ b/util/ectool.c
@@ -949,6 +949,9 @@ int cmd_flash_pd(int argc, char *argv[])
 	rv = ec_command(EC_CMD_USB_PD_FW_UPDATE, 0,
 			p, p->size + sizeof(*p), NULL, 0);
 
+	/* 3 secs should allow ample time for 2KB page erases at 40ms */
+	usleep(3000000);
+
 	if (rv < 0)
 		goto pd_flash_error;