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>

3 files changed, 91 insertions, 103 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,