From a98f627ad2d4c6f061945eeebab8e4bddf5ac2a4 Mon Sep 17 00:00:00 2001 From: Daisuke Nojiri Date: Wed, 14 Feb 2018 15:20:09 -0800 Subject: 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 where is a numeric value assigned to each data item, is the number of bytes used for . 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 Reviewed-on: https://chromium-review.googlesource.com/920905 Reviewed-by: Aaron Durbin Reviewed-on: https://chromium-review.googlesource.com/929311 --- common/cbi.c | 265 ++++++++++++++++++++++++++++------------------ include/cros_board_info.h | 37 ++++--- include/ec_commands.h | 21 ++-- util/ectool.c | 68 +++++++----- 4 files changed, 237 insertions(+), 154 deletions(-) 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 . (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 e1573e85b9..5943f306df 100644 --- a/include/ec_commands.h +++ b/include/ec_commands.h @@ -4582,14 +4582,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, }; /* @@ -4601,7 +4598,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_* */ }; @@ -4617,10 +4614,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 a706ec12e8..6661e51936 100644 --- a/util/ectool.c +++ b/util/ectool.c @@ -6231,11 +6231,13 @@ static void cmd_cbi_help(char *cmd) { fprintf(stderr, " Usage: %s get [get_flag]\n" - " Usage: %s set value [set_flag]\n" + " Usage: %s set [set_flag]\n" " is one of:\n" " 0: BOARD_VERSION\n" " 1: OEM_ID\n" " 2: SKU_ID\n" + " is the size of the data" + " 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" @@ -6250,7 +6252,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; @@ -6260,17 +6262,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) { @@ -6279,45 +6282,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"); -- cgit v1.2.1