g: introduce versioning and backwards compatibility of usb_upgrade

The original version of usb_upgrade does not provide for any error
recovery and also does not support any protocol version notion.

Real life experience has shown that error recovery is essential, it
needs to be introduced as a protocol enhancement. We want to stay
backwards compatible though, so there is a need for protocol
versioning.

In the original version of the protocol target response is always the
same: a 4 byte number which is the error code (and zero means no
error). This patch modifies response to the very first packet from the
host, the startup packet. The startup response is 8 bytes long. The
first 4 bytes is still the same as before, the second 4 bytes carry
the protocol version supported by the target, an integer in network
byte order.

Thus, receiving a 4 byte reply to the startup message tells the host
that the target is running protocol version zero, 8 byte reply carries
the actual version number in the last 4 bytes.

The USB transfer function on the host is enhanced to accept responses
shorter then expected, when allowed.

BRANCH=none
BUG=chrome-os-partner:52856
TEST=usb_updater can successfully update both old and new cr50 images,
     properly reporting protocol version as 0 or 1 respectively.

Change-Id: I9920d2708b21f29615282161fc0eb027018f9378
Signed-off-by: Vadim Bendebury <vbendeb@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/341617
Reviewed-by: Mary Ruthven <mruthven@chromium.org>

1 files changed, 33 insertions, 4 deletions

diff --git a/chip/g/usb_upgrade.c b/chip/g/usb_upgrade.c
index df3d34ca11..634556cffb 100644
--- a/chip/g/usb_upgrade.c
+++ b/chip/g/usb_upgrade.c
@@ -97,6 +97,8 @@ static uint32_t block_index;
  */
 static uint64_t prev_activity_timestamp;
 
+#define UPGRADE_PROTOCOL_VERSION 1
+
 /* Called to deal with data from the host */
 static void upgrade_out_handler(struct consumer const *consumer, size_t count)
 {
@@ -124,6 +126,29 @@ static void upgrade_out_handler(struct consumer const *consumer, size_t count)
 	}
 
 	if (rx_state_ == rx_idle) {
+		/*
+		 * When responding to the very first packet of the upgrade
+		 * sequence, the original implementation was responding with a
+		 * four byte value, just as to any other block of the transfer
+		 * sequence.
+		 *
+		 * It became clear that there is a need to be able to enhance
+		 * the upgrade protocol, while stayng backwards compatible. To
+		 * achieve that we respond to the very first packet with an 8
+		 * byte value, the first 4 bytes the same as before, the
+		 * second 4 bytes - the protocol version number.
+		 *
+		 * This way if on the host side receiving of a four byte value
+		 * in response to the first packet is an indication of the
+		 * 'legacy' protocol, version 0. Receiving of an 8 byte
+		 * response would communicate the protocol version in the
+		 * second 4 bytes.
+		 */
+		struct {
+			uint32_t value;
+			uint32_t version;
+		} startup_resp;
+
 		/* This better be the first block, of zero size. */
 		if (count != sizeof(struct update_pdu_header)) {
 			CPRINTS("FW update: wrong first block size %d",
@@ -140,15 +165,19 @@ static void upgrade_out_handler(struct consumer const *consumer, size_t count)
 					   &resp_size);
 
 		if (resp_size == 4) {
-			resp_value = updu.resp; /* Already in network order. */
+			/* Already in network order. */
+			startup_resp.value = updu.resp;
 			rx_state_ = rx_outside_block;
 		} else {
 			/* This must be a single byte error code. */
-			resp_value = htobe32(*((uint8_t *)&updu.resp));
+			startup_resp.value = htobe32(*((uint8_t *)&updu.resp));
 		}
+
+		startup_resp.version = htobe32(UPGRADE_PROTOCOL_VERSION);
+
 		/* Let the host know what upgrader had to say. */
-		QUEUE_ADD_UNITS(&upgrade_to_usb, &resp_value,
-				sizeof(resp_value));
+		QUEUE_ADD_UNITS(&upgrade_to_usb, &startup_resp,
+				sizeof(startup_resp));
 		return;
 	}