diff options
author | Louis Yung-Chieh Lo <yjlou@chromium.org> | 2014-04-14 15:51:42 -0700 |
---|---|---|
committer | chrome-internal-fetch <chrome-internal-fetch@google.com> | 2014-04-21 20:58:32 +0000 |
commit | 58878e79e07f33050d6ff8d280e5db63b5902e10 (patch) | |
tree | 05abf3306c786062bdf843c53c515f9cac0a2e2d | |
parent | bd1a3ffeaf6d40d96b43c1169b150ae68a6d1291 (diff) | |
download | chrome-ec-stabilize-5784.0.B.tar.gz |
Fixed the stack overflow bug in 'battery' console command.stabilize-5784.0.B
On the Nyan EC, we almost run out of the stack of console task.
Instead of making that struct static or global, we print the cached data.
Read the issue tracker for more detailed discussion.
BUG=chrome-os-partner:28027
BRANCH=tot
TEST=verified on nyan with/without battery.
The "battery" console command doesn't crash the system.
Change-Id: Id5246724760aed4cf1df827baf115007b2ffb48e
Signed-off-by: Louis Yung-Chieh Lo <yjlou@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/194875
Reviewed-by: Dave Parker <dparker@chromium.org>
Reviewed-by: Bill Richardson <wfrichar@chromium.org>
-rw-r--r-- | common/battery.c | 39 | ||||
-rw-r--r-- | common/charge_state_v1.c | 5 | ||||
-rw-r--r-- | common/charge_state_v2.c | 5 | ||||
-rw-r--r-- | common/pmu_tps65090_charger.c | 20 | ||||
-rw-r--r-- | include/charge_state.h | 7 |
5 files changed, 61 insertions, 15 deletions
diff --git a/common/battery.c b/common/battery.c index 1f8b757929..25df2e17a3 100644 --- a/common/battery.c +++ b/common/battery.c @@ -6,6 +6,7 @@ */ #include "battery.h" +#include "charge_state.h" #include "common.h" #include "console.h" #include "gpio.h" @@ -96,41 +97,51 @@ static void print_battery_strings(void) static void print_battery_params(void) { - struct batt_params batt; +#if defined(HAS_TASK_CHARGER) + /* Ask charger so that we don't need to ask battery again. */ + const struct batt_params *batt = charger_current_battery_params(); +#else + /* This is for test code, where doesn't have charger task. */ + struct batt_params _batt; + const struct batt_params *batt = &_batt; + + battery_get_params(&_batt); +#endif - battery_get_params(&batt); print_item_name("Param flags:"); - ccprintf("%08x\n", batt.flags); + ccprintf("%08x\n", batt->flags); print_item_name("Temp:"); ccprintf("0x%04x = %.1d K (%.1d C)\n", - batt.temperature, batt.temperature, batt.temperature - 2731); + batt->temperature, + batt->temperature, + batt->temperature - 2731); print_item_name("V:"); - ccprintf("0x%04x = %d mV\n", batt.voltage, batt.voltage); + ccprintf("0x%04x = %d mV\n", batt->voltage, batt->voltage); print_item_name("V-desired:"); - ccprintf("0x%04x = %d mV\n", batt.desired_voltage, - batt.desired_voltage); + ccprintf("0x%04x = %d mV\n", batt->desired_voltage, + batt->desired_voltage); print_item_name("I:"); - ccprintf("0x%04x = %d mA", batt.current & 0xffff, batt.current); - if (batt.current > 0) + ccprintf("0x%04x = %d mA", batt->current & 0xffff, batt->current); + if (batt->current > 0) ccputs("(CHG)"); - else if (batt.current < 0) + else if (batt->current < 0) ccputs("(DISCHG)"); ccputs("\n"); print_item_name("I-desired:"); - ccprintf("0x%04x = %d mA\n", batt.desired_current, - batt.desired_current); + ccprintf("0x%04x = %d mA\n", batt->desired_current, + batt->desired_current); print_item_name("Charging:"); ccprintf("%sAllowed\n", - batt.flags & BATT_FLAG_WANT_CHARGE ? "" : "Not "); + batt->flags & BATT_FLAG_WANT_CHARGE ? "" : "Not "); print_item_name("Charge:"); - ccprintf("%d %%\n", batt.state_of_charge); + ccprintf("%d %%\n", batt->state_of_charge); } static void print_battery_info(void) diff --git a/common/charge_state_v1.c b/common/charge_state_v1.c index 1b1ed8272d..fc1fe28da3 100644 --- a/common/charge_state_v1.c +++ b/common/charge_state_v1.c @@ -695,6 +695,11 @@ static int charge_force_idle(int enable) return EC_SUCCESS; } +const struct batt_params *charger_current_battery_params(void) +{ + return &task_ctx.curr.batt; +} + /** * Battery charging task */ diff --git a/common/charge_state_v2.c b/common/charge_state_v2.c index ef8d3becb8..25cde8eefc 100644 --- a/common/charge_state_v2.c +++ b/common/charge_state_v2.c @@ -398,6 +398,11 @@ static void notify_host_of_low_battery(void) host_set_single_event(EC_HOST_EVENT_BATTERY_CRITICAL); } +const struct batt_params *charger_current_battery_params(void) +{ + return &curr.batt; +} + /* Main loop */ void charger_task(void) { diff --git a/common/pmu_tps65090_charger.c b/common/pmu_tps65090_charger.c index ff66a9c4d8..69ce2a9c26 100644 --- a/common/pmu_tps65090_charger.c +++ b/common/pmu_tps65090_charger.c @@ -52,6 +52,10 @@ static int has_pending_event; static enum charging_state current_state = ST_IDLE0; +/* Cached version of battery parameter */ +static struct batt_params batt_params_copy; + + #ifdef CONFIG_PMU_TPS65090_CHARGING_LED static void update_battery_led(void) { @@ -177,12 +181,26 @@ static int rsoc_moving_average(int state_of_charge) return moving_average; } +/* + * This saves battery parameters for charger_current_battery_params(). + */ +static void battery_get_params_and_save_a_copy(struct batt_params *batt) +{ + battery_get_params(&batt_params_copy); + memcpy(batt, &batt_params_copy, sizeof(*batt)); +} + +struct batt_params *charger_current_battery_params(void) +{ + return &batt_params_copy; +} + static int calc_next_state(int state) { struct batt_params batt; int alarm; - battery_get_params(&batt); + battery_get_params_and_save_a_copy(&batt); switch (state) { case ST_IDLE0: diff --git a/include/charge_state.h b/include/charge_state.h index 35c2041d45..0da544422f 100644 --- a/include/charge_state.h +++ b/include/charge_state.h @@ -103,6 +103,13 @@ int charge_want_shutdown(void); */ int charge_temp_sensor_get_val(int idx, int *temp_ptr); +/** + * Get the pointer to the battery parameters we saved in charge state. + * + * Use this carefully. Other threads can modify data while you are reading. + */ +const struct batt_params *charger_current_battery_params(void); + /* Pick the right implementation */ #ifdef CONFIG_CHARGER_V1 |