tcpmv2: cleanup auto discharge disconnect

This CL is based on the inability to boot trembyle when a
battery is not connected.  The failure looks like it is always
right after AutoDischargeDisconnect has been set.  I asked
Nuvoton if we could go back to using ForcedDischarge and
veer away from AutoDischargeDisconnect and I was told no
that it is now required.

My first step was to determine how close the existing
AutoDischargeDisconnect was to the spec and found enough
difference that I wanted to get that in line before
trying to pinpoint the issues causing this problem.

BUG=none
BRANCH=none
TEST=get tcpmv2 to work without a battery attached

Signed-off-by: Denis Brockus <dbrockus@google.com>
Change-Id: I44b86ae5dfcf6b547c742c1af0228a0ed8e3520d
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2105935
Tested-by: Denis Brockus <dbrockus@chromium.org>
Reviewed-by: Edward Hill <ecgh@chromium.org>
Reviewed-by: Keith Short <keithshort@chromium.org>
Commit-Queue: Denis Brockus <dbrockus@chromium.org>
Auto-Submit: Denis Brockus <dbrockus@chromium.org>

4 files changed, 169 insertions, 105 deletions

diff --git a/driver/tcpm/nct38xx.c b/driver/tcpm/nct38xx.c
index 59f5ad9c63..654ae6a999 100644
--- a/driver/tcpm/nct38xx.c
+++ b/driver/tcpm/nct38xx.c
@@ -29,8 +29,8 @@ static int nct38xx_tcpm_init(int port)
 	int reg;
 
 	rv = tcpci_tcpm_init(port);
-		if (rv)
-			return rv;
+	if (rv)
+		return rv;
 
 	/*
 	 * Write to the CONTROL_OUT_EN register to enable:
@@ -79,13 +79,15 @@ static int nct38xx_tcpm_init(int port)
 	/* Start VBus monitor */
 	rv = tcpc_write(port, TCPC_REG_COMMAND,
 			TCPC_REG_COMMAND_ENABLE_VBUS_DETECT);
+	if (rv)
+		return rv;
 
 	/*
 	 * Enable the Vendor Define alert event only when the IO expander
 	 * feature is defined
 	 */
 	if (IS_ENABLED(CONFIG_IO_EXPANDER_NCT38XX))
-		rv |= tcpc_update16(port,
+		rv = tcpc_update16(port,
 				    TCPC_REG_ALERT_MASK,
 				    TCPC_REG_ALERT_VENDOR_DEF,
 				    MASK_SET);
@@ -118,101 +120,26 @@ static void nct38xx_tcpc_alert(int port)
 
 }
 
-#ifdef CONFIG_USB_PD_DUAL_ROLE_AUTO_TOGGLE
-static int nct38xx_set_new_connection(int port,
-	enum tcpc_cc_pull pull)
+static __maybe_unused int nct3807_tcpc_drp_toggle(int port)
 {
 	int rv;
-	int role;
 
-	/* Get the ROLE CONTROL value */
-	rv = tcpc_read(port, TCPC_REG_ROLE_CTRL, &role);
+	/* DRP will already be set with the correct pull on both CC lines */
+
+	/* Set up to catch LOOK4CONNECTION alerts */
+	rv = tcpc_update8(port,
+			  TCPC_REG_TCPC_CTRL,
+			  TCPC_REG_TCPC_CTRL_EN_LOOK4CONNECTION_ALERT,
+			  MASK_SET);
 	if (rv)
 		return rv;
 
-	if (role & TCPC_REG_ROLE_CTRL_DRP_MASK) {
-		/*
-		 * If DRP is set, the CC pins shall stay in
-		 * Potential_Connect_as_Src or Potential_Connect_as_Sink
-		 * until directed otherwise.
-		 *
-		 * Set RC.CC1 & RC.CC2 per potential decision
-		 * Set RC.DRP=0
-		 */
-		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:
-			cc1_pull = TYPEC_CC_OPEN;
-			break;
-		case TYPEC_CC_VOLT_RA:
-			cc1_pull = TYPEC_CC_RA;
-			break;
-		case TYPEC_CC_VOLT_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:
-			cc1_pull = TYPEC_CC_RD;
-			break;
-		default:
-			return EC_ERROR_UNKNOWN;
-		}
-
-		switch (cc2) {
-		case TYPEC_CC_VOLT_OPEN:
-			cc2_pull = TYPEC_CC_OPEN;
-			break;
-		case TYPEC_CC_VOLT_RA:
-			cc2_pull = TYPEC_CC_RA;
-			break;
-		case TYPEC_CC_VOLT_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:
-			cc2_pull = TYPEC_CC_RD;
-			break;
-		default:
-			return EC_ERROR_UNKNOWN;
-		}
-
-		/* Set the CC lines */
-		rv = tcpc_write(port, TCPC_REG_ROLE_CTRL,
-				TCPC_REG_ROLE_CTRL_SET(0,
-						CONFIG_USB_PD_PULLUP,
-						cc1_pull, cc2_pull));
-		if (rv)
-			return rv;
-	} else {
-		/*
-		 * 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)
-			return rv;
+	/* Set Look4Connection command */
+	rv = tcpc_write(port, TCPC_REG_COMMAND,
+			TCPC_REG_COMMAND_LOOK4CONNECTION);
 
-		/* Set the CC lines */
-		rv = nct38xx_tcpm_drv.set_cc(port, pull);
-		if (rv)
-			return rv;
-	}
-	return EC_SUCCESS;
+	return rv;
 }
-#endif
 
 const struct tcpm_drv nct38xx_tcpm_drv = {
 	.init			= &nct38xx_tcpm_init,
@@ -236,8 +163,8 @@ const struct tcpm_drv nct38xx_tcpm_drv = {
 	.tcpc_enable_auto_discharge_disconnect =
 				  &tcpci_tcpc_enable_auto_discharge_disconnect,
 #ifdef CONFIG_USB_PD_DUAL_ROLE_AUTO_TOGGLE
-	.drp_toggle		= &tcpci_tcpc_drp_toggle,
-	.set_new_connection	= &nct38xx_set_new_connection,
+	.drp_toggle		= &nct3807_tcpc_drp_toggle,
+	.set_connection		= &tcpci_tcpc_set_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 dd64ebb157..95c8d9bb3e 100644
--- a/driver/tcpm/tcpci.c
+++ b/driver/tcpm/tcpci.c
@@ -16,6 +16,7 @@
 #include "tcpm.h"
 #include "timer.h"
 #include "usb_charge.h"
+#include "usb_common.h"
 #include "usb_mux.h"
 #include "usb_pd.h"
 #include "usb_pd_tcpc.h"
@@ -379,13 +380,147 @@ int tcpci_tcpc_drp_toggle(int port)
 {
 	int rv;
 
-	/* Set auto drp toggle */
+	/*
+	 * Set auto drp toggle
+	 * NOTE: This should be done according to the last connection
+	 * that we are disconnecting from. TCPCI Rev 2 spec figures
+	 * 4-21 and 4-22 show:
+	 *     SNK => DRP should set CC lines to Rd/Rd
+	 *     SRC => DRP should set CC lines to Rp/Rp
+	 * The function tcpci_tcpc_set_connection performs this action
+	 * and it may be wise as chips can use this to make this the
+	 * standard and remove this set_role_ctrl call.
+	 */
 	rv = tcpci_set_role_ctrl(port, 1, TYPEC_RP_USB, TYPEC_CC_RD);
+	if (rv)
+		return rv;
+
+	/* Set up to catch LOOK4CONNECTION alerts */
+	rv = tcpc_update8(port,
+			  TCPC_REG_TCPC_CTRL,
+			  TCPC_REG_TCPC_CTRL_EN_LOOK4CONNECTION_ALERT,
+			  MASK_SET);
+	if (rv)
+		return rv;
 
 	/* Set Look4Connection command */
-	rv |= tcpc_write(port, TCPC_REG_COMMAND,
-			 TCPC_REG_COMMAND_LOOK4CONNECTION);
+	rv = tcpc_write(port, TCPC_REG_COMMAND,
+			TCPC_REG_COMMAND_LOOK4CONNECTION);
+
+	return rv;
+}
+
+int tcpci_tcpc_set_connection(int port,
+			      enum tcpc_cc_pull pull,
+			      int connect)
+{
+	int rv;
+	int role;
+
+	/*
+	 * Disconnecting will set the following and then return
+	 *     Set RC.DRP=1b (DRP)
+	 *     Set RC.RpValue=00b (smallest Rp to save power)
+	 *     Set RC.CC1=pull (Rd or Rp)
+	 *     Set RC.CC2=pull (Rd or Rp)
+	 */
+	if (!connect) {
+		tcpci_set_role_ctrl(port, 1, TYPEC_RP_USB, pull);
+		return EC_SUCCESS;
+	}
+
+	/* Get the ROLE CONTROL value */
+	rv = tcpc_read(port, TCPC_REG_ROLE_CTRL, &role);
+	if (rv)
+		return rv;
+
+	if (role & TCPC_REG_ROLE_CTRL_DRP_MASK) {
+		enum tcpc_cc_pull cc1_pull, cc2_pull;
+		enum tcpc_cc_voltage_status cc1, cc2;
+
+		enum tcpc_cc_polarity polarity;
 
+		/*
+		 * If DRP is set, the CC pins shall stay in
+		 * Potential_Connect_as_Src or Potential_Connect_as_Sink
+		 * until directed otherwise.
+		 *
+		 * Set RC.CC1 & RC.CC2 per potential decision
+		 * Set RC.DRP=0
+		 */
+		rv = tcpm_get_cc(port, &cc1, &cc2);
+		if (rv)
+			return rv;
+
+		switch (cc1) {
+		case TYPEC_CC_VOLT_OPEN:
+			cc1_pull = TYPEC_CC_OPEN;
+			break;
+		case TYPEC_CC_VOLT_RA:
+			cc1_pull = TYPEC_CC_RA;
+			break;
+		case TYPEC_CC_VOLT_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:
+			cc1_pull = TYPEC_CC_RD;
+			break;
+		default:
+			return EC_ERROR_UNKNOWN;
+		}
+
+		switch (cc2) {
+		case TYPEC_CC_VOLT_OPEN:
+			cc2_pull = TYPEC_CC_OPEN;
+			break;
+		case TYPEC_CC_VOLT_RA:
+			cc2_pull = TYPEC_CC_RA;
+			break;
+		case TYPEC_CC_VOLT_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:
+			cc2_pull = TYPEC_CC_RD;
+			break;
+		default:
+			return EC_ERROR_UNKNOWN;
+		}
+
+		/* Set the CC lines */
+		rv = tcpc_write(port, TCPC_REG_ROLE_CTRL,
+				TCPC_REG_ROLE_CTRL_SET(0,
+						CONFIG_USB_PD_PULLUP,
+						cc1_pull, cc2_pull));
+		if (rv)
+			return rv;
+
+		/* Set TCPC_CONTROl.PlugOrientation */
+		if (pull == TYPEC_CC_RD)
+			polarity = polarity_rm_dts(
+					get_snk_polarity(cc1, cc2));
+		else
+			polarity = get_src_polarity(cc1, cc2);
+
+		rv = tcpc_update8(port, TCPC_REG_TCPC_CTRL,
+				  TCPC_REG_TCPC_CTRL_SET(1),
+				  (polarity == POLARITY_CC1) ? MASK_CLR
+							     : MASK_SET);
+	} else {
+		/*
+		 * 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);
+	}
 	return rv;
 }
 #endif
@@ -852,6 +987,7 @@ void tcpci_tcpc_alert(int port)
 		int fault;
 
 		if (tcpci_get_fault(port, &fault) == EC_SUCCESS &&
+		    fault != 0 &&
 		    tcpci_handle_fault(port, fault) == EC_SUCCESS &&
 		    tcpci_clear_fault(port, fault) == EC_SUCCESS)
 			CPRINTS("C%d FAULT 0x%02X handled", port, fault);
@@ -1030,7 +1166,6 @@ int tcpci_tcpm_init(int port)
 	int error;
 	int power_status;
 	int tries = TCPM_INIT_TRIES;
-	int regval;
 
 	/* Start with an unknown connection */
 	tcpci_set_cached_pull(port, TYPEC_CC_OPEN);
@@ -1058,9 +1193,9 @@ int tcpci_tcpm_init(int port)
 	 * Alert assertion when CC_STATUS.Looking4Connection changes state.
 	 */
 	if (tcpc_config[port].flags & TCPC_FLAGS_TCPCI_REV2_0) {
-		error = tcpc_read(port, TCPC_REG_TCPC_CTRL, &regval);
-		regval |= TCPC_REG_TCPC_CTRL_EN_LOOK4CONNECTION_ALERT;
-		error |= tcpc_write(port, TCPC_REG_TCPC_CTRL, regval);
+		error = tcpc_update8(port, TCPC_REG_TCPC_CTRL,
+				TCPC_REG_TCPC_CTRL_EN_LOOK4CONNECTION_ALERT,
+				MASK_SET);
 		if (error)
 			CPRINTS("C%d: Failed to init TCPC_CTRL!", port);
 	}
diff --git a/driver/tcpm/tcpci.h b/driver/tcpm/tcpci.h
index 60bca01107..813c3026d4 100644
--- a/driver/tcpm/tcpci.h
+++ b/driver/tcpm/tcpci.h
@@ -222,6 +222,8 @@ int tcpci_tcpm_release(int port);
 #ifdef CONFIG_USB_PD_DUAL_ROLE_AUTO_TOGGLE
 int tcpci_set_role_ctrl(int port, int toggle, int rp, int pull);
 int tcpci_tcpc_drp_toggle(int port);
+int tcpci_tcpc_set_connection(int port, enum tcpc_cc_pull pull,
+			      int connect);
 #endif
 #ifdef CONFIG_USB_PD_TCPC_LOW_POWER
 int tcpci_enter_low_power_mode(int port);
diff --git a/driver/tcpm/tcpm.h b/driver/tcpm/tcpm.h
index 373fbf545f..f9453d418c 100644
--- a/driver/tcpm/tcpm.h
+++ b/driver/tcpm/tcpm.h
@@ -178,16 +178,16 @@ 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)
+static inline int tcpm_set_connection(int port,
+				      enum tcpc_cc_pull pull,
+				      int connect)
 {
 	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);
+	    tcpc->set_connection)
+		return tcpc->set_connection(port, pull, connect);
+	return EC_SUCCESS;
 }
 
 static inline int tcpm_set_polarity(int port, enum tcpc_cc_polarity polarity)