tcpmv2: Only issue SOP' soft reset following PR swap

Entry to the PE_SNK_Startup and PE_SRC_Startup states resets the
protocol layer, and clears the message IDs for all SOP types.

These states are entered during initial connect, hard reset, and after a
power role swap. The cable should automatically clear it's message IDs
on the initial connect (because it didn't have power) and on a hard
reset, so modify the PE policy to only send SOP' soft reset after power
role swaps.

BUG=b:179325862
BRANCH=volteer
TEST=Connect TBT loopback device, verify no SOP' soft reset is sent and
that TBT entry is successful.
TEST=Connect USB+DP monitor when EC starts as SRC/DFP and VCONN source.
Verify EC sends SOP' soft reset is sent after power role swap.

Signed-off-by: Keith Short <keithshort@chromium.org>
Change-Id: I2c133eec0e76c1ecb7b79cea94a541fdb55ee9c1
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2691423
Reviewed-by: Abe Levkoy <alevkoy@chromium.org>
Commit-Queue: Abe Levkoy <alevkoy@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2697852
Tested-by: Abe Levkoy <alevkoy@chromium.org>

3 files changed, 26 insertions, 32 deletions

diff --git a/common/usbc/usb_pe_drp_sm.c b/common/usbc/usb_pe_drp_sm.c
index 7d1e7ddc19..e56ada0a48 100644
--- a/common/usbc/usb_pe_drp_sm.c
+++ b/common/usbc/usb_pe_drp_sm.c
@@ -2168,13 +2168,6 @@ static void pe_src_startup_entry(int port)
 	/* Reset the protocol layer */
 	prl_reset_soft(port);
 
-	/*
-	 * Protocol layer reset clears the message IDs for all SOP types.
-	 * Indicate that a SOP' soft reset is required before any other
-	 * messages are sent to the cable.
-	 */
-	pd_dpm_request(port, DPM_REQUEST_SOP_PRIME_SOFT_RESET_SEND);
-
 	/* Set initial data role */
 	pe[port].data_role = pd_get_data_role(port);
 
@@ -2186,6 +2179,19 @@ static void pe_src_startup_entry(int port)
 
 	if (PE_CHK_FLAG(port, PE_FLAGS_PR_SWAP_COMPLETE)) {
 		PE_CLR_FLAG(port, PE_FLAGS_PR_SWAP_COMPLETE);
+		/*
+		 * Protocol layer reset clears the message IDs for all SOP
+		 * types. Indicate that a SOP' soft reset is required before any
+		 * other messages are sent to the cable.
+		 *
+		 * Note that other paths into this state are for the initial
+		 * connection and for a hard reset. In both cases the cable
+		 * should also automatically clear the message IDs so don't
+		 * generate an SOP' soft reset for those cases. Sending
+		 * unnecessary SOP' soft resets causes bad behavior with
+		 * some devices. See b/179325862.
+		 */
+		pd_dpm_request(port, DPM_REQUEST_SOP_PRIME_SOFT_RESET_SEND);
 
 		/* Start SwapSourceStartTimer */
 		pe[port].swap_source_start_timer = get_time().val +
@@ -2994,13 +3000,6 @@ static void pe_snk_startup_entry(int port)
 	/* Reset the protocol layer */
 	prl_reset_soft(port);
 
-	/*
-	 * Protocol layer reset clears the message IDs for all SOP types.
-	 * Indicate that a SOP' soft reset is required before any other
-	 * messages are sent to the cable.
-	 */
-	pd_dpm_request(port, DPM_REQUEST_SOP_PRIME_SOFT_RESET_SEND);
-
 	/* Set initial data role */
 	pe[port].data_role = pd_get_data_role(port);
 
@@ -3012,6 +3011,19 @@ static void pe_snk_startup_entry(int port)
 
 	if (PE_CHK_FLAG(port, PE_FLAGS_PR_SWAP_COMPLETE)) {
 		PE_CLR_FLAG(port, PE_FLAGS_PR_SWAP_COMPLETE);
+		/*
+		 * Protocol layer reset clears the message IDs for all SOP
+		 * types. Indicate that a SOP' soft reset is required before any
+		 * other messages are sent to the cable.
+		 *
+		 * Note that other paths into this state are for the initial
+		 * connection and for a hard reset. In both cases the cable
+		 * should also automatically clear the message IDs so don't
+		 * generate an SOP' soft reset for those cases. Sending
+		 * unnecessary SOP' soft resets causes bad behavior with
+		 * some devices. See b/179325862.
+		 */
+		pd_dpm_request(port, DPM_REQUEST_SOP_PRIME_SOFT_RESET_SEND);
 
 		/*
 		 * Some port partners may violate spec and attempt to
diff --git a/test/usb_pe_drp.c b/test/usb_pe_drp.c
index beb54c82cb..2644ded63b 100644
--- a/test/usb_pe_drp.c
+++ b/test/usb_pe_drp.c
@@ -110,16 +110,6 @@ test_static int finish_src_discovery(int startup_cable_probes)
 		   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.
 	 */
diff --git a/test/usb_tcpmv2_compliance_common.c b/test/usb_tcpmv2_compliance_common.c
index 0a1449ede3..5273948de3 100644
--- a/test/usb_tcpmv2_compliance_common.c
+++ b/test/usb_tcpmv2_compliance_common.c
@@ -379,14 +379,6 @@ int handle_attach_expected_msgs(enum pd_data_role data_role)
 		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");
-		mock_set_alert(TCPC_REG_ALERT_TX_SUCCESS);
-		task_wait_event(10 * MSEC);
-		partner_send_msg(PD_MSG_SOP_PRIME, PD_CTRL_NOT_SUPPORTED, 0, 0,
-			NULL);
-
 		TEST_EQ(verify_tcpci_transmit(TCPC_TX_SOP_PRIME, 0,
 				PD_DATA_VENDOR_DEF),
 			EC_SUCCESS, "%d");