pd: implement TCPC RX Detect register to enable/disable TCPC RX

Implement the TCPC RX Detect register and use it for the TCPM to
enable and disable PD communication. When no type-C connection,
disable TCPC RX so that we don't send goodCRC when we are not
ready. Once TCPM establishes a type-C connection, enable TCPC RX.

BUG=none
BRANCH=none
TEST=tested on glados and on samus. On glados, without this change,
sometimes when you plug in zinger, we get into a hard reset loop
because TCPC is sending goodCRC to source cap while TCPM is still
debouncing CC and is not ready. With this change, we reliably form
PD contract.

Also tested enabling and disabling PD comms from the TCPM console
with "pd enable 0|1".

Change-Id: I8c9e696f2597978436f6ceccfe06ffb021c95ea3
Signed-off-by: Alec Berg <alecaberg@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/274811
Reviewed-by: Vincent Palatin <vpalatin@chromium.org>

5 files changed, 86 insertions, 44 deletions

diff --git a/common/usb_pd_protocol.c b/common/usb_pd_protocol.c
index 0b86991349..b1e8a103c1 100644
--- a/common/usb_pd_protocol.c
+++ b/common/usb_pd_protocol.c
@@ -251,6 +251,8 @@ static inline void set_state(int port, enum pd_states next_state)
 #ifdef CONFIG_USBC_VCONN
 		tcpm_set_vconn(port, 0);
 #endif
+		/* Disable TCPC RX */
+		tcpm_set_rx_enable(port, 0);
 	}
 
 #ifdef CONFIG_LOW_POWER_IDLE
@@ -288,6 +290,10 @@ static int pd_transmit(int port, enum tcpm_transmit_type type,
 {
 	int evt;
 
+	/* If comms are disabled, do not transmit, return error */
+	if (!pd_comm_enabled)
+		return -1;
+
 	tcpm_transmit(port, type, header, data);
 
 	/* Wait until TX is complete */
@@ -1198,23 +1204,26 @@ int pd_get_partner_data_swap_capable(int port)
 #ifdef CONFIG_COMMON_RUNTIME
 void pd_comm_enable(int enable)
 {
+	int i;
+
 	pd_comm_enabled = enable;
+
+	for (i = 0; i < CONFIG_USB_PD_PORT_COUNT; i++) {
+		/* If type-C connection, then update the TCPC RX enable */
+		if (pd_is_connected(i))
+			tcpm_set_rx_enable(i, enable);
+
 #ifdef CONFIG_USB_PD_DUAL_ROLE
-	/*
-	 * If communications are enabled, start hard reset timer for
-	 * any port in PD_SNK_DISCOVERY.
-	 */
-	if (enable) {
-		int i;
-		for (i = 0; i < CONFIG_USB_PD_PORT_COUNT; i++) {
-			if (pd[i].task_state == PD_STATE_SNK_DISCOVERY)
-				set_state_timeout(i,
-						  get_time().val +
-						  PD_T_SINK_WAIT_CAP,
-						  PD_STATE_HARD_RESET_SEND);
-		}
-	}
+		/*
+		 * If communications are enabled, start hard reset timer for
+		 * any port in PD_SNK_DISCOVERY.
+		 */
+		if (enable && pd[i].task_state == PD_STATE_SNK_DISCOVERY)
+			set_state_timeout(i,
+					  get_time().val + PD_T_SINK_WAIT_CAP,
+					  PD_STATE_HARD_RESET_SEND);
 #endif
+	}
 }
 #endif
 
@@ -1296,6 +1305,9 @@ void pd_task(void)
 	tcpc_init(port);
 #endif
 
+	/* Disable TCPC RX until connection is established */
+	tcpm_set_rx_enable(port, 0);
+
 	/* Initialize PD protocol state variables for each port. */
 	pd[port].power_role = PD_ROLE_DEFAULT;
 	pd[port].vdm_state = VDM_STATE_DONE;
@@ -1442,6 +1454,9 @@ void pd_task(void)
 					break;
 				}
 #endif
+				/* If PD comm is enabled, enable TCPC RX */
+				if (pd_comm_enabled)
+					tcpm_set_rx_enable(port, 1);
 
 #ifdef CONFIG_USBC_VCONN
 				tcpm_set_vconn(port, 1);
@@ -1855,6 +1870,10 @@ void pd_task(void)
 			typec_set_input_current_limit(
 				port, typec_curr, TYPE_C_VOLTAGE);
 #endif
+			/* If PD comm is enabled, enable TCPC RX */
+			if (pd_comm_enabled)
+				tcpm_set_rx_enable(port, 1);
+
 			/*
 			 * fake set data role swapped flag so we send
 			 * discover identity when we enter SRC_READY
diff --git a/common/usb_pd_tcpc.c b/common/usb_pd_tcpc.c
index bd99bebcec..fa740ba4c4 100644
--- a/common/usb_pd_tcpc.c
+++ b/common/usb_pd_tcpc.c
@@ -34,19 +34,10 @@
  */
 static int debug_level;
 
-/*
- * TODO: disable in RO? can we remove enable var from protocol layer?
- * do we need to send a hard reset when we transition to enabled because
- * source could have given up sending source cap and may need hard reset
- * in order to establish a contract.
- */
-static uint8_t pd_comm_enabled = 1;
-
 static struct mutex pd_crc_lock;
 #else
 #define CPRINTF(format, args...)
 static const int debug_level;
-static const int pd_comm_enabled = 1;
 #endif
 
 /* Encode 5 bits using Biphase Mark Coding */
@@ -216,6 +207,8 @@ static struct pd_port_controller {
 	uint8_t cc_status[2];
 	/* TCPC alert status */
 	uint8_t alert[2];
+	/* RX enabled */
+	uint8_t rx_enabled;
 
 	/* Last received */
 	int rx_head;
@@ -711,10 +704,8 @@ void tcpc_init(int port)
 	/* Initialize physical layer */
 	pd_hw_init(port, PD_ROLE_DEFAULT);
 
-	/* make sure PD monitoring is enabled to wake on PD RX */
-	if (pd_comm_enabled)
-		pd_rx_enable_monitoring(port);
-
+	/* make sure PD monitoring is disabled initially */
+	pd[port].rx_enabled = 0;
 }
 
 int tcpc_run(int port, int evt)
@@ -722,7 +713,7 @@ int tcpc_run(int port, int evt)
 	int cc, i, res;
 
 	/* incoming packet ? */
-	if (pd_rx_started(port) && pd_comm_enabled) {
+	if (pd_rx_started(port) && pd[port].rx_enabled) {
 		pd[port].rx_head = pd_analyze_rx(port,
 						 pd[port].rx_payload);
 		pd_rx_complete(port);
@@ -738,7 +729,7 @@ int tcpc_run(int port, int evt)
 	}
 
 	/* outgoing packet ? */
-	if ((evt & PD_EVENT_TX) && pd_comm_enabled) {
+	if ((evt & PD_EVENT_TX) && pd[port].rx_enabled) {
 		switch (pd[port].tx_type) {
 		case TRANSMIT_SOP:
 			res = send_validate_message(port,
@@ -786,7 +777,7 @@ int tcpc_run(int port, int evt)
 	}
 
 	/* make sure PD monitoring is enabled to wake on PD RX */
-	if (pd_comm_enabled)
+	if (pd[port].rx_enabled)
 		pd_rx_enable_monitoring(port);
 
 	/* TODO: adjust timeout based on how often to sample CC */
@@ -883,6 +874,16 @@ int tcpc_set_vconn(int port, int enable)
 	return EC_SUCCESS;
 }
 
+int tcpc_set_rx_enable(int port, int enable)
+{
+	pd[port].rx_enabled = enable;
+
+	if (!enable)
+		pd_rx_disable_monitoring(port);
+
+	return EC_SUCCESS;
+}
+
 int tcpc_transmit(int port, enum tcpm_transmit_type type, uint16_t header,
 		  const uint32_t *data)
 {
@@ -935,6 +936,10 @@ static void tcpc_i2c_write(int port, int reg, int len, uint8_t *payload)
 	case TCPC_REG_ALERT2:
 		/* TODO: clear alert status reg when writtent to */
 		break;
+	case TCPC_REG_RX_DETECT:
+		tcpc_set_rx_enable(port, payload[1] &
+					 TCPC_REG_RX_DETECT_SOP_HRST_MASK);
+		break;
 	case TCPC_REG_TX_HDR:
 		pd[port].tx_head = (payload[2] << 8) | payload[1];
 		break;
@@ -980,6 +985,10 @@ static int tcpc_i2c_read(int port, int reg, uint8_t *payload)
 	case TCPC_REG_ALERT2:
 		tcpc_alert_status(port, reg, payload);
 		return 1;
+	case TCPC_REG_RX_DETECT:
+		payload[0] = pd[port].rx_enabled ?
+				TCPC_REG_RX_DETECT_SOP_HRST_MASK : 0;
+		return 1;
 	case TCPC_REG_RX_BYTE_CNT:
 		payload[0] = 4*PD_HEADER_CNT(pd[port].rx_head);
 		return 1;
@@ -1067,18 +1076,6 @@ static int command_tcpc(int argc, char **argv)
 			debug_level = level;
 		}
 		return EC_SUCCESS;
-	} else if (!strcasecmp(argv[1], "enable")) {
-		int enable;
-
-		if (argc < 3)
-			return EC_ERROR_PARAM_COUNT;
-
-		enable = strtoi(argv[2], &e, 10);
-		if (*e)
-			return EC_ERROR_PARAM3;
-		pd_comm_enabled = enable;
-		ccprintf("Ports %s\n", enable ? "enabled" : "disabled");
-		return EC_SUCCESS;
 	}
 
 	/* command: pd <port> <subcmd> [args] */
@@ -1103,7 +1100,7 @@ static int command_tcpc(int argc, char **argv)
 	} else if (!strncasecmp(argv[2], "state", 5)) {
 		ccprintf("Port C%d, %s - CC:%d, CC0:%d, CC1:%d, "
 			 "Alert: 0x%02x 0x%02x\n", port,
-			 pd_comm_enabled ? "Ena" : "Dis",
+			 pd[port].rx_enabled ? "Ena" : "Dis",
 			 pd[port].cc_pull,
 			 pd[port].cc_status[0], pd[port].cc_status[1],
 			 pd[port].alert[0], pd[port].alert[1]);
@@ -1112,7 +1109,7 @@ static int command_tcpc(int argc, char **argv)
 	return EC_SUCCESS;
 }
 DECLARE_CONSOLE_COMMAND(tcpc, command_tcpc,
-			"dump|enable [0|1]\n\t<port> [clock|state]",
+			"dump [0|1]\n\t<port> [clock|state]",
 			"Type-C Port Controller",
 			NULL);
 #endif
diff --git a/common/usb_pd_tcpm.c b/common/usb_pd_tcpm.c
index b5b841c209..d24931e692 100644
--- a/common/usb_pd_tcpm.c
+++ b/common/usb_pd_tcpm.c
@@ -80,6 +80,14 @@ int tcpm_alert_status(int port, int alert_reg, uint8_t *alert)
 			 alert_reg, (int *)alert);
 }
 
+int tcpm_set_rx_enable(int port, int enable)
+{
+	/* If enable, then set RX detect for SOP and HRST */
+	return i2c_write8(I2C_PORT_TCPC, I2C_ADDR_TCPC(port),
+			  TCPC_REG_RX_DETECT,
+			  enable ? TCPC_REG_RX_DETECT_SOP_HRST_MASK : 0);
+}
+
 int tcpm_get_message(int port, uint32_t *payload, int *head)
 {
 	int rv, cnt, reg = TCPC_REG_RX_DATA;
diff --git a/common/usb_pd_tcpm_stub.c b/common/usb_pd_tcpm_stub.c
index 10882db920..bb8ba57045 100644
--- a/common/usb_pd_tcpm_stub.c
+++ b/common/usb_pd_tcpm_stub.c
@@ -14,6 +14,7 @@ extern int tcpc_set_cc(int port, int pull);
 extern int tcpc_set_polarity(int port, int polarity);
 extern int tcpc_set_vconn(int port, int enable);
 extern int tcpc_set_msg_header(int port, int power_role, int data_role);
+extern int tcpc_set_rx_enable(int port, int enable);
 
 extern int tcpc_get_message(int port, uint32_t *payload, int *head);
 extern int tcpc_transmit(int port, enum tcpm_transmit_type type,
@@ -49,6 +50,11 @@ int tcpm_alert_status(int port, int alert_reg, uint8_t *alert)
 	return tcpc_alert_status(port, alert_reg, alert);
 }
 
+int tcpm_set_rx_enable(int port, int enable)
+{
+	return tcpc_set_rx_enable(port, enable);
+}
+
 int tcpm_get_message(int port, uint32_t *payload, int *head)
 {
 	return tcpc_get_message(port, payload, head);
diff --git a/include/usb_pd_tcpm.h b/include/usb_pd_tcpm.h
index 47b60622f7..951de5572f 100644
--- a/include/usb_pd_tcpm.h
+++ b/include/usb_pd_tcpm.h
@@ -102,6 +102,8 @@ enum tcpc_cc_pull {
 #define TCPC_REG_RX_BYTE_CNT       0x2f
 #define TCPC_REG_RX_STATUS         0x30
 #define TCPC_REG_RX_DETECT         0x31
+#define TCPC_REG_RX_DETECT_SOP_HRST_MASK 0x21
+
 #define TCPC_REG_RX_HDR            0x32
 #define TCPC_REG_RX_DATA           0x34 /* through 0x4f */
 
@@ -211,6 +213,16 @@ int tcpm_set_vconn(int port, int enable);
 int tcpm_set_msg_header(int port, int power_role, int data_role);
 
 /**
+ * Set RX enable flag
+ *
+ * @param port Type-C port number
+ * @enable true for enable, false for disable
+ *
+ * @return EC_SUCCESS or error
+ */
+int tcpm_set_rx_enable(int port, int enable);
+
+/**
  * Read last received PD message.
  *
  * @param port Type-C port number