mcdp28x0: Add checksum verification to rx_serial.

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

BRANCH=samus
BUG=chrome-os-partner:35939
TEST=compiles, MCDP_READY gpio still asserted.

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

2 files changed, 22 insertions, 13 deletions

diff --git a/driver/mcdp28x0.c b/driver/mcdp28x0.c
index 7fba0bd035..a3a2043619 100644
--- a/driver/mcdp28x0.c
+++ b/driver/mcdp28x0.c
@@ -50,15 +50,15 @@ USART_CONFIG(usart_mcdp, CONFIG_MCDP28X0, 115200, rx_queue, tx_queue,
 /**
  * Compute checksum.
  *
+ * @seed initial value of checksum.
  * @msg message bytes to compute checksum on.
  * @cnt count of message bytes.
  * @return partial checksum.
  */
-static uint8_t compute_checksum(const uint8_t *msg, int cnt)
+static uint8_t compute_checksum(uint8_t seed, const uint8_t *msg, int cnt)
 {
 	int i;
-	/* 1st byte (not in msg) is always cnt + 2, so seed chksum with that */
-	uint8_t chksum = cnt + 2;
+	uint8_t chksum = seed;
 
 	for (i = 0; i < cnt; i++)
 		chksum += msg[i];
@@ -69,8 +69,8 @@ static uint8_t compute_checksum(const uint8_t *msg, int cnt)
  * transmit message over serial
  *
  * Packet consists of:
- *   msg[0]     == length including this byte + cnt + chksum
- *   msg[1]     == 1st message byte
+ *   msg[0]     == length of entire packet
+ *   msg[1]     == 1st message byte (typically command)
  *   msg[cnt+1] == last message byte
  *   msg[cnt+2] == checksum
  *
@@ -81,7 +81,8 @@ static uint8_t compute_checksum(const uint8_t *msg, int cnt)
 static int tx_serial(const uint8_t *msg, int cnt)
 {
 	uint8_t out = cnt + 2;
-	uint8_t chksum = compute_checksum(msg, cnt);
+	/* 1st byte (not in msg) is always cnt + 2, so seed chksum with that */
+	uint8_t chksum = compute_checksum(cnt + 2, msg, cnt);
 
 	if (out_stream_write(&usart_out.out, &out, 1) != 1)
 		return EC_ERROR_UNKNOWN;
@@ -100,6 +101,15 @@ static int tx_serial(const uint8_t *msg, int cnt)
 /**
  * receive message over serial
  *
+ * While definitive documentation is lacking its believed the following receive
+ * packet is always true.
+ *
+ * Packet consists of:
+ *   msg[0]     == length of entire packet
+ *   msg[1]     == 1st message byte (typically command)
+ *   msg[cnt+2] == last message byte
+ *   msg[cnt+3] == checksum
+ *
  * @msg pointer to buffer to read message into
  * @cnt count of message bytes
  * @return zero if success, error code otherwise
@@ -119,6 +129,9 @@ static int rx_serial(uint8_t *msg, int cnt)
 
 	print_buffer(msg, cnt);
 
+	if (msg[cnt-1] != compute_checksum(0, msg, cnt-1))
+		return EC_ERROR_UNKNOWN;
+
 	return !(read == cnt);
 }
 
@@ -143,11 +156,6 @@ int mcdp_get_info(struct mcdp_info  *info)
 	if (rx_serial(inbuf, sizeof(inbuf)))
 		return EC_ERROR_UNKNOWN;
 
-	/* check length & returned command ... no checksum provided */
-	if (((inbuf[0] - 1) != sizeof(inbuf)) ||
-	    (inbuf[1] != MCDP_CMD_GETINFO))
-		return EC_ERROR_UNKNOWN;
-
 	memcpy(info, &inbuf[2], MCDP_LEN_GETINFO);
 
 #ifdef MCDP_DEBUG
diff --git a/driver/mcdp28x0.h b/driver/mcdp28x0.h
index c32c9fe4aa..a2af5d8494 100644
--- a/driver/mcdp28x0.h
+++ b/driver/mcdp28x0.h
@@ -12,10 +12,11 @@
 #define MCDP_INBUF_MAX 16
 
 #define MCDP_CMD_GETINFO 0x40
+
+/* packet header (2 bytes: length + cmd) + data + footer (1byte: checksum) */
+#define MCDP_RSP_LEN(len) (len + 3)
 #define MCDP_LEN_GETINFO 12
 
-/* length byte + cmd byte + data len */
-#define MCDP_RSP_LEN(len) (len + 2)
 
 /**
  * Enable mcdp driver.