summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorScott Collyer <scollyer@google.com>2017-11-15 17:18:20 -0800
committerchrome-bot <chrome-bot@chromium.org>2017-11-21 15:58:42 -0800
commit0da531fae0099080b7dd472ade0788c18162cc19 (patch)
tree7aabf3e458e8da5ab72990e78344f52bb88c686a
parent267320bd477f9758bb7d16e32985c671cc92475a (diff)
downloadchrome-ec-0da531fae0099080b7dd472ade0788c18162cc19.tar.gz
coral: Fix corner case for battery_present_timer_started flag
With the SMP and Celxpert batteries for Robo systems, following battery cutoff, there is a race condition resulting from a failed sb_read operation when the battery status is read. If this read fails, then the flag battery_report_present is reset to 0. Since this flag gets set in a hook task deferred callback, if the sb_read fails after battery_report_present is set, but before batt_pres is set to BP_YES, then the deferred call would not be restarted. This results in the battery_is_present call returning BP_NO indefinitely. To fix this condition, this CL ensures that battery_report_present_timer_started is cleared for any case that could result in BP_NO. This addressed both this corner case and makes redundant a previous change that was put to handle the case where the battery is disconnected and reconnected while the system remains powered. The CL also adjusts the timer to 0.5 sec so that in the event it has to be called twice, it doesn't exceed the previous 1 second timer and delay boot time even longer. BUG=b:69151530 BRANCH=coral TEST=With the DUT powered, removed and reconnected the battery. Ensured that the battery again reports present. Bitland also verified that with this CL they can no longer reproduce the issue. For the corner case, had additional debug console prints. (Note this is with 1 second timer) The deferred call is started. [0.156135 battery timer hook call] This shows where the sb_read error happens in batt_init() [1.085627 Battery FET: reg 0x0018 mask 0x0010 disc 0x0000] [1.092251 battery: pres 0, prev_pres 0, cutoff 0, init 1, rep 0] [1.160969 battery will now report present] This shows where the batt_report present gets cleared [1.184993 ******** batt_report_present 1 -> 0 *****] [1.185840 Battery read status failed] [1.186540 report = 0, batt_init 1->0: stat = 0x200c6bea 1 get 1] [1.187544 battery: pres 0, prev_pres 0, cutoff 0, init 0, rep 0] [1.193796 Battery read status failed] [1.194886 battery: pres 0, prev_pres 0, cutoff 0, init 0, rep 0] batt_init() no longer returns 0, so deferred call is restarted [1.290559 Battery FET: reg 0x0018 mask 0x0010 disc 0x0000] [1.292942 battery timer hook call] battery reports present now [2.293436 battery will now report present] Change-Id: I89d69cf133365affc4cc538328daeaaf9ac05ed9 Signed-off-by: Scott Collyer <scollyer@google.com> Reviewed-on: https://chromium-review.googlesource.com/773623 Tested-by: Scott Collyer <scollyer@chromium.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org> Reviewed-by: Furquan Shaikh <furquan@chromium.org> Commit-Queue: Scott Collyer <scollyer@chromium.org> (cherry picked from commit 535080115d20dc223dab6288c5fea02da67c8e9c) Reviewed-on: https://chromium-review.googlesource.com/782599 Commit-Ready: Scott Collyer <scollyer@chromium.org>
-rw-r--r--board/coral/battery.c22
1 files changed, 11 insertions, 11 deletions
diff --git a/board/coral/battery.c b/board/coral/battery.c
index b80bac3645..ebc8071c0d 100644
--- a/board/coral/battery.c
+++ b/board/coral/battery.c
@@ -641,28 +641,28 @@ enum battery_present battery_is_present(void)
battery_check_disconnect() != BATTERY_NOT_DISCONNECTED ||
battery_init() == 0)) {
battery_report_present = 0;
+ /*
+ * When this path is taken, the _timer_started flag must be
+ * reset so the 'else if' path will be entered and the
+ * battery_report_present flag can be set by the deferred
+ * call. This handles the case of the battery being disconected
+ * and reconnected while running or if battery_init() returns an
+ * error due to a failed sb_read.
+ */
+ battery_report_present_timer_started = 0;
} else if (batt_pres == BP_YES && batt_pres_prev == BP_NO &&
!battery_report_present_timer_started) {
/*
- * Wait 1 second before reporting present if it was
+ * Wait 1/2 second before reporting present if it was
* previously reported as not present
*/
battery_report_present_timer_started = 1;
battery_report_present = 0;
- hook_call_deferred(&battery_now_present_data, SECOND);
+ hook_call_deferred(&battery_now_present_data, 500 * MSEC);
}
if (!battery_report_present)
batt_pres = BP_NO;
- /*
- * If battery was present, but has been disabled or removed, then reset
- * the timer_started variable, so when battery is reconnected it can
- * report as present.
- */
- if (batt_pres_prev == BP_YES && batt_pres == BP_NO) {
- battery_report_present_timer_started = 0;
- CPRINTS("Battery was present, but is now disconnected");
- }
batt_pres_prev = batt_pres;