summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLouis Yung-Chieh Lo <yjlou@chromium.org>2014-04-14 15:51:42 -0700
committerchrome-internal-fetch <chrome-internal-fetch@google.com>2014-04-21 20:58:32 +0000
commit58878e79e07f33050d6ff8d280e5db63b5902e10 (patch)
tree05abf3306c786062bdf843c53c515f9cac0a2e2d
parentbd1a3ffeaf6d40d96b43c1169b150ae68a6d1291 (diff)
downloadchrome-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.c39
-rw-r--r--common/charge_state_v1.c5
-rw-r--r--common/charge_state_v2.c5
-rw-r--r--common/pmu_tps65090_charger.c20
-rw-r--r--include/charge_state.h7
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