From 9cc05de8fe3dbad33de5c544255d4d69dab23249 Mon Sep 17 00:00:00 2001 From: Edward Hill Date: Wed, 15 Aug 2018 11:13:53 -0600 Subject: pd: Don't auto toggle when DRP state is not dual role. 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 Reviewed-on: https://chromium-review.googlesource.com/1188936 Reviewed-by: Aseda Aboagye Commit-Queue: Aseda Aboagye Tested-by: Aseda Aboagye (cherry picked from commit e86ebfa5121414cc67df3a09f2c4636569a3d617) Reviewed-on: https://chromium-review.googlesource.com/1200568 Trybot-Ready: Aseda Aboagye --- common/usb_pd_protocol.c | 10 +++++++- driver/tcpm/anx74xx.c | 64 ++++++++++++++++-------------------------------- 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 }; -- cgit v1.2.1