diff options
author | Shawn Nematbakhsh <shawnn@chromium.org> | 2017-06-23 17:35:27 -0700 |
---|---|---|
committer | chrome-bot <chrome-bot@chromium.org> | 2017-07-05 14:24:49 -0700 |
commit | 67759f3553555a7ea6c3a5296934cbbed2297c68 (patch) | |
tree | d5ce3d98cca1c0ff9cb7ac2f94dc1bf2d79fe566 | |
parent | e3336f4c8d4fb59137d35f87f4a42d22848aabcd (diff) | |
download | chrome-ec-67759f3553555a7ea6c3a5296934cbbed2297c68.tar.gz |
virtual_battery: Remove direct i2c access
Virtual battery implements a smart battery interface, but the actual
battery on the system may speak a different protocol. Support such
batteries by removing direct i2c access from the virtual battery driver.
Fetch data from storage when available, and call generic battery
routines when not.
BUG=chromium:717753
BRANCH=None
TEST=Manual on kevin, boot and verify "Unhandled VB reg" prints are not
seen. Verify by-eye that all regs in cros 4.4 kernel sbs-battery.c are
handled (except REG_MANUFACTURER_DATA). Verify that sysfs manufacturer,
model name, time_to_full_avg and time_to_empty_avg values are all sane.
Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org>
Change-Id: Ia8fc0a80ac7576fa8bdcc3b7dac0609d9d754234
Reviewed-on: https://chromium-review.googlesource.com/547004
Commit-Ready: Shawn N <shawnn@chromium.org>
Tested-by: Shawn N <shawnn@chromium.org>
Reviewed-by: Shawn N <shawnn@chromium.org>
-rw-r--r-- | common/i2c_master.c | 11 | ||||
-rw-r--r-- | common/virtual_battery.c | 133 |
2 files changed, 84 insertions, 60 deletions
diff --git a/common/i2c_master.c b/common/i2c_master.c index 1dc1a09b64..25b31613fa 100644 --- a/common/i2c_master.c +++ b/common/i2c_master.c @@ -541,6 +541,7 @@ static int i2c_command_passthru(struct host_cmd_handler_args *args) const uint8_t *out; int in_len; int ret, i; + int port_is_locked = 0; #ifdef CONFIG_BATTERY_CUT_OFF /* @@ -571,8 +572,6 @@ static int i2c_command_passthru(struct host_cmd_handler_args *args) out = args->params + sizeof(*params) + params->num_msgs * sizeof(*msg); in_len = 0; - i2c_lock(params->port, 1); - for (resp->num_msgs = 0, msg = params->msg; resp->num_msgs < params->num_msgs; resp->num_msgs++, msg++) { @@ -609,10 +608,13 @@ static int i2c_command_passthru(struct host_cmd_handler_args *args) #ifdef CONFIG_I2C_PASSTHRU_RESTRICTED if (system_is_locked() && !board_allow_i2c_passthru(params->port)) { - i2c_lock(params->port, 0); + if (port_is_locked) + i2c_lock(params->port, 0); return EC_RES_ACCESS_DENIED; } #endif + if (!port_is_locked) + i2c_lock(params->port, (port_is_locked = 1)); rv = i2c_xfer(params->port, addr, out, write_len, &resp->data[in_len], read_len, xferflags); } @@ -632,7 +634,8 @@ static int i2c_command_passthru(struct host_cmd_handler_args *args) args->response_size = sizeof(*resp) + in_len; /* Unlock port */ - i2c_lock(params->port, 0); + if (port_is_locked) + i2c_lock(params->port, 0); /* * Return success even if transfer failed so response is sent. Host diff --git a/common/virtual_battery.c b/common/virtual_battery.c index 63abe5b510..c9c600a745 100644 --- a/common/virtual_battery.c +++ b/common/virtual_battery.c @@ -71,38 +71,15 @@ int virtual_battery_handler(struct ec_response_i2c_passthru *resp, *err_code = 0; } else { sb_cmd_state = READ_VB; - /* Test if the reg is cached. */ *err_code = virtual_battery_operation(batt_cmd_head, NULL, 0, 0); /* - * If the reg is not cached in the virtual memory, - * we need to physically write the reg index to - * the battery. + * If the reg is not handled by virtual battery, we + * do not support it. */ - if (*err_code) { - *err_code = i2c_xfer( - I2C_PORT_VIRTUAL_BATTERY, - VIRTUAL_BATTERY_ADDR, - batt_cmd_head, - 1, - NULL, - 0, - I2C_XFER_START); - /* sent a stop bit here */ - if (*err_code) { - if (*err_code == EC_ERROR_TIMEOUT) { - resp->i2c_status = - EC_I2C_STATUS_TIMEOUT; - } else { - resp->i2c_status = - EC_I2C_STATUS_NAK; - } - reset_parse_state(); - return EC_ERROR_INVAL; - } - *err_code = 1; - } else - cache_hit = 1; + if (*err_code) + return EC_ERROR_INVAL; + cache_hit = 1; } break; case WRITE_VB: @@ -173,6 +150,25 @@ void reset_parse_state(void) acc_write_len = 0; } +/* + * Copy memmap string data from offset to dest, up to size len, in the format + * expected by SBS (first byte of dest contains strlen). + */ +void copy_memmap_string(uint8_t *dest, int offset, int len) +{ + uint8_t *memmap_str; + uint8_t memmap_strlen; + + if (len == 0) + return; + memmap_str = host_get_memmap(offset); + /* memmap_str might not be NULL terminated */ + memmap_strlen = *(memmap_str + EC_MEMMAP_TEXT_MAX - 1) == '\0' ? + strlen(memmap_str) : EC_MEMMAP_TEXT_MAX; + dest[0] = memmap_strlen; + memcpy(dest + 1, memmap_str, MIN(memmap_strlen, len - 1)); +} + int virtual_battery_operation(const uint8_t *batt_cmd_head, uint8_t *dest, int read_len, @@ -186,16 +182,14 @@ int virtual_battery_operation(const uint8_t *batt_cmd_head, * Note that we don't update the cached capacity: We do a real-time * conversion and return the converted values. */ - static uint16_t batt_mode_cache; + static int batt_mode_cache; const struct batt_params *curr_batt; - /* - * All of the smart battery reg indexes supported by this virtual - * battery implementation are two bytes long. So we should limit - * the range of memory access accordingly. + * Don't allow host reads into arbitrary memory space, most params + * are two bytes. */ - if (read_len > 2) - read_len = 2; + int bounded_read_len = MIN(read_len, 2); + curr_batt = charger_current_battery_params(); switch (*batt_cmd_head) { case SB_BATTERY_MODE: @@ -203,69 +197,96 @@ int virtual_battery_operation(const uint8_t *batt_cmd_head, batt_mode_cache = batt_cmd_head[1] | (batt_cmd_head[2] << 8); } else if (read_len > 0) { - if (batt_mode_cache == 0) { + if (batt_mode_cache == 0) /* * Read the battery operational mode from * the battery to initialize batt_mode_cache. + * This may cause an i2c transaction. */ - i2c_xfer(I2C_PORT_VIRTUAL_BATTERY, - VIRTUAL_BATTERY_ADDR, - batt_cmd_head, - 1, - (uint8_t *)&batt_mode_cache, - 2, - I2C_XFER_SINGLE); - } - memcpy(dest, &batt_mode_cache, read_len); + if (battery_get_mode(&batt_mode_cache) == + EC_ERROR_UNIMPLEMENTED) + /* + * Register not supported, choose + * typical SB defaults. + */ + batt_mode_cache = + MODE_INTERNAL_CHARGE_CONTROLLER | + MODE_ALARM | + MODE_CHARGER; + + memcpy(dest, &batt_mode_cache, bounded_read_len); } break; case SB_SERIAL_NUMBER: val = strtoi(host_get_memmap(EC_MEMMAP_BATT_SERIAL), NULL, 16); - memcpy(dest, &val, read_len); + memcpy(dest, &val, bounded_read_len); break; case SB_VOLTAGE: - memcpy(dest, &(curr_batt->voltage), read_len); + memcpy(dest, &(curr_batt->voltage), bounded_read_len); break; case SB_RELATIVE_STATE_OF_CHARGE: - memcpy(dest, &(curr_batt->state_of_charge), read_len); + memcpy(dest, &(curr_batt->state_of_charge), bounded_read_len); break; case SB_TEMPERATURE: - memcpy(dest, &(curr_batt->temperature), read_len); + memcpy(dest, &(curr_batt->temperature), bounded_read_len); break; case SB_CURRENT: - memcpy(dest, &(curr_batt->current), read_len); + memcpy(dest, &(curr_batt->current), bounded_read_len); break; case SB_FULL_CHARGE_CAPACITY: val = curr_batt->full_capacity; if (batt_mode_cache & MODE_CAPACITY) val = val * curr_batt->voltage / 10000; - memcpy(dest, &val, read_len); + memcpy(dest, &val, bounded_read_len); break; case SB_BATTERY_STATUS: - memcpy(dest, &(curr_batt->status), read_len); + memcpy(dest, &(curr_batt->status), bounded_read_len); break; case SB_CYCLE_COUNT: memcpy(dest, (int *)host_get_memmap(EC_MEMMAP_BATT_CCNT), - read_len); + bounded_read_len); break; case SB_DESIGN_CAPACITY: val = *(int *)host_get_memmap(EC_MEMMAP_BATT_DCAP); if (batt_mode_cache & MODE_CAPACITY) val = val * curr_batt->voltage / 10000; - memcpy(dest, &val, read_len); + memcpy(dest, &val, bounded_read_len); break; case SB_DESIGN_VOLTAGE: memcpy(dest, (int *)host_get_memmap(EC_MEMMAP_BATT_DVLT), - read_len); + bounded_read_len); break; case SB_REMAINING_CAPACITY: val = curr_batt->remaining_capacity; if (batt_mode_cache & MODE_CAPACITY) val = val * curr_batt->voltage / 10000; - memcpy(dest, &val, read_len); + memcpy(dest, &val, bounded_read_len); + break; + case SB_MANUFACTURER_NAME: + copy_memmap_string(dest, EC_MEMMAP_BATT_MFGR, read_len); + break; + case SB_DEVICE_NAME: + copy_memmap_string(dest, EC_MEMMAP_BATT_MODEL, read_len); + break; + case SB_AVERAGE_TIME_TO_FULL: + /* This may cause an i2c transaction */ + if (battery_time_to_full(&val)) + return EC_ERROR_INVAL; + memcpy(dest, &val, bounded_read_len); break; + case SB_AVERAGE_TIME_TO_EMPTY: + /* This may cause an i2c transaction */ + if (battery_time_to_empty(&val)) + return EC_ERROR_INVAL; + memcpy(dest, &val, bounded_read_len); + break; + case SB_MANUFACTURER_ACCESS: + /* No manuf. access reg access allowed over VB interface */ + return EC_ERROR_INVAL; default: + CPRINTS("Unhandled VB reg %x", *batt_cmd_head); return EC_ERROR_INVAL; } return EC_SUCCESS; } + |