tcpm: anx74xx: Take ANX3429 out of suspend state before I2C access

The cable_det signal is used to signal to the EC that the tcpc has
detected a cable being connected when low power mode is
configured. The driver then needs to take the tcpc out of suspend
state by setting PWR_EN and RESET_N high. Then bit 7 of analog_ctrl_0
needs to be set properly.

The code that is handling this transition was attempting to access the
tcpc via I2C before putting the chip in normal mode when
transitioning from suspend to normal. In addition there are issues
with calling the driver function directly from the hook task (in the
delayed ISR handler) and from the USB PD task.

This CL changes the delay ISR handler to only set the TCPC_RESET
indication so that the call to put the ANX3429 into normal from
standby to normal mode only happens in the USB PD task. The TCPC_RESET
event is only set if cable_det is high, but reset_n (to the ANX3429)
is low which indicates that the ANX3429 is currently in standby mode.

BUG=b:35775019
BRANCH=reef
TEST=Manual
Tested with various adapters in both S0 and S3/G3 chipset
states. Verified that adapters connected as expected. When in S3/G3
connecting a sink only adapter still causes an indefinite toggle, but
all calls into the driver are executed from within the USB PD 0 task
and all ANX3429 I2C accesses work as expected.

Change-Id: I6e4843e43f59afbf5ca3251feb68981b815c1c78
Signed-off-by: Scott <scollyer@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/457103
Commit-Ready: Scott Collyer <scollyer@chromium.org>
Tested-by: Scott Collyer <scollyer@chromium.org>
Reviewed-by: Nicolas Boichat <drinkcat@chromium.org>

4 files changed, 82 insertions, 56 deletions

diff --git a/board/eve/board.c b/board/eve/board.c
index 31e97ce75c..338d982d7b 100644
--- a/board/eve/board.c
+++ b/board/eve/board.c
@@ -95,42 +95,40 @@ void trackpad_interrupt(enum gpio_signal signal)
 #ifdef CONFIG_USB_PD_TCPC_LOW_POWER
 static void anx74xx_c0_cable_det_handler(void)
 {
-	int level = gpio_get_level(GPIO_USB_C0_CABLE_DET);
+	int cable_det = gpio_get_level(GPIO_USB_C0_CABLE_DET);
+	int reset_n = gpio_get_level(GPIO_USB_C0_PD_RST_L);
 
 	/*
-	 * Setting the low power is handled by DRP status hence
-	 * handle only the attach event.
+	 * A cable_det low->high transition was detected. If following the
+	 * debounce time, cable_det is high, and reset_n is low, then ANX3429 is
+	 * currently in standby mode and needs to be woken up. Set the
+	 * TCPC_RESET event which will bring the ANX3429 out of standby
+	 * mode. Setting this event is gated on reset_n being low because the
+	 * ANX3429 will always set cable_det when transitioning to normal mode
+	 * and if in normal mode, then there is no need to trigger a tcpc reset.
 	 */
-	if (level)
-		anx74xx_handle_power_mode(I2C_PORT_TCPC0,
-					ANX74XX_NORMAL_MODE);
-
-	/* confirm if cable_det is asserted */
-	if (!level || gpio_get_level(GPIO_USB_C0_PD_RST_L))
-		return;
-
-	task_set_event(TASK_ID_PD_C0, PD_EVENT_TCPC_RESET, 0);
+	if (cable_det && !reset_n)
+		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)
 {
-	int level = gpio_get_level(GPIO_USB_C1_CABLE_DET);
+	int cable_det = gpio_get_level(GPIO_USB_C1_CABLE_DET);
+	int reset_n = gpio_get_level(GPIO_USB_C1_PD_RST_L);
 
 	/*
-	 * Setting the low power is handled by DRP status hence
-	 * handle only the attach event.
+	 * A cable_det low->high transition was detected. If following the
+	 * debounce time, cable_det is high, and reset_n is low, then ANX3429 is
+	 * currently in standby mode and needs to be woken up. Set the
+	 * TCPC_RESET event which will bring the ANX3429 out of standby
+	 * mode. Setting this event is gated on reset_n being low because the
+	 * ANX3429 will always set cable_det when transitioning to normal mode
+	 * and if in normal mode, then there is no need to trigger a tcpc reset.
 	 */
-	if (level)
-		anx74xx_handle_power_mode(I2C_PORT_TCPC1,
-					ANX74XX_NORMAL_MODE);
-
-	/* confirm if cable_det is asserted */
-	if (!level || gpio_get_level(GPIO_USB_C1_PD_RST_L))
-		return;
-
-	task_set_event(TASK_ID_PD_C1, PD_EVENT_TCPC_RESET, 0);
+	if (cable_det && !reset_n)
+		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);
diff --git a/board/poppy/board.c b/board/poppy/board.c
index b6afe92153..40c6022dcb 100644
--- a/board/poppy/board.c
+++ b/board/poppy/board.c
@@ -97,28 +97,27 @@ void usb1_evt(enum gpio_signal signal)
 #ifdef CONFIG_USB_PD_TCPC_LOW_POWER
 static void anx74xx_cable_det_handler(void)
 {
-	int level = gpio_get_level(GPIO_USB_C0_CABLE_DET);
+	int cable_det = gpio_get_level(GPIO_USB_C0_CABLE_DET);
+	int reset_n = gpio_get_level(GPIO_USB_C0_PD_RST_L);
 
 	/*
-	 * Setting the low power is handled by DRP status hence
-	 * handle only the attach event.
+	 * A cable_det low->high transition was detected. If following the
+	 * debounce time, cable_det is high, and reset_n is low, then ANX3429 is
+	 * currently in standby mode and needs to be woken up. Set the
+	 * TCPC_RESET event which will bring the ANX3429 out of standby
+	 * mode. Setting this event is gated on reset_n being low because the
+	 * ANX3429 will always set cable_det when transitioning to normal mode
+	 * and if in normal mode, then there is no need to trigger a tcpc reset.
 	 */
-	if (level)
-		anx74xx_handle_power_mode(NPCX_I2C_PORT0_0,
-					ANX74XX_NORMAL_MODE);
-
-	/* confirm if cable_det is asserted */
-	if (!level || gpio_get_level(GPIO_USB_C0_PD_RST_L))
-		return;
-
-	task_set_event(TASK_ID_PD_C0, PD_EVENT_TCPC_RESET, 0);
+	if (cable_det && !reset_n)
+		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)
 {
-	/* debounce for 2ms */
+	/* debounce for 2 msec */
 	hook_call_deferred(&anx74xx_cable_det_handler_data, (2 * MSEC));
 }
 #endif
diff --git a/board/reef/board.c b/board/reef/board.c
index c39a038aa4..cbba33e83d 100644
--- a/board/reef/board.c
+++ b/board/reef/board.c
@@ -83,28 +83,27 @@ static void tcpc_alert_event(enum gpio_signal signal)
 #ifdef CONFIG_USB_PD_TCPC_LOW_POWER
 static void anx74xx_cable_det_handler(void)
 {
-	int level = gpio_get_level(GPIO_USB_C0_CABLE_DET);
+	int cable_det = gpio_get_level(GPIO_USB_C0_CABLE_DET);
+	int reset_n = gpio_get_level(GPIO_USB_C0_PD_RST_L);
 
 	/*
-	 * Setting the low power is handled by DRP status hence
-	 * handle only the attach event.
+	 * A cable_det low->high transition was detected. If following the
+	 * debounce time, cable_det is high, and reset_n is low, then ANX3429 is
+	 * currently in standby mode and needs to be woken up. Set the
+	 * TCPC_RESET event which will bring the ANX3429 out of standby
+	 * mode. Setting this event is gated on reset_n being low because the
+	 * ANX3429 will always set cable_det when transitioning to normal mode
+	 * and if in normal mode, then there is no need to trigger a tcpc reset.
 	 */
-	if (level)
-		anx74xx_handle_power_mode(USB_PD_PORT_ANX74XX,
-					  ANX74XX_NORMAL_MODE);
-
-	/* confirm if cable_det is asserted */
-	if (!level || gpio_get_level(GPIO_USB_C0_PD_RST_L))
-		return;
-
-	task_set_event(TASK_ID_PD_C0, PD_EVENT_TCPC_RESET, 0);
+	if (cable_det && !reset_n)
+		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)
 {
-	/* debounce for 2ms */
+	/* debounce for 2 msec */
 	hook_call_deferred(&anx74xx_cable_det_handler_data, (2 * MSEC));
 }
 #endif
diff --git a/driver/tcpm/anx74xx.c b/driver/tcpm/anx74xx.c
index 128890a8dd..d9317abbb7 100644
--- a/driver/tcpm/anx74xx.c
+++ b/driver/tcpm/anx74xx.c
@@ -52,13 +52,20 @@ static void anx74xx_tcpm_set_auto_good_crc(int port, int enable)
 		   enable ? ANX74XX_REG_REPLY_SOP_EN : 0);
 }
 
-static void anx74xx_set_power_mode(int port, int mode)
+static void anx74xx_update_cable_det(int port, int mode)
 {
 #ifdef CONFIG_USB_PD_TCPC_LOW_POWER
 	int reg;
+
+	if (anx[port].prev_mode == mode)
+		return;
+
+	/* Update power mode */
 	anx[port].prev_mode = mode;
 
-	tcpc_read(port, ANX74XX_REG_ANALOG_CTRL_0, &reg);
+	/* Get ANALOG_CTRL_0 for cable det bit */
+	if (tcpc_read(port, ANX74XX_REG_ANALOG_CTRL_0, &reg))
+		return;
 
 	/*
 	 * When ANX3429 needs to enter ANX74XX_STANDBY_MODE, Cable det pin
@@ -72,12 +79,35 @@ static void anx74xx_set_power_mode(int port, int mode)
 		reg |= ANX74XX_REG_R_PIN_CABLE_DET;
 
 	tcpc_write(port, ANX74XX_REG_ANALOG_CTRL_0, reg);
-
-	/* Delay recommended by Analogix for CABLE_DET setup time */
-	msleep(2);
 #endif
+}
 
-	board_set_tcpc_power_mode(port, mode == ANX74XX_NORMAL_MODE);
+static void anx74xx_set_power_mode(int port, int mode)
+{
+	/*
+	 * Update PWR_EN and RESET_N signals to the correct level. High for
+	 * Normal mode and low for Standby mode. When transitioning from standby
+	 * to normal mode, must set the PWR_EN and RESET_N before attempting to
+	 * modify cable_det bit of analog_ctrl_0. If going from Normal to
+	 * Standby, updating analog_ctrl_0 must happen before setting PWR_EN and
+	 * RESET_N low.
+	 */
+	if (mode == ANX74XX_NORMAL_MODE) {
+		/* Take chip out of standby mode */
+		board_set_tcpc_power_mode(port, mode);
+		/* Update the cable det signal */
+		anx74xx_update_cable_det(port, mode);
+	} else {
+		/* Update cable cable det signal */
+		anx74xx_update_cable_det(port, mode);
+		/*
+		 * 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);
+	}
 }
 
 void anx74xx_tcpc_set_vbus(int port, int enable)