Move protocol v2 constants to ec_commands.h

These constants are scattered around the various interface
implementations and should be in one place.  This will also clean up
the u-boot side when ec_commands.h is copied there.

BUG=chrome-os-partner:20257
BRANCH=none
TEST=build link, spring, pit; test 'ectool hello'

Change-Id: Ib1425db00ec8220538d8c5c65107ac9548009516
Signed-off-by: Randall Spangler <rspangler@chromium.org>
Reviewed-on: https://gerrit.chromium.org/gerrit/60810
Reviewed-by: Bill Richardson <wfrichar@chromium.org>
Reviewed-by: Simon Glass <sjg@chromium.org>

8 files changed, 158 insertions, 142 deletions

diff --git a/chip/lm4/lpc.c b/chip/lm4/lpc.c
index 6648a56dad..7712d13cd4 100644
--- a/chip/lm4/lpc.c
+++ b/chip/lm4/lpc.c
@@ -202,7 +202,7 @@ static void lpc_send_response(struct host_cmd_handler_args *args)
 	lpc_host_args->checksum = (uint8_t)csum;
 
 	/* Fail if response doesn't fit in the param buffer */
-	if (size > EC_HOST_PARAM_SIZE)
+	if (size > EC_PROTO2_MAX_PARAM_SIZE)
 		args->result = EC_RES_INVALID_RESPONSE;
 	else if (host_cmd_args.response != cmd_params)
 		memcpy(cmd_params, args->response, size);
@@ -532,11 +532,11 @@ static void handle_host_write(int is_cmd)
 		host_cmd_args.params = params_copy;
 		host_cmd_args.params_size = size;
 		host_cmd_args.response = cmd_params;
-		host_cmd_args.response_max = EC_HOST_PARAM_SIZE;
+		host_cmd_args.response_max = EC_PROTO2_MAX_PARAM_SIZE;
 		host_cmd_args.response_size = 0;
 
 		/* Verify params size */
-		if (size > EC_HOST_PARAM_SIZE) {
+		if (size > EC_PROTO2_MAX_PARAM_SIZE) {
 			host_cmd_args.result = EC_RES_INVALID_PARAM;
 		} else {
 			const uint8_t *src = cmd_params;
diff --git a/chip/stm32/i2c-stm32f.c b/chip/stm32/i2c-stm32f.c
index 3cdcbe84fd..4fbacc1713 100644
--- a/chip/stm32/i2c-stm32f.c
+++ b/chip/stm32/i2c-stm32f.c
@@ -100,7 +100,7 @@ static const struct dma_option dma_rx_option[I2C_PORT_COUNT] = {
 static uint16_t i2c_sr1[I2C_PORT_COUNT];
 
 /* Buffer for host commands (including version, error code and checksum) */
-static uint8_t host_buffer[EC_HOST_PARAM_SIZE + 4];
+static uint8_t host_buffer[EC_PROTO2_MAX_REQUEST_SIZE];
 static struct host_cmd_handler_args host_cmd_args;
 static uint8_t i2c_old_response;  /* Send an old-style response */
 
@@ -220,9 +220,15 @@ static void i2c_process_command(void)
 		buff += 3;
 		i2c_old_response = 0;
 	} else {
-		/* Old style command */
+		/*
+		 * Old style (version 1) command.
+		 *
+		 * TODO(rspangler): Nothing sends these anymore, since this was
+		 * superseded by version 2 before snow launched.  This code
+		 * should be safe to remove.
+		 */
 		args->version = 0;
-		args->params_size = EC_HOST_PARAM_SIZE;	/* unknown */
+		args->params_size = EC_PROTO2_MAX_PARAM_SIZE;	/* unknown */
 		buff++;
 		i2c_old_response = 1;
 	}
@@ -232,7 +238,7 @@ static void i2c_process_command(void)
 	args->params = buff;
 	/* skip room for error code, arglen */
 	args->response = host_buffer + 2;
-	args->response_max = EC_HOST_PARAM_SIZE;
+	args->response_max = EC_PROTO2_MAX_PARAM_SIZE;
 	args->response_size = 0;
 
 	host_command_received(args);
diff --git a/chip/stm32/spi.c b/chip/stm32/spi.c
index 4f65820d9a..cbe90d4816 100644
--- a/chip/stm32/spi.c
+++ b/chip/stm32/spi.c
@@ -33,61 +33,49 @@ static const struct dma_option dma_rx_option = {
 	STM32_DMA_CCR_MSIZE_8_BIT | STM32_DMA_CCR_PSIZE_16_BIT
 };
 
+/* Special byte values */
+#define SPI_STALL_BYTE   0xfd  /* Bytes sent when EC isn't ready to respond */
+#define SPI_PAD_BYTE     0xff  /* Bytes which precede the framing byte */
+#define SPI_FRAMING_BYTE 0xec  /* Used by AP to find response start */
+
 /*
- * Since message.c no longer supports our protocol, we must do it all here.
- *
- * We allow a preamble and a header byte so that SPI can function at all.
- * We also add an 8-bit length so that we can tell that we got the whole
- * message, since the master decides how many bytes to read.
+ * Timeout to wait for SPI request packet
  *
- * TODO: move these constants to ec_commands.h
+ * TODO(sjg@chromium.org): Support much slower SPI clocks. For 4096 we have a
+ * delay of 4ms. For the largest message (68 bytes) this is 130KhZ, assuming
+ * that the master starts sending bytes as soon as it drops NSS. In practice,
+ * this timeout seems adequately high for a 1MHz clock which is as slow as we
+ * would reasonably want it.
  */
-enum {
-	/* The bytes which appear before the header in a message */
-	SPI_MSG_PREAMBLE_BYTE	= 0xff,
-
-	/* The header byte, which follows the preamble */
-	SPI_MSG_HEADER_BYTE1	= 0xfe,
-	SPI_MSG_HEADER_BYTE2	= 0xec,
+#define SPI_CMD_RX_TIMEOUT_US 8192
 
-	SPI_MSG_HEADER_LEN	= 4,
-	SPI_MSG_TRAILER_LEN	= 2,
-	SPI_MSG_PROTO_LEN	= SPI_MSG_HEADER_LEN + SPI_MSG_TRAILER_LEN,
+/* Offset of output parameters needs to account for pad and framing bytes */
+#define SPI_PROTO2_OFFSET (EC_PROTO2_RESPONSE_HEADER_BYTES + 2)
+#define SPI_PROTO2_OVERHEAD (SPI_PROTO2_OFFSET +		\
+			     EC_PROTO2_RESPONSE_TRAILER_BYTES)
 
-	/*
-	 * Timeout to wait for SPI command
-	 * TODO(sjg@chromium.org): Support much slower SPI clocks. For 4096
-	 * we have a delay of 4ms. For the largest message (68 bytes) this
-	 * is 130KhZ, assuming that the master starts sending bytes as soon
-	 * as it drops NSS. In practice, this timeout seems adequately high
-	 * for a 1MHz clock which is as slow as we would reasonably want it.
-	 */
-	SPI_CMD_RX_TIMEOUT_US	= 8192,
-
-	/*
-	 * Max data size for request/response packet.  This is big enough
-	 * to handle a request/respose header, flash write offset/size, and
-	 * 512 bytes of flash data.
-	 */
-	SPI_MAX_REQUEST_SIZE = 0x220,
-	SPI_MAX_RESPONSE_SIZE = 0x220,
-};
+/*
+ * Max data size for a version 3 request/response packet.  This is big enough
+ * to handle a request/response header, flash write offset/size, and 512 bytes
+ * of flash data.
+ */
+#define SPI_MAX_REQUEST_SIZE 0x220
+#define SPI_MAX_RESPONSE_SIZE 0x220
 
 /*
- * The AP blindly clocks back bytes over the SPI interface looking for the
- * header bytes.  So this preamble must always precede the actual response
- * packet.  The preamble must be 32-bit aligned so that the response buffer is
- * also 32-bit aligned.
+ * The AP blindly clocks back bytes over the SPI interface looking for a
+ * framing byte.  So this preamble must always precede the actual response
+ * packet.  Search for "spi-frame-header" in U-boot to see how that's
+ * implemented.
  *
- * Really, only HEADER_BYTE2 matters, since the SPI driver ignores everything
- * until it sees that byte code.  Search for "spi-frame-header" in U-boot to
- * see how that's implemented.
+ * The preamble must be 32-bit aligned so that the response buffer is also
+ * 32-bit aligned.
  */
 static const uint8_t out_preamble[4] = {
-	SPI_MSG_PREAMBLE_BYTE,
-	SPI_MSG_PREAMBLE_BYTE,
-	SPI_MSG_HEADER_BYTE1,
-	SPI_MSG_HEADER_BYTE2,  /* This is the byte which matters */
+	SPI_PAD_BYTE,
+	SPI_PAD_BYTE,
+	SPI_PAD_BYTE,
+	SPI_FRAMING_BYTE,  /* This is the byte which matters */
 };
 
 /*
@@ -123,7 +111,7 @@ static int wait_for_bytes(stm32_dma_chan_t *rxdma, int needed,
 
 	ASSERT(needed <= sizeof(in_msg));
 	deadline.val = 0;
-	for (;;) {
+	while (1) {
 		if (dma_bytes_done(rxdma, sizeof(in_msg)) >= needed)
 			return 0;
 		if (REG16(nss_reg) & nss_mask)
@@ -170,42 +158,40 @@ static int wait_for_bytes(stm32_dma_chan_t *rxdma, int needed,
  * @param txdma		TX DMA channel to send on
  * @param status	Status result to send
  * @param msg_ptr	Message payload to send, which normally starts
- *			SPI_MSG_HEADER_LEN bytes into out_msg
+ *			SPI_PROTO2_OFFSET bytes into out_msg
  * @param msg_len	Number of message bytes to send
  */
 static void reply(stm32_dma_chan_t *txdma,
 		  enum ec_status status, char *msg_ptr, int msg_len)
 {
-	char *msg;
+	char *msg = out_msg;
+	int need_copy = msg_ptr != msg + SPI_PROTO2_OFFSET;
 	int sum, i;
-	int copy;
 
-	msg = out_msg;
-	copy = msg_ptr != msg + SPI_MSG_HEADER_LEN;
+	ASSERT(msg_len + SPI_PROTO2_OVERHEAD <= sizeof(out_msg));
 
 	/* Add our header bytes - the first one might not actually be sent */
-	msg[0] = SPI_MSG_HEADER_BYTE1;
-	msg[1] = SPI_MSG_HEADER_BYTE2;
+	msg[0] = SPI_PAD_BYTE;
+	msg[1] = SPI_FRAMING_BYTE;
 	msg[2] = status;
 	msg[3] = msg_len & 0xff;
-	sum = status + msg_len;
 
-	/* Calculate the checksum */
+	/*
+	 * Calculate the checksum; includes the status and message length bytes
+	 * but not the pad and framing bytes since those are stripped by the AP
+	 * driver.
+	 */
+	sum = status + msg_len;
 	for (i = 0; i < msg_len; i++) {
-		int ch;
-
-		ch = msg_ptr[i];
+		int ch = msg_ptr[i];
 		sum += ch;
-		if (copy)
-			msg[i + SPI_MSG_HEADER_LEN] = ch;
+		if (need_copy)
+			msg[i + SPI_PROTO2_OFFSET] = ch;
 	}
-	msg_len += SPI_MSG_PROTO_LEN;
-	ASSERT(msg_len <= sizeof(out_msg));
 
 	/* Add the checksum and get ready to send */
-	msg[msg_len - 2] = sum & 0xff;
-	msg[msg_len - 1] = SPI_MSG_PREAMBLE_BYTE;
-	dma_prepare_tx(&dma_tx_option, msg_len, msg);
+	msg[SPI_PROTO2_OFFSET + msg_len] = sum & 0xff;
+	dma_prepare_tx(&dma_tx_option, msg_len + SPI_PROTO2_OVERHEAD, msg);
 
 	/* Kick off the DMA to send the data */
 	dma_go(txdma);
@@ -225,10 +211,10 @@ static void setup_for_transaction(void)
 	/* We are no longer actively processing a transaction */
 	active = 0;
 
-	/* write 0xfd which will be our default output value */
-	spi->dr = 0xfd;
+	/* Output stall byte by default */
+	spi->dr = SPI_STALL_BYTE;
 	dma_disable(STM32_DMAC_SPI1_TX);
-	*in_msg = 0xff;
+	*in_msg = SPI_PAD_BYTE;
 
 	/* read a byte in case there is one, and the rx dma gets it */
 	dmac = spi->dr;
@@ -251,12 +237,12 @@ static void spi_send_response(struct host_cmd_handler_args *args)
 	if (!active)
 		return;
 
-	if (args->response_size > EC_HOST_PARAM_SIZE)
+	if (args->response_size > EC_PROTO2_MAX_PARAM_SIZE)
 		result = EC_RES_INVALID_RESPONSE;
 
 	if ((uint8_t *)args->response >= out_msg &&
 			(uint8_t *)args->response < out_msg + sizeof(out_msg))
-		ASSERT(args->response == out_msg + SPI_MSG_HEADER_LEN);
+		ASSERT(args->response == out_msg + SPI_PROTO2_OFFSET);
 
 	/* Transmit the reply */
 	txdma = dma_get_channel(STM32_DMAC_SPI1_TX);
@@ -308,20 +294,16 @@ void spi_event(enum gpio_signal signal)
 	 * for the next.
 	 */
 	nss_reg = gpio_get_level_reg(GPIO_SPI1_NSS, &nss_mask);
-	if (REG16(nss_reg) & nss_mask) {
-		setup_for_transaction();
-		return;
-	}
+	if (REG16(nss_reg) & nss_mask)
+		goto spi_event_error;
 
 	/* Otherwise, NSS is low and we're now inside a transaction */
 	active = 1;
 	rxdma = dma_get_channel(STM32_DMAC_SPI1_RX);
 
 	/* Wait for version, command, length bytes */
-	if (wait_for_bytes(rxdma, 3, nss_reg, nss_mask)) {
-		setup_for_transaction();
-		return;
-	}
+	if (wait_for_bytes(rxdma, 3, nss_reg, nss_mask))
+		goto spi_event_error;
 
 	if (in_msg[0] == EC_HOST_REQUEST_VERSION) {
 		/* Protocol version 3 */
@@ -329,10 +311,8 @@ void spi_event(enum gpio_signal signal)
 		int pkt_size;
 
 		/* Wait for the rest of the command header */
-		if (wait_for_bytes(rxdma, sizeof(*r), nss_reg, nss_mask)) {
-			setup_for_transaction();
-			return;
-		}
+		if (wait_for_bytes(rxdma, sizeof(*r), nss_reg, nss_mask))
+			goto spi_event_error;
 
 		/*
 		 * Check how big the packet should be.  We can't just wait to
@@ -340,16 +320,12 @@ void spi_event(enum gpio_signal signal)
 		 * sending dummy data until we respond.
 		 */
 		pkt_size = host_request_expected_size(r);
-		if (pkt_size == 0 || pkt_size > sizeof(in_msg)) {
-			setup_for_transaction();
-			return;
-		}
+		if (pkt_size == 0 || pkt_size > sizeof(in_msg))
+			goto spi_event_error;
 
 		/* Wait for the packet data */
-		if (wait_for_bytes(rxdma, pkt_size, nss_reg, nss_mask)) {
-			setup_for_transaction();
-			return;
-		}
+		if (wait_for_bytes(rxdma, pkt_size, nss_reg, nss_mask))
+			goto spi_event_error;
 
 		spi_packet.send_response = spi_send_response_packet;
 
@@ -374,40 +350,40 @@ void spi_event(enum gpio_signal signal)
 		/*
 		 * Protocol version 2
 		 *
-		 * TODO: remove once all systems upgraded to version 3 */
+		 * TODO: remove once all systems upgraded to version 3
+		 */
 		args.version = in_msg[0] - EC_CMD_VERSION0;
 		args.command = in_msg[1];
 		args.params_size = in_msg[2];
 		args.params = in_msg + 3;
-	} else {
+
+		/* Wait for parameters */
+		if (wait_for_bytes(rxdma, 3 + args.params_size,
+				   nss_reg, nss_mask))
+			goto spi_event_error;
+
 		/*
-		 * Protocol version 1
-		 *
-		 * TODO: remove; nothing sends this.  Ignore this packet?
-		 * Send back an error response?
+		 * TODO: params are not 32-bit aligned in protocol version 2.
+		 * As a workaround, memmove them to the beginning of the input
+		 * buffer so they are aligned.
 		 */
-		args.version = 0;
-		args.command = in_msg[0];
-		args.params = in_msg + 1;
-		args.params_size = 0;
-	}
 
-	/* Wait for parameters */
-	if (wait_for_bytes(rxdma, 3 + args.params_size, nss_reg, nss_mask)) {
-		setup_for_transaction();
-		return;
-	}
+		/* Process the command and send the reply */
+		args.send_response = spi_send_response;
 
-	/* Process the command and send the reply */
-	args.send_response = spi_send_response;
+		/* Allow room for the header bytes */
+		args.response = out_msg + SPI_PROTO2_OFFSET;
+		args.response_max = sizeof(out_msg) - SPI_PROTO2_OVERHEAD;
+		args.response_size = 0;
+		args.result = EC_RES_SUCCESS;
 
-	/* Allow room for the header bytes */
-	args.response = out_msg + SPI_MSG_HEADER_LEN;
-	args.response_max = sizeof(out_msg) - SPI_MSG_PROTO_LEN;
-	args.response_size = 0;
-	args.result = EC_RES_SUCCESS;
+		host_command_received(&args);
+		return;
+	}
 
-	host_command_received(&args);
+ spi_event_error:
+	/* Error, or protocol we can't handle.  Set up for next transaction */
+	setup_for_transaction();
 }
 
 static void spi_init(void)
diff --git a/common/host_command.c b/common/host_command.c
index e482658580..2f8d514194 100644
--- a/common/host_command.c
+++ b/common/host_command.c
@@ -631,7 +631,7 @@ static int command_host_command(int argc, char **argv)
 	int rv;
 
 	/* Use shared memory for command params space */
-	if (shared_mem_acquire(EC_HOST_PARAM_SIZE, &cmd_params)) {
+	if (shared_mem_acquire(EC_PROTO2_MAX_PARAM_SIZE, &cmd_params)) {
 		ccputs("Can't acquire shared memory buffer.\n");
 		return EC_ERROR_UNKNOWN;
 	}
@@ -670,7 +670,7 @@ static int command_host_command(int argc, char **argv)
 	}
 
 	args.response = cmd_params;
-	args.response_max = EC_HOST_PARAM_SIZE;
+	args.response_max = EC_PROTO2_MAX_PARAM_SIZE;
 	args.response_size = 0;
 
 	res = host_command_process(&args);
diff --git a/include/ec_commands.h b/include/ec_commands.h
index 0359bb1270..5d0df934ce 100644
--- a/include/ec_commands.h
+++ b/include/ec_commands.h
@@ -43,11 +43,13 @@
 #define EC_LPC_ADDR_HOST_CMD   0x204
 
 /* I/O addresses for host command args and params */
-#define EC_LPC_ADDR_HOST_ARGS  0x800    /* and 0x801, 0x802, 0x803 */
-#define EC_LPC_ADDR_HOST_PARAM 0x804
-#define EC_HOST_PARAM_SIZE     0x0fc   /* Size of param area in bytes */
-#define EC_LPC_ADDR_HOST_PACKET 0x800  /* Offset of version 3 packet */
-#define EC_LPC_HOST_PACKET_SIZE 0x100  /* Max size of version 3 packet */
+/* Protocol version 2 */
+#define EC_LPC_ADDR_HOST_ARGS    0x800  /* And 0x801, 0x802, 0x803 */
+#define EC_LPC_ADDR_HOST_PARAM   0x804  /* For version 2 params; size is
+					 * EC_PROTO2_MAX_PARAM_SIZE */
+/* Protocol version 3 */
+#define EC_LPC_ADDR_HOST_PACKET  0x800  /* Offset of version 3 packet */
+#define EC_LPC_HOST_PACKET_SIZE  0x100  /* Max size of version 3 packet */
 
 /* The actual block is 0x800-0x8ff, but some BIOSes think it's 0x880-0x8ff
  * and they tell the kernel that so we have to think of it as two parts. */
@@ -286,6 +288,43 @@ struct ec_lpc_host_args {
 /*****************************************************************************/
 
 /*
+ * Protocol version 2 for I2C and SPI send a request this way:
+ *
+ *	0	EC_CMD_VERSION0 + (command version)
+ *	1	Command number
+ *	2	Length of params = N
+ *	3..N+2	Params, if any
+ *	N+3	8-bit checksum of bytes 0..N+2
+ *
+ * The corresponding response is:
+ *
+ *	0	Result code (EC_RES_*)
+ *	1	Length of params = M
+ *	2..M+1	Params, if any
+ *	M+2	8-bit checksum of bytes 0..M+1
+ */
+#define EC_PROTO2_REQUEST_HEADER_BYTES 3
+#define EC_PROTO2_REQUEST_TRAILER_BYTES 1
+#define EC_PROTO2_REQUEST_OVERHEAD (EC_PROTO2_REQUEST_HEADER_BYTES +	\
+				    EC_PROTO2_REQUEST_TRAILER_BYTES)
+
+#define EC_PROTO2_RESPONSE_HEADER_BYTES 2
+#define EC_PROTO2_RESPONSE_TRAILER_BYTES 1
+#define EC_PROTO2_RESPONSE_OVERHEAD (EC_PROTO2_RESPONSE_HEADER_BYTES +	\
+				     EC_PROTO2_RESPONSE_TRAILER_BYTES)
+
+/* Parameter length was limited by the LPC interface */
+#define EC_PROTO2_MAX_PARAM_SIZE 0xfc
+
+/* Maximum request and response packet sizes for protocol version 2 */
+#define EC_PROTO2_MAX_REQUEST_SIZE (EC_PROTO2_REQUEST_OVERHEAD +	\
+				    EC_PROTO2_MAX_PARAM_SIZE)
+#define EC_PROTO2_MAX_RESPONSE_SIZE (EC_PROTO2_RESPONSE_OVERHEAD +	\
+				     EC_PROTO2_MAX_PARAM_SIZE)
+
+/*****************************************************************************/
+
+/*
  * Value written to legacy command port / prefix byte to indicate protocol
  * 3+ structs are being used.  Usage is bus-dependent.
  */
diff --git a/util/comm-dev.c b/util/comm-dev.c
index c6bd6b4176..fb5fb31ae6 100644
--- a/util/comm-dev.c
+++ b/util/comm-dev.c
@@ -110,7 +110,7 @@ int comm_init_dev(void)
 	 * TODO: need a way to get this from the driver and EC.  For now,
 	 * pick a magic lowest common denominator value.
 	 */
-	ec_max_outsize = EC_HOST_PARAM_SIZE - 8;
+	ec_max_outsize = EC_PROTO2_MAX_PARAM_SIZE - 8;
 
 	return 0;
 }
diff --git a/util/comm-i2c.c b/util/comm-i2c.c
index eab48d2a99..91ceb555e2 100644
--- a/util/comm-i2c.c
+++ b/util/comm-i2c.c
@@ -32,15 +32,8 @@
 #define debug(...)
 #endif
 
-/* v2 protocol bytes
- *   OUT: (version, command, size, ... request ..., checksum) */
-#define PROTO_V2_IN    4
-/*   IN:  (command, size, ... response ..., checkcum) */
-#define PROTO_V2_OUT   3
-
 static int i2c_fd = -1;
 
-
 /*
  * Sends a command to the EC (protocol v2).  Returns the command status code, or
  * -1 if other error.
@@ -90,7 +83,7 @@ static int ec_command_i2c(int command, int version,
 	 * allocate larger packet
 	 * (version, command, size, ..., checksum)
 	 */
-	req_len = outsize + PROTO_V2_IN;
+	req_len = outsize + EC_PROTO2_REQUEST_OVERHEAD;
 	req_buf = calloc(1, req_len);
 	if (!req_buf)
 		goto done;
@@ -115,7 +108,7 @@ static int ec_command_i2c(int command, int version,
 	 * allocate larger packet
 	 * (result, size, ..., checksum)
 	 */
-	resp_len = insize + PROTO_V2_OUT;
+	resp_len = insize + EC_PROTO2_RESPONSE_OVERHEAD;
 	resp_buf = calloc(1, resp_len);
 	if (!resp_buf)
 		goto done;
@@ -213,7 +206,7 @@ int comm_init_i2c(void)
 	free(file_path);
 
 	ec_command = ec_command_i2c;
-	ec_max_outsize = ec_max_insize = EC_HOST_PARAM_SIZE;
+	ec_max_outsize = ec_max_insize = EC_PROTO2_MAX_PARAM_SIZE;
 
 	return 0;
 }
diff --git a/util/comm-lpc.c b/util/comm-lpc.c
index f2c635770e..adbbb89f10 100644
--- a/util/comm-lpc.c
+++ b/util/comm-lpc.c
@@ -266,13 +266,13 @@ int comm_init_lpc(void)
 	 */
 	byte &= inb(EC_LPC_ADDR_HOST_CMD);
 	byte &= inb(EC_LPC_ADDR_HOST_DATA);
-	for (i = 0; i < EC_HOST_PARAM_SIZE && byte == 0xff; ++i)
+	for (i = 0; i < EC_PROTO2_MAX_PARAM_SIZE && byte == 0xff; ++i)
 		byte &= inb(EC_LPC_ADDR_HOST_PARAM + i);
 	if (byte == 0xff) {
 		fprintf(stderr, "Port 0x%x,0x%x,0x%x-0x%x are all 0xFF.\n",
 			EC_LPC_ADDR_HOST_CMD, EC_LPC_ADDR_HOST_DATA,
 			EC_LPC_ADDR_HOST_PARAM,
-			EC_LPC_ADDR_HOST_PARAM + EC_HOST_PARAM_SIZE - 1);
+			EC_LPC_ADDR_HOST_PARAM + EC_PROTO2_MAX_PARAM_SIZE - 1);
 		fprintf(stderr,
 			"Very likely this board doesn't have a Chromium EC.\n");
 		return -4;
@@ -296,6 +296,7 @@ int comm_init_lpc(void)
 	i = inb(EC_LPC_ADDR_MEMMAP + EC_MEMMAP_HOST_CMD_FLAGS);
 
 	if (i & EC_HOST_CMD_FLAG_VERSION_3) {
+		/* Protocol version 3 */
 		ec_command = ec_command_lpc_3;
 		ec_max_outsize = EC_LPC_HOST_PACKET_SIZE -
 			sizeof(struct ec_host_request);
@@ -303,8 +304,9 @@ int comm_init_lpc(void)
 			sizeof(struct ec_host_response);
 
 	} else if (i & EC_HOST_CMD_FLAG_LPC_ARGS_SUPPORTED) {
+		/* Protocol version 2*/
 		ec_command = ec_command_lpc;
-		ec_max_outsize = ec_max_insize = EC_HOST_PARAM_SIZE;
+		ec_max_outsize = ec_max_insize = EC_PROTO2_MAX_PARAM_SIZE;
 
 	} else {
 		fprintf(stderr, "EC doesn't support protocols we need.\n");