From 318557af840588d1c0e95b505f828abbf39363e4 Mon Sep 17 00:00:00 2001 From: Ting Shen Date: Mon, 2 Dec 2019 15:27:57 +0800 Subject: kodama: overwrite bad battery params to known good value Kodama's bitbang driver fails randomly, and there's no way to notify kernel side that bitbang read failed (batt->flags does not propagate into kernel). Thus, if any value in batt_params is bad, replace it with a cached good value, to make sure we never send random numbers to kernel side. BUG=b:144195782 TEST=Modify smart battery driver to make sb_read has 50% fail rate, and monitor /sys/class/power_supply/sbs*/*, make sure the bad values does not observable in kernel. BRANCH=kukui Change-Id: Idf4691eb743f1ef785593b308b8f07a34e5ea642 Signed-off-by: Ting Shen Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/1943637 Reviewed-by: Eric Yilun Lin Reviewed-by: Daisuke Nojiri Commit-Queue: Ting Shen Tested-by: Ting Shen Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/3872709 Tested-by: Daisuke Nojiri Auto-Submit: Daisuke Nojiri Reviewed-by: Ting Shen Commit-Queue: Daisuke Nojiri --- board/kodama/battery.c | 227 ++++++++++++++++++++++++++++++++++++++++ common/battery.c | 4 + driver/battery/smart.c | 1 + include/battery.h | 5 + test/battery_get_params_smart.c | 4 + 5 files changed, 241 insertions(+) create mode 100644 board/kodama/battery.c diff --git a/board/kodama/battery.c b/board/kodama/battery.c new file mode 100644 index 0000000000..4328ff02d8 --- /dev/null +++ b/board/kodama/battery.c @@ -0,0 +1,227 @@ +/* Copyright 2019 The Chromium OS Authors. All rights reserved. + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#include "battery.h" +#include "battery_fuel_gauge.h" +#include "charge_state.h" +#include "charger_mt6370.h" +#include "console.h" +#include "driver/charger/rt946x.h" +#include "gpio.h" +#include "power.h" +#include "timer.h" +#include "usb_pd.h" +#include "util.h" + +#define CPRINTS(format, args...) cprints(CC_CHARGER, format, ## args) + +const struct board_batt_params board_battery_info[] = { + [BATTERY_SIMPLO] = { + .fuel_gauge = { + .manuf_name = "SMP", + .device_name = "L19M3PG0", + .ship_mode = { + .reg_addr = 0x34, + .reg_data = { 0x0000, 0x1000 }, + }, + .fet = { + .reg_addr = 0x34, + .reg_mask = 0x0100, + .disconnect_val = 0x0100, + } + }, + .batt_info = { + .voltage_max = 4400, + .voltage_normal = 3840, + .voltage_min = 3000, + .precharge_current = 256, + .start_charging_min_c = 0, + .start_charging_max_c = 45, + .charging_min_c = 0, + .charging_max_c = 60, + .discharging_min_c = -20, + .discharging_max_c = 60, + }, + }, + [BATTERY_CELXPERT] = { + .fuel_gauge = { + .manuf_name = "Celxpert", + .device_name = "L19C3PG0", + .ship_mode = { + .reg_addr = 0x34, + .reg_data = { 0x0000, 0x1000 }, + }, + .fet = { + .reg_addr = 0x34, + .reg_mask = 0x0100, + .disconnect_val = 0x0100, + } + }, + .batt_info = { + .voltage_max = 4400, + .voltage_normal = 3840, + .voltage_min = 2800, + .precharge_current = 404, + .start_charging_min_c = 0, + .start_charging_max_c = 45, + .charging_min_c = 0, + .charging_max_c = 60, + .discharging_min_c = -20, + .discharging_max_c = 60, + }, + }, +}; +BUILD_ASSERT(ARRAY_SIZE(board_battery_info) == BATTERY_TYPE_COUNT); + +const enum battery_type DEFAULT_BATTERY_TYPE = BATTERY_SIMPLO; + +enum battery_present battery_hw_present(void) +{ + return gpio_get_level(GPIO_EC_BATT_PRES_ODL) ? BP_NO : BP_YES; +} + +static void fix_single_param(int flag, int *cached, int *curr) +{ + if (flag) + *curr = *cached; + else + *cached = *curr; +} + +#define CACHE_INVALIDATION_TIME_US (5 * SECOND) + +/* + * b:144195782: bitbang fails randomly, and there's no way to + * notify kernel side that bitbang read failed. + * Thus, if any value in batt_params is bad, replace it with a cached + * good value, to make sure we never send random numbers to kernel + * side. + */ +__override void board_battery_compensate_params(struct batt_params *batt) +{ + static struct batt_params batt_cache = { 0 }; + static timestamp_t deadline; + + /* + * If battery keeps failing for 5 seconds, stop hiding the error and + * report back to host. + */ + if (batt->flags & BATT_FLAG_BAD_ANY) { + if (timestamp_expired(deadline, NULL)) + return; + } else { + deadline.val = get_time().val + CACHE_INVALIDATION_TIME_US; + } + + /* return cached values for at most CACHE_INVALIDATION_TIME_US */ + fix_single_param(batt->flags & BATT_FLAG_BAD_STATE_OF_CHARGE, + &batt_cache.state_of_charge, + &batt->state_of_charge); + fix_single_param(batt->flags & BATT_FLAG_BAD_VOLTAGE, + &batt_cache.voltage, + &batt->voltage); + fix_single_param(batt->flags & BATT_FLAG_BAD_CURRENT, + &batt_cache.current, + &batt->current); + fix_single_param(batt->flags & BATT_FLAG_BAD_DESIRED_VOLTAGE, + &batt_cache.desired_voltage, + &batt->desired_voltage); + fix_single_param(batt->flags & BATT_FLAG_BAD_DESIRED_CURRENT, + &batt_cache.desired_current, + &batt->desired_current); + fix_single_param(batt->flags & BATT_FLAG_BAD_REMAINING_CAPACITY, + &batt_cache.remaining_capacity, + &batt->remaining_capacity); + fix_single_param(batt->flags & BATT_FLAG_BAD_FULL_CAPACITY, + &batt_cache.full_capacity, + &batt->full_capacity); + fix_single_param(batt->flags & BATT_FLAG_BAD_STATUS, + &batt_cache.status, + &batt->status); + fix_single_param(batt->flags & BATT_FLAG_BAD_TEMPERATURE, + &batt_cache.temperature, + &batt->temperature); + /* + * If battery_compensate_params() didn't calculate display_charge + * for us, also update it with last good value. + */ + fix_single_param(batt->display_charge == 0, + &batt_cache.display_charge, + &batt->display_charge); + + /* remove bad flags after applying cached values */ + batt->flags &= ~BATT_FLAG_BAD_ANY; +} + +int charger_profile_override(struct charge_state_data *curr) +{ + const struct battery_info *batt_info = battery_get_info(); + static int normal_charge_lock, over_discharge_lock; + /* battery temp in 0.1 deg C */ + int bat_temp_c = curr->batt.temperature - 2731; + + /* + * SMP battery uses HW pre-charge circuit and pre-charge current is + * limited to ~50mA. Once the charge current is lower than IEOC level + * within CHG_TEDG_EOC, and TE is enabled, the charging power path will + * be turned off. Disable EOC and TE when battery stays over discharge + * state, otherwise enable EOC and TE. + */ + if (!(curr->batt.flags & BATT_FLAG_BAD_VOLTAGE)) { + if (curr->batt.voltage < batt_info->voltage_min) { + normal_charge_lock = 0; + + if (!over_discharge_lock && curr->state == ST_CHARGE) { + over_discharge_lock = 1; + rt946x_enable_charge_eoc(0); + rt946x_enable_charge_termination(0); + } + } else { + over_discharge_lock = 0; + + if (!normal_charge_lock) { + normal_charge_lock = 1; + rt946x_enable_charge_eoc(1); + rt946x_enable_charge_termination(1); + } + } + } + +#ifdef VARIANT_KUKUI_CHARGER_MT6370 + mt6370_charger_profile_override(curr); +#endif /* CONFIG_CHARGER_MT6370 */ + + /* + * When smart battery temperature is more than 45 deg C, the max + * charging voltage is 4100mV. + */ + if (curr->state == ST_CHARGE && bat_temp_c >= 450 + && !(curr->batt.flags & BATT_FLAG_BAD_TEMPERATURE)) + curr->requested_voltage = 4100; + else + curr->requested_voltage = batt_info->voltage_max; + + /* + * mt6370's minimum regulated current is 500mA REG17[7:2] 0b100, + * values below 0b100 are preserved. In the other hand, it makes sure + * mt6370's VOREG set as 4400mV and minimum value of mt6370's ICHG + * is limited as 500mA. + */ + curr->requested_current = MAX(500, curr->requested_current); + + return 0; +} + +enum ec_status charger_profile_override_get_param(uint32_t param, + uint32_t *value) +{ + return EC_RES_INVALID_PARAM; +} + +enum ec_status charger_profile_override_set_param(uint32_t param, + uint32_t value) +{ + return EC_RES_INVALID_PARAM; +} diff --git a/common/battery.c b/common/battery.c index c58d0b11b7..92ceafa8ff 100644 --- a/common/battery.c +++ b/common/battery.c @@ -602,6 +602,10 @@ void battery_compensate_params(struct batt_params *batt) batt->display_charge = 1000; } +__overridable void board_battery_compensate_params(struct batt_params *batt) +{ +} + __attribute__((weak)) int get_battery_manufacturer_name(char *dest, int size) { strzcpy(dest, "", size); diff --git a/driver/battery/smart.c b/driver/battery/smart.c index cc3ef73c29..51a04d3b6b 100644 --- a/driver/battery/smart.c +++ b/driver/battery/smart.c @@ -398,6 +398,7 @@ void battery_get_params(struct batt_params *batt) #ifdef HAS_TASK_HOSTCMD /* if there is no host, we don't care about compensation */ battery_compensate_params(&batt_new); + board_battery_compensate_params(&batt_new); #endif /* Update visible battery parameters */ diff --git a/include/battery.h b/include/battery.h index df5aee5990..55ea92a39e 100644 --- a/include/battery.h +++ b/include/battery.h @@ -439,4 +439,9 @@ extern struct i2c_stress_test_dev battery_i2c_stress_test_dev; */ void battery_compensate_params(struct batt_params *batt); +/** + * board-specific battery_compensate_params + */ +__override_proto void board_battery_compensate_params(struct batt_params *batt); + #endif /* __CROS_EC_BATTERY_H */ diff --git a/test/battery_get_params_smart.c b/test/battery_get_params_smart.c index c44cd64e56..751a719ba1 100644 --- a/test/battery_get_params_smart.c +++ b/test/battery_get_params_smart.c @@ -24,6 +24,10 @@ void battery_compensate_params(struct batt_params *batt) { } +void board_battery_compensate_params(struct batt_params *batt) +{ +} + static void reset_and_fail_on(int first, int last) { /* We're not initializing the fake battery, so everything reads zero */ -- cgit v1.2.1