diff options
author | Jett Rink <jettrink@chromium.org> | 2020-06-05 17:00:34 -0600 |
---|---|---|
committer | Jett Rink <jettrink@chromium.org> | 2020-06-10 20:56:29 +0000 |
commit | 99c3604670a6db488a126ef35076e810c859ef42 (patch) | |
tree | 4f160643f6b42380807fa0570050c815a7462bc3 | |
parent | 454fda0f0c1eea8884a82f1f251e0c8cf72f65df (diff) | |
download | chrome-ec-99c3604670a6db488a126ef35076e810c859ef42.tar.gz |
tcpmv2: retool start/end AMS
- Drop start and end ams function in favor of a flag based approach
- Don't clear RX queue on TX reset. We are supposed to drop any pending
TX messages (not RX messages). This should also help us to process
partner messages if we get a collision
- Drop prl_tx_phy_layer_reset_run and call next state directly in entry
- Dropping retry_counter reset to 0 since that happens in entry method
- Dropping flags reset to 0 because it is most likely dropping more
flags than we want.
BRANCH=none
BUG=b:158248741,b:157228506,b:157661566
TEST=DUT accepts soft resets
Change-Id: Ice8721a6c81452584f8d4ec474cb4f4a487b713b
Signed-off-by: Jett Rink <jettrink@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2233794
Tested-by: Diana Z <dzigterman@chromium.org>
Tested-by: Denis Brockus <dbrockus@chromium.org>
Reviewed-by: Diana Z <dzigterman@chromium.org>
Reviewed-by: Denis Brockus <dbrockus@chromium.org>
-rw-r--r-- | common/usbc/usb_pe_ctvpd_sm.c | 6 | ||||
-rw-r--r-- | common/usbc/usb_pe_drp_sm.c | 53 | ||||
-rw-r--r-- | common/usbc/usb_prl_sm.c | 135 | ||||
-rw-r--r-- | include/usb_pe_sm.h | 11 | ||||
-rw-r--r-- | include/usb_prl_sm.h | 14 | ||||
-rw-r--r-- | test/fake_prl.c | 3 | ||||
-rw-r--r-- | test/usb_prl.c | 15 |
7 files changed, 109 insertions, 128 deletions
diff --git a/common/usbc/usb_pe_ctvpd_sm.c b/common/usbc/usb_pe_ctvpd_sm.c index 7e215e4113..e4f3aeabd3 100644 --- a/common/usbc/usb_pe_ctvpd_sm.c +++ b/common/usbc/usb_pe_ctvpd_sm.c @@ -53,6 +53,12 @@ static void pe_init(int port) set_state_pe(port, PE_REQUEST); } +bool pe_in_local_ams(int port) +{ + /* We never start a local AMS */ + return false; +} + void pe_run(int port, int evt, int en) { static enum sm_local_state local_state[CONFIG_USB_PD_PORT_MAX_COUNT]; diff --git a/common/usbc/usb_pe_drp_sm.c b/common/usbc/usb_pe_drp_sm.c index 443b0eb8d2..483699a428 100644 --- a/common/usbc/usb_pe_drp_sm.c +++ b/common/usbc/usb_pe_drp_sm.c @@ -651,9 +651,8 @@ static inline void send_data_msg(int port, enum tcpm_transmit_type type, prl_send_data_msg(port, type, msg); } - -static inline void send_ext_data_msg(int port, enum tcpm_transmit_type type, - enum pd_ext_msg_type msg) +static __maybe_unused inline void send_ext_data_msg( + int port, enum tcpm_transmit_type type, enum pd_ext_msg_type msg) { /* Clear any previous TX status before sending a new message */ PE_CLR_FLAG(port, PE_FLAGS_TX_COMPLETE); @@ -696,6 +695,11 @@ int pe_is_running(int port) return local_state[port] == SM_RUN; } +bool pe_in_local_ams(int port) +{ + return !!PE_CHK_FLAG(port, PE_FLAGS_LOCALLY_INITIATED_AMS); +} + void pe_set_debug_level(enum debug_level debug_level) { #ifndef CONFIG_USB_PD_DEBUG_LEVEL @@ -975,7 +979,6 @@ void pe_report_error(int port, enum pe_error e, enum tcpm_transmit_type type) * Error during an Interruptible AMS. */ else { - PE_SET_FLAG(port, PE_FLAGS_PROTOCOL_ERROR); if (pe[port].power_role == PD_ROLE_SINK) set_state_pe(port, PE_SNK_READY); else @@ -1907,21 +1910,10 @@ static void pe_src_ready_entry(int port) { print_current_state(port); + /* Ensure any aborted message sequence is properly cleaned up */ PE_CLR_FLAG(port, PE_FLAGS_LOCALLY_INITIATED_AMS); /* - * If the transition into PE_SRC_Ready is the result of Protocol Error - * that has not caused a Soft Reset (see Section 8.3.3.4.1) then the - * notification to the Protocol Layer of the end of the AMS Shall Not - * be sent since there is a Message to be processed. - */ - if (PE_CHK_FLAG(port, PE_FLAGS_PROTOCOL_ERROR)) { - PE_CLR_FLAG(port, PE_FLAGS_PROTOCOL_ERROR); - } else { - prl_end_ams(port); - } - - /* * Wait and add jitter if we are operating in PD2.0 mode and no messages * have been sent since enter this state. */ @@ -2110,18 +2102,6 @@ static void pe_src_ready_run(int port) } } -static void pe_src_ready_exit(int port) -{ - /* - * If the Source is initiating an AMS then the Policy Engine Shall - * notify the Protocol Layer that the first Message in an AMS will - * follow. - */ - if (PE_CHK_FLAG(port, PE_FLAGS_LOCALLY_INITIATED_AMS)) - prl_start_ams(port); - -} - /** * PE_SRC_Disabled */ @@ -2670,9 +2650,8 @@ static void pe_snk_ready_entry(int port) { print_current_state(port); + /* Ensure any aborted message sequence is properly cleaned up */ PE_CLR_FLAG(port, PE_FLAGS_LOCALLY_INITIATED_AMS); - prl_end_ams(port); - /* * On entry to the PE_SNK_Ready state as the result of a wait, * then do the following: @@ -2882,16 +2861,6 @@ static void pe_snk_ready_run(int port) } } -static void pe_snk_ready_exit(int port) -{ - /* - * If the Sink is initiating an AMS then notify the Protocol Layer - * that the first Message in the AMS will follow - */ - if (PE_CHK_FLAG(port, PE_FLAGS_LOCALLY_INITIATED_AMS)) - prl_start_ams(port); -} - /** * PE_SNK_Hard_Reset */ @@ -3973,7 +3942,7 @@ static void pe_frs_snk_src_start_ams_entry(int port) pe_invalidate_explicit_contract(port); /* Inform Protocol Layer this is start of AMS */ - prl_start_ams(port); + PE_SET_FLAG(port, PE_FLAGS_LOCALLY_INITIATED_AMS); /* Shared PRS/FRS code, indicate FRS path */ PE_SET_FLAG(port, PE_FLAGS_FAST_ROLE_SWAP_PATH); @@ -5737,7 +5706,6 @@ static const struct usb_state pe_states[] = { [PE_SRC_READY] = { .entry = pe_src_ready_entry, .run = pe_src_ready_run, - .exit = pe_src_ready_exit, }, [PE_SRC_DISABLED] = { .entry = pe_src_disabled_entry, @@ -5780,7 +5748,6 @@ static const struct usb_state pe_states[] = { [PE_SNK_READY] = { .entry = pe_snk_ready_entry, .run = pe_snk_ready_run, - .exit = pe_snk_ready_exit, }, [PE_SNK_HARD_RESET] = { .entry = pe_snk_hard_reset_entry, diff --git a/common/usbc/usb_prl_sm.c b/common/usbc/usb_prl_sm.c index a3cf347ea8..955fb0a978 100644 --- a/common/usbc/usb_prl_sm.c +++ b/common/usbc/usb_prl_sm.c @@ -68,10 +68,10 @@ */ /* Flag to note message transmission completed */ #define PRL_FLAGS_TX_COMPLETE BIT(0) -/* Flag to note an AMS is being started by PE */ -#define PRL_FLAGS_START_AMS BIT(1) -/* Flag to note an AMS is being stopped by PE */ -#define PRL_FLAGS_END_AMS BIT(2) +/* Flag to note that PRL requested to set SINK_NG CC state */ +#define PRL_FLAGS_SINK_NG BIT(1) +/* Flag to note PRL waited for SINK_OK CC state before transmitting */ +#define PRL_FLAGS_WAIT_SINK_OK BIT(2) /* Flag to note transmission error occurred */ #define PRL_FLAGS_TX_ERROR BIT(3) /* Flag to note PE triggered a hard reset */ @@ -80,7 +80,10 @@ #define PRL_FLAGS_HARD_RESET_COMPLETE BIT(5) /* Flag to note port partner sent a hard reset */ #define PRL_FLAGS_PORT_PARTNER_HARD_RESET BIT(6) -/* Flag to note a message transmission has been requested */ +/* + * Flag to note a message transmission has been requested. It is only cleared + * when we send the message to the TCPC layer. + */ #define PRL_FLAGS_MSG_XMIT BIT(7) /* Flag to note a message was received */ #define PRL_FLAGS_MSG_RECEIVED BIT(8) @@ -531,16 +534,6 @@ void prl_set_debug_level(enum debug_level debug_level) #endif } -void prl_start_ams(int port) -{ - PRL_TX_SET_FLAG(port, PRL_FLAGS_START_AMS); -} - -void prl_end_ams(int port) -{ - PRL_TX_SET_FLAG(port, PRL_FLAGS_END_AMS); -} - void prl_hard_reset_complete(int port) { PRL_HR_SET_FLAG(port, PRL_FLAGS_HARD_RESET_COMPLETE); @@ -702,7 +695,7 @@ static void prl_copy_msg_to_buffer(int port) pdmsg[port].data_objs = (tx_emsg[port].len + 3) >> 2; } -static int pdmsg_xmit_type_is_rev30(const int port) +static __maybe_unused int pdmsg_xmit_type_is_rev30(const int port) { if (IS_ENABLED(CONFIG_USB_PD_REV30)) return ((pdmsg[port].xmit_type < NUM_SOP_STAR_TYPES) @@ -711,6 +704,13 @@ static int pdmsg_xmit_type_is_rev30(const int port) return 0; } +/* Returns true if the SOP port partner operates at PD rev3.0 */ +static bool is_sop_rev30(const int port) +{ + return IS_ENABLED(CONFIG_USB_PD_REV30) && + prl_get_rev(port, TCPC_TX_SOP) == PD_REV30; +} + /* Common Protocol Layer Message Transmission */ static void prl_tx_phy_layer_reset_entry(const int port) { @@ -720,13 +720,11 @@ static void prl_tx_phy_layer_reset_entry(const int port) || IS_ENABLED(CONFIG_USB_VPD)) { vpd_rx_enable(pd_is_connected(port)); } else { - tcpm_clear_pending_messages(port); + /* Purge any outstanding TX message */ + PRL_TX_CLR_FLAG(port, PRL_FLAGS_MSG_XMIT); + /* TODO(b/157228506): notify pe if needed */ tcpm_set_rx_enable(port, pd_is_connected(port)); } -} - -static void prl_tx_phy_layer_reset_run(const int port) -{ set_state_prl_tx(port, PRL_TX_WAIT_FOR_MESSAGE_REQUEST); } @@ -740,38 +738,40 @@ static void prl_tx_wait_for_message_request_entry(const int port) static void prl_tx_wait_for_message_request_run(const int port) { - if (IS_ENABLED(CONFIG_USB_PD_REV30) && - pdmsg_xmit_type_is_rev30(port) && PRL_TX_CHK_FLAG(port, - (PRL_FLAGS_START_AMS | PRL_FLAGS_END_AMS))) { + /* Clear any AMS flags and state if we are no longer in an AMS */ + if (IS_ENABLED(CONFIG_USB_PD_REV30) && !pe_in_local_ams(port)) { + /* Note PRL_Tx_Src_Sink_Tx is embedded here. */ + if (PRL_TX_CHK_FLAG(port, PRL_FLAGS_SINK_NG)) { + tcpm_select_rp_value(port, SINK_TX_OK); + tcpm_set_cc(port, TYPEC_CC_RP); + } + PRL_TX_CLR_FLAG(port, + PRL_FLAGS_SINK_NG | PRL_FLAGS_WAIT_SINK_OK); + } + + /* + * Check if we are starting an AMS and need to wait and/or set the CC + * lines appropriately. + */ + if (IS_ENABLED(CONFIG_USB_PD_REV30) && is_sop_rev30(port) && + pe_in_local_ams(port)) { if (pd_get_power_role(port) == PD_ROLE_SOURCE) { /* * Start of SRC AMS notification received from * Policy Engine */ - if (PRL_TX_CHK_FLAG(port, PRL_FLAGS_START_AMS)) { - PRL_TX_CLR_FLAG(port, PRL_FLAGS_START_AMS); + if (!PRL_TX_CHK_FLAG(port, PRL_FLAGS_SINK_NG)) { + PRL_TX_SET_FLAG(port, PRL_FLAGS_SINK_NG); set_state_prl_tx(port, PRL_TX_SRC_SOURCE_TX); return; } - /* - * End of SRC AMS notification received from - * Policy Engine - */ - else if (PRL_TX_CHK_FLAG(port, PRL_FLAGS_END_AMS)) { - /* Set Rp = SinkTxOk */ - tcpm_select_rp_value(port, SINK_TX_OK); - tcpm_set_cc(port, TYPEC_CC_RP); - prl_tx[port].retry_counter = 0; - /* PRL_FLAGS_END AMS is cleared here */ - prl_tx[port].flags = 0; - } } else { /* * Start of SNK AMS notification received from * Policy Engine */ - if (PRL_TX_CHK_FLAG(port, PRL_FLAGS_START_AMS)) { - PRL_TX_CLR_FLAG(port, PRL_FLAGS_START_AMS); + if (!PRL_TX_CHK_FLAG(port, PRL_FLAGS_WAIT_SINK_OK)) { + PRL_TX_SET_FLAG(port, PRL_FLAGS_WAIT_SINK_OK); /* * First Message in AMS notification * received from Policy Engine. @@ -779,18 +779,11 @@ static void prl_tx_wait_for_message_request_run(const int port) set_state_prl_tx(port, PRL_TX_SNK_START_AMS); return; } - /* - * End of SNK AMS notification received from - * Policy Engine - */ - else if (PRL_TX_CHK_FLAG(port, PRL_FLAGS_END_AMS)) { - prl_tx[port].retry_counter = 0; - /* PRL_FLAGS_END AMS is cleared here */ - prl_tx[port].flags = 0; - } - } - } else if (PRL_TX_CHK_FLAG(port, PRL_FLAGS_MSG_XMIT)) { + } + + /* Handle non Rev 3.0 or subsequent messages in AMS sequence */ + if (PRL_TX_CHK_FLAG(port, PRL_FLAGS_MSG_XMIT)) { PRL_TX_CLR_FLAG(port, PRL_FLAGS_MSG_XMIT); /* * Soft Reset Message Message pending @@ -851,8 +844,10 @@ static void prl_tx_src_source_tx_entry(const int port) static void prl_tx_src_source_tx_run(const int port) { if (PRL_TX_CHK_FLAG(port, PRL_FLAGS_MSG_XMIT)) { - PRL_TX_CLR_FLAG(port, PRL_FLAGS_MSG_XMIT); - + /* + * Don't clear pending XMIT flag here. Wait until we send so + * we can detect if we dropped this message or not. + */ set_state_prl_tx(port, PRL_TX_SRC_PENDING); } } @@ -868,8 +863,10 @@ static void prl_tx_snk_start_ams_entry(const int port) static void prl_tx_snk_start_ams_run(const int port) { if (PRL_TX_CHK_FLAG(port, PRL_FLAGS_MSG_XMIT)) { - PRL_TX_CLR_FLAG(port, PRL_FLAGS_MSG_XMIT); - + /* + * Don't clear pending XMIT flag here. Wait until we send so + * we can detect if we dropped this message or not. + */ set_state_prl_tx(port, PRL_TX_SNK_PENDING); } } @@ -1054,9 +1051,14 @@ static void prl_tx_src_pending_entry(const int port) static void prl_tx_src_pending_run(const int port) { - if (get_time().val > prl_tx[port].sink_tx_timer) { /* + * We clear the pending XMIT flag here right before we send so + * we can detect if we discarded this message or not + */ + PRL_TX_CLR_FLAG(port, PRL_FLAGS_MSG_XMIT); + + /* * Soft Reset Message pending & * SinkTxTimer timeout */ @@ -1088,9 +1090,16 @@ static void prl_tx_snk_pending_run(const int port) { enum tcpc_cc_voltage_status cc1, cc2; + /* Wait unit the SRC applies SINK_TX_OK so we can transmit */ tcpm_get_cc(port, &cc1, &cc2); if (cc1 == TYPEC_CC_VOLT_RP_3_0 || cc2 == TYPEC_CC_VOLT_RP_3_0) { /* + * We clear the pending XMIT flag here right before we send so + * we can detect if we discarded this message or not + */ + PRL_TX_CLR_FLAG(port, PRL_FLAGS_MSG_XMIT); + + /* * Soft Reset Message Message pending & * Rp = SinkTxOk */ @@ -1910,6 +1919,7 @@ static void prl_rx_wait_for_phy_message(const int port, int evt) PD_HEADER_PROLE(header) == PD_PLUG_FROM_DFP_UFP) return; + /* Handle incoming soft reset as special case */ if (cnt == 0 && type == PD_CTRL_SOFT_RESET) { int i; @@ -1920,9 +1930,6 @@ static void prl_rx_wait_for_phy_message(const int port, int evt) prl_rx[port].msg_id[i] = -1; } - /* Inform Policy Engine of Soft Reset */ - pe_got_soft_reset(port); - /* Soft Reset occurred */ set_state_prl_tx(port, PRL_TX_PHY_LAYER_RESET); @@ -1932,6 +1939,15 @@ static void prl_rx_wait_for_phy_message(const int port, int evt) set_state_tch(port, TCH_WAIT_FOR_MESSAGE_REQUEST_FROM_PE); } + + /* + * Inform Policy Engine of Soft Reset. Note perform this after + * performing the protocol layer reset, otherwise we will lose + * the PE's outgoing ACCEPT message to the soft reset. + */ + pe_got_soft_reset(port); + + return; } /* @@ -2010,7 +2026,6 @@ static void prl_rx_wait_for_phy_message(const int port, int evt) static const struct usb_state prl_tx_states[] = { [PRL_TX_PHY_LAYER_RESET] = { .entry = prl_tx_phy_layer_reset_entry, - .run = prl_tx_phy_layer_reset_run, }, [PRL_TX_WAIT_FOR_MESSAGE_REQUEST] = { .entry = prl_tx_wait_for_message_request_entry, diff --git a/include/usb_pe_sm.h b/include/usb_pe_sm.h index 90069f6cf6..e69eff572f 100644 --- a/include/usb_pe_sm.h +++ b/include/usb_pe_sm.h @@ -178,5 +178,16 @@ void pe_set_sysjump(void); * @param port USB-C port number */ void pe_invalidate_explicit_contract(int port); + +/* + * Return true if the PE is is within an atomic + * messaging sequence that it initiated with a SOP* port partner. + * + * Note the PRL layer polls this instead of using AMS_START and AMS_END + * notification from the PE that is called out by the spec + * + * @param port USB-C port number + */ +bool pe_in_local_ams(int port); #endif /* __CROS_EC_USB_PE_H */ diff --git a/include/usb_prl_sm.h b/include/usb_prl_sm.h index 2d24143221..ee357ccba2 100644 --- a/include/usb_prl_sm.h +++ b/include/usb_prl_sm.h @@ -112,20 +112,6 @@ void prl_hard_reset_complete(int port); */ void prl_execute_hard_reset(int port); -/** - * Informs the Protocol Layer to start an Atomic Message Sequence - * - * @param port USB-C port number - */ -void prl_start_ams(int port); - -/** - * Informs the Protocol Layer to end an Atomic Message Sequence - * - * @param port USB-C port number - */ -void prl_end_ams(int port); - #ifdef TEST_BUILD /** * Fake to track the last sent control message diff --git a/test/fake_prl.c b/test/fake_prl.c index ac64594e61..b4e2aaa11a 100644 --- a/test/fake_prl.c +++ b/test/fake_prl.c @@ -52,9 +52,6 @@ void prl_set_rev(int port, enum tcpm_transmit_type partner, enum pd_rev_type rev) {} -void prl_start_ams(int port) -{} - enum pd_ctrl_msg_type fake_prl_get_last_sent_ctrl_msg(int port) { diff --git a/test/usb_prl.c b/test/usb_prl.c index 3ada5adfdb..51b40d25d7 100644 --- a/test/usb_prl.c +++ b/test/usb_prl.c @@ -769,6 +769,12 @@ void pe_got_soft_reset(int port) pd_port[port].mock_got_soft_reset = 1; } +bool pe_in_local_ams(int port) +{ + /* We will probably want to change this in the future */ + return false; +} + static int test_prl_reset(void) { int port = PORT0; @@ -1072,11 +1078,6 @@ static int test_send_extended_data_msg(void) static int test_receive_soft_reset_msg(void) { int port = PORT0; - int expected_header = PD_HEADER(PD_CTRL_SOFT_RESET, - pd_port[port].power_role, - pd_port[port].data_role, - pd_port[port].msg_rx_id, - 0, pd_port[port].rev, 0); enable_prl(port, 1); @@ -1103,9 +1104,7 @@ static int test_receive_soft_reset_msg(void) TEST_ASSERT(pd_port[port].mock_got_soft_reset); TEST_ASSERT(pd_port[port].mock_pe_error < 0); - TEST_ASSERT(pd_port[port].mock_pe_message_received); - TEST_ASSERT(expected_header == rx_emsg[port].header); - TEST_ASSERT(rx_emsg[port].len == 0); + TEST_EQ(pd_port[port].mock_pe_message_received, 0, "%d"); enable_prl(port, 0); |