nct3807: potential connection cleanup

I changed TCPMv2 to call tcpm_set_new_connection
instead of tcpm_set_cc when connecting at the
parent state for a new connection type.  This
allows the NCT3807 to clear out DRP and set the
correct connection instead of clobbering what
the hardware determined to be correct and setting
it to an open listen.

BUG=b:149593609
BRANCH=none
TEST=verify USB-C

Change-Id: I7402d3417a14fdc4158636e4716ef7fbdf4fa4a3
Signed-off-by: Denis Brockus <dbrockus@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2064184
Commit-Queue: Edward Hill <ecgh@chromium.org>
Reviewed-by: Edward Hill <ecgh@chromium.org>

5 files changed, 66 insertions, 121 deletions

diff --git a/common/usbc/usb_tc_drp_acc_trysrc_sm.c b/common/usbc/usb_tc_drp_acc_trysrc_sm.c
index f21568fb39..2ad9bfa881 100644
--- a/common/usbc/usb_tc_drp_acc_trysrc_sm.c
+++ b/common/usbc/usb_tc_drp_acc_trysrc_sm.c
@@ -2804,23 +2804,9 @@ static void tc_drp_auto_toggle_run(const int port)
 		set_state_tc(port, PD_DEFAULT_STATE(port));
 		break;
 	case DRP_TC_UNATTACHED_SNK:
-		/*
-		 * Some TCPCI compliant TCPCs come out of auto toggle with
-		 * a prospective connection.  They are expecting us to set
-		 * the CC lines to what it is thinking is best or it goes
-		 * directly back to unattached.
-		 */
-		tcpm_drp_toggle_connection(port, cc1, cc2);
 		set_state_tc(port, TC_ATTACH_WAIT_SNK);
 		break;
 	case DRP_TC_UNATTACHED_SRC:
-		/*
-		 * Some TCPCI compliant TCPCs come out of auto toggle with
-		 * a prospective connection.  They are expecting us to set
-		 * the CC lines to what it is thinking is best or it goes
-		 * directly back to unattached.
-		 */
-		tcpm_drp_toggle_connection(port, cc1, cc2);
 		set_state_tc(port, TC_ATTACH_WAIT_SRC);
 		break;
 	case DRP_TC_DRP_AUTO_TOGGLE:
@@ -3148,7 +3134,7 @@ static void tc_cc_rd_entry(const int port)
 	 * Both CC1 and CC2 pins shall be independently terminated to
 	 * ground through Rd.
 	 */
-	tcpm_set_cc(port, TYPEC_CC_RD);
+	tcpm_set_new_connection(port, TYPEC_CC_RD);
 }
 
 
@@ -3170,7 +3156,7 @@ static void tc_cc_rp_entry(const int port)
 	 * up through Rp.
 	 */
 	tcpm_select_rp_value(port, CONFIG_USB_PD_PULLUP);
-	tcpm_set_cc(port, TYPEC_CC_RP);
+	tcpm_set_new_connection(port, TYPEC_CC_RP);
 }
 
 /**
diff --git a/driver/tcpm/nct38xx.c b/driver/tcpm/nct38xx.c
index 0f27661371..59f5ad9c63 100644
--- a/driver/tcpm/nct38xx.c
+++ b/driver/tcpm/nct38xx.c
@@ -119,40 +119,32 @@ static void nct38xx_tcpc_alert(int port)
 }
 
 #ifdef CONFIG_USB_PD_DUAL_ROLE_AUTO_TOGGLE
-static void nct38xx_drp_toggle_connection(int port,
-	enum tcpc_cc_voltage_status cc1,
-	enum tcpc_cc_voltage_status cc2)
+static int nct38xx_set_new_connection(int port,
+	enum tcpc_cc_pull pull)
 {
 	int rv;
 	int role;
 
 	/* Get the ROLE CONTROL value */
 	rv = tcpc_read(port, TCPC_REG_ROLE_CTRL, &role);
-	if (rv) {
-		CPRINTS("C%d: %s failed to read ROLE",
-			port, __func__);
-		return;
-	}
+	if (rv)
+		return rv;
 
 	if (role & TCPC_REG_ROLE_CTRL_DRP_MASK) {
-		/* TODO(b/149593609) get an understanding from Nuvoton
-		 * why this is the way it works.
-		 *
+		/*
 		 * If DRP is set, the CC pins shall stay in
 		 * Potential_Connect_as_Src or Potential_Connect_as_Sink
 		 * until directed otherwise.
 		 *
-		 * From TCPCIr2 figure 4-20 DRP Connection Detection
-		 *     Determine CC & VCONN:
-		 *         Set RC.CC1 & RC.CC2 per potential connect decision
-		 *         Set RC.DRP=0
-		 *         Set TCPC_CONTROl.PlugOrientation
-		 *         Set PC.AutoDischargeDisconnect=1 & PC.EnableVconn
+		 * Set RC.CC1 & RC.CC2 per potential decision
+		 * Set RC.DRP=0
 		 */
-		int ctrl;
-		enum tcpc_cc_polarity polarity;
-		enum tcpc_cc_voltage_status cc1_pull, cc2_pull;
-		enum tcpc_rp_value rp = TYPEC_RP_USB;
+		enum tcpc_cc_pull cc1_pull, cc2_pull;
+		enum tcpc_cc_voltage_status cc1, cc2;
+
+		rv = nct38xx_tcpm_drv.get_cc(port, &cc1, &cc2);
+		if (rv)
+			return rv;
 
 		switch (cc1) {
 		case TYPEC_CC_VOLT_OPEN:
@@ -162,18 +154,15 @@ static void nct38xx_drp_toggle_connection(int port,
 			cc1_pull = TYPEC_CC_RA;
 			break;
 		case TYPEC_CC_VOLT_RD:
-			cc1_pull = TYPEC_CC_RD;
+			cc1_pull = TYPEC_CC_RP;
 			break;
 		case TYPEC_CC_VOLT_RP_DEF:
 		case TYPEC_CC_VOLT_RP_1_5:
 		case TYPEC_CC_VOLT_RP_3_0:
-			rp = cc1 - TYPEC_CC_VOLT_RP_DEF;
-			cc1_pull = TYPEC_CC_RP;
+			cc1_pull = TYPEC_CC_RD;
 			break;
 		default:
-			CPRINTS("C%d: %s Invalid CC1 Voltage presented",
-				port, __func__);
-			return;
+			return EC_ERROR_UNKNOWN;
 		}
 
 		switch (cc2) {
@@ -184,75 +173,44 @@ static void nct38xx_drp_toggle_connection(int port,
 			cc2_pull = TYPEC_CC_RA;
 			break;
 		case TYPEC_CC_VOLT_RD:
-			cc2_pull = TYPEC_CC_RD;
+			cc2_pull = TYPEC_CC_RP;
 			break;
 		case TYPEC_CC_VOLT_RP_DEF:
 		case TYPEC_CC_VOLT_RP_1_5:
 		case TYPEC_CC_VOLT_RP_3_0:
-			rp = cc2 - TYPEC_CC_VOLT_RP_DEF;
-			cc2_pull = TYPEC_CC_RP;
+			cc2_pull = TYPEC_CC_RD;
 			break;
 		default:
-			CPRINTS("C%d: %s Invalid CC2 Voltage presented",
-				port, __func__);
-			return;
+			return EC_ERROR_UNKNOWN;
 		}
 
 		/* Set the CC lines */
 		rv = tcpc_write(port, TCPC_REG_ROLE_CTRL,
 				TCPC_REG_ROLE_CTRL_SET(0,
-						rp, cc1_pull, cc2_pull));
-		if (rv) {
-			CPRINTS("C%d: %s failed to write ROLE",
-				port, __func__);
-			return;
-		}
-
-		/* Set the polarity */
-		if (cc_is_rp(cc1) || cc_is_rp(cc2))
-			polarity = get_snk_polarity(cc1, cc2);
-		else
-			polarity = get_src_polarity(cc1, cc2);
-		nct38xx_tcpm_drv.set_polarity(port, polarity);
-
-		/* Set/Clear auto discharge disconnect */
-		rv = tcpc_read(port, TCPC_REG_POWER_CTRL, &ctrl);
-		if (rv) {
-			CPRINTS("C%d: %s failed to read POWER_CTRL",
-				port, __func__);
-			return;
-		}
-
-		if (TCPC_REG_POWER_CTRL_VCONN(ctrl))
-			ctrl |= TCPC_REG_POWER_CTRL_AUTO_DISCHARGE_DISCONNECT;
-		else
-			ctrl &= ~TCPC_REG_POWER_CTRL_AUTO_DISCHARGE_DISCONNECT;
-
-		rv = tcpc_write(port,
-				TCPC_REG_POWER_CTRL,
-				ctrl);
-		if (rv) {
-			CPRINTS("C%d: %s failed to write POWER_CTRL",
-				port, __func__);
-			return;
-		}
+						CONFIG_USB_PD_PULLUP,
+						cc1_pull, cc2_pull));
+		if (rv)
+			return rv;
 	} else {
 		/*
-		 * We left auto-toggle and no longer have DRP set. This
-		 * would happen if DRP was turned off and we did not have
-		 * a connection.  We have to manually turn off that we
-		 * are looking for a connection.
+		 * DRP is not set. This would happen if DRP is not enabled or
+		 * was turned off and we did not have a connection.  We have
+		 * to manually turn off that we are looking for a connection
+		 * and set both CC lines to the pull value.
 		 */
 		rv = tcpc_update8(port,
 				  TCPC_REG_TCPC_CTRL,
 				  TCPC_REG_TCPC_CTRL_EN_LOOK4CONNECTION_ALERT,
 				  MASK_CLR);
-		if (rv) {
-			CPRINTS("C%d: %s failed to clear Look4Connection",
-				port, __func__);
-			return;
-		}
+		if (rv)
+			return rv;
+
+		/* Set the CC lines */
+		rv = nct38xx_tcpm_drv.set_cc(port, pull);
+		if (rv)
+			return rv;
 	}
+	return EC_SUCCESS;
 }
 #endif
 
@@ -279,7 +237,7 @@ const struct tcpm_drv nct38xx_tcpm_drv = {
 				  &tcpci_tcpc_enable_auto_discharge_disconnect,
 #ifdef CONFIG_USB_PD_DUAL_ROLE_AUTO_TOGGLE
 	.drp_toggle		= &tcpci_tcpc_drp_toggle,
-	.drp_toggle_connection	= &nct38xx_drp_toggle_connection,
+	.set_new_connection	= &nct38xx_set_new_connection,
 #endif
 #ifdef CONFIG_USBC_PPC
 	.set_snk_ctrl		= &tcpci_tcpm_set_snk_ctrl,
diff --git a/driver/tcpm/tcpci.c b/driver/tcpm/tcpci.c
index 2e1b535055..0935f338c2 100644
--- a/driver/tcpm/tcpci.c
+++ b/driver/tcpm/tcpci.c
@@ -359,17 +359,13 @@ int tcpci_tcpm_get_cc(int port, enum tcpc_cc_voltage_status *cc1,
 
 int tcpci_tcpm_set_cc(int port, int pull)
 {
-	int cc1, cc2;
-
-	cc1 = cc2 = pull;
-
 	/* Keep track of current CC pull value */
 	tcpci_set_cached_pull(port, pull);
 
 	return tcpc_write(port, TCPC_REG_ROLE_CTRL,
 			  TCPC_REG_ROLE_CTRL_SET(0,
 						 tcpci_get_cached_rp(port),
-						 cc1, cc2));
+						 pull, pull));
 }
 
 #ifdef CONFIG_USB_PD_DUAL_ROLE_AUTO_TOGGLE
diff --git a/driver/tcpm/tcpm.h b/driver/tcpm/tcpm.h
index 61a3a0f8e3..373fbf545f 100644
--- a/driver/tcpm/tcpm.h
+++ b/driver/tcpm/tcpm.h
@@ -178,6 +178,18 @@ static inline int tcpm_set_cc(int port, int pull)
 	return tcpc_config[port].drv->set_cc(port, pull);
 }
 
+static inline int tcpm_set_new_connection(int port,
+	enum tcpc_cc_pull pull)
+{
+	const struct tcpm_drv *tcpc = tcpc_config[port].drv;
+
+	if (IS_ENABLED(CONFIG_USB_PD_DUAL_ROLE_AUTO_TOGGLE) &&
+	    tcpc->set_new_connection)
+		return tcpc->set_new_connection(port, pull);
+	else
+		return tcpc->set_cc(port, pull);
+}
+
 static inline int tcpm_set_polarity(int port, enum tcpc_cc_polarity polarity)
 {
 	return tcpc_config[port].drv->set_polarity(port, polarity);
@@ -257,16 +269,6 @@ static inline int tcpm_enable_drp_toggle(int port)
 {
 	return tcpc_config[port].drv->drp_toggle(port);
 }
-
-static inline void tcpm_drp_toggle_connection(int port,
-	enum tcpc_cc_voltage_status cc1,
-	enum tcpc_cc_voltage_status cc2)
-{
-	const struct tcpm_drv *tcpc = tcpc_config[port].drv;
-
-	if (tcpc->drp_toggle_connection)
-		tcpc->drp_toggle_connection(port, cc1, cc2);
-}
 #endif
 
 #ifdef CONFIG_USB_PD_TCPC_LOW_POWER
diff --git a/include/usb_pd_tcpm.h b/include/usb_pd_tcpm.h
index 527388d73e..4589fc4839 100644
--- a/include/usb_pd_tcpm.h
+++ b/include/usb_pd_tcpm.h
@@ -319,29 +319,32 @@ struct tcpm_drv {
 	void (*tcpc_enable_auto_discharge_disconnect)(int port,
 						      int enable);
 
-#ifdef CONFIG_USB_PD_DUAL_ROLE_AUTO_TOGGLE
 	/**
-	 * Enable TCPC auto DRP toggling.
+	 * Set new connection
+	 * There is a new connection. May have to handle differently
+	 * if we were performing auto-toggle. Allow a driver to do
+	 * any work required to leave the unattached auto-toggle mode
+	 * as well as setting the CC lines.  If auto-toggle is not
+	 * being used or was not the cause of the new connection
+	 * detection then set both CC lines to the passed pull.
 	 *
 	 * @param port Type-C port number
+	 * @param pull enum tcpc_cc_pull of CC lines
 	 *
 	 * @return EC_SUCCESS or error
 	 */
-	int (*drp_toggle)(int port);
+	int (*set_new_connection)(int port,
+				enum tcpc_cc_pull pull);
 
+#ifdef CONFIG_USB_PD_DUAL_ROLE_AUTO_TOGGLE
 	/**
-	 * Auto Toggle Connection
-	 * There is a connection while performing auto-toggle.
-	 * Allow a driver to do any work required to leave the
-	 * unattached auto-toggle mode
+	 * Enable TCPC auto DRP toggling.
 	 *
 	 * @param port Type-C port number
-	 * @param cc1 enum tcpc_cc_pull of CC1
-	 * @param cc2 enum tcpc_cc_pull of CC2
+	 *
+	 * @return EC_SUCCESS or error
 	 */
-	void (*drp_toggle_connection)(int port,
-				enum tcpc_cc_voltage_status cc1,
-				enum tcpc_cc_voltage_status cc2);
+	int (*drp_toggle)(int port);
 #endif
 
 	/**