From c069177ccc29b4373d5587e1dcea966aeee0a856 Mon Sep 17 00:00:00 2001 From: Keith Short Date: Thu, 27 Aug 2020 14:51:41 -0600 Subject: tcpmv2: disable receive of SOP' during VCONN swap The USB PD specification indicates that the initial VCONN source shall cease source VCONN within tVCONNSourceOff (25 ms) after receiving PS_RDY. Not all partners wait after sending PS_RDY before trying to communicate with the cable. BUG=b:163478172, b:163143427 BRANCH=none TEST=make buildall TEST=Connect 2 Volteers together and force VCONN swaps confirming the sequence completes normally. Signed-off-by: Keith Short Change-Id: I14720f033c5f6e9caed9c4fe3bfa11e5c046116e Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2381030 Reviewed-by: Denis Brockus Commit-Queue: Denis Brockus --- common/usbc/usb_pe_drp_sm.c | 27 ++++++++++++- driver/tcpm/anx7447.c | 3 ++ driver/tcpm/anx7688.c | 3 ++ driver/tcpm/mt6370.c | 3 ++ driver/tcpm/nct38xx.c | 3 ++ driver/tcpm/ps8xxx.c | 3 ++ driver/tcpm/raa489000.c | 3 ++ driver/tcpm/rt1715.c | 3 ++ driver/tcpm/tcpci.c | 99 ++++++++++++++++++++++++++------------------- driver/tcpm/tcpci.h | 1 + driver/tcpm/tcpm.h | 9 +++++ driver/tcpm/tusb422.c | 3 ++ include/usb_pd_tcpm.h | 13 ++++++ 13 files changed, 130 insertions(+), 43 deletions(-) diff --git a/common/usbc/usb_pe_drp_sm.c b/common/usbc/usb_pe_drp_sm.c index c90d98a75c..7e76c15e1c 100644 --- a/common/usbc/usb_pe_drp_sm.c +++ b/common/usbc/usb_pe_drp_sm.c @@ -5269,6 +5269,23 @@ static void pe_vcs_evaluate_swap_entry(int port) /* NOTE: PE_VCS_Accept_Swap State embedded here */ PE_SET_FLAG(port, PE_FLAGS_ACCEPT); send_ctrl_msg(port, TCPC_TX_SOP, PD_CTRL_ACCEPT); + + /* + * The USB PD 3.0 spec indicates that the initial VCONN source + * shall cease sourcing VCONN within tVCONNSourceOff (25ms) + * after receiving the PS_RDY message. However, some partners + * begin sending SOP' messages only 1 ms after sending PS_RDY + * during VCONN swap. + * + * Preemptively disable receipt of SOP' and SOP'' messages while + * we wait for PS_RDY so we don't attempt to process messages + * directed at the cable. If the partner fails to send PS_RDY we + * perform a hard reset so no need to re-enable SOP' messages. + * + * We continue to source VCONN while we wait as required by the + * spec. + */ + tcpm_sop_prime_disable(port); } } @@ -5333,12 +5350,18 @@ static void pe_vcs_send_swap_run(int port) */ if (type == PD_CTRL_ACCEPT) { pe[port].vconn_swap_counter = 0; - if (tc_is_vconn_src(port)) + if (tc_is_vconn_src(port)) { + /* + * Prevent receiving any SOP' and SOP'' + * messages while a swap is in progress. + */ + tcpm_sop_prime_disable(port); set_state_pe(port, PE_VCS_WAIT_FOR_VCONN_SWAP); - else + } else { set_state_pe(port, PE_VCS_TURN_ON_VCONN_SWAP); + } return; } /* diff --git a/driver/tcpm/anx7447.c b/driver/tcpm/anx7447.c index f1fda87cf1..de9fb5dcfc 100644 --- a/driver/tcpm/anx7447.c +++ b/driver/tcpm/anx7447.c @@ -809,6 +809,9 @@ const struct tcpm_drv anx7447_tcpm_drv = { .select_rp_value = &tcpci_tcpm_select_rp_value, .set_cc = &anx7447_set_cc, .set_polarity = &anx7447_set_polarity, +#ifdef CONFIG_USB_PD_DECODE_SOP + .sop_prime_disable = &tcpci_tcpm_sop_prime_disable, +#endif .set_vconn = &tcpci_tcpm_set_vconn, .set_msg_header = &tcpci_tcpm_set_msg_header, .set_rx_enable = &tcpci_tcpm_set_rx_enable, diff --git a/driver/tcpm/anx7688.c b/driver/tcpm/anx7688.c index af585db562..1c6785f406 100644 --- a/driver/tcpm/anx7688.c +++ b/driver/tcpm/anx7688.c @@ -200,6 +200,9 @@ const struct tcpm_drv anx7688_tcpm_drv = { .select_rp_value = &tcpci_tcpm_select_rp_value, .set_cc = &tcpci_tcpm_set_cc, .set_polarity = &tcpci_tcpm_set_polarity, +#ifdef CONFIG_USB_PD_DECODE_SOP + .sop_prime_disable = &tcpci_tcpm_sop_prime_disable, +#endif .set_vconn = &tcpci_tcpm_set_vconn, .set_msg_header = &tcpci_tcpm_set_msg_header, .set_rx_enable = &tcpci_tcpm_set_rx_enable, diff --git a/driver/tcpm/mt6370.c b/driver/tcpm/mt6370.c index af11bcb11d..07ebeac7f9 100644 --- a/driver/tcpm/mt6370.c +++ b/driver/tcpm/mt6370.c @@ -195,6 +195,9 @@ const struct tcpm_drv mt6370_tcpm_drv = { .select_rp_value = &tcpci_tcpm_select_rp_value, .set_cc = &mt6370_set_cc, .set_polarity = &mt6370_set_polarity, +#ifdef CONFIG_USB_PD_DECODE_SOP + .sop_prime_disable = &tcpci_tcpm_sop_prime_disable, +#endif .set_vconn = &tcpci_tcpm_set_vconn, .set_msg_header = &tcpci_tcpm_set_msg_header, .set_rx_enable = &tcpci_tcpm_set_rx_enable, diff --git a/driver/tcpm/nct38xx.c b/driver/tcpm/nct38xx.c index ffd2a15b68..308cefe928 100644 --- a/driver/tcpm/nct38xx.c +++ b/driver/tcpm/nct38xx.c @@ -201,6 +201,9 @@ const struct tcpm_drv nct38xx_tcpm_drv = { .select_rp_value = &tcpci_tcpm_select_rp_value, .set_cc = &nct38xx_tcpm_set_cc, .set_polarity = &tcpci_tcpm_set_polarity, +#ifdef CONFIG_USB_PD_DECODE_SOP + .sop_prime_disable = &tcpci_tcpm_sop_prime_disable, +#endif .set_vconn = &tcpci_tcpm_set_vconn, .set_msg_header = &tcpci_tcpm_set_msg_header, .set_rx_enable = &tcpci_tcpm_set_rx_enable, diff --git a/driver/tcpm/ps8xxx.c b/driver/tcpm/ps8xxx.c index 713977b1e7..9fb6d31544 100644 --- a/driver/tcpm/ps8xxx.c +++ b/driver/tcpm/ps8xxx.c @@ -572,6 +572,9 @@ const struct tcpm_drv ps8xxx_tcpm_drv = { .select_rp_value = &tcpci_tcpm_select_rp_value, .set_cc = &tcpci_tcpm_set_cc, .set_polarity = &tcpci_tcpm_set_polarity, +#ifdef CONFIG_USB_PD_DECODE_SOP + .sop_prime_disable = &tcpci_tcpm_sop_prime_disable, +#endif .set_vconn = &tcpci_tcpm_set_vconn, .set_msg_header = &tcpci_tcpm_set_msg_header, .set_rx_enable = &tcpci_tcpm_set_rx_enable, diff --git a/driver/tcpm/raa489000.c b/driver/tcpm/raa489000.c index ae6aef2269..632c17c78b 100644 --- a/driver/tcpm/raa489000.c +++ b/driver/tcpm/raa489000.c @@ -207,6 +207,9 @@ const struct tcpm_drv raa489000_tcpm_drv = { .select_rp_value = &tcpci_tcpm_select_rp_value, .set_cc = &raa489000_tcpm_set_cc, .set_polarity = &tcpci_tcpm_set_polarity, +#ifdef CONFIG_USB_PD_DECODE_SOP + .sop_prime_disable = &tcpci_tcpm_sop_prime_disable, +#endif .set_vconn = &tcpci_tcpm_set_vconn, .set_msg_header = &tcpci_tcpm_set_msg_header, .set_rx_enable = &tcpci_tcpm_set_rx_enable, diff --git a/driver/tcpm/rt1715.c b/driver/tcpm/rt1715.c index 970af18984..cf98666058 100644 --- a/driver/tcpm/rt1715.c +++ b/driver/tcpm/rt1715.c @@ -55,6 +55,9 @@ const struct tcpm_drv rt1715_tcpm_drv = { .select_rp_value = &tcpci_tcpm_select_rp_value, .set_cc = &tcpci_tcpm_set_cc, .set_polarity = &tcpci_tcpm_set_polarity, +#ifdef CONFIG_USB_PD_DECODE_SOP + .sop_prime_disable = &tcpci_tcpm_sop_prime_disable, +#endif .set_vconn = &tcpci_tcpm_set_vconn, .set_msg_header = &tcpci_tcpm_set_msg_header, .set_rx_enable = &tcpci_tcpm_set_rx_enable, diff --git a/driver/tcpm/tcpci.c b/driver/tcpm/tcpci.c index 008f3bd589..64de918dff 100644 --- a/driver/tcpm/tcpci.c +++ b/driver/tcpm/tcpci.c @@ -25,10 +25,10 @@ #define CPRINTF(format, args...) cprintf(CC_USBPD, format, ## args) #define CPRINTS(format, args...) cprints(CC_USBPD, format, ## args) -#ifdef CONFIG_USB_PD_DECODE_SOP -static int vconn_en[CONFIG_USB_PD_PORT_MAX_COUNT]; -static int rx_en[CONFIG_USB_PD_PORT_MAX_COUNT]; -#endif +STATIC_IF(CONFIG_USB_PD_DECODE_SOP) + int sop_prime_en[CONFIG_USB_PD_PORT_MAX_COUNT]; +STATIC_IF(CONFIG_USB_PD_DECODE_SOP) + int rx_en[CONFIG_USB_PD_PORT_MAX_COUNT]; #define TCPC_FLAGS_VSAFE0V(_flags) \ ((_flags & TCPC_FLAGS_TCPCI_REV2_0) && \ @@ -608,17 +608,10 @@ int tcpci_tcpm_set_src_ctrl(int port, int enable) } #endif -int tcpci_tcpm_set_vconn(int port, int enable) +__maybe_unused static int tpcm_set_sop_prime_enable(int port, int enable) { - int reg, rv; - - rv = tcpc_read(port, TCPC_REG_POWER_CTRL, ®); - if (rv) - return rv; - -#ifdef CONFIG_USB_PD_DECODE_SOP - /* save vconn */ - vconn_en[port] = enable; + /* save SOP'/SOP'' enable state */ + sop_prime_en[port] = enable; if (rx_en[port]) { int detect_sop_en = TCPC_REG_RX_DETECT_SOP_HRST_MASK; @@ -628,9 +621,31 @@ int tcpci_tcpm_set_vconn(int port, int enable) TCPC_REG_RX_DETECT_SOP_SOPP_SOPPP_HRST_MASK; } - tcpc_write(port, TCPC_REG_RX_DETECT, detect_sop_en); + return tcpc_write(port, TCPC_REG_RX_DETECT, detect_sop_en); } -#endif + + return EC_SUCCESS; +} + +__maybe_unused int tcpci_tcpm_sop_prime_disable(int port) +{ + return tpcm_set_sop_prime_enable(port, 0); +} + +int tcpci_tcpm_set_vconn(int port, int enable) +{ + int reg, rv; + + rv = tcpc_read(port, TCPC_REG_POWER_CTRL, ®); + if (rv) + return rv; + + if (IS_ENABLED(CONFIG_USB_PD_DECODE_SOP)) { + rv = tpcm_set_sop_prime_enable(port, enable); + if (rv) + return rv; + } + reg &= ~TCPC_REG_POWER_CTRL_VCONN(1); reg |= TCPC_REG_POWER_CTRL_VCONN(enable); return tcpc_write(port, TCPC_REG_POWER_CTRL, reg); @@ -664,24 +679,24 @@ int tcpci_tcpm_set_rx_enable(int port, int enable) { int detect_sop_en = 0; -#ifdef CONFIG_USB_PD_DECODE_SOP - /* save rx_on */ - rx_en[port] = enable; -#endif + if (IS_ENABLED(CONFIG_USB_PD_DECODE_SOP)) { + /* save rx_on */ + rx_en[port] = enable; + } + if (enable) { detect_sop_en = TCPC_REG_RX_DETECT_SOP_HRST_MASK; -#ifdef CONFIG_USB_PD_DECODE_SOP - /* - * Only the VCONN Source is allowed to communicate - * with the Cable Plugs. - */ - - if (vconn_en[port]) + if (IS_ENABLED(CONFIG_USB_PD_DECODE_SOP) && + sop_prime_en[port]) { + /* + * Only the VCONN Source is allowed to communicate + * with the Cable Plugs. + */ detect_sop_en = TCPC_REG_RX_DETECT_SOP_SOPP_SOPPP_HRST_MASK; -#endif + } } /* If enable, then set RX detect for SOP and HRST */ @@ -765,9 +780,7 @@ static int tcpci_rev1_0_tcpm_get_message_raw(int port, uint32_t *payload, int *head) { int rv, cnt, reg = TCPC_REG_RX_DATA; -#ifdef CONFIG_USB_PD_DECODE_SOP int frm; -#endif rv = tcpc_read(port, TCPC_REG_RX_BYTE_CNT, &cnt); @@ -782,21 +795,22 @@ static int tcpci_rev1_0_tcpm_get_message_raw(int port, uint32_t *payload, goto clear; } -#ifdef CONFIG_USB_PD_DECODE_SOP - rv = tcpc_read(port, TCPC_REG_RX_BUF_FRAME_TYPE, &frm); - if (rv != EC_SUCCESS) { - rv = EC_ERROR_UNKNOWN; - goto clear; + if (IS_ENABLED(CONFIG_USB_PD_DECODE_SOP)) { + rv = tcpc_read(port, TCPC_REG_RX_BUF_FRAME_TYPE, &frm); + if (rv != EC_SUCCESS) { + rv = EC_ERROR_UNKNOWN; + goto clear; + } } -#endif rv = tcpc_read16(port, TCPC_REG_RX_HDR, (int *)head); -#ifdef CONFIG_USB_PD_DECODE_SOP - /* Encode message address in bits 31 to 28 */ - *head &= 0x0000ffff; - *head |= PD_HEADER_SOP(frm); -#endif + if (IS_ENABLED(CONFIG_USB_PD_DECODE_SOP)) { + /* Encode message address in bits 31 to 28 */ + *head &= 0x0000ffff; + *head |= PD_HEADER_SOP(frm); + } + if (rv == EC_SUCCESS && cnt > 0) { tcpc_read_block(port, reg, (uint8_t *)payload, cnt); } @@ -1726,6 +1740,9 @@ const struct tcpm_drv tcpci_tcpm_drv = { .select_rp_value = &tcpci_tcpm_select_rp_value, .set_cc = &tcpci_tcpm_set_cc, .set_polarity = &tcpci_tcpm_set_polarity, +#ifdef CONFIG_USB_PD_DECODE_SOP + .sop_prime_disable = &tcpci_tcpm_sop_prime_disable, +#endif .set_vconn = &tcpci_tcpm_set_vconn, .set_msg_header = &tcpci_tcpm_set_msg_header, .set_rx_enable = &tcpci_tcpm_set_rx_enable, diff --git a/driver/tcpm/tcpci.h b/driver/tcpm/tcpci.h index 914b143c41..db05c978e8 100644 --- a/driver/tcpm/tcpci.h +++ b/driver/tcpm/tcpci.h @@ -221,6 +221,7 @@ bool tcpci_tcpm_check_vbus_level(int port, enum vbus_level level); int tcpci_tcpm_select_rp_value(int port, int rp); int tcpci_tcpm_set_cc(int port, int pull); int tcpci_tcpm_set_polarity(int port, enum tcpc_cc_polarity polarity); +int tcpci_tcpm_sop_prime_disable(int port); int tcpci_tcpm_set_vconn(int port, int enable); int tcpci_tcpm_set_msg_header(int port, int power_role, int data_role); int tcpci_tcpm_set_rx_enable(int port, int enable); diff --git a/driver/tcpm/tcpm.h b/driver/tcpm/tcpm.h index 4ab993dcc2..e1db30ecb8 100644 --- a/driver/tcpm/tcpm.h +++ b/driver/tcpm/tcpm.h @@ -193,6 +193,15 @@ static inline int tcpm_set_polarity(int port, enum tcpc_cc_polarity polarity) return tcpc_config[port].drv->set_polarity(port, polarity); } +static inline int tcpm_sop_prime_disable(int port) +{ +#ifdef CONFIG_USB_PD_DECODE_SOP + return tcpc_config[port].drv->sop_prime_disable(port); +#else + return EC_SUCCESS; +#endif +} + static inline int tcpm_set_vconn(int port, int enable) { return tcpc_config[port].drv->set_vconn(port, enable); diff --git a/driver/tcpm/tusb422.c b/driver/tcpm/tusb422.c index 3ab95c9721..75ffcd1d43 100644 --- a/driver/tcpm/tusb422.c +++ b/driver/tcpm/tusb422.c @@ -158,6 +158,9 @@ const struct tcpm_drv tusb422_tcpm_drv = { .select_rp_value = &tcpci_tcpm_select_rp_value, .set_cc = &tusb422_tcpm_set_cc, .set_polarity = &tcpci_tcpm_set_polarity, +#ifdef CONFIG_USB_PD_DECODE_SOP + .sop_prime_disable = &tcpci_tcpm_sop_prime_disable, +#endif .set_vconn = &tcpci_tcpm_set_vconn, .set_msg_header = &tcpci_tcpm_set_msg_header, .set_rx_enable = &tcpci_tcpm_set_rx_enable, diff --git a/include/usb_pd_tcpm.h b/include/usb_pd_tcpm.h index bccea184e4..d6f89553fc 100644 --- a/include/usb_pd_tcpm.h +++ b/include/usb_pd_tcpm.h @@ -245,6 +245,19 @@ struct tcpm_drv { */ int (*set_polarity)(int port, enum tcpc_cc_polarity polarity); +#ifdef CONFIG_USB_PD_DECODE_SOP + /** + * Disable receive of SOP' and SOP'' messages. This is provided + * separately from set_vconn so that we can preemptively disable + * receipt of SOP' messages during a VCONN swap. + * + * @param port Type-C port number + * + * @return EC_SUCCESS or error + */ + int (*sop_prime_disable)(int port); +#endif + /** * Set Vconn. * -- cgit v1.2.1