From 6cc992c3d703ecee5e44966a2a3f6580e85b60c3 Mon Sep 17 00:00:00 2001 From: Alec Berg Date: Tue, 9 Dec 2014 17:28:25 -0800 Subject: [PATCH] samus: fix charge state machine's handling of low power chargers On samus it is possible to have AC plugged in but have the battery discharging. So, add a new variable to charge state machine for battery charging status and use that where necessary. For example, the low battery shutdown code should now be based on whether or not battery is charging rather than if AC is present. This also changes the hibernate behavior when battery is low. The change is to wait 30 seconds in G3 of low battery with no charging before hibernating because for some chargers, like a USB PD charger, the charger may increase it's current limit after a little bit of time. BUG=chrome-os-partner:34485 BRANCH=samus TEST=test on samus. use low power charger and make sure that ectool battery shows the "DISCHARGING" flag. use zinger and see "CHARGING" flag. also use power_supply_info to make sure that the battery state accurately reflects reality. Change-Id: I8ac0267dd393071c4ca1fa24fbc9a13bf27848a9 Signed-off-by: Alec Berg Reviewed-on: https://chromium-review.googlesource.com/235491 Reviewed-by: Bill Richardson --- common/charge_state_v2.c | 58 +++++++++++++++++++-------------------- include/charge_state_v2.h | 1 + test/sbs_charging_v2.c | 4 +++ 3 files changed, 34 insertions(+), 29 deletions(-) diff --git a/common/charge_state_v2.c b/common/charge_state_v2.c index 2346ddd131..74d8e910f5 100644 --- a/common/charge_state_v2.c +++ b/common/charge_state_v2.c @@ -200,17 +200,8 @@ static void update_dynamic_battery_info(void) curr.batt.state_of_charge <= BATTERY_LEVEL_CRITICAL) tmp |= EC_BATT_FLAG_LEVEL_CRITICAL; - switch (curr.state) { - case ST_DISCHARGE: - tmp |= EC_BATT_FLAG_DISCHARGING; - break; - case ST_CHARGE: - tmp |= EC_BATT_FLAG_CHARGING; - break; - default: - /* neither charging nor discharging */ - break; - } + tmp |= curr.batt_is_charging ? EC_BATT_FLAG_CHARGING : + EC_BATT_FLAG_DISCHARGING; /* Tell the AP to re-read battery status if charge state changes */ if (*memmap_flags != tmp) @@ -237,6 +228,7 @@ static void dump_charge_state(void) #define DUMP_BATT(FLD, FMT) ccprintf("\t" #FLD " = " FMT "\n", curr.batt. FLD) ccprintf("state = %s\n", state_list[curr.state]); DUMP(ac, "%d"); + DUMP(batt_is_charging, "%d"); ccprintf("chg.*:\n"); DUMP_CHG(voltage, "%dmV"); DUMP_CHG(current, "%dmA"); @@ -272,8 +264,7 @@ static void show_charging_progress(void) { int rv, minutes, to_full; - if (curr.state == ST_IDLE || - curr.state == ST_DISCHARGE) { + if (!curr.batt_is_charging) { rv = battery_time_to_empty(&minutes); to_full = 0; } else { @@ -414,7 +405,7 @@ static inline int battery_too_low(void) /* Shut everything down before the battery completely dies. */ static void prevent_deep_discharge(void) { - if (!battery_too_low()) { + if (!battery_too_low() || curr.batt_is_charging) { /* Reset shutdown warning time */ shutdown_warning_time.val = 0; return; @@ -423,22 +414,24 @@ static void prevent_deep_discharge(void) CPRINTS("Low battery: %d%%, %dmV", curr.batt.state_of_charge, curr.batt.voltage); - if (chipset_in_state(CHIPSET_STATE_ANY_OFF)) { -#ifdef CONFIG_HIBERNATE - /* AP is off, so shut down the EC now */ - CPRINTS("charge force EC hibernate due to low battery"); - system_hibernate(0, 0); -#endif - } else if (!shutdown_warning_time.val) { - /* Warn AP battery level is so low we'll shut down */ + if (!shutdown_warning_time.val) { CPRINTS("charge warn shutdown due to low battery"); shutdown_warning_time = get_time(); - host_set_single_event(EC_HOST_EVENT_BATTERY_SHUTDOWN); + if (!chipset_in_state(CHIPSET_STATE_ANY_OFF)) + host_set_single_event(EC_HOST_EVENT_BATTERY_SHUTDOWN); } else if (get_time().val > shutdown_warning_time.val + LOW_BATTERY_SHUTDOWN_TIMEOUT_US) { - /* Timeout waiting for AP to shut down, so kill it */ - CPRINTS("charge force shutdown due to low battery"); - chipset_force_shutdown(); + if (chipset_in_state(CHIPSET_STATE_ANY_OFF)) { +#ifdef CONFIG_HIBERNATE + /* Timeout waiting for charger to provide more power */ + CPRINTS("charge force EC hibernate due to low battery"); + system_hibernate(0, 0); +#endif + } else { + /* Timeout waiting for AP to shut down, so kill it */ + CPRINTS("charge force shutdown due to low battery"); + chipset_force_shutdown(); + } } } @@ -574,11 +567,18 @@ void charger_task(void) if (curr.batt.flags & BATT_FLAG_BAD_ANY) problem(PR_BATT_FLAGS, curr.batt.flags); + /* + * If AC is present, check if input current is sufficient to + * actually charge battery. + */ + curr.batt_is_charging = curr.ac && (curr.batt.current >= 0); + + /* Don't let the battery hurt itself. */ + prevent_hot_discharge(); + prevent_deep_discharge(); + if (!curr.ac) { curr.state = ST_DISCHARGE; - /* Don't let the battery hurt itself. */ - prevent_hot_discharge(); - prevent_deep_discharge(); goto wait_for_it; } diff --git a/include/charge_state_v2.h b/include/charge_state_v2.h index b8503fcc9c..37b88326c9 100644 --- a/include/charge_state_v2.h +++ b/include/charge_state_v2.h @@ -27,6 +27,7 @@ enum charge_state_v2 { struct charge_state_data { timestamp_t ts; int ac; + int batt_is_charging; struct charger_params chg; struct batt_params batt; enum charge_state_v2 state; diff --git a/test/sbs_charging_v2.c b/test/sbs_charging_v2.c index 31dfb34e25..bb15a8d18b 100644 --- a/test/sbs_charging_v2.c +++ b/test/sbs_charging_v2.c @@ -285,6 +285,8 @@ static int test_low_battery(void) mock_chipset_state = CHIPSET_STATE_SOFT_OFF; hook_notify(HOOK_CHIPSET_SHUTDOWN); wait_charging_state(); + /* after a while, the EC should hibernate */ + sleep(LOW_BATTERY_SHUTDOWN_TIMEOUT); TEST_ASSERT(is_hibernated); ccprintf("[CHARGING TEST] Low battery shutdown S5\n"); @@ -293,6 +295,8 @@ static int test_low_battery(void) wait_charging_state(); sb_write(SB_RELATIVE_STATE_OF_CHARGE, 2); wait_charging_state(); + /* after a while, the EC should hibernate */ + sleep(LOW_BATTERY_SHUTDOWN_TIMEOUT); TEST_ASSERT(is_hibernated); ccprintf("[CHARGING TEST] Low battery AP shutdown\n");