CBI: Make data offset and size variable

Currently CBI data offset and size are fixed. This patch makes them
variable. Each data item consists of <tag><size><value> where <tag>
is a numeric value assigned to each data item, <size> is the number
of bytes used for <value>.

BUG=b:70294260
BRANCH=none
TEST=Use 'ectool cbi set' to set board version, oem, sku.
Verify the contents by cbi console command and ectool cbi get.
1. ectool cbi set 0 0x202 2 2 (Init CBI and write board ver. of size 2)
2. ectool cbi set 1 1 1 (write oem id of size 1)
3. ectool cbi set 2 2 1 (write sku id of size 1)
4. ectool cbi get 0
 514 (0x202)
5. ectool cbi get 1
 1 (0x1)
6. ectool cbi get 2
 2 (0x2)
7. Run cbi console command:
 CBI_VERSION: 0x0000
 TOTAL_SIZE: 18
 BOARD_VERSION: 514 (0x202)
 OEM_ID: 1 (0x1)
 SKU_ID: 2 (0x2)
  43 42 49 8c 00 00 12 00 00 02 02 02 01 01 01 02
  01 02

Change-Id: I5a30a4076e3eb448f4808d2af8ec4ef4c016ae5e
Signed-off-by: Daisuke Nojiri <dnojiri@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/920905
Reviewed-by: Aaron Durbin <adurbin@chromium.org>

5 files changed, 239 insertions, 156 deletions

diff --git a/board/fizz/board.c b/board/fizz/board.c
index d3de09dc05..cc4450cbcc 100644
--- a/board/fizz/board.c
+++ b/board/fizz/board.c
@@ -503,7 +503,7 @@ static void set_charge_limit(int charge_ma)
 void board_set_charge_limit(int port, int supplier, int charge_ma,
 			    int max_ma, int charge_mv)
 {
-	uint32_t brd_ver = 0;
+	uint32_t ver = 0;
 	int p87w = 0, p65w = 0, p60w = 0;
 
 	/*
@@ -521,7 +521,7 @@ void board_set_charge_limit(int port, int supplier, int charge_ma,
 	 * If CBI isn't initialized or contains invalid data, we assume it's
 	 * a new board.
 	 */
-	if (cbi_get_board_version(&brd_ver) == EC_SUCCESS && brd_ver < 0x0202)
+	if (cbi_get_board_version(&ver) == EC_SUCCESS && ver < 0x0202)
 		return set_charge_limit(charge_ma);
 	/*
 	 * We have three FETs connected to three registers: PR257, PR258,
diff --git a/common/cbi.c b/common/cbi.c
index ac7708c12c..1eb50d2661 100644
--- a/common/cbi.c
+++ b/common/cbi.c
@@ -12,18 +12,23 @@
 #include "gpio.h"
 #include "host_command.h"
 #include "i2c.h"
+#include "timer.h"
 #include "util.h"
 
 #define CPRINTS(format, args...) cprints(CC_SYSTEM, "CBI " format, ## args)
 
 #define EEPROM_PAGE_WRITE_SIZE	16
+#define EEPROM_PAGE_WRITE_MS	5
 #define EC_ERROR_CBI_CACHE_INVALID	EC_ERROR_INTERNAL_FIRST
-static struct board_info bi;
+
 static int cached_read_result = EC_ERROR_CBI_CACHE_INVALID;
+static uint8_t cbi[CBI_EEPROM_SIZE];
+static struct cbi_header * const head = (struct cbi_header *)cbi;
 
-static uint8_t cbi_crc8(const struct board_info *bi)
+static uint8_t cbi_crc8(const struct cbi_header *h)
 {
-	return crc8((uint8_t *)&bi->head.crc + 1, bi->head.total_size - 4);
+	return crc8((uint8_t *)&h->crc + 1,
+		    h->total_size - sizeof(h->magic) - sizeof(h->crc));
 }
 
 /*
@@ -31,7 +36,6 @@ static uint8_t cbi_crc8(const struct board_info *bi)
  */
 static int do_read_board_info(void)
 {
-	uint8_t buf[256];
 	uint8_t offset;
 
 	CPRINTS("Reading board info");
@@ -39,71 +43,123 @@ static int do_read_board_info(void)
 	/* Read header */
 	offset = 0;
 	if (i2c_xfer(I2C_PORT_EEPROM, I2C_ADDR_EEPROM,
-		     &offset, 1, buf, sizeof(bi.head), I2C_XFER_SINGLE)) {
+		     &offset, 1, cbi, sizeof(*head), I2C_XFER_SINGLE)) {
 		CPRINTS("Failed to read header");
 		return EC_ERROR_INVAL;
 	}
-	memcpy(&bi.head, buf, sizeof(bi.head));
 
 	/* Check magic */
-	if (memcmp(bi.head.magic, cbi_magic, sizeof(bi.head.magic))) {
+	if (memcmp(head->magic, cbi_magic, sizeof(head->magic))) {
 		CPRINTS("Bad magic");
 		return EC_ERROR_INVAL;
 	}
 
 	/* check version */
-	if (bi.head.major_version > CBI_VERSION_MAJOR) {
+	if (head->major_version > CBI_VERSION_MAJOR) {
 		CPRINTS("Version mismatch");
 		return EC_ERROR_INVAL;
 	}
 
 	/* Check the data size. It's expected to support up to 64k but our
 	 * buffer has practical limitation. */
-	if (bi.head.total_size < sizeof(bi) ||
-			bi.head.total_size > sizeof(buf)) {
-		CPRINTS("Bad size: %d", bi.head.total_size);
+	if (head->total_size < sizeof(*head) ||
+			sizeof(cbi) < head->total_size) {
+		CPRINTS("Bad size: %d", head->total_size);
 		return EC_ERROR_OVERFLOW;
 	}
 
-	/* Read the rest */
-	offset = sizeof(bi.head);
+	/* Read the data */
+	offset = sizeof(*head);
 	if (i2c_xfer(I2C_PORT_EEPROM, I2C_ADDR_EEPROM, &offset, 1,
-		     buf + sizeof(bi.head),
-		     bi.head.total_size - sizeof(bi.head),
+		     head->data, head->total_size - sizeof(*head),
 		     I2C_XFER_SINGLE)) {
 		CPRINTS("Failed to read body");
 		return EC_ERROR_INVAL;
 	}
 
 	/* Check CRC. This supports new fields unknown to this parser. */
-	if (cbi_crc8((struct board_info *)&buf) != bi.head.crc) {
+	if (cbi_crc8(head) != head->crc) {
 		CPRINTS("Bad CRC");
 		return EC_ERROR_INVAL;
 	}
 
-	/* Save only the data we understand. */
-	memcpy(&bi.head + 1, &buf[sizeof(bi.head)],
-	       sizeof(bi) - sizeof(bi.head));
-	/* If we're handling previous version, clear all new fields */
-
 	return EC_SUCCESS;
 }
 
 static int read_board_info(void)
 {
-	if (cached_read_result != EC_ERROR_CBI_CACHE_INVALID)
-		/* We already tried and know the result. Return the cached
-		 * error code immediately to avoid wasteful reads. */
-		return cached_read_result;
-
-	cached_read_result = do_read_board_info();
-	if (cached_read_result)
-		/* On error (I2C or bad contents), retry a read */
+	if (cached_read_result == EC_ERROR_CBI_CACHE_INVALID) {
 		cached_read_result = do_read_board_info();
-
+		if (cached_read_result)
+			/* On error (I2C or bad contents), retry a read */
+			cached_read_result = do_read_board_info();
+	}
+	/* Else, we already tried and know the result. Return the cached
+	 * error code immediately to avoid wasteful reads. */
 	return cached_read_result;
 }
 
+static struct cbi_data *find_tag(enum cbi_data_tag tag)
+{
+	struct cbi_data *d;
+	uint8_t *p;
+	for (p = head->data; p + sizeof(*d) < cbi + head->total_size;) {
+		d = (struct cbi_data *)p;
+		if (d->tag == tag)
+			return d;
+		p += sizeof(*d) + d->size;
+	}
+	return NULL;
+}
+
+int cbi_get_board_info(enum cbi_data_tag tag, uint8_t *buf, uint8_t *size)
+{
+	const struct cbi_data *d;
+
+	if (read_board_info())
+		return EC_ERROR_UNKNOWN;
+
+	d = find_tag(tag);
+	if (!d)
+		/* Not found */
+		return EC_ERROR_UNKNOWN;
+	if (*size < d->size)
+		/* Insufficient buffer size */
+		return EC_ERROR_INVAL;
+
+	/* Clear the buffer in case len < *size */
+	memset(buf, 0, *size);
+	/* Copy the value */
+	memcpy(buf, d->value, d->size);
+	*size = d->size;
+	return EC_SUCCESS;
+}
+
+int cbi_set_board_info(enum cbi_data_tag tag, const uint8_t *buf, uint8_t size)
+{
+	struct cbi_data *d;
+
+	d = find_tag(tag);
+	if (!d) {
+		/* Not found. Check if new item would fit */
+		if (sizeof(cbi) < head->total_size + sizeof(*d) + size)
+			return EC_ERROR_OVERFLOW;
+		/* Append new item */
+		d = (struct cbi_data *)&cbi[head->total_size];
+		d->tag = tag;
+		d->size = size;
+		memcpy(d->value, buf, d->size);
+		head->total_size += (sizeof(*d) + size);
+		return EC_SUCCESS;
+	}
+	/* No expand or shrink. Items are tightly packed. */
+	if (d->size != size)
+		return EC_ERROR_INVAL;
+	/* Overwrite existing item */
+	memcpy(d->value, buf, d->size);
+	return EC_SUCCESS;
+}
+
 static int eeprom_is_write_protected(void)
 {
 	return !gpio_get_level(GPIO_WP_L);
@@ -111,13 +167,12 @@ static int eeprom_is_write_protected(void)
 
 static int write_board_info(void)
 {
-	uint8_t buf[sizeof(bi) + 1];
 	/* The code is only tested for ST M24C02, whose page size for a single
 	 * write is 16 byte. To support different EEPROMs, you may need to
 	 * craft the i2c packets accordingly. */
-	_Static_assert(sizeof(bi) <= EEPROM_PAGE_WRITE_SIZE,
-		       "struct board_info exceeds page write size");
-	int rv;
+	uint8_t buf[EEPROM_PAGE_WRITE_SIZE + 1];  /* '1' for offset byte */
+	uint8_t *p = cbi;
+	int rest = head->total_size;
 
 	if (eeprom_is_write_protected()) {
 		CPRINTS("Failed to write for WP");
@@ -125,74 +180,68 @@ static int write_board_info(void)
 	}
 
 	buf[0] = 0;	/* Offset 0 */
-	memcpy(&buf[1], &bi, sizeof(bi));
-	rv = i2c_xfer(I2C_PORT_EEPROM, I2C_ADDR_EEPROM, buf,
-		     sizeof(bi) + 1, NULL, 0, I2C_XFER_SINGLE);
-	if (rv) {
-		CPRINTS("Failed to write for %d", rv);
-		return rv;
+	while (rest > 0) {
+		int size = MIN(EEPROM_PAGE_WRITE_SIZE, rest);
+		int rv;
+		rest -= size;
+		memcpy(&buf[1], p, size);
+		rv = i2c_xfer(I2C_PORT_EEPROM, I2C_ADDR_EEPROM, buf, size + 1,
+			      NULL, 0, I2C_XFER_SINGLE);
+		if (rv) {
+			CPRINTS("Failed to write for %d", rv);
+			return rv;
+		}
+		/* Wait for internal write cycle completion */
+		msleep(EEPROM_PAGE_WRITE_MS);
+		buf[0] += size;
+		p += size;
 	}
 
 	return EC_SUCCESS;
 }
 
-int cbi_get_board_version(uint32_t *version)
+int cbi_get_board_version(uint32_t *ver)
 {
-	if (read_board_info())
-		return EC_ERROR_UNKNOWN;
-	*version = bi.version;
-	return EC_SUCCESS;
+	uint8_t size = sizeof(*ver);
+	return cbi_get_board_info(CBI_TAG_BOARD_VERSION, (uint8_t *)ver, &size);
 }
 
-int cbi_get_sku_id(uint32_t *sku_id)
+int cbi_get_sku_id(uint32_t *id)
 {
-	if (read_board_info())
-		return EC_ERROR_UNKNOWN;
-	*sku_id = bi.sku_id;
-	return EC_SUCCESS;
+	uint8_t size = sizeof(*id);
+	return cbi_get_board_info(CBI_TAG_SKU_ID, (uint8_t *)id, &size);
 }
 
-int cbi_get_oem_id(uint32_t *oem_id)
+int cbi_get_oem_id(uint32_t *id)
 {
-	if (read_board_info())
-		return EC_ERROR_UNKNOWN;
-	*oem_id = bi.oem_id;
-	return EC_SUCCESS;
+	uint8_t size = sizeof(*id);
+	return cbi_get_board_info(CBI_TAG_OEM_ID, (uint8_t *)id, &size);
 }
 
+/*
+ * For backward compatibility. New code should use cbi_get_board_version.
+ */
 int board_get_version(void)
 {
-	uint32_t version;
-	if (cbi_get_board_version(&version))
+	uint32_t ver;
+	uint8_t size = sizeof(ver);
+	if (cbi_get_board_info(CBI_TAG_BOARD_VERSION, (uint8_t *)&ver, &size))
 		return -1;
-	return version;
+	return ver;
 }
 
 static int hc_cbi_get(struct host_cmd_handler_args *args)
 {
 	const struct __ec_align4 ec_params_get_cbi *p = args->params;
+	uint8_t size = MIN(args->response_max, UINT8_MAX);
 
 	if (p->flag & CBI_GET_RELOAD)
 		cached_read_result = EC_ERROR_CBI_CACHE_INVALID;
 
-	if (read_board_info())
-		return EC_RES_ERROR;
-
-	switch (p->type) {
-	case CBI_DATA_BOARD_VERSION:
-		*(uint32_t *)args->response = bi.version;
-		break;
-	case CBI_DATA_OEM_ID:
-		*(uint32_t *)args->response = bi.oem_id;
-		break;
-	case CBI_DATA_SKU_ID:
-		*(uint32_t *)args->response = bi.sku_id;
-		break;
-	default:
+	if (cbi_get_board_info(p->tag, args->response, &size))
 		return EC_RES_INVALID_PARAM;
-	}
-	args->response_size = sizeof(uint32_t);
 
+	args->response_size = size;
 	return EC_RES_SUCCESS;
 }
 DECLARE_HOST_COMMAND(EC_CMD_GET_CROS_BOARD_INFO,
@@ -205,40 +254,23 @@ static int hc_cbi_set(struct host_cmd_handler_args *args)
 	int rv;
 
 	if (p->flag & CBI_SET_INIT) {
-		memset(&bi, 0, sizeof(bi));
-		memcpy(&bi.head.magic, cbi_magic, sizeof(cbi_magic));
+		memset(cbi, 0, sizeof(cbi));
+		memcpy(head->magic, cbi_magic, sizeof(cbi_magic));
+		head->total_size = sizeof(*head);
 		cached_read_result = EC_SUCCESS;
 	} else {
 		if (read_board_info())
 			return EC_RES_ERROR;
 	}
 
-	switch (p->type) {
-	case CBI_DATA_BOARD_VERSION:
-		if (p->data > UINT16_MAX)
-			return EC_RES_INVALID_PARAM;
-		bi.version = p->data;
-		break;
-	case CBI_DATA_OEM_ID:
-		if (p->data > UINT8_MAX)
-			return EC_RES_INVALID_PARAM;
-		bi.oem_id = p->data;
-		break;
-	case CBI_DATA_SKU_ID:
-		if (p->data > UINT8_MAX)
-			return EC_RES_INVALID_PARAM;
-		bi.sku_id = p->data;
-		break;
-	default:
+	if (cbi_set_board_info(p->tag, p->data, p->size))
 		return EC_RES_INVALID_PARAM;
-	}
 
 	/* Whether we're modifying existing data or creating new one,
 	 * we take over the format. */
-	bi.head.major_version = CBI_VERSION_MAJOR;
-	bi.head.minor_version = CBI_VERSION_MINOR;
-	bi.head.total_size = sizeof(bi);
-	bi.head.crc = cbi_crc8(&bi);
+	head->major_version = CBI_VERSION_MAJOR;
+	head->minor_version = CBI_VERSION_MINOR;
+	head->crc = cbi_crc8(head);
 
 	/* Skip write if client asks so. */
 	if (p->flag & CBI_SET_NO_SYNC)
@@ -255,11 +287,42 @@ DECLARE_HOST_COMMAND(EC_CMD_SET_CROS_BOARD_INFO,
 		     hc_cbi_set,
 		     EC_VER_MASK(0));
 
+static void dump_cbi(void)
+{
+	int i;
+	for (i = 0; i < head->total_size; i++) {
+		ccprintf(" %02x", cbi[i]);
+		if (i % 16 == 15)
+			ccprintf("\n");
+	}
+	ccprintf("\n");
+}
+
 static int command_dump_cbi(int argc, char **argv)
 {
-	ccprintf("BOARD_VERSION: %u (0x%x)\n", bi.version, bi.version);
-	ccprintf("OEM_ID: %u (0x%x)\n", bi.oem_id, bi.oem_id);
-	ccprintf("SKU_ID: %u (0x%x)\n", bi.sku_id, bi.sku_id);
+	uint32_t val;
+
+	ccprintf("CBI_VERSION: 0x%04x\n", head->version);
+	ccprintf("TOTAL_SIZE: %u\n", head->total_size);
+	ccprintf("BOARD_VERSION: ");
+	if (cbi_get_board_version(&val) == EC_SUCCESS)
+		ccprintf("%u (0x%x)\n", val, val);
+	else
+		ccprintf("UNKNOWN\n");
+
+	ccprintf("OEM_ID: ");
+	if (cbi_get_oem_id(&val) == EC_SUCCESS)
+		ccprintf("%u (0x%x)\n", val, val);
+	else
+		ccprintf("UNKNOWN\n");
+
+	ccprintf("SKU_ID: ");
+	if (cbi_get_sku_id(&val) == EC_SUCCESS)
+		ccprintf("%u (0x%x)\n", val, val);
+	else
+		ccprintf("UNKNOWN\n");
+
+	dump_cbi();
 	return EC_SUCCESS;
 }
 DECLARE_CONSOLE_COMMAND(cbi, command_dump_cbi, NULL, NULL);
diff --git a/include/cros_board_info.h b/include/cros_board_info.h
index 8d140f1d25..eb239f9696 100644
--- a/include/cros_board_info.h
+++ b/include/cros_board_info.h
@@ -8,9 +8,11 @@
 #define __CROS_EC_CROS_BOARD_INFO_H
 
 #include "common.h"
+#include "ec_commands.h"
 
-#define CBI_VERSION_MAJOR 0
-#define CBI_VERSION_MINOR 0
+#define CBI_VERSION_MAJOR	0
+#define CBI_VERSION_MINOR	0
+#define CBI_EEPROM_SIZE		256
 static const uint8_t cbi_magic[] = { 0x43, 0x42, 0x49 };  /* 'C' 'B' 'I' */
 
 struct cbi_header {
@@ -29,22 +31,14 @@ struct cbi_header {
 	/* Total size of data. It can be larger than sizeof(struct board_info)
 	 * if future versions add additional fields. */
 	uint16_t total_size;
+	/* List of data items (i.e. struct cbi_data[]) */
+	uint8_t data[];
 } __attribute__((packed));
 
-struct board_info {
-	struct cbi_header head;
-	/* Board version */
-	union {
-		struct {
-			uint8_t minor_version;
-			uint8_t major_version;
-		};
-		uint16_t version;
-	};
-	/* OEM ID */
-	uint8_t oem_id;
-	/* SKU ID */
-	uint8_t sku_id;
+struct cbi_data {
+	uint8_t tag;		/* enum cbi_data_tag */
+	uint8_t size;		/* size of value[] */
+	uint8_t value[];	/* data value */
 } __attribute__((packed));
 
 /**
@@ -57,4 +51,15 @@ int cbi_get_board_version(uint32_t *version);
 int cbi_get_sku_id(uint32_t *sku_id);
 int cbi_get_oem_id(uint32_t *oem_id);
 
+/**
+ * Primitive accessors
+ *
+ * @param tag   Tag of the target data.
+ * @param buf   Buffer where data is passed.
+ * @param size  (IN) Size of <buf>. (OUT) Size of the data returned.
+ * @return EC_SUCCESS on success or EC_ERROR_* otherwise.
+ */
+int cbi_get_board_info(enum cbi_data_tag tag, uint8_t *buf, uint8_t *size);
+int cbi_set_board_info(enum cbi_data_tag tag, const uint8_t *buf, uint8_t size);
+
 #endif /* __CROS_EC_CROS_BOARD_INFO_H */
diff --git a/include/ec_commands.h b/include/ec_commands.h
index 1d8761be8b..bc654209f0 100644
--- a/include/ec_commands.h
+++ b/include/ec_commands.h
@@ -4587,14 +4587,11 @@ struct __ec_align1 ec_params_efs_verify {
  */
 #define EC_CMD_SET_CROS_BOARD_INFO	0x0120
 
-enum cbi_data_type {
-	/* integer types */
-	CBI_DATA_BOARD_VERSION = 0,
-	CBI_DATA_OEM_ID = 1,
-	CBI_DATA_SKU_ID = 2,
-	/* string types */
-	CBI_FIRST_STRING_PARAM = 0x1000,
-	CBI_DATA_COUNT,
+enum cbi_data_tag {
+	CBI_TAG_BOARD_VERSION = 0, /* uint16_t or uint8_t[] = {minor,major} */
+	CBI_TAG_OEM_ID = 1,        /* uint8_t */
+	CBI_TAG_SKU_ID = 2,        /* uint8_t */
+	CBI_TAG_COUNT,
 };
 
 /*
@@ -4606,7 +4603,7 @@ enum cbi_data_type {
 #define CBI_GET_RELOAD		(1 << 0)
 
 struct __ec_align4 ec_params_get_cbi {
-	uint32_t type;		/* enum cbi_data_type */
+	uint32_t tag;		/* enum cbi_data_tag */
 	uint32_t flag;		/* CBI_GET_* */
 };
 
@@ -4622,10 +4619,10 @@ struct __ec_align4 ec_params_get_cbi {
 #define CBI_SET_INIT		(1 << 1)
 
 struct __ec_align1 ec_params_set_cbi {
-	uint32_t type;		/* enum cbi_data_type */
+	uint32_t tag;		/* enum cbi_data_tag */
 	uint32_t flag;		/* CBI_SET_* */
-	uint32_t data;		/* For numeric value */
-	uint8_t raw[];		/* For string and raw data */
+	uint32_t size;		/* Data size */
+	uint8_t data[];		/* For string and raw data */
 };
 
 /*****************************************************************************/
diff --git a/util/ectool.c b/util/ectool.c
index 15b904c23d..cdd5e2c2d3 100644
--- a/util/ectool.c
+++ b/util/ectool.c
@@ -6370,11 +6370,13 @@ static void cmd_cbi_help(char *cmd)
 {
 	fprintf(stderr,
 		"  Usage: %s get <type> [get_flag]\n"
-		"  Usage: %s set <type> value [set_flag]\n"
+		"  Usage: %s set <type> <value> <size> [set_flag]\n"
 		"    <type> is one of:\n"
 		"      0: BOARD_VERSION\n"
 		"      1: OEM_ID\n"
 		"      2: SKU_ID\n"
+		"    <size> is the size of the data"
+		"    <value> is integer to be set. No raw data support yet."
 		"    [get_flag] is combination of:\n"
 		"      01b: Invalidate cache and reload data from EEPROM\n"
 		"    [set_flag] is combination of:\n"
@@ -6389,7 +6391,7 @@ static void cmd_cbi_help(char *cmd)
  */
 static int cmd_cbi(int argc, char *argv[])
 {
-	enum cbi_data_type type;
+	enum cbi_data_tag tag;
 	char *e;
 	int rv;
 
@@ -6399,17 +6401,18 @@ static int cmd_cbi(int argc, char *argv[])
 		return -1;
 	}
 
-	/* Type */
-	type = strtol(argv[2], &e, 0);
+	/* Tag */
+	tag = strtol(argv[2], &e, 0);
 	if (e && *e) {
-		fprintf(stderr, "Bad type\n");
+		fprintf(stderr, "Bad tag\n");
 		return -1;
 	}
 
 	if (!strcasecmp(argv[1], "get")) {
 		struct ec_params_get_cbi p;
-		uint32_t r;
-		p.type = type;
+		uint8_t *r;
+		int i;
+		p.tag = tag;
 		if (argc > 3) {
 			p.flag = strtol(argv[3], &e, 0);
 			if (e && *e) {
@@ -6418,45 +6421,60 @@ static int cmd_cbi(int argc, char *argv[])
 			}
 		}
 		rv = ec_command(EC_CMD_GET_CROS_BOARD_INFO, 0, &p, sizeof(p),
-				&r, sizeof(r));
+				ec_inbuf, ec_max_insize);
 		if (rv < 0) {
 			fprintf(stderr, "Error code: %d\n", rv);
 			return rv;
 		}
-		if (type < CBI_FIRST_STRING_PARAM) { 	/* integer fields */
-			if (rv < sizeof(uint32_t)) {
-				fprintf(stderr, "Invalid size: %d\n", rv);
-				return -1;
-			}
-			printf("%u (0x%x)\n", r, r);
-		} else {
-			fprintf(stderr, "Invalid type: %x\n", type);
+		if (rv < sizeof(uint8_t)) {
+			fprintf(stderr, "Invalid size: %d\n", rv);
 			return -1;
 		}
+		r = ec_inbuf;
+		if (rv <= sizeof(uint32_t))
+			printf("As integer: %u (0x%x)\n", r[0], r[0]);
+		printf("As binary:");
+		for (i = 0; i < rv; i++) {
+			if (i % 32 == 31)
+				printf("\n");
+			printf(" %02x", r[i]);
+		}
+		printf("\n");
 		return 0;
 	} else if (!strcasecmp(argv[1], "set")) {
-		struct ec_params_set_cbi p;
-		if (argc < 4) {
+		struct ec_params_set_cbi *p =
+				(struct ec_params_set_cbi *)ec_outbuf;
+		uint32_t val;
+		uint8_t size;
+		if (argc < 5) {
 			fprintf(stderr, "Invalid number of params\n");
 			cmd_cbi_help(argv[0]);
 			return -1;
 		}
-		memset(&p, 0, sizeof(p));
-		p.type = type;
-		p.data = strtol(argv[3], &e, 0);
+		memset(p, 0, ec_max_outsize);
+		p->tag = tag;
+		val = strtol(argv[3], &e, 0);
 		if (e && *e) {
 			fprintf(stderr, "Bad value\n");
 			return -1;
 		}
-		if (argc > 4) {
-			p.flag = strtol(argv[4], &e, 0);
+		size = strtol(argv[4], &e, 0);
+		if ((e && *e) || val >= (1 << size*8)) {
+			fprintf(stderr, "Bad size\n");
+			return -1;
+		}
+		/* Little endian */
+		memcpy(p->data, &val, size);
+		p->size = size;
+		if (argc > 5) {
+			p->flag = strtol(argv[5], &e, 0);
 			if (e && *e) {
 				fprintf(stderr, "Bad flag\n");
 				return -1;
 			}
 		}
-		rv = ec_command(EC_CMD_SET_CROS_BOARD_INFO, 0, &p, sizeof(p),
-				NULL, 0);
+		rv = ec_command(EC_CMD_SET_CROS_BOARD_INFO, 0,
+				p, sizeof(*p) + size, NULL, 0);
 		if (rv < 0) {
 			if (rv == -EC_RES_ACCESS_DENIED - EECRESULT)
 				fprintf(stderr, "Write failed. WP enabled?\n");