cr50: Refactor Rdd state machine

The code to mirror Rdd detect into CCD_MODE_L and handle keepalive is
now inside chip/g/rdd.c It uses a HOOK_SECOND state machine similar to
what's coming for EC/AP/Servo.

This also removes the explicit 'ccd enable' / 'ccd disable' commands,
since they'd be overridden by the HOOK_SECOND handler.  If you need to
force CCD enabled, use 'ccd keepalive enable'.

BUG=b:64799106
BRANCH=cr50
TEST=With a CR50_DEV=1 images:
	Disconnect CCD cable (pull RDCC1 and RDCC2 outside 0.2-2.0V)
	gpioget --> CCD_MODE_L = 1
	ccd --> CCD disabled

	Connect CCD cable --> see 'Debug accessory connected'
	gpioget --> CCD_MODE_L = 0
	ccd --> CCD enabled

	Briefly disconnect and reconnect CCD cable --> No debug output
	gpioget --> CCD_MODE_L = 0
	ccd --> CCD enabled

	Disconnect CCD cable and wait a second --> 'disconnected'
	gpioget --> CCD_MODE_L = 1
	ccd --> CCD disabled

	Force CCD_MODE_L = 0 externally, wait a second
	gpioget --> CCD_MODE_L = 0
	ccd --> CCD enabled

	Stop forcing CCD_MODE_L externally, wait a second
	gpioget --> CCD_MODE_L = 1
	ccd --> CCD disabled

	ccd keepalive enable
	gpioget --> CCD_MODE_L = 0
	ccd --> CCD enabled

	ccd keepalive disable
	gpioget --> CCD_MODE_L = 1
	ccd --> CCD disabled

Change-Id: I65110b45e76f60390828e0fbbac8f36fc2cc9b37
Signed-off-by: Randall Spangler <rspangler@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/619393
Reviewed-by: Mary Ruthven <mruthven@chromium.org>

8 files changed, 246 insertions, 96 deletions

diff --git a/board/cr50/board.c b/board/cr50/board.c
index c70fbd5268..d00a007536 100644
--- a/board/cr50/board.c
+++ b/board/cr50/board.c
@@ -709,6 +709,7 @@ static void board_init(void)
 	 * used for battery cutoff software support on detachable devices.
 	 */
 	init_ac_detect();
+	init_rdd_state();
 
 	/*
 	 * The interrupt is enabled by default, but we only want it enabled when
@@ -1101,15 +1102,18 @@ DECLARE_DEFERRED(ec_deferred);
 /* Note: this must EXACTLY match enum device_type! */
 struct device_config device_states[] = {
 	[DEVICE_SERVO] = {
+		.state = DEVICE_STATE_UNKNOWN,
 		.deferred = &servo_deferred_data,
 		.detect = GPIO_DETECT_SERVO,
 		.name = "Servo"
 	},
 	[DEVICE_AP] = {
+		.state = DEVICE_STATE_UNKNOWN,
 		.deferred = &ap_deferred_data,
 		.name = "AP"
 	},
 	[DEVICE_EC] = {
+		.state = DEVICE_STATE_UNKNOWN,
 		.deferred = &ec_deferred_data,
 		.detect = GPIO_DETECT_EC,
 		.name = "EC"
diff --git a/board/cr50/board.h b/board/cr50/board.h
index 27faa9341a..b851f98209 100644
--- a/board/cr50/board.h
+++ b/board/cr50/board.h
@@ -180,6 +180,43 @@ enum device_type {
 	DEVICE_COUNT
 };
 
+/*
+ * Device states
+ *
+ * Note that not all states are used by all devices.
+ */
+enum device_state {
+	/* Initial state at boot */
+	DEVICE_STATE_INIT = 0,
+
+	/*
+	 * Detect was not asserted at boot, but we're not willing to give up on
+	 * the device right away so we're debouncing to see if it shows up.
+	 */
+	DEVICE_STATE_INIT_DEBOUNCING,
+
+	/* Disconnected or off, because detect is deasserted */
+	DEVICE_STATE_DISCONNECTED,
+	DEVICE_STATE_OFF,
+
+	/* Device state is not knowable because we're driving detect */
+	DEVICE_STATE_UNDETECTABLE,
+
+	/* Connected or on, because detect is asserted */
+	DEVICE_STATE_CONNECTED,
+	DEVICE_STATE_ON,
+
+	/*
+	 * Device was connected, but we saw detect deasserted and are
+	 * debouncing to see if it stays deasserted - at which point we'll
+	 * decide that it's disconnected.
+	 */
+	DEVICE_STATE_DEBOUNCING,
+
+	/* Device state is unknown.  Used only by legacy device_state code. */
+	DEVICE_STATE_UNKNOWN,
+};
+
 /* NVMem variables. */
 enum nvmem_vars {
 	NVMEM_VAR_CONSOLE_LOCKED = 0,
diff --git a/board/cr50/gpio.inc b/board/cr50/gpio.inc
index de46005af5..1be250e379 100644
--- a/board/cr50/gpio.inc
+++ b/board/cr50/gpio.inc
@@ -141,7 +141,13 @@ PINMUX(GPIO(EN_PP3300_INA_L), B7, DIO_INPUT)
  * only change it to an output when we want to assert the signal.
  */
 PINMUX(GPIO(SYS_RST_L_OUT),   M0, DIO_INPUT)
-PINMUX(GPIO(CCD_MODE_L),      M1, DIO_INPUT)
+/*
+ * CCD_MODE_L is an input above, but we need to be able to drive it as
+ * an output if Rdd detects the debug cable.  So set DIO_OUTPUT as
+ * well.  It will be tristated initially, because we don't set
+ * GPIO_OUTPUT above.
+ */
+PINMUX(GPIO(CCD_MODE_L),      M1, DIO_INPUT | DIO_OUTPUT)
 PINMUX(GPIO(BATT_PRES_L),     M2, 0)
 PINMUX(GPIO(I2C_SCL_INA),     B0, DIO_INPUT)
 PINMUX(GPIO(I2C_SDA_INA),     B1, DIO_INPUT)
diff --git a/board/cr50/rdd.c b/board/cr50/rdd.c
index 75c8a202ce..074ffac858 100644
--- a/board/cr50/rdd.c
+++ b/board/cr50/rdd.c
@@ -19,8 +19,6 @@
 
 #define CPRINTS(format, args...) cprints(CC_USB, format, ## args)
 
-static int keep_ccd_enabled;
-
 struct uart_config {
 	const char *name;
 	enum device_type device;
@@ -127,38 +125,11 @@ static void configure_ccd(int enable)
 	CPRINTS("CCD is now %sabled.", enable ? "en" : "dis");
 }
 
-void rdd_attached(void)
-{
-	/* Change CCD_MODE_L to an output which follows the internal GPIO. */
-	GWRITE(PINMUX, DIOM1_SEL, GC_PINMUX_GPIO0_GPIO5_SEL);
-	/* Indicate case-closed debug mode (active low) */
-	gpio_set_flags(GPIO_CCD_MODE_L, GPIO_OUT_LOW);
-}
-
-void rdd_detached(void)
-{
-	/*
-	 * Done with case-closed debug mode, therefore re-setup the CCD_MODE_L
-	 * pin as an input only if CCD mode isn't being forced enabled.
-	 *
-	 * NOTE: A pull up is required on this pin, however it was already
-	 * configured during the set up of the pinmux in gpio_pre_init().  The
-	 * chip-specific GPIO module will ignore any pull up/down configuration
-	 * anyways.
-	 */
-	if (!keep_ccd_enabled)
-		gpio_set_flags(GPIO_CCD_MODE_L, GPIO_INPUT);
-}
-
 static void rdd_check_pin(void)
 {
 	/* The CCD mode pin is active low. */
 	int enable = !gpio_get_level(GPIO_CCD_MODE_L);
 
-	/* Keep CCD enabled if it's being forced enabled. */
-	if (keep_ccd_enabled)
-		enable = 1;
-
 	if (enable == rdd_is_connected())
 		return;
 
@@ -190,12 +161,6 @@ static void rdd_ccd_change_hook(void)
 }
 DECLARE_HOOK(HOOK_CCD_CHANGE, rdd_ccd_change_hook, HOOK_PRIO_DEFAULT);
 
-static void clear_keepalive(void)
-{
-	keep_ccd_enabled = 0;
-	ccprintf("Cleared CCD keepalive\n");
-}
-
 static int command_ccd(int argc, char **argv)
 {
 	int val;
@@ -215,34 +180,19 @@ static int command_ccd(int argc, char **argv)
 			else
 				usb_i2c_board_disable();
 		} else if (!strcasecmp("keepalive", argv[1])) {
+			force_rdd_detect(val);
 			if (val) {
-				/* Make sure ccd is enabled */
-				if (!rdd_is_connected())
-					rdd_attached();
-
-				keep_ccd_enabled = 1;
 				ccprintf("Warning CCD will remain "
 					 "enabled until it is "
 					 "explicitly disabled.\n");
-			} else {
-				clear_keepalive();
-			}
-		} else if (argc == 2) {
-			if (val) {
-				rdd_attached();
-			} else {
-				if (keep_ccd_enabled)
-					clear_keepalive();
-
-				rdd_detached();
 			}
 		} else
 			return EC_ERROR_PARAM1;
 	}
 
 	ccprintf("CCD:     %s\n",
-		keep_ccd_enabled ? "forced enable" :
-		rdd_is_connected() ? "enabled" : "disabled");
+		 rdd_detect_is_forced() ? "forced enable" :
+		 rdd_is_connected() ? "enabled" : "disabled");
 	ccprintf("AP UART: %s\n",
 		 uartn_is_enabled(UART_AP) ?
 		 uart_tx_is_connected(UART_AP) ? "RX+TX" : "RX" : "disabled");
diff --git a/board/mn50/board.c b/board/mn50/board.c
index 39746a1745..bc04a459db 100644
--- a/board/mn50/board.c
+++ b/board/mn50/board.c
@@ -10,7 +10,6 @@
 #include "common.h"
 #include "console.h"
 #include "dcrypto/dcrypto.h"
-#include "device_state.h"
 #include "ec_version.h"
 #include "extension.h"
 #include "flash.h"
diff --git a/chip/g/rdd.c b/chip/g/rdd.c
index bd0d2360d8..30ea671238 100644
--- a/chip/g/rdd.c
+++ b/chip/g/rdd.c
@@ -5,6 +5,7 @@
 
 #include "clock.h"
 #include "console.h"
+#include "gpio.h"
 #include "hooks.h"
 #include "rdd.h"
 #include "registers.h"
@@ -21,9 +22,26 @@
  * debug accessory.
  */
 #define DETECT_DEBUG 0x420
+
+/*
+ * The interrupt only triggers when the debug state is detected.  If we want to
+ * trigger an interrupt when the debug state is *not* detected, we need to
+ * program the bit-inverse.
+ */
 #define DETECT_DISCONNECT (~DETECT_DEBUG & 0xffff)
 
-int debug_cable_is_attached(void)
+/* State of RDD CC detection */
+static enum device_state state = DEVICE_STATE_DISCONNECTED;
+
+/* Force detecting a debug accessory (ignore RDD CC detect hardware) */
+static int force_detected;
+
+/**
+ * Get instantaneous cable detect state
+ *
+ * @return 1 if debug accessory is detected, 0 if not detected
+ */
+static int rdd_is_detected(void)
 {
 	uint8_t cc1 = GREAD_FIELD(RDD, INPUT_PIN_VALUES, CC1);
 	uint8_t cc2 = GREAD_FIELD(RDD, INPUT_PIN_VALUES, CC2);
@@ -31,58 +49,186 @@ int debug_cable_is_attached(void)
 	return (cc1 == cc2 && (cc1 == 3 || cc1 == 1));
 }
 
-static void rdd_disconnected(void)
+/**
+ * Handle debug accessory disconnecting
+ */
+static void rdd_disconnect(void)
 {
-	CPRINTS("Debug Accessory disconnected");
-
-	rdd_detached();
+	CPRINTS("Debug accessory disconnect");
+	state = DEVICE_STATE_DISCONNECTED;
+
+	/*
+	 * Stop pulling CCD_MODE_L low.  The internal pullup configured in the
+	 * pinmux will pull the signal back high, unless the EC is also pulling
+	 * it low.
+	 *
+	 * This disables the SBUx muxes, if we were the only one driving
+	 * CCD_MODE_L.
+	 */
+	gpio_set_flags(GPIO_CCD_MODE_L, GPIO_INPUT);
 }
-DECLARE_DEFERRED(rdd_disconnected);
 
-void rdd_interrupt(void)
+/**
+ * Handle debug accessory connecting
+ *
+ * This can be deferred from both rdd_detect() and the interrupt handler, so
+ * it needs to check the current state to determine whether we're already
+ * connected.
+ */
+static void rdd_connect(void)
 {
-	delay_sleep_by(1 * SECOND);
+	/* If we were debouncing, we're done, and still connected */
+	if (state == DEVICE_STATE_DEBOUNCING)
+		state = DEVICE_STATE_CONNECTED;
 
-	if (debug_cable_is_attached()) {
-		/* cancel pending rdd disconnect */
-		hook_call_deferred(&rdd_disconnected_data, -1);
+	/* If we're already connected, done */
+	if (state == DEVICE_STATE_CONNECTED)
+		return;
 
-		CPRINTS("Debug Accessory connected");
+	/* We were previously disconnected, so connect */
+	CPRINTS("Debug accessory connect");
+	state = DEVICE_STATE_CONNECTED;
 
-		/* Detect when debug cable is disconnected */
+	/* Start pulling CCD_MODE_L low to enable the SBUx muxes */
+	gpio_set_flags(GPIO_CCD_MODE_L, GPIO_OUT_LOW);
+}
+DECLARE_DEFERRED(rdd_connect);
+
+/**
+ * Debug accessory detect interrupt
+ */
+static void rdd_interrupt(void)
+{
+	/*
+	 * The Rdd detector is level-sensitive with debounce.  That is, it
+	 * samples the RDCCx pin states.  If they're different, it resets the
+	 * wait counter.  If they're the same, it decrements the wait counter.
+	 * Then if the counter is zero, and the state we're looking for matches
+	 * the map, it fires the interrupt.
+	 *
+	 * Note that the counter *remains* zero until the pin states change.
+	 *
+	 * If we want to be able to wake on Rdd change, then interrupts need to
+	 * remain enabled.  Each time we get an interrupt, we'll toggle the map
+	 * we're looking for to the opposite state.  That stops the interrupt
+	 * from continuing to fire on the current state.  When the pins settle
+	 * into a new state, we'll fire the interrupt again.
+	 *
+	 * Even with that, we can still get a double interrupt now and then,
+	 * because the Rdd module runs on a different clock than we do.  So the
+	 * write we do to change the state map may not be picked up until the
+	 * next clock, when the Rdd module has already generated its next
+	 * interrupt based on the old map.  This is harmless, because we're
+	 * unlikely to actually trigger the deferred function twice, and it
+	 * doesn't care if we do anyway because on the second call it'll
+	 * already be in the connected state.
+	 *
+	 */
+	if (rdd_is_detected()) {
+		/* Accessory detected; toggle to looking for disconnect */
 		GWRITE(RDD, PROG_DEBUG_STATE_MAP, DETECT_DISCONNECT);
 
-		rdd_attached();
+		/*
+		 * Trigger the deferred handler so that we move back into the
+		 * connected state before our debounce interval expires.
+		 */
+		hook_call_deferred(&rdd_connect_data, 0);
 	} else {
-		/* Detect when debug cable is connected */
-		GWRITE(RDD, PROG_DEBUG_STATE_MAP, DETECT_DEBUG);
-
 		/*
-		 * Debounce the RDD disconnect for 2 seconds so rdd events
-		 * won't be triggered by any PD negotiation the EC does during
-		 * reset or sysjump.
+		 * Not detected; toggle to looking for connect.  We'll start
+		 * debouncing disconnect the next time HOOK_SECOND triggers
+		 * rdd_detect() below.
 		 */
-		hook_call_deferred(&rdd_disconnected_data, 2 * SECOND);
+		GWRITE(RDD, PROG_DEBUG_STATE_MAP, DETECT_DEBUG);
 	}
 
-	/* Clear interrupt */
+	/* Make sure we stay awake long enough to advance the state machine */
+	delay_sleep_by(1 * SECOND);
+
+	/* Clear the interrupt */
 	GWRITE_FIELD(RDD, INT_STATE, INTR_DEBUG_STATE_DETECTED, 1);
 }
 DECLARE_IRQ(GC_IRQNUM_RDD0_INTR_DEBUG_STATE_DETECTED_INT, rdd_interrupt, 1);
 
-void rdd_init(void)
+/**
+ * RDD CC detect state machine
+ */
+static void rdd_detect(void)
+{
+	/* Handle detecting device */
+	if (force_detected || rdd_is_detected()) {
+		rdd_connect();
+		return;
+	}
+
+	/* CC wasn't detected.  If we're already disconnected, done. */
+	if (state == DEVICE_STATE_DISCONNECTED)
+		return;
+
+	/* If we were debouncing, we're now sure we're disconnected */
+	if (state == DEVICE_STATE_DEBOUNCING) {
+		rdd_disconnect();
+		return;
+	}
+
+	/*
+	 * Otherwise, we were connected but the accessory seems to be
+	 * disconnected right now.  PD negotiation (e.g. during EC reset or
+	 * sysjump) can alter the RDCCx voltages, so we need to debounce this
+	 * signal for longer than the Rdd hardware does to make sure it's
+	 * really disconnected before we deassert CCD_MODE_L.
+	 */
+	state = DEVICE_STATE_DEBOUNCING;
+}
+/*
+ * Bump up priority so this runs before the CCD_MODE_L state machine, because
+ * we can change CCD_MODE_L.
+ */
+DECLARE_HOOK(HOOK_SECOND, rdd_detect, HOOK_PRIO_DEFAULT - 1);
+
+void init_rdd_state(void)
 {
-	/* Enable RDD */
+	/* Enable RDD hardware */
 	clock_enable_module(MODULE_RDD, 1);
 	GWRITE(RDD, POWER_DOWN_B, 1);
 
-	GWRITE(RDD, PROG_DEBUG_STATE_MAP, DETECT_DEBUG);
+	/*
+	 * Note that there is currently (ha, see what I did there) a leakage
+	 * path out of Cr50 into the CC lines.  On some systems, this can cause
+	 * false Rdd detection when the TCPCs are turned off.  This may require
+	 * a software workaround where RDD hardware must be powered down
+	 * whenever the TCPCs are off, and can only be powered up for brief
+	 * periods to do a quick check.  See b/38019839 and b/64582597.
+	 */
 
-	/* Initialize the debug state based on the current cc values */
-	rdd_interrupt();
+	/* Configure to detect accessory connected */
+	GWRITE(RDD, PROG_DEBUG_STATE_MAP, DETECT_DEBUG);
 
-	/* Enable RDD interrupts */
+	/*
+	 * Enable interrupt for detecting CC.  This minimizes the time before
+	 * we transition to cable-detected at boot, and will cause us to wake
+	 * from deep sleep if a cable is plugged in.
+	 */
 	task_enable_irq(GC_IRQNUM_RDD0_INTR_DEBUG_STATE_DETECTED_INT);
+	GWRITE_FIELD(RDD, INT_STATE, INTR_DEBUG_STATE_DETECTED, 1);
 	GWRITE_FIELD(RDD, INT_ENABLE, INTR_DEBUG_STATE_DETECTED, 1);
 }
-DECLARE_HOOK(HOOK_INIT, rdd_init, HOOK_PRIO_DEFAULT);
+
+void force_rdd_detect(int enable)
+{
+	force_detected = enable;
+
+	/*
+	 * If we're forcing detection, trigger then connect handler early.
+	 *
+	 * Otherwise, we'll revert to the normal logic of checking the RDD
+	 * hardware CC state.
+	 */
+	if (force_detected)
+		hook_call_deferred(&rdd_connect_data, 0);
+}
+
+int rdd_detect_is_forced(void)
+{
+	return force_detected;
+}
diff --git a/chip/g/rdd.h b/chip/g/rdd.h
index 7fd5549ec5..0da5cbef0e 100644
--- a/chip/g/rdd.h
+++ b/chip/g/rdd.h
@@ -6,10 +6,26 @@
 #ifndef __CROS_RDD_H
 #define __CROS_RDD_H
 
-/* Detach from debug cable */
-void rdd_detached(void);
+/**
+ * Initialize RDD module
+ */
+void init_rdd_state(void);
+
+/**
+ * Enable/disable forcing debug accessory detection.
+ *
+ * When enabled, the RDD module will assert CCD_MODE_L even if the CC value
+ * does not indicate a debug accessory is present.
+ *
+ * @param enable	Enable (1) or disable (0) keepalive.
+ */
+void force_rdd_detect(int enable);
 
-/* Attach to debug cable */
-void rdd_attached(void);
+/**
+ * Check if debug accessory detection is forced.
+ *
+ * @return 1 if keepalive is enabled, 0 if disabled.
+ */
+int rdd_detect_is_forced(void);
 
 #endif  /* __CROS_RDD_H */
diff --git a/include/device_state.h b/include/device_state.h
index 7d22846d9e..e7894ba998 100644
--- a/include/device_state.h
+++ b/include/device_state.h
@@ -6,14 +6,6 @@
 #ifndef __CROS_DEVICE_STATE_H
 #define __CROS_DEVICE_STATE_H
 
-/* Device state indexes */
-enum device_state {
-	DEVICE_STATE_UNKNOWN = 0,
-	DEVICE_STATE_OFF,
-	DEVICE_STATE_ON,
-	DEVICE_STATE_COUNT,
-};
-
 enum gpio_signal;
 
 /* Device configuration */