Comm interface now provides max I/O sizes and preallocated buffers

The maximum packet / param size differs depending on interface and
protocol version.  Commands can now ask the comm interface what the
limits are, and can use preallocated buffers to avoid needless
malloc/free.

BUG=chrome-os-partner:20571
BRANCH=none
TEST=the following all work on link
  burn_my_ec
  ectool version
  ectool chargedump
  ectool console
  ectool i2cxfer 5 0x41 2

Change-Id: Ib847994da3f79721e7fb4e347231b9147a3f485f
Signed-off-by: Randall Spangler <rspangler@chromium.org>
Reviewed-on: https://gerrit.chromium.org/gerrit/60275

8 files changed, 76 insertions, 42 deletions

diff --git a/util/burn_my_ec.c b/util/burn_my_ec.c
index 1cac34bec5..7d9a2dd2f3 100644
--- a/util/burn_my_ec.c
+++ b/util/burn_my_ec.c
@@ -21,21 +21,21 @@ static const char * const part_name[] = {"unknown", "RO", "RW"};
 enum ec_current_image get_version(enum ec_current_image *version_ptr)
 {
 	struct ec_response_get_version r;
-	char build_info[EC_HOST_PARAM_SIZE];
+	char *build_info = (char *)ec_inbuf;
 	int res;
 
 	res = ec_command(EC_CMD_GET_VERSION, 0, NULL, 0, &r, sizeof(r));
 	if (res < 0)
 		return res;
-	res = ec_command(EC_CMD_GET_BUILD_INFO, 0, NULL, 0, build_info,
-			 sizeof(build_info));
+	res = ec_command(EC_CMD_GET_BUILD_INFO, 0, NULL, 0,
+			 ec_inbuf, ec_max_insize);
 	if (res < 0)
 		return res;
 
 	/* Ensure versions are null-terminated before we print them */
 	r.version_string_ro[sizeof(r.version_string_ro) - 1] = '\0';
 	r.version_string_rw[sizeof(r.version_string_rw) - 1] = '\0';
-	build_info[sizeof(build_info) - 1] = '\0';
+	build_info[ec_max_insize - 1] = '\0';
 
 	/* Print versions */
 	printf("RO version:    %s\n", r.version_string_ro);
diff --git a/util/comm-dev.c b/util/comm-dev.c
index 4dae913a82..6dac13fde3 100644
--- a/util/comm-dev.c
+++ b/util/comm-dev.c
@@ -85,5 +85,11 @@ int comm_init_dev(void)
 	if (ec_readmem_dev(EC_MEMMAP_ID, 2, version) == 2)
 		ec_readmem = ec_readmem_dev;
 
+	/*
+	 * 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;
+
 	return 0;
 }
diff --git a/util/comm-host.c b/util/comm-host.c
index bf94927361..d0ab376aa9 100644
--- a/util/comm-host.c
+++ b/util/comm-host.c
@@ -17,6 +17,10 @@ int (*ec_command)(int command, int version,
 
 int (*ec_readmem)(int offset, int bytes, void *dest);
 
+int ec_max_outsize, ec_max_insize;
+void *ec_outbuf;
+void *ec_inbuf;
+
 int comm_init_dev(void) __attribute__((weak));
 int comm_init_lpc(void) __attribute__((weak));
 int comm_init_i2c(void) __attribute__((weak));
@@ -61,17 +65,29 @@ int comm_init(void)
 
 	/* Prefer new /dev method */
 	if (comm_init_dev && !comm_init_dev())
-		return 0;
+		goto init_ok;
 
 	/* Fallback to direct LPC on x86 */
 	if (comm_init_lpc && !comm_init_lpc())
-		return 0;
+		goto init_ok;
 
 	/* Fallback to direct i2c on ARM */
 	if (comm_init_i2c && !comm_init_i2c())
-		return 0;
+		goto init_ok;
 
 	/* Give up */
 	fprintf(stderr, "Unable to establish host communication\n");
 	return 1;
+
+ init_ok:
+	/* Allocate shared I/O buffers */
+	ec_outbuf = malloc(ec_max_outsize);
+	ec_inbuf = malloc(ec_max_insize);
+	if (!ec_outbuf || !ec_inbuf) {
+		fprintf(stderr, "Unable to allocate buffers\n");
+		return 1;
+	}
+
+	return 0;
+
 }
diff --git a/util/comm-host.h b/util/comm-host.h
index d890156af0..695fc33c4f 100644
--- a/util/comm-host.h
+++ b/util/comm-host.h
@@ -12,12 +12,23 @@
 #include "common.h"
 #include "ec_commands.h"
 
-/* Perform initializations needed for subsequent requests
+/* Maximum output and input sizes for EC command, in bytes */
+extern int ec_max_outsize, ec_max_insize;
+
+/*
+ * Maximum-size output and input buffers, for use by callers.  This saves each
+ * caller needing to allocate/free its own buffers.
+ */
+extern void *ec_outbuf;
+extern void *ec_inbuf;
+
+/**
+ * Perform initializations needed for subsequent requests
  *
  * returns 0 in case of success or error code. */
 int comm_init(void);
 
-/*
+/**
  * Send a command to the EC.  Returns the length of output data returned (0 if
  * none), or negative on error.
  */
@@ -25,7 +36,7 @@ extern int (*ec_command)(int command, int version,
 			 const void *outdata, int outsize, /* to the EC */
 			 void *indata, int insize);	   /* from the EC */
 
-/*
+/**
  * Return the content of the EC information area mapped as "memory".
  * The offsets are defined by the EC_MEMMAP_ constants. Returns the number
  * of bytes read, or negative on error. Specifying bytes=0 will read a
diff --git a/util/comm-i2c.c b/util/comm-i2c.c
index 8f75320b48..eab48d2a99 100644
--- a/util/comm-i2c.c
+++ b/util/comm-i2c.c
@@ -213,5 +213,7 @@ int comm_init_i2c(void)
 	free(file_path);
 
 	ec_command = ec_command_i2c;
+	ec_max_outsize = ec_max_insize = EC_HOST_PARAM_SIZE;
+
 	return 0;
 }
diff --git a/util/comm-lpc.c b/util/comm-lpc.c
index 3290bc7c7c..f2c635770e 100644
--- a/util/comm-lpc.c
+++ b/util/comm-lpc.c
@@ -297,14 +297,21 @@ int comm_init_lpc(void)
 
 	if (i & EC_HOST_CMD_FLAG_VERSION_3) {
 		ec_command = ec_command_lpc_3;
+		ec_max_outsize = EC_LPC_HOST_PACKET_SIZE -
+			sizeof(struct ec_host_request);
+		ec_max_insize = EC_LPC_HOST_PACKET_SIZE -
+			sizeof(struct ec_host_response);
+
 	} else if (i & EC_HOST_CMD_FLAG_LPC_ARGS_SUPPORTED) {
 		ec_command = ec_command_lpc;
+		ec_max_outsize = ec_max_insize = EC_HOST_PARAM_SIZE;
+
 	} else {
 		fprintf(stderr, "EC doesn't support protocols we need.\n");
 		return -5;
 	}
 
-	/* Either one supports reading mapped memory directly */
+	/* Either one supports reading mapped memory directly. */
 	ec_readmem = ec_readmem_lpc;
 	return 0;
 }
diff --git a/util/ec_flash.c b/util/ec_flash.c
index f776240e58..0c9c698600 100644
--- a/util/ec_flash.c
+++ b/util/ec_flash.c
@@ -15,22 +15,20 @@
 int ec_flash_read(uint8_t *buf, int offset, int size)
 {
 	struct ec_params_flash_read p;
-	/* TODO(rspangler): need better way to determine max read size */
-	uint8_t rdata[EC_HOST_PARAM_SIZE - sizeof(struct ec_host_response)];
 	int rv;
 	int i;
 
 	/* Read data in chunks */
-	for (i = 0; i < size; i += sizeof(rdata)) {
+	for (i = 0; i < size; i += ec_max_insize) {
 		p.offset = offset + i;
-		p.size = MIN(size - i, sizeof(rdata));
+		p.size = MIN(size - i, ec_max_insize);
 		rv = ec_command(EC_CMD_FLASH_READ, 0,
-				&p, sizeof(p), rdata, sizeof(rdata));
+				&p, sizeof(p), ec_inbuf, p.size);
 		if (rv < 0) {
 			fprintf(stderr, "Read error at offset %d\n", i);
 			return rv;
 		}
-		memcpy(buf + i, rdata, p.size);
+		memcpy(buf + i, ec_inbuf, p.size);
 	}
 
 	return 0;
diff --git a/util/ectool.c b/util/ectool.c
index 3ea88e5260..953b5b9a6d 100644
--- a/util/ectool.c
+++ b/util/ectool.c
@@ -323,17 +323,16 @@ int cmd_cmdversions(int argc, char *argv[])
 int cmd_version(int argc, char *argv[])
 {
 	struct ec_response_get_version r;
-	char build_string[EC_HOST_PARAM_SIZE];
+	char *build_string = (char *)ec_inbuf;
 	int rv;
 
-	rv = ec_command(EC_CMD_GET_VERSION, 0,
-			NULL, 0, &r, sizeof(r));
+	rv = ec_command(EC_CMD_GET_VERSION, 0, NULL, 0, &r, sizeof(r));
 	if (rv < 0) {
 		fprintf(stderr, "ERROR: EC_CMD_GET_VERSION failed: %d\n", rv);
 		return rv;
 	}
 	rv = ec_command(EC_CMD_GET_BUILD_INFO, 0,
-			NULL, 0, build_string, sizeof(build_string));
+			NULL, 0, ec_inbuf, ec_max_insize);
 	if (rv < 0) {
 		fprintf(stderr, "ERROR: EC_CMD_GET_BUILD_INFO failed: %d\n",
 				rv);
@@ -343,7 +342,7 @@ int cmd_version(int argc, char *argv[])
 	/* Ensure versions are null-terminated before we print them */
 	r.version_string_ro[sizeof(r.version_string_ro) - 1] = '\0';
 	r.version_string_rw[sizeof(r.version_string_rw) - 1] = '\0';
-	build_string[sizeof(build_string) - 1] = '\0';
+	build_string[ec_max_insize - 1] = '\0';
 
 	/* Print versions */
 	printf("RO version:    %s\n", r.version_string_ro);
@@ -1599,15 +1598,15 @@ static void print_panic_reg(int regnum, const uint32_t *regs, int index)
 
 int cmd_panic_info(int argc, char *argv[])
 {
-	char out[EC_HOST_PARAM_SIZE];
 	int rv;
-	struct panic_data *pdata = (struct panic_data *)out;
+	struct panic_data *pdata = (struct panic_data *)ec_inbuf;
 	const uint32_t *lregs = pdata->regs;
 	const uint32_t *sregs = NULL;
 	int in_handler;
 	int i;
 
-	rv = ec_command(EC_CMD_GET_PANIC_INFO, 0, NULL, 0, out, sizeof(out));
+	rv = ec_command(EC_CMD_GET_PANIC_INFO, 0, NULL, 0,
+			ec_inbuf, ec_max_insize);
 	if (rv < 0)
 		return rv;
 
@@ -2151,16 +2150,10 @@ int cmd_i2c_write(int argc, char *argv[])
 
 int cmd_i2c_xfer(int argc, char *argv[])
 {
-	union {
-		struct ec_params_i2c_passthru p;
-		uint8_t outbuf[EC_HOST_PARAM_SIZE];
-	} params;
-	union {
-		struct ec_response_i2c_passthru r;
-		uint8_t inbuf[EC_HOST_PARAM_SIZE];
-	} response;
-	struct ec_params_i2c_passthru *p = &params.p;
-	struct ec_response_i2c_passthru *r = &response.r;
+	struct ec_params_i2c_passthru *p =
+		(struct ec_params_i2c_passthru *)ec_outbuf;
+	struct ec_response_i2c_passthru *r =
+		(struct ec_response_i2c_passthru *)ec_inbuf;
 	struct ec_params_i2c_passthru_msg *msg = p->msg;
 	unsigned int addr;
 	uint8_t *pdata;
@@ -2201,11 +2194,11 @@ int cmd_i2c_xfer(int argc, char *argv[])
 	p->num_msgs = (read_len != 0) + (write_len != 0);
 
 	size = sizeof(*p) + p->num_msgs * sizeof(*msg);
-	if (size + write_len > sizeof(params)) {
+	if (size + write_len > ec_max_outsize) {
 		fprintf(stderr, "Params too large for buffer\n");
 		return -1;
 	}
-	if (sizeof(*r) + read_len > sizeof(response)) {
+	if (sizeof(*r) + read_len > ec_max_insize) {
 		fprintf(stderr, "Read length too big for buffer\n");
 		return -1;
 	}
@@ -2364,10 +2357,11 @@ int cmd_charge_force_idle(int argc, char *argv[])
 
 int cmd_charge_dump(int argc, char *argv[])
 {
-	unsigned char out[EC_HOST_PARAM_SIZE];
+	unsigned char *out = ec_inbuf;
 	int rv, i;
 
-	rv = ec_command(EC_CMD_CHARGE_DUMP, 0, NULL, 0, out, sizeof(out));
+	rv = ec_command(EC_CMD_CHARGE_DUMP, 0, NULL, 0,
+			ec_inbuf, ec_max_insize);
 
 	if (rv < 0)
 		return rv;
@@ -2749,7 +2743,7 @@ int cmd_rtc_set(int argc, char *argv[])
 
 int cmd_console(int argc, char *argv[])
 {
-	char out[EC_HOST_PARAM_SIZE];
+	char *out = (char *)ec_inbuf;
 	int rv;
 
 	/* Snapshot the EC console */
@@ -2760,7 +2754,7 @@ int cmd_console(int argc, char *argv[])
 	/* Loop and read from the snapshot until it's done */
 	while (1) {
 		rv = ec_command(EC_CMD_CONSOLE_READ, 0,
-				NULL, 0, out, sizeof(out));
+				NULL, 0, ec_inbuf, ec_max_insize);
 		if (rv < 0)
 			return rv;
 
@@ -2768,7 +2762,7 @@ int cmd_console(int argc, char *argv[])
 			break;  /* Empty response means done */
 
 		/* Make sure output is null-terminated, then dump it */
-		out[sizeof(out) - 1] = '\0';
+		out[ec_max_insize - 1] = '\0';
 		fputs(out, stdout);
 	}
 	printf("\n");