PD 3.0: Add AMS start detection to collision avoidance

When an explicit contract is in place for PD 3.0, the source shall
control the Rp value in order to facilitate collision avoidance.  When
the value is set to 1.5 A, the sink can only respond to an existing AMS,
and not start a new one.

An Atomic Message Sequence (AMS) is defined as "a Message sequence that
starts and/or ends in either the PE_SRC_Ready, PE_SNK_Ready or
PE_CBL_Ready
states."  This means any given PD message may be starting an AMS
(requiring the source to set Rp, and sink to check the CC level) or it
may be a response within an AMS (in which case, sink may send regardless
of CC levels).

This change adjusts the pd_transmit() calls to indicate whether any
given PD message is the beginning of an AMS.  There are many AMS's
defined, which may be found in section 8.3.2 of the PD 3.0 spec.
Anytime the source returns to its ready state, it will reset Rp to
reflect that the sink may start an AMS.

Additionally, this removes the buffer for sending PD messages.  If an
AMS cannot be started, then it's better to fail the send so the pd_task
state machine can handle the unsent message.

BRANCH=None
BUG=b:64411727, b:147476471
TEST=Tested with bip board with PD 3.0 config using 2 different PD 3.0
hubs. Monitored Twinkie output with hubs acting as source, sink, and
power swapping to source.  Also turned off PD 3.0 to ensure PD 2.0
behavior with the hubs was unaffected.
TEST=Tested on volteer board with PD 3.0 config using Thunderbolt
capable dock.

Change-Id: Ib02670add1a125217a981a846e6e2c31681de169
Signed-off-by: Diana Z <dzigterman@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/1246273
Tested-by: Keith Short <keithshort@chromium.org>
Reviewed-by: Jett Rink <jettrink@chromium.org>
Commit-Queue: Keith Short <keithshort@chromium.org>

2 files changed, 113 insertions, 110 deletions

diff --git a/common/usb_pd_protocol.c b/common/usb_pd_protocol.c
index 2e501cfe62..edf6cee255 100644
--- a/common/usb_pd_protocol.c
+++ b/common/usb_pd_protocol.c
@@ -113,6 +113,11 @@ static const int debug_level;
  */
 #define SRC_READY_HOLD_OFF_US (400 * MSEC)
 
+enum ams_seq {
+	AMS_START,
+	AMS_RESPONSE,
+};
+
 enum vdm_states {
 	VDM_STATE_ERR_BUSY = -3,
 	VDM_STATE_ERR_SEND = -2,
@@ -253,11 +258,6 @@ static struct pd_protocol {
 	uint32_t dev_rw_hash[PD_RW_HASH_SIZE/4];
 	enum ec_current_image current_image;
 #ifdef CONFIG_USB_PD_REV30
-	/* PD Collision avoidance buffer */
-	uint16_t ca_buffered;
-	uint16_t ca_header;
-	uint32_t ca_buffer[PDO_MAX_OBJECTS];
-	enum tcpm_transmit_type ca_type;
 	/* protocol revision */
 	uint8_t rev;
 #endif
@@ -428,6 +428,19 @@ static void set_vconn(int port, int enable)
 }
 #endif /* defined(CONFIG_USBC_VCONN) */
 
+#ifdef CONFIG_USB_PD_REV30
+/* Note: rp should be set to either SINK_TX_OK or SINK_TX_NG */
+static void sink_can_xmit(int port, int rp)
+{
+	tcpm_select_rp_value(port, rp);
+	tcpm_set_cc(port, TYPEC_CC_RP);
+
+	/* We must wait tSinkTx before sending a message */
+	if (rp == SINK_TX_NG)
+		usleep(PD_T_SINK_TX);
+}
+#endif
+
 #ifdef CONFIG_USB_PD_TCPC_LOW_POWER
 
 /* 10 ms is enough time for any TCPC transaction to complete. */
@@ -863,6 +876,15 @@ static inline void set_state(int port, enum pd_states next_state)
 			hook_notify(HOOK_USB_PD_DISCONNECT);
 	}
 
+#ifdef CONFIG_USB_PD_REV30
+	/* Upon entering SRC_READY, it is safe for the sink to transmit */
+	if (next_state == PD_STATE_SRC_READY) {
+		if (pd[port].rev == PD_REV30 &&
+			pd[port].flags & PD_FLAGS_EXPLICIT_CONTRACT)
+			sink_can_xmit(port, SINK_TX_OK);
+	}
+#endif
+
 #ifdef CONFIG_LOW_POWER_IDLE
 	/* If a PD device is attached then disable deep sleep */
 	for (i = 0; i < board_get_usb_pd_port_count(); i++) {
@@ -888,19 +910,6 @@ static void inc_id(int port)
 	pd[port].msg_id = (pd[port].msg_id + 1) & PD_MESSAGE_ID_COUNT;
 }
 
-#ifdef CONFIG_USB_PD_REV30
-static void sink_can_xmit(int port, int rp)
-{
-	tcpm_select_rp_value(port, rp);
-	tcpm_set_cc(port, TYPEC_CC_RP);
-}
-
-static inline void pd_ca_reset(int port)
-{
-	pd[port].ca_buffered = 0;
-}
-#endif
-
 void pd_transmit_complete(int port, int status)
 {
 	if (status == TCPC_TX_COMPLETE_SUCCESS)
@@ -911,9 +920,13 @@ void pd_transmit_complete(int port, int status)
 }
 
 static int pd_transmit(int port, enum tcpm_transmit_type type,
-		       uint16_t header, const uint32_t *data)
+		       uint16_t header, const uint32_t *data, enum ams_seq ams)
 {
 	int evt;
+	int res;
+#ifdef CONFIG_USB_PD_REV30
+	int sink_ng = 0;
+#endif
 
 	/* If comms are disabled, do not transmit, return error */
 	if (!pd_comm_is_enabled(port))
@@ -928,32 +941,37 @@ static int pd_transmit(int port, enum tcpm_transmit_type type,
 #ifdef CONFIG_USB_PD_REV30
 	/* Source-coordinated collision avoidance */
 	/*
+	 * USB PD Rev 3.0, Version 2.0: Section 2.7.3.2
+	 * Collision Avoidance - Protocol Layer
+	 *
 	 * In order to avoid message collisions due to asynchronous Messaging
-	 * sent from the Sink, the Source sets Rp to SinkTxOk to indicate to
-	 * the Sink that it is ok to initiate an AMS. When the Source wishes
-	 * to initiate an AMS it sets Rp to SinkTxNG. When the Sink detects
-	 * that Rp is set to SinkTxOk it May initiate an AMS. When the Sink
-	 * detects that Rp is set to SinkTxNG it Shall Not initiate an AMS
-	 * and Shall only send Messages that are part of an AMS the Source has
-	 * initiated. Note that this restriction applies to SOP* AMS’s i.e.
-	 * for both Port to Port and Port to Cable Plug communications.
+	 * sent from the Sink, the Source sets Rp to SinkTxOk (3A) to indicate
+	 * to the Sink that it is ok to initiate an AMS. When the Source wishes
+	 * to initiate an AMS, it sets Rp to SinkTxNG (1.5A).
+	 * When the Sink detects that Rp is set to SinkTxOk, it May initiate an
+	 * AMS. When the Sink detects that Rp is set to SinkTxNG it Shall Not
+	 * initiate an AMS and Shall only send Messages that are part of an AMS
+	 * the Source has initiated.
+	 * Note that this restriction applies to SOP* AMS’s i.e. for both Port
+	 * to Port and Port to Cable Plug communications.
 	 *
-	 * This starts after an Explicit Contract is in place
-	 * PD R3 V1.1 Section 2.5.2.
+	 * This starts after an Explicit Contract is in place (see section 2.5.2
+	 * SOP* Collision Avoidance).
 	 *
 	 * Note: a Sink can still send Hard Reset signaling at any time.
 	 */
-	if ((pd[port].rev == PD_REV30) &&
+	if ((pd[port].rev == PD_REV30) && ams == AMS_START &&
 		(pd[port].flags & PD_FLAGS_EXPLICIT_CONTRACT)) {
 		if (pd[port].power_role == PD_ROLE_SOURCE) {
 			/*
 			 * Inform Sink that it can't transmit. If a sink
-			 * transmition is in progress and a collsion occurs,
+			 * transmission is in progress and a collision occurs,
 			 * a reset is generated. This should be rare because
 			 * all extended messages are chunked. This effectively
 			 * defaults to PD REV 2.0 collision avoidance.
 			 */
 			sink_can_xmit(port, SINK_TX_NG);
+			sink_ng = 1;
 		} else if (type != TCPC_TX_HARD_RESET) {
 			enum tcpc_cc_voltage_status cc1, cc2;
 
@@ -961,18 +979,8 @@ static int pd_transmit(int port, enum tcpm_transmit_type type,
 			if (cc1 == TYPEC_CC_VOLT_RP_1_5 ||
 				cc2 == TYPEC_CC_VOLT_RP_1_5) {
 				/* Sink can't transmit now. */
-				/* Check if message is already buffered. */
-				if (pd[port].ca_buffered)
-					return -1;
-
-				/* Buffer message and send later. */
-				pd[port].ca_type = type;
-				pd[port].ca_header = header;
-				memcpy(pd[port].ca_buffer,
-					data, sizeof(uint32_t) *
-					PD_HEADER_CNT(header));
-				pd[port].ca_buffered = 1;
-				return 1;
+				/* Return failure, pd_task can retry later */
+				return -1;
 			}
 		}
 	}
@@ -982,46 +990,19 @@ static int pd_transmit(int port, enum tcpm_transmit_type type,
 	/* Wait until TX is complete */
 	evt = task_wait_event_mask(PD_EVENT_TX, PD_T_TCPC_TX_TIMEOUT);
 
-#ifdef CONFIG_USB_PD_REV30
-	/*
-	 * If the source just completed a transmit, tell
-	 * the sink it can transmit if it wants to.
-	 */
-	if ((pd[port].rev == PD_REV30) &&
-			(pd[port].power_role == PD_ROLE_SOURCE) &&
-			(pd[port].flags & PD_FLAGS_EXPLICIT_CONTRACT)) {
-		sink_can_xmit(port, SINK_TX_OK);
-	}
-#endif
-
 	if (evt & TASK_EVENT_TIMER)
 		return -1;
 
 	/* TODO: give different error condition for failed vs discarded */
-	return pd[port].tx_status == TCPC_TX_COMPLETE_SUCCESS ? 1 : -1;
-}
+	res = pd[port].tx_status == TCPC_TX_COMPLETE_SUCCESS ? 1 : -1;
 
 #ifdef CONFIG_USB_PD_REV30
-static void pd_ca_send_pending(int port)
-{
-	enum tcpc_cc_voltage_status cc1, cc2;
-
-	/* Check if a message has been buffered. */
-	if (!pd[port].ca_buffered)
-		return;
-
-	tcpm_get_cc(port, &cc1, &cc2);
-	if ((cc1 != TYPEC_CC_VOLT_RP_1_5) &&
-			(cc2 != TYPEC_CC_VOLT_RP_1_5))
-		if (pd_transmit(port, pd[port].ca_type,
-				pd[port].ca_header,
-				pd[port].ca_buffer) < 0)
-			return;
-
-	/* Message was sent, so free up the buffer. */
-	pd[port].ca_buffered = 0;
-}
+	/* If the AMS transaction failed to start, reset CC to OK */
+	if (res < 0 && sink_ng)
+		sink_can_xmit(port, SINK_TX_OK);
 #endif
+	return res;
+}
 
 static void pd_update_roles(int port)
 {
@@ -1035,15 +1016,27 @@ static int send_control(int port, int type)
 	uint16_t header = PD_HEADER(type, pd[port].power_role,
 				pd[port].data_role, pd[port].msg_id, 0,
 				pd_get_rev(port), 0);
+	/*
+	 * For PD 3.0, collision avoidance logic needs to know if this message
+	 * will begin a new Atomic Message Sequence (AMS)
+	 */
+	enum ams_seq ams = ((1 << type) & PD_CTRL_AMS_START_MASK)
+			    ? AMS_START : AMS_RESPONSE;
 
-	bit_len = pd_transmit(port, TCPC_TX_SOP, header, NULL);
+
+	bit_len = pd_transmit(port, TCPC_TX_SOP, header, NULL, ams);
 	if (debug_level >= 2)
 		CPRINTF("C%d CTRL[%d]>%d\n", port, type, bit_len);
 
 	return bit_len;
 }
 
-static int send_source_cap(int port)
+/*
+ * Note: Source capabilities may either be in an existing AMS (ex. as a
+ * response to Get_Source_Cap), or the beginning of an AMS for a power
+ * negotiation.
+ */
+static int send_source_cap(int port, enum ams_seq ams)
 {
 	int bit_len;
 #if defined(CONFIG_USB_PD_DYNAMIC_SRC_CAP) || \
@@ -1066,7 +1059,7 @@ static int send_source_cap(int port)
 			pd[port].data_role, pd[port].msg_id, src_pdo_cnt,
 			pd_get_rev(port), 0);
 
-	bit_len = pd_transmit(port, TCPC_TX_SOP, header, src_pdo);
+	bit_len = pd_transmit(port, TCPC_TX_SOP, header, src_pdo, ams);
 	if (debug_level >= 2)
 		CPRINTF("C%d srcCAP>%d\n", port, bit_len);
 
@@ -1153,7 +1146,8 @@ static int send_battery_cap(int port, uint32_t *payload)
 		}
 	}
 
-	bit_len = pd_transmit(port, TCPC_TX_SOP, header, (uint32_t *)msg);
+	bit_len = pd_transmit(port, TCPC_TX_SOP, header, (uint32_t *)msg,
+			      AMS_RESPONSE);
 	if (debug_level >= 2)
 		CPRINTF("C%d batCap>%d\n", port, bit_len);
 	return bit_len;
@@ -1220,7 +1214,7 @@ static int send_battery_status(int port,  uint32_t *payload)
 		msg = BSDO_CAP(BSDO_CAP_UNKNOWN);
 	}
 
-	bit_len = pd_transmit(port, TCPC_TX_SOP, header, &msg);
+	bit_len = pd_transmit(port, TCPC_TX_SOP, header, &msg, AMS_RESPONSE);
 	if (debug_level >= 2)
 		CPRINTF("C%d batStat>%d\n", port, bit_len);
 
@@ -1236,7 +1230,8 @@ static void send_sink_cap(int port)
 			pd[port].data_role, pd[port].msg_id, pd_snk_pdo_cnt,
 			pd_get_rev(port), 0);
 
-	bit_len = pd_transmit(port, TCPC_TX_SOP, header, pd_snk_pdo);
+	bit_len = pd_transmit(port, TCPC_TX_SOP, header, pd_snk_pdo,
+			      AMS_RESPONSE);
 	if (debug_level >= 2)
 		CPRINTF("C%d snkCAP>%d\n", port, bit_len);
 }
@@ -1248,7 +1243,8 @@ static int send_request(int port, uint32_t rdo)
 			pd[port].data_role, pd[port].msg_id, 1,
 			pd_get_rev(port), 0);
 
-	bit_len = pd_transmit(port, TCPC_TX_SOP, header, &rdo);
+	/* Note: ams will need to be AMS_START if used for PPS keep alive */
+	bit_len = pd_transmit(port, TCPC_TX_SOP, header, &rdo, AMS_RESPONSE);
 	if (debug_level >= 2)
 		CPRINTF("C%d REQ>%d\n", port, bit_len);
 
@@ -1267,7 +1263,7 @@ static int send_bist_cmd(int port)
 			pd[port].data_role, pd[port].msg_id, 1,
 			pd_get_rev(port), 0);
 
-	bit_len = pd_transmit(port, TCPC_TX_SOP, header, &bdo);
+	bit_len = pd_transmit(port, TCPC_TX_SOP, header, &bdo, AMS_START);
 	CPRINTF("C%d BIST>%d\n", port, bit_len);
 
 	return bit_len;
@@ -1301,6 +1297,12 @@ static void handle_vdm_request(int port, int cnt, uint32_t *payload,
 			return;
 		} else {
 			pd[port].vdm_state = VDM_STATE_DONE;
+#ifdef CONFIG_USB_PD_REV30
+			if (pd[port].rev == PD_REV30 &&
+			    pd[port].power_role == PD_ROLE_SOURCE &&
+			    pd[port].flags & PD_FLAGS_EXPLICIT_CONTRACT)
+				sink_can_xmit(port, SINK_TX_OK);
+#endif
 		}
 	}
 
@@ -1380,7 +1382,6 @@ void pd_execute_hard_reset(int port)
 
 #ifdef CONFIG_USB_PD_REV30
 	pd[port].rev = PD_REV30;
-	pd_ca_reset(port);
 #endif
 	/*
 	 * Fake set last state to hard reset to make sure that the next
@@ -1649,15 +1650,6 @@ static void handle_data_request(int port, uint16_t head,
 				pd_update_saved_port_flags(
 					port, PD_BBRMFLG_EXPLICIT_CONTRACT, 1);
 #endif /* CONFIG_USB_PD_DUAL_ROLE */
-#ifdef CONFIG_USB_PD_REV30
-				/*
-				 * Start Source-coordinated collision
-				 * avoidance
-				 */
-				if (pd[port].rev == PD_REV30 &&
-					pd[port].power_role == PD_ROLE_SOURCE)
-					sink_can_xmit(port, SINK_TX_OK);
-#endif
 				pd[port].requested_idx = RDO_POS(payload[0]);
 				set_state(port, PD_STATE_SRC_ACCEPTED);
 				return;
@@ -1677,7 +1669,7 @@ static void handle_data_request(int port, uint16_t head,
 			if ((payload[0] >> 28) == 5) {
 				/* bist data object mode is 2 */
 				pd_transmit(port, TCPC_TX_BIST_MODE_2, 0,
-					    NULL);
+					    NULL, AMS_RESPONSE);
 				/* Set to appropriate port disconnected state */
 				set_state(port, DUAL_ROLE_IF_ELSE(port,
 						PD_STATE_SNK_DISCONNECTED,
@@ -1777,7 +1769,7 @@ static void handle_ctrl_request(int port, uint16_t head,
 		if (pd[port].task_state == PD_STATE_SRC_READY)
 			set_state(port, PD_STATE_SRC_DISCOVERY);
 		else {
-			res = send_source_cap(port);
+			res = send_source_cap(port, AMS_RESPONSE);
 			if ((res >= 0) &&
 			    (pd[port].task_state == PD_STATE_SRC_DISCOVERY))
 				set_state(port, PD_STATE_SRC_NEGOCIATE);
@@ -2216,7 +2208,7 @@ static void pd_vdm_send_state_machine(int port)
 				pd_get_rev(port),
 				0);
 			res = pd_transmit(port, TCPC_TX_SOP_PRIME, header,
-					  pd[port].vdo_data);
+					  pd[port].vdo_data, AMS_START);
 			/*
 			 * If there is no ack from the cable, its a non-emark
 			 * cable and since, the pd flow should continue
@@ -2233,7 +2225,7 @@ static void pd_vdm_send_state_machine(int port)
 				pd[port].vdo_data[0] =
 					VDO(USB_SID_PD, 1, CMD_DISCOVER_SVID);
 				res = pd_transmit(port, TCPC_TX_SOP, header,
-						  pd[port].vdo_data);
+						  pd[port].vdo_data, AMS_START);
 				reset_pd_cable(port);
 			}
 		} else {
@@ -2245,7 +2237,7 @@ static void pd_vdm_send_state_machine(int port)
 					   (int)pd[port].vdo_count,
 					   pd_get_rev(port), 0);
 			res = pd_transmit(port, TCPC_TX_SOP, header,
-					  pd[port].vdo_data);
+					  pd[port].vdo_data, AMS_START);
 		}
 
 		if (res < 0) {
@@ -2888,7 +2880,6 @@ void pd_task(void *u)
 #ifdef CONFIG_USB_PD_REV30
 	/* Set Revision to highest */
 	pd[port].rev = PD_REV30;
-	pd_ca_reset(port);
 #endif
 
 #ifdef CONFIG_USB_PD_DUAL_ROLE
@@ -3016,10 +3007,6 @@ void pd_task(void *u)
 #endif
 
 	while (1) {
-#ifdef CONFIG_USB_PD_REV30
-		/* send any pending messages */
-		pd_ca_send_pending(port);
-#endif
 		/* process VDM messages last */
 		pd_vdm_send_state_machine(port);
 
@@ -3029,7 +3016,8 @@ void pd_task(void *u)
 			/* cut the power */
 			pd_execute_hard_reset(port);
 			/* notify the other side of the issue */
-			pd_transmit(port, TCPC_TX_HARD_RESET, 0, NULL);
+			pd_transmit(port, TCPC_TX_HARD_RESET, 0, NULL,
+				    AMS_START);
 		}
 
 		/* wait for next event/packet or timeout expiration */
@@ -3507,7 +3495,7 @@ void pd_task(void *u)
 			if (caps_count < PD_CAPS_COUNT  &&
 						next_src_cap <= now.val) {
 				/* Query capabilities of the other side */
-				res = send_source_cap(port);
+				res = send_source_cap(port, AMS_START);
 				/* packet was acked => PD capable device) */
 				if (res >= 0) {
 					set_state(port,
@@ -3608,7 +3596,7 @@ void pd_task(void *u)
 
 			/* Send updated source capabilities to our partner */
 			if (pd[port].flags & PD_FLAGS_UPDATE_SRC_CAPS) {
-				res = send_source_cap(port);
+				res = send_source_cap(port, AMS_START);
 				if (res >= 0) {
 					set_state(port,
 						  PD_STATE_SRC_NEGOCIATE);
@@ -4493,8 +4481,8 @@ void pd_task(void *u)
 			if (hard_reset_sent)
 				break;
 
-			if (pd_transmit(port, TCPC_TX_HARD_RESET, 0, NULL) <
-			    0) {
+			if (pd_transmit(port, TCPC_TX_HARD_RESET, 0, NULL,
+				AMS_START) < 0) {
 				/*
 				 * likely a non-idle channel
 				 * TCPCI r2.0 v1.0 4.4.15:
@@ -4563,7 +4551,8 @@ void pd_task(void *u)
 						PD_STATE_SRC_DISCONNECTED));
 			break;
 		case PD_STATE_BIST_TX:
-			pd_transmit(port, TCPC_TX_BIST_MODE_2, 0, NULL);
+			pd_transmit(port, TCPC_TX_BIST_MODE_2, 0, NULL,
+				    AMS_START);
 			/* Delay at least enough to finish sending BIST */
 			timeout = PD_T_BIST_TRANSMIT + 20*MSEC;
 			/* Set to appropriate port disconnected state */
diff --git a/include/usb_pd.h b/include/usb_pd.h
index c1e4ff5583..994ffb9422 100644
--- a/include/usb_pd.h
+++ b/include/usb_pd.h
@@ -951,6 +951,20 @@ enum pd_ctrl_msg_type {
 	/* 22-31 Reserved */
 };
 
+/* Control message types which always mark the start of an AMS */
+#define PD_CTRL_AMS_START_MASK ((1 << PD_CTRL_GOTO_MIN) | \
+				(1 << PD_CTRL_GET_SOURCE_CAP) | \
+				(1 << PD_CTRL_GET_SINK_CAP) | \
+				(1 << PD_CTRL_DR_SWAP) | \
+				(1 << PD_CTRL_PR_SWAP) | \
+				(1 << PD_CTRL_VCONN_SWAP) | \
+				(1 << PD_CTRL_GET_SOURCE_CAP_EXT) | \
+				(1 << PD_CTRL_GET_STATUS) | \
+				(1 << PD_CTRL_FR_SWAP) | \
+				(1 << PD_CTRL_GET_PPS_STATUS) | \
+				(1 << PD_CTRL_GET_COUNTRY_CODES))
+
+
 /* Battery Status Data Object fields for REV 3.0 */
 #define BSDO_CAP_UNKNOWN 0xffff
 #define BSDO_CAP(n)      (((n) & 0xffff) << 16)