Change ec_command() to return negative values for errors

This is more compatible with kernel and u-boot, and will make it
easier to share code between the ec project and those.

BUG=none
TEST=manual:

  ectool version -> should work normally

on ec, do 'hostevent set 0x40000'.  Then at root shell
  ectool queryec
EC returned error result code 19

  ectool flashread 0x100000 16 foo
Reading 16 bytes at offset 1048576...
EC returned error result code 2
Read error at offset 0

Signed-off-by: Randall Spangler <rspangler@chromium.org>

Change-Id: I5e2a85f96c874d0730c14e1438a533649cd594f8
Reviewed-on: https://gerrit.chromium.org/gerrit/26359
Commit-Ready: Randall Spangler <rspangler@chromium.org>
Reviewed-by: Randall Spangler <rspangler@chromium.org>
Tested-by: Randall Spangler <rspangler@chromium.org>

1 files changed, 32 insertions, 23 deletions

diff --git a/util/comm-lpc.c b/util/comm-lpc.c
index 4f3aae3912..9e296ba866 100644
--- a/util/comm-lpc.c
+++ b/util/comm-lpc.c
@@ -12,7 +12,6 @@
 #include "comm-host.h"
 #include "ec_commands.h"
 
-
 #define INITIAL_UDELAY 5     /* 5 us */
 #define MAXIMUM_UDELAY 10000 /* 10 ms */
 
@@ -27,8 +26,15 @@ int comm_init(void)
 		return -3;
 	}
 
-	/* Test if the I/O port has been configured for GEC.
-	 * If they all are 0xff, then very possible you cannot access GEC. */
+	/*
+	 * Test if the I/O port has been configured for Chromium EC LPC
+	 * interface.  If all the bytes are 0xff, very likely that Chromium EC
+	 * is not present.
+	 *
+	 * TODO: (crosbug.com/p/10963) Should only need to look at the command
+	 * byte, since we don't support ACPI burst mode and thus bit 4 should
+	 * be 0.
+	 */
 	byte &= inb(EC_LPC_ADDR_USER_CMD);
 	byte &= inb(EC_LPC_ADDR_USER_DATA);
 	for (i = 0; i < EC_PARAM_SIZE && byte == 0xff; ++i)
@@ -38,27 +44,32 @@ int comm_init(void)
 			EC_LPC_ADDR_USER_CMD, EC_LPC_ADDR_USER_DATA,
 			EC_LPC_ADDR_USER_PARAM,
 			EC_LPC_ADDR_USER_PARAM + EC_PARAM_SIZE - 1);
-		fprintf(stderr, "Very likely this board doesn't have GEC.\n");
+		fprintf(stderr,
+			"Very likely this board doesn't have a Chromium EC.\n");
 		return -4;
 	}
 
 	return 0;
 }
 
-
-/* Waits for the EC to be unbusy.  Returns 0 if unbusy, non-zero if
- * timeout. */
+/*
+ * Wait for the EC to be unbusy.  Returns 0 if unbusy, non-zero if
+ * timeout.
+ */
 static int wait_for_ec(int status_addr, int timeout_usec)
 {
 	int i;
 	int delay = INITIAL_UDELAY;
 
 	for (i = 0; i < timeout_usec; i += delay) {
-		/* Delay first, in case we just sent out a command but
-		 * the EC hasn't raise the busy flag. However, I think
-		 * this doesn't happen since the LPC commands are executed
-		 * in order and the busy flag is set by hardware.
-		 * TODO: move this delay after inb(status). */
+		/*
+		 * Delay first, in case we just sent out a command but the EC
+		 * hasn't raise the busy flag. However, I think this doesn't
+		 * happen since the LPC commands are executed in order and the
+		 * busy flag is set by hardware.
+		 *
+		 * TODO: move this delay after inb(status).
+		 */
 		usleep(MIN(delay, timeout_usec - i));
 
 		if (!(inb(status_addr) & EC_LPC_STATUS_BUSY_MASK))
@@ -71,8 +82,6 @@ static int wait_for_ec(int status_addr, int timeout_usec)
 	return -1;  /* Timeout */
 }
 
-/* Sends a command to the EC.  Returns the command status code, or
- * -1 if other error. */
 int ec_command(int command, const void *indata, int insize,
 	       void *outdata, int outsize) {
 	uint8_t *d;
@@ -85,12 +94,12 @@ int ec_command(int command, const void *indata, int insize,
 
 	if (insize > EC_PARAM_SIZE || outsize > EC_PARAM_SIZE) {
 		fprintf(stderr, "Data size too big\n");
-		return -1;
+		return -EC_RES_ERROR;
 	}
 
 	if (wait_for_ec(cmd_addr, 1000000)) {
 		fprintf(stderr, "Timeout waiting for EC ready\n");
-		return -1;
+		return -EC_RES_ERROR;
 	}
 
 	/* Write data, if any */
@@ -102,14 +111,14 @@ int ec_command(int command, const void *indata, int insize,
 
 	if (wait_for_ec(cmd_addr, 1000000)) {
 		fprintf(stderr, "Timeout waiting for EC response\n");
-		return -1;
+		return -EC_RES_ERROR;
 	}
 
 	/* Check result */
 	i = inb(data_addr);
 	if (i) {
 		fprintf(stderr, "EC returned error result code %d\n", i);
-		return i;
+		return -i;
 	}
 
 	/* Read data, if any */
@@ -117,28 +126,28 @@ int ec_command(int command, const void *indata, int insize,
 	for (i = 0, d = (uint8_t *)outdata; i < outsize; i++, d++)
 		*d = inb(param_addr + i);
 
-	return 0;
+	/*
+	 * LPC protocol doesn't have a way to communicate the true output
+	 * size, so assume we got everything we asked for.
+	 */
+	return outsize;
 }
 
-
 uint8_t read_mapped_mem8(uint8_t offset)
 {
 	return inb(EC_LPC_ADDR_MEMMAP + offset);
 }
 
-
 uint16_t read_mapped_mem16(uint8_t offset)
 {
 	return inw(EC_LPC_ADDR_MEMMAP + offset);
 }
 
-
 uint32_t read_mapped_mem32(uint8_t offset)
 {
 	return inl(EC_LPC_ADDR_MEMMAP + offset);
 }
 
-
 int read_mapped_string(uint8_t offset, char *buf)
 {
 	int c;