tcpm: anx74xx: Improvements to low power mode for S3/G3 chipset state

Before entering standby mode, bit 7 (cable_det) or analog_ctrl_0 is
cleared. This allows a new cable detect event to trigger the EC to
put the ANX4329 into normal mode. However, in some cases such as when
in S3/S5/G3 chipset power states a port will be in sink only mode and
therefore won't attach when a sink only adapter is connected to the
port. This results in the an indefinite toggle of standby<->normal
mode transitions. This constant toggle floods the EC console and more
importantly prevents the ANX4329 from remaining in standby mode and
the power consumption remains at ~9.5 mW instead of ~1.2 mW when no
adapter is connected.

This CL adds logic around clearing the cable_det bit so that it's only
cleared if both CC lines are open or if an Emark cable is
attached. Emark cable is determined from the presence of Ra on one CC
line and no Rd on the other CC line. The special consideration for an
Emark cable is reqiured because when an Emark cable is connected, the
USB PD auto toggle state may require some number of iterations until
the attach event is recognized.

In order to support handle cases where the drp mode is changed via
either the EC console command or host command, added a call to
tcpm_set_drp_toggle whenever the drp state is updated. Since the drp
mode is updated upon chipset resume events, the chipset resume hook in
board.c for anx74xx_cable_det_handler() became redundant and hence it
was removed.

BUG=b:35775019,b:35586188
BRANCH=reef
TEST=Tested the following cases:
- Sink, source adapters in chipset S0. Verified that adapters
connected as expected. Also, used Emark cable to connect to both USB
PD chargers (source)  and Pixel phone (sink).

- Run 'lidclose' on EC console then connect Type C -> A adatper
[3490.370125 TCPC p0 reset!]
[3490.389588 TCPC p0 Low Power Mode]
It no longer toggles indefintely and after running 'lidopen' verified
that port 0 is now in SRC_DISCOVERY.

- In S3/G3 connected USB PD charger with regular and Emark typeC
  cable. Verified that port 0 is in SNK_READY state.

- use 'pd dualrole sink' and test with Type C -> A adatper. Verified
  that tcpc wakes up, but goes into standby mode until entering 'pd
  dualrole on'.

- When sink only adapter is connected in S3/G3, measure power level
  ~1.2 mW as opposed to ~10 mW in S0.

- Repeated similar tests on port 1 (parade tcpc) to verify that
  adapters connected as expected.

Change-Id: Ib8de666f72723934186fee7869f9dda01381c7a8
Signed-off-by: Scott <scollyer@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/463991
Commit-Ready: Scott Collyer <scollyer@chromium.org>
Tested-by: Scott Collyer <scollyer@chromium.org>
Reviewed-by: S Wang <swang@analogix.corp-partner.google.com>
Reviewed-by: Duncan Laurie <dlaurie@chromium.org>
Reviewed-by: Nicolas Boichat <drinkcat@chromium.org>

6 files changed, 67 insertions, 17 deletions

diff --git a/board/eve/board.c b/board/eve/board.c
index 338d982d7b..fc87d1d1dc 100644
--- a/board/eve/board.c
+++ b/board/eve/board.c
@@ -111,7 +111,6 @@ static void anx74xx_c0_cable_det_handler(void)
 		task_set_event(TASK_ID_PD_C0, PD_EVENT_TCPC_RESET, 0);
 }
 DECLARE_DEFERRED(anx74xx_c0_cable_det_handler);
-DECLARE_HOOK(HOOK_CHIPSET_RESUME, anx74xx_c0_cable_det_handler, HOOK_PRIO_LAST);
 
 static void anx74xx_c1_cable_det_handler(void)
 {
@@ -131,7 +130,6 @@ static void anx74xx_c1_cable_det_handler(void)
 		task_set_event(TASK_ID_PD_C1, PD_EVENT_TCPC_RESET, 0);
 }
 DECLARE_DEFERRED(anx74xx_c1_cable_det_handler);
-DECLARE_HOOK(HOOK_CHIPSET_RESUME, anx74xx_c1_cable_det_handler, HOOK_PRIO_LAST);
 
 void anx74xx_cable_det_interrupt(enum gpio_signal signal)
 {
diff --git a/board/poppy/board.c b/board/poppy/board.c
index 40c6022dcb..b9b5875273 100644
--- a/board/poppy/board.c
+++ b/board/poppy/board.c
@@ -113,7 +113,6 @@ static void anx74xx_cable_det_handler(void)
 		task_set_event(TASK_ID_PD_C0, PD_EVENT_TCPC_RESET, 0);
 }
 DECLARE_DEFERRED(anx74xx_cable_det_handler);
-DECLARE_HOOK(HOOK_CHIPSET_RESUME, anx74xx_cable_det_handler, HOOK_PRIO_LAST);
 
 void anx74xx_cable_det_interrupt(enum gpio_signal signal)
 {
diff --git a/board/reef/board.c b/board/reef/board.c
index cbba33e83d..5db387f6b9 100644
--- a/board/reef/board.c
+++ b/board/reef/board.c
@@ -99,7 +99,6 @@ static void anx74xx_cable_det_handler(void)
 		task_set_event(TASK_ID_PD_C0, PD_EVENT_TCPC_RESET, 0);
 }
 DECLARE_DEFERRED(anx74xx_cable_det_handler);
-DECLARE_HOOK(HOOK_CHIPSET_RESUME, anx74xx_cable_det_handler, HOOK_PRIO_LAST);
 
 void anx74xx_cable_det_interrupt(enum gpio_signal signal)
 {
diff --git a/common/usb_pd_protocol.c b/common/usb_pd_protocol.c
index b92641e5c8..ee4ae31ab0 100644
--- a/common/usb_pd_protocol.c
+++ b/common/usb_pd_protocol.c
@@ -1428,6 +1428,12 @@ void pd_update_dual_role_config(int port)
 		set_state(port, PD_STATE_SRC_DISCONNECTED);
 		tcpm_set_cc(port, TYPEC_CC_RP);
 	}
+
+#if defined(CONFIG_USB_PD_DUAL_ROLE_AUTO_TOGGLE) && \
+	defined(CONFIG_USB_PD_TCPC_LOW_POWER)
+	/* When switching drp mode, make sure tcpc is out of standby mode */
+	tcpm_set_drp_toggle(port, 0);
+#endif
 }
 
 int pd_get_role(int port)
diff --git a/driver/tcpm/anx74xx.c b/driver/tcpm/anx74xx.c
index d9317abbb7..4a32fc4864 100644
--- a/driver/tcpm/anx74xx.c
+++ b/driver/tcpm/anx74xx.c
@@ -67,16 +67,56 @@ static void anx74xx_update_cable_det(int port, int mode)
 	if (tcpc_read(port, ANX74XX_REG_ANALOG_CTRL_0, &reg))
 		return;
 
-	/*
-	 * When ANX3429 needs to enter ANX74XX_STANDBY_MODE, Cable det pin
-	 * shall be pulled low first by ANX3429`s register, in this way,
-	 * for use case of E-mark cable only, EC can find cable det pin is
-	 * pulled high again.
-	 */
-	if (mode == ANX74XX_STANDBY_MODE)
-		reg &= ~ANX74XX_REG_R_PIN_CABLE_DET;
-	else
+	if (mode == ANX74XX_STANDBY_MODE) {
+		int cc_reg;
+
+		/*
+		 * The ANX4329 enters standby mode by setting PWR_EN signal
+		 * low. In addition, RESET_L must be set low to keep the ANX3429
+		 * in standby mode.
+		 *
+		 * Clearing bit 7 of ANX74XX_REG_ANALOG_CTRL_0 will cause the
+		 * ANX3429 to clear the cable_det signal that goes from the
+		 * ANX3429 to the EC. If this bit is cleared when a cable is
+		 * attached then cable_det will go high once standby is entered.
+		 *
+		 * In some cases, such as when the chipset power state is
+		 * S3/S5/G3 and a sink only adapter is connected to the port,
+		 * this behavior is undesirable. The constant toggling between
+		 * standby and normal mode means that effectively the ANX3429 is
+		 * not in standby mode only consumes ~1 mW less than just
+		 * remaining in normal mode. However when an E mark cable is
+		 * connected, clearing bit 7 is required so that while the E
+		 * mark cable configuration happens, the USB PD state machine
+		 * will continue to wake up until the USB PD attach event can be
+		 * regtistered.
+		 *
+		 * Therefore, the decision to clear bit 7 is based on the
+		 * current CC status of the port. If the CC status is open for
+		 * both CC lines OR if either CC line is showing Ra, then clear
+		 * bit 7. Not clearing bit 7 has no impact for normal cables and
+		 * prevents the constant toggle of standby<->normal when an
+		 * adapter is connected that isn't allowed to attach. Clearing
+		 * bit 7 when CC status reads Ra for either CC line allows the
+		 * USB PD state machine to be woken until the attach event can
+		 * happen. Note that in the case an E mark cable is connected
+		 * and can't attach (i.e. sink only port <- Emark cable -> sink
+		 * only adapter), then the ANX3429 will toggle indefinitely,
+		 * until either the cable is removed, or the port drp status
+		 * changes so the attach event can occur.
+		 * .
+		 */
+
+		/* Read CC status to see if cable_det bit should be cleared */
+		if (tcpc_read(port, ANX74XX_REG_CC_STATUS, &cc_reg))
+			return;
+		/* If open or either CC line is Ra, then clear cable_det */
+		if (!cc_reg || (cc_reg & ANX74XX_CC_RA_MASK &&
+				!(cc_reg & ANX74XX_CC_RD_MASK)))
+			reg &= ~ANX74XX_REG_R_PIN_CABLE_DET;
+	} else {
 		reg |= ANX74XX_REG_R_PIN_CABLE_DET;
+	}
 
 	tcpc_write(port, ANX74XX_REG_ANALOG_CTRL_0, reg);
 #endif
@@ -664,10 +704,14 @@ void anx74xx_handle_power_mode(int port, int mode)
 
 static int anx74xx_tcpc_drp_toggle(int port, int enable)
 {
-	if (!enable)
-		/* TODO: Switch to normal mode here (Issue 702277) */
-		return EC_SUCCESS;
-	anx74xx_handle_power_mode(port, ANX74XX_STANDBY_MODE);
+	/*
+	 * 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 */
diff --git a/driver/tcpm/anx74xx.h b/driver/tcpm/anx74xx.h
index 127af4e0b5..06720650bd 100644
--- a/driver/tcpm/anx74xx.h
+++ b/driver/tcpm/anx74xx.h
@@ -181,6 +181,10 @@
 #define BIT_VALUE_OF_SNK_CC_DEFAULT 0x04
 #define BIT_VALUE_OF_SNK_CC_1_P_5   0x08
 #define BIT_VALUE_OF_SNK_CC_3_P_0   0x0C
+#define ANX74XX_CC_RA_MASK (BIT_VALUE_OF_SRC_CC_RA | \
+			    (BIT_VALUE_OF_SRC_CC_RA << 4))
+#define ANX74XX_CC_RD_MASK (BIT_VALUE_OF_SRC_CC_RD | \
+			    (BIT_VALUE_OF_SRC_CC_RD << 4))
 
 extern const struct tcpm_drv anx74xx_tcpm_drv;
 extern const struct usb_mux_driver anx74xx_tcpm_usb_mux_driver;