pd: Don't auto toggle when DRP state is not dual role.stabilize-nocturne-10986.B

When in PD_STATE_DRP_AUTO_TOGGLE, we were always enabling DRP
auto-toggle, even when drp_state is PD_DRP_TOGGLE_OFF or
PD_DRP_FORCE_SINK, which prevents us from acting as source.

This caused an infinite loop of entering and exiting low power
mode when a sink-only device was plugged into a PS8751 port when
the system is in S5/G3.

To fix this, only enable DRP auto-toggle when drp_state allows
us to be dual-role.

One problem with doing this is that the ANX3429 doesn't support
low power mode with auto-toggle disabled. Luckily, the ANX3429
will stay in low power mode when a sink-only device is connected.

BRANCH=none
BUG=b:72007056,b:111663127
TEST=sink device + ANX3429: low power mode [1]
TEST=sink device + PS8751: low power mode [1]
TEST=charger via hub + ANX3429: 50% chance fail to charge [2]
TEST=charger via hub + PS8751: starts charging
(all tests with Grunt in G3)

[1] dut-control pp3300_tcpc_mw -> 3.5
[2] b/72007056 still remains to be fixed.

Change-Id: I22b9ed5ce93e1eeeb867fcc8bcd6dd803c0bc19b
Signed-off-by: Edward Hill <ecgh@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1188936
Reviewed-by: Aseda Aboagye <aaboagye@chromium.org>
Commit-Queue: Aseda Aboagye <aaboagye@chromium.org>
Tested-by: Aseda Aboagye <aaboagye@chromium.org>
(cherry picked from commit e86ebfa5121414cc67df3a09f2c4636569a3d617)
Reviewed-on: https://chromium-review.googlesource.com/1200568
Trybot-Ready: Aseda Aboagye <aaboagye@chromium.org>

2 files changed, 30 insertions, 44 deletions

diff --git a/common/usb_pd_protocol.c b/common/usb_pd_protocol.c
index 0e879b05cb..b64b74afcf 100644
--- a/common/usb_pd_protocol.c
+++ b/common/usb_pd_protocol.c
@@ -3749,7 +3749,15 @@ void pd_task(void *u)
 				pd_set_power_role(port, PD_ROLE_SOURCE);
 				timeout = 2*MSEC;
 			} else {
-				pd_set_drp_toggle(port, 1);
+				/*
+				 * Staying in PD_STATE_DRP_AUTO_TOGGLE,
+				 * always enter low power mode, and auto-toggle
+				 * while in low power mode if drp_state allows
+				 * us to be dual role.
+				 */
+				if (drp_state[port] == PD_DRP_TOGGLE_ON)
+					tcpm_set_drp_toggle(port, 1);
+				request_low_power_mode(port, 1);
 				pd[port].flags |= PD_FLAGS_TCPC_DRP_TOGGLE;
 				timeout = -1;
 			}
diff --git a/driver/tcpm/anx74xx.c b/driver/tcpm/anx74xx.c
index f892f05920..25bb91eb54 100644
--- a/driver/tcpm/anx74xx.c
+++ b/driver/tcpm/anx74xx.c
@@ -141,27 +141,36 @@ static void anx74xx_set_power_mode(int port, int mode)
 		/* Update cable cable det signal */
 		anx74xx_update_cable_det(port, mode);
 		/*
-		 * The final piece of entering low power mode is setting the
-		 * RESET_L signal low, which is done via
-		 * anx74xx_enter_low_power_mode which is called at least 10ms
-		 * after setting cable_det low (above).
+		 * Delay between setting cable_det low and setting RESET_L low
+		 * as recommended the ANX3429 datasheet.
 		 */
+		msleep(1);
+		/* Put chip into standby mode */
+		board_set_tcpc_power_mode(port, mode);
 	}
 }
 
-#ifdef CONFIG_USB_PD_TCPC_LOW_POWER
-static int anx74xx_enter_low_power_mode(int port)
+#if defined(CONFIG_USB_PD_DUAL_ROLE_AUTO_TOGGLE) && \
+	defined(CONFIG_USB_PD_TCPC_LOW_POWER)
+
+static int anx74xx_tcpc_drp_toggle(int port, int enable)
 {
 	/*
-	 * The delay between setting cable_det low (in anx74xx_set_power_mode)
-	 * and setting RESET_L low in (board_set_tcpc_power_mode) is at least
-	 * 1 ms. This should be taken care of by the PD_LM_DEBOUCE_US delay of
-	 * 10ms, but we want to protect against any future tweaks of that value.
+	 * The ANX3429 always auto-toggles when in low power mode. Since this is
+	 * not configurable, there is nothing to do here. DRP auto-toggle will
+	 * happen once low power mode is set via anx74xx_enter_low_power_mode().
+	 * Note: this means the ANX3429 auto-toggles in PD_DRP_FORCE_SINK mode,
+	 * which is undesirable (b/72007056).
 	 */
-	msleep(1);
-	board_set_tcpc_power_mode(port, ANX74XX_STANDBY_MODE);
 	return EC_SUCCESS;
 }
+
+static int anx74xx_enter_low_power_mode(int port)
+{
+	anx74xx_set_power_mode(port, ANX74XX_STANDBY_MODE);
+	return EC_SUCCESS;
+}
+
 #endif
 
 void anx74xx_tcpc_set_vbus(int port, int enable)
@@ -714,35 +723,6 @@ static int anx74xx_tcpm_set_cc(int port, int pull)
 	return rv;
 }
 
-#if defined(CONFIG_USB_PD_DUAL_ROLE_AUTO_TOGGLE) && \
-	defined(CONFIG_USB_PD_TCPC_LOW_POWER)
-static void anx74xx_handle_power_mode(int port, int mode)
-{
-	if (mode == ANX74XX_STANDBY_MODE) {
-		anx74xx_set_power_mode(port, mode);
-	} else if (anx[port].prev_mode != ANX74XX_NORMAL_MODE) {
-		/*
-		 * TODO: Interrupt high follows CC line hence ignore multiple
-		 * interrupts.
-		 */
-		anx74xx_tcpm_init(port);
-	}
-}
-
-static int anx74xx_tcpc_drp_toggle(int port, int enable)
-{
-	/*
-	 * When using low power mode, this function is an entry to point to
-	 * bring the ANX3429 in to or out of standby mode. DRP toggle is
-	 * associated with the chip being in standby mode.
-	 */
-	anx74xx_handle_power_mode(port, enable ? ANX74XX_STANDBY_MODE :
-				  ANX74XX_NORMAL_MODE);
-
-	return EC_SUCCESS;
-}
-#endif /* CONFIG_USB_PD_DUAL_ROLE_AUTO_TOGGLE && CONFIG_USB_PD_TCPC_LOW_POWER */
-
 static int anx74xx_tcpm_set_polarity(int port, int polarity)
 {
 	int reg, mux_state, rv = EC_SUCCESS;
@@ -1108,8 +1088,6 @@ const struct tcpm_drv anx74xx_tcpm_drv = {
 #if defined(CONFIG_USB_PD_DUAL_ROLE_AUTO_TOGGLE) && \
 		defined(CONFIG_USB_PD_TCPC_LOW_POWER)
 	.drp_toggle		= &anx74xx_tcpc_drp_toggle,
-#endif
-#ifdef CONFIG_USB_PD_TCPC_LOW_POWER
 	.enter_low_power_mode	= &anx74xx_enter_low_power_mode,
 #endif
 };