TCPMv2: Process DPMs before discovery

Some devices may reply to discovery differently if they're not yet being
offered sufficient current.  Process DPM requests before running
discovery in order to get these devices into the best state for
discovery probing.

This also moves the cable soft reset DPM request up to the common DPM
request function.  Otherwise, it will be considered an unhandled request
and dropped before attempting discovery.

BRANCH=volteer
BUG=b:177303467,b:177001425
TEST=verify we can reliably receive DiscoverSVIDs ACK from LaCie SSD

Signed-off-by: Diana Z <dzigterman@chromium.org>
Change-Id: I0917476c2aeb0cb9551edde3df14ddd1fd47fcab
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2627805
Tested-by: Utkarsh H Patel <utkarsh.h.patel@intel.com>
Reviewed-by: Abe Levkoy <alevkoy@chromium.org>
Reviewed-by: Keith Short <keithshort@chromium.org>

3 files changed, 55 insertions, 79 deletions

diff --git a/common/usbc/usb_pe_drp_sm.c b/common/usbc/usb_pe_drp_sm.c
index 39a335188f..76bdb82a1a 100644
--- a/common/usbc/usb_pe_drp_sm.c
+++ b/common/usbc/usb_pe_drp_sm.c
@@ -1556,7 +1556,15 @@ static bool common_src_snk_dpm_requests(int port)
 					DPM_REQUEST_GET_SNK_CAPS);
 		set_state_pe(port, PE_DR_GET_SINK_CAP);
 		return true;
+	} else if (PE_CHK_DPM_REQUEST(port,
+				      DPM_REQUEST_SOP_PRIME_SOFT_RESET_SEND)) {
+		pe_set_dpm_curr_request(port,
+			DPM_REQUEST_SOP_PRIME_SOFT_RESET_SEND);
+		pe[port].tx_type = TCPC_TX_SOP_PRIME;
+		set_state_pe(port, PE_VCS_CBL_SEND_SOFT_RESET);
+		return true;
 	}
+
 	return false;
 }
 
@@ -1724,18 +1732,6 @@ __maybe_unused static bool pe_attempt_port_discovery(int port)
 		}
 	}
 
-	/*
-	 * TODO(b/177001425): TCPMv2 - move SOP' soft reset check into
-	 * common_src_snk_dpm_requests()
-	 */
-	if (PE_CHK_DPM_REQUEST(port, DPM_REQUEST_SOP_PRIME_SOFT_RESET_SEND)) {
-		pe_set_dpm_curr_request(port,
-			DPM_REQUEST_SOP_PRIME_SOFT_RESET_SEND);
-		pe[port].tx_type = TCPC_TX_SOP_PRIME;
-		set_state_pe(port, PE_VCS_CBL_SEND_SOFT_RESET);
-		return true;
-	}
-
 	/* If mode entry was successful, disable the timer */
 	if (PE_CHK_FLAG(port, PE_FLAGS_VDM_SETUP_DONE)) {
 		pe[port].discover_identity_timer = TIMER_DISABLED;
@@ -2566,13 +2562,6 @@ static void pe_src_ready_run(int port)
 		pe[port].wait_and_add_jitter_timer = TIMER_DISABLED;
 
 		/*
-		 * Attempt discovery if possible, and return if state was
-		 * changed for that discovery.
-		 */
-		if (pe_attempt_port_discovery(port))
-			return;
-
-		/*
 		 * Handle Device Policy Manager Requests
 		 */
 
@@ -2634,6 +2623,13 @@ static void pe_src_ready_run(int port)
 			return;
 		}
 
+		/*
+		 * Attempt discovery if possible, and return if state was
+		 * changed for that discovery.
+		 */
+		if (pe_attempt_port_discovery(port))
+			return;
+
 		/* No DPM requests; attempt mode entry/exit if needed */
 		dpm_run(port);
 	}
@@ -3406,13 +3402,6 @@ static void pe_snk_ready_run(int port)
 		}
 
 		/*
-		 * Attempt discovery if possible, and return if state was
-		 * changed for that discovery.
-		 */
-		if (pe_attempt_port_discovery(port))
-			return;
-
-		/*
 		 * Handle Device Policy Manager Requests
 		 */
 		/*
@@ -3464,6 +3453,13 @@ static void pe_snk_ready_run(int port)
 			return;
 		}
 
+		/*
+		 * Attempt discovery if possible, and return if state was
+		 * changed for that discovery.
+		 */
+		if (pe_attempt_port_discovery(port))
+			return;
+
 		/* No DPM requests; attempt mode entry/exit if needed */
 		dpm_run(port);
 
diff --git a/test/usb_pe_drp.c b/test/usb_pe_drp.c
index 8138bb5ddf..0b22c41c9c 100644
--- a/test/usb_pe_drp.c
+++ b/test/usb_pe_drp.c
@@ -78,8 +78,11 @@ test_static void rx_message(enum pd_msg_type sop,
 /*
  * This sequence is used by multiple tests, so pull out into a function to
  * avoid duplication.
+ *
+ * Send in how many SOP' DiscoverIdentity requests have been processed so far,
+ * as this may vary depending on startup sequencing as a source.
  */
-test_static int finish_src_discovery(void)
+test_static int finish_src_discovery(int startup_cable_probes)
 {
 	int i;
 
@@ -106,10 +109,20 @@ test_static int finish_src_discovery(void)
 		   PDO_FIXED(5000, 500, PDO_FIXED_COMM_CAP));
 
 	/*
+	 * Cable soft reset is always issued after entry into Src/Snk_Ready
+	 * simulate no cable response.
+	 */
+	TEST_EQ(mock_prl_wait_for_tx_msg(PORT0, TCPC_TX_SOP_PRIME,
+					 PD_CTRL_SOFT_RESET, 0,
+					 60 * MSEC),
+					 EC_SUCCESS, "%d");
+	mock_prl_report_error(PORT0, ERR_TCH_XMIT, TCPC_TX_SOP_PRIME);
+
+	/*
 	 * Cable identity discovery is attempted 6 times total. 1 was done
 	 * above, so expect 5 more now.
 	 */
-	for (i = 0; i < 5; i++) {
+	for (i = startup_cable_probes; i < 6; i++) {
 		TEST_EQ(mock_prl_wait_for_tx_msg(PORT0, TCPC_TX_SOP_PRIME,
 						 0, PD_DATA_VENDOR_DEF,
 						 60 * MSEC),
@@ -189,17 +202,7 @@ test_static int test_send_caps_error_before_connected(void)
 		EC_SUCCESS, "%d");
 	mock_prl_message_sent(PORT0);
 
-	/*
-	 * Cable soft reset is always issued after entry into Src/Snk_Ready
-	 * simulate no cable response.
-	 */
-	TEST_EQ(mock_prl_wait_for_tx_msg(PORT0, TCPC_TX_SOP_PRIME,
-					 PD_CTRL_SOFT_RESET, 0,
-					 60 * MSEC),
-					 EC_SUCCESS, "%d");
-	mock_prl_report_error(PORT0, ERR_TCH_XMIT, TCPC_TX_SOP_PRIME);
-
-	TEST_EQ(finish_src_discovery(), EC_SUCCESS, "%d");
+	TEST_EQ(finish_src_discovery(1), EC_SUCCESS, "%d");
 
 	task_wait_event(5 * SECOND);
 
@@ -234,30 +237,7 @@ test_static int test_send_caps_error_when_connected(void)
 		EC_SUCCESS, "%d");
 	mock_prl_message_sent(PORT0);
 
-	/*
-	 * Cable soft reset is always issued after entry into Src/Snk_Ready
-	 * simulate no cable response.
-	 */
-	TEST_EQ(mock_prl_wait_for_tx_msg(PORT0, TCPC_TX_SOP_PRIME,
-					 PD_CTRL_SOFT_RESET, 0,
-					 60 * MSEC),
-					 EC_SUCCESS, "%d");
-	mock_prl_report_error(PORT0, ERR_TCH_XMIT, TCPC_TX_SOP_PRIME);
-
-	/*
-	 * Expect VENDOR_DEF for cable identity, simulate no cable (so no
-	 * GoodCRC, so ERR_TCH_XMIT). Don't reply NOT_SUPPORTED, since the spec
-	 * says a cable never does that.
-	 * TODO: Add tests for cable replying to identity, and replying
-	 * NOT_SUPPORTED (since we should be robust to cables doing the wrong
-	 * thing).
-	 */
-	TEST_EQ(mock_prl_wait_for_tx_msg(PORT0, TCPC_TX_SOP_PRIME,
-					 0, PD_DATA_VENDOR_DEF, 10 * MSEC),
-		EC_SUCCESS, "%d");
-	mock_prl_report_error(PORT0, ERR_TCH_XMIT, TCPC_TX_SOP_PRIME);
-
-	TEST_EQ(finish_src_discovery(), EC_SUCCESS, "%d");
+	TEST_EQ(finish_src_discovery(0), EC_SUCCESS, "%d");
 
 	task_wait_event(5 * SECOND);
 
diff --git a/test/usb_tcpmv2_compliance_common.c b/test/usb_tcpmv2_compliance_common.c
index a5ff2348d4..0a1449ede3 100644
--- a/test/usb_tcpmv2_compliance_common.c
+++ b/test/usb_tcpmv2_compliance_common.c
@@ -365,6 +365,20 @@ int proc_pd_e3(void)
 int handle_attach_expected_msgs(enum pd_data_role data_role)
 {
 	if (data_role == PD_ROLE_DFP) {
+		TEST_EQ(verify_tcpci_transmit(TCPC_TX_SOP,
+				PD_CTRL_GET_SOURCE_CAP, 0),
+			EC_SUCCESS, "%d");
+		mock_set_alert(TCPC_REG_ALERT_TX_SUCCESS);
+		task_wait_event(10 * MSEC);
+		partner_send_msg(PD_MSG_SOP, PD_DATA_SOURCE_CAP, 1, 0, &pdo);
+
+		TEST_EQ(verify_tcpci_transmit(TCPC_TX_SOP,
+				PD_CTRL_GET_SINK_CAP, 0),
+			EC_SUCCESS, "%d");
+		mock_set_alert(TCPC_REG_ALERT_TX_SUCCESS);
+		task_wait_event(10 * MSEC);
+		partner_send_msg(PD_MSG_SOP, PD_DATA_SINK_CAP, 1, 0, &pdo);
+
 		TEST_EQ(verify_tcpci_transmit(TCPC_TX_SOP_PRIME,
 				PD_CTRL_SOFT_RESET, 0),
 			EC_SUCCESS, "%d");
@@ -387,13 +401,8 @@ int handle_attach_expected_msgs(enum pd_data_role data_role)
 		mock_set_alert(TCPC_REG_ALERT_TX_SUCCESS);
 		task_wait_event(10 * MSEC);
 		partner_send_msg(PD_MSG_SOP, PD_CTRL_NOT_SUPPORTED, 0, 0, NULL);
-
-		TEST_EQ(verify_tcpci_transmit(TCPC_TX_SOP,
-				PD_CTRL_GET_SOURCE_CAP, 0),
-			EC_SUCCESS, "%d");
-		mock_set_alert(TCPC_REG_ALERT_TX_SUCCESS);
-		task_wait_event(10 * MSEC);
-		partner_send_msg(PD_MSG_SOP, PD_DATA_SOURCE_CAP, 1, 0, &pdo);
+	} else if (data_role == PD_ROLE_UFP) {
+		int vcs;
 
 		TEST_EQ(verify_tcpci_transmit(TCPC_TX_SOP,
 				PD_CTRL_GET_SINK_CAP, 0),
@@ -401,8 +410,6 @@ int handle_attach_expected_msgs(enum pd_data_role data_role)
 		mock_set_alert(TCPC_REG_ALERT_TX_SUCCESS);
 		task_wait_event(10 * MSEC);
 		partner_send_msg(PD_MSG_SOP, PD_DATA_SINK_CAP, 1, 0, &pdo);
-	} else if (data_role == PD_ROLE_UFP) {
-		int vcs;
 
 		TEST_EQ(verify_tcpci_transmit(TCPC_TX_SOP, PD_CTRL_DR_SWAP, 0),
 			EC_SUCCESS, "%d");
@@ -419,13 +426,6 @@ int handle_attach_expected_msgs(enum pd_data_role data_role)
 			partner_send_msg(PD_MSG_SOP, PD_CTRL_REJECT, 0, 0,
 				NULL);
 		}
-
-		TEST_EQ(verify_tcpci_transmit(TCPC_TX_SOP,
-				PD_CTRL_GET_SINK_CAP, 0),
-			EC_SUCCESS, "%d");
-		mock_set_alert(TCPC_REG_ALERT_TX_SUCCESS);
-		task_wait_event(10 * MSEC);
-		partner_send_msg(PD_MSG_SOP, PD_DATA_SINK_CAP, 1, 0, &pdo);
 	}
 	task_wait_event(1 * SECOND);
 	return EC_SUCCESS;