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>

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;