poppy: Prevent base detection from falling into reverse detection trap

The problem here is that both normal detection range (120-300mV)
and reverse connection detection range (450-500mV) are on the same
side of the interrupt pin TTL level.

So if we accidentally sample the ADC at the wrong time (either on
connection, or after a side-band wake signal from base), we may
fall into a trap where we assume that the base is connected in
reverse, while the value may just be a transient.

We have seen this case in the field, due to side-band wake pulse
from base EC, but this could potentially happen on attach as well.

The code debounces the reverse detection signal a little longer
(this has no impact as the base is not functional anyway), so that
we only consider that the base is connected in reverse if the ADC
value is within the range twice in a row.

BRANCH=poppy
BUG=b:77828249
TEST=With HACK CL to emulate bug:
 - Press a few keys, hammer gets disconnected
 - Connect/disconnect hammer a few times, sometimes it does not
   get disconnected.
TEST=Base connection, normal and reverse, still works properly.

Change-Id: I2ccb911472dd591146e3b0e68400c8bd55368dba
Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1010044
Reviewed-by: Aseda Aboagye <aaboagye@chromium.org>
(cherry picked from commit cdc9d417612275cdcc3b62f47c0a2cc6155b8c41)
Reviewed-on: https://chromium-review.googlesource.com/1013757
Reviewed-by: Furquan Shaikh <furquan@chromium.org>

1 files changed, 34 insertions, 12 deletions

diff --git a/board/poppy/base_detect_poppy.c b/board/poppy/base_detect_poppy.c
index 4a220af993..83f395f43c 100644
--- a/board/poppy/base_detect_poppy.c
+++ b/board/poppy/base_detect_poppy.c
@@ -71,6 +71,7 @@ enum base_status {
 	BASE_UNKNOWN = 0,
 	BASE_DISCONNECTED = 1,
 	BASE_CONNECTED = 2,
+	BASE_CONNECTED_REVERSE = 3,
 };
 
 static enum base_status current_base_status;
@@ -112,6 +113,7 @@ static void base_detect_deferred(void)
 	uint64_t time_now = get_time().val;
 	int v;
 	uint32_t tmp_pulse_width = pulse_width;
+	static int reverse_debounce = 1;
 
 	if (base_detect_debounce_time > time_now) {
 		hook_call_deferred(&base_detect_deferred_data,
@@ -123,29 +125,49 @@ static void base_detect_deferred(void)
 	if (v == ADC_READ_ERROR)
 		return;
 
+	print_base_detect_value(v, tmp_pulse_width);
+
+	if (v >= BASE_DETECT_REVERSE_MIN_MV &&
+	    v <= BASE_DETECT_REVERSE_MAX_MV) {
+		/*
+		 * If we are unlucky when we sample the ADC, we may think that
+		 * the base is connected in reverse, while this may just be a
+		 * transient. Force debouncing a little longer in that case.
+		 */
+		if (current_base_status == BASE_CONNECTED_REVERSE)
+			return;
+
+		if (reverse_debounce == 0) {
+			base_detect_change(BASE_CONNECTED_REVERSE);
+			return;
+		}
+
+		reverse_debounce = 0;
+		hook_call_deferred(&base_detect_deferred_data,
+				   BASE_DETECT_DEBOUNCE_US);
+		return;
+	}
+	/* Reset reverse debounce */
+	reverse_debounce = 1;
+
 	if (v >= BASE_DETECT_MIN_MV && v <= BASE_DETECT_MAX_MV) {
 		if (current_base_status != BASE_CONNECTED) {
-			print_base_detect_value(v, tmp_pulse_width);
 			base_detect_change(BASE_CONNECTED);
 		} else if (tmp_pulse_width >= BASE_DETECT_PULSE_MIN_US &&
 			   tmp_pulse_width <= BASE_DETECT_PULSE_MAX_US) {
-			print_base_detect_value(v, tmp_pulse_width);
 			CPRINTS("Sending event to AP");
 			host_set_single_event(EC_HOST_EVENT_KEY_PRESSED);
 		}
-	} else if ((v >= BASE_DETECT_REVERSE_MIN_MV &&
-		    v <= BASE_DETECT_REVERSE_MAX_MV) ||
-		   v >= BASE_DETECT_DISCONNECT_MIN_MV) {
-		/* TODO(b/35585396): Handle reverse connection separately. */
-
-		print_base_detect_value(v, tmp_pulse_width);
+		return;
+	}
 
+	if (v >= BASE_DETECT_DISCONNECT_MIN_MV) {
 		base_detect_change(BASE_DISCONNECTED);
-	} else {
-		/* Unclear base status, schedule again in a while. */
-		hook_call_deferred(&base_detect_deferred_data,
-				   BASE_DETECT_RETRY_US);
+		return;
 	}
+
+	/* Unclear base status, schedule again in a while. */
+	hook_call_deferred(&base_detect_deferred_data, BASE_DETECT_RETRY_US);
 }
 
 static inline int detect_pin_connected(enum gpio_signal det_pin)