[PATCH v3 0/2] tcpm: AMS and Collision Avoidance

[PATCH v3 0/2] tcpm: AMS and Collision Avoidance

[Kyle Tso]

*** BLURB HERE ***

Kyle Tso (2):
  usb: typec: tcpm: AMS and Collision Avoidance
  usb: typec: tcpm: AMS for PD2.0

 drivers/usb/typec/tcpm/tcpm.c | 523 ++++++++++++++++++++++++++++++----
 include/linux/usb/pd.h        |   1 +
 include/linux/usb/tcpm.h      |   4 +
 3 files changed, 468 insertions(+), 60 deletions(-)

-- 
2.23.0.351.gc4317032e6-goog

[PATCH v3 1/2] usb: typec: tcpm: AMS and Collision Avoidance

[Kyle Tso]

This patch provides the implementation of Collision Avoidance introduced
in PD3.0. The start of each Atomic Message Sequence (AMS) initiated by
the port will be denied if the current AMS is not interruptible. The
Source port will set the CC to SinkTxNG if it is going to initiate an
AMS, and SinkTxOk otherwise. Meanwhile, any AMS initiated by a Sink port
will be denied in TCPM if the port partner (Source) sets SinkTxNG except
for HARD_RESET and SOFT_RESET.

Signed-off-by: Kyle Tso <kyletso@google.com>
---
Changelog since v2:
- (nit) changed some return values from constant 0 to variable "ret" in
  function tcpm_ams_start

 drivers/usb/typec/tcpm/tcpm.c | 538 ++++++++++++++++++++++++++++++----
 include/linux/usb/pd.h        |   1 +
 include/linux/usb/tcpm.h      |   4 +
 3 files changed, 483 insertions(+), 60 deletions(-)

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 96562744101c..7d1c30c33097 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -73,6 +73,8 @@
 	S(SNK_HARD_RESET_SINK_ON),		\
 						\
 	S(SOFT_RESET),				\
+	S(SRC_SOFT_RESET_WAIT_SNK_TX),		\
+	S(SNK_SOFT_RESET),			\
 	S(SOFT_RESET_SEND),			\
 						\
 	S(DR_SWAP_ACCEPT),			\
@@ -126,7 +128,45 @@
 						\
 	S(ERROR_RECOVERY),			\
 	S(PORT_RESET),				\
-	S(PORT_RESET_WAIT_OFF)
+	S(PORT_RESET_WAIT_OFF),			\
+						\
+	S(AMS_START)
+
+#define FOREACH_AMS(S)				\
+	S(NONE_AMS),				\
+	S(POWER_NEGOTIATION),			\
+	S(GOTOMIN),				\
+	S(SOFT_RESET_AMS),			\
+	S(HARD_RESET),				\
+	S(CABLE_RESET),				\
+	S(GET_SOURCE_CAPABILITIES),		\
+	S(GET_SINK_CAPABILITIES),		\
+	S(POWER_ROLE_SWAP),			\
+	S(FAST_ROLE_SWAP),			\
+	S(DATA_ROLE_SWAP),			\
+	S(VCONN_SWAP),				\
+	S(SOURCE_ALERT),			\
+	S(GETTING_SOURCE_EXTENDED_CAPABILITIES),\
+	S(GETTING_SOURCE_SINK_STATUS),		\
+	S(GETTING_BATTERY_CAPABILITIES),	\
+	S(GETTING_BATTERY_STATUS),		\
+	S(GETTING_MANUFACTURER_INFORMATION),	\
+	S(SECURITY),				\
+	S(FIRMWARE_UPDATE),			\
+	S(DISCOVER_IDENTITY),			\
+	S(SOURCE_STARTUP_CABLE_PLUG_DISCOVER_IDENTITY),	\
+	S(DISCOVER_SVIDS),			\
+	S(DISCOVER_MODES),			\
+	S(DFP_TO_UFP_ENTER_MODE),		\
+	S(DFP_TO_UFP_EXIT_MODE),		\
+	S(DFP_TO_CABLE_PLUG_ENTER_MODE),	\
+	S(DFP_TO_CABLE_PLUG_EXIT_MODE),		\
+	S(ATTENTION),				\
+	S(BIST),				\
+	S(UNSTRUCTURED_VDMS),			\
+	S(STRUCTURED_VDMS),			\
+	S(COUNTRY_INFO),			\
+	S(COUNTRY_CODES)
 
 #define GENERATE_ENUM(e)	e
 #define GENERATE_STRING(s)	#s
@@ -139,6 +179,14 @@ static const char * const tcpm_states[] = {
 	FOREACH_STATE(GENERATE_STRING)
 };
 
+enum tcpm_ams {
+	FOREACH_AMS(GENERATE_ENUM)
+};
+
+static const char * const tcpm_ams_str[] = {
+	FOREACH_AMS(GENERATE_STRING)
+};
+
 enum vdm_states {
 	VDM_STATE_ERR_BUSY = -3,
 	VDM_STATE_ERR_SEND = -2,
@@ -148,6 +196,7 @@ enum vdm_states {
 	VDM_STATE_READY = 1,
 	VDM_STATE_BUSY = 2,
 	VDM_STATE_WAIT_RSP_BUSY = 3,
+	VDM_STATE_SEND_MESSAGE = 4,
 };
 
 enum pd_msg_request {
@@ -322,6 +371,11 @@ struct tcpm_port {
 	/* port belongs to a self powered device */
 	bool self_powered;
 
+	/* Collision Avoidance and Atomic Message Sequence */
+	enum tcpm_state upcoming_state;
+	enum tcpm_ams ams;
+	bool in_ams;
+
 #ifdef CONFIG_DEBUG_FS
 	struct dentry *dentry;
 	struct mutex logbuffer_lock;	/* log buffer access lock */
@@ -373,6 +427,12 @@ struct pd_rx_event {
 	((port)->try_src_count == 0 && (port)->try_role == TYPEC_SOURCE && \
 	(port)->port_type == TYPEC_PORT_DRP)
 
+#define tcpm_sink_tx_ok(port) \
+	(tcpm_port_is_sink(port) && \
+	((port)->cc1 == TYPEC_CC_RP_3_0 || (port)->cc2 == TYPEC_CC_RP_3_0))
+
+#define support_ams(port)       ((port)->negotiated_rev >= PD_REV30)
+
 static enum tcpm_state tcpm_default_state(struct tcpm_port *port)
 {
 	if (port->port_type == TYPEC_PORT_DRP) {
@@ -608,6 +668,33 @@ static void tcpm_debugfs_exit(const struct tcpm_port *port) { }
 
 #endif
 
+static void tcpm_set_cc(struct tcpm_port *port, enum typec_cc_status cc)
+{
+	tcpm_log(port, "cc:=%d", cc);
+	port->cc_req = cc;
+	port->tcpc->set_cc(port->tcpc, cc);
+}
+
+static int tcpm_ams_finish(struct tcpm_port *port)
+{
+	int ret = 0;
+
+	if (!support_ams(port)) {
+		port->upcoming_state = INVALID_STATE;
+		return -EOPNOTSUPP;
+	}
+
+	tcpm_log(port, "AMS %s finished", tcpm_ams_str[port->ams]);
+
+	if (port->pwr_role == TYPEC_SOURCE)
+		tcpm_set_cc(port, SINK_TX_OK);
+
+	port->in_ams = false;
+	port->ams = NONE_AMS;
+
+	return ret;
+}
+
 static int tcpm_pd_transmit(struct tcpm_port *port,
 			    enum tcpm_transmit_type type,
 			    const struct pd_message *msg)
@@ -635,13 +722,29 @@ static int tcpm_pd_transmit(struct tcpm_port *port,
 	switch (port->tx_status) {
 	case TCPC_TX_SUCCESS:
 		port->message_id = (port->message_id + 1) & PD_HEADER_ID_MASK;
-		return 0;
+		/*
+		 * USB PD rev 3.0, 8.3.2.1.3:
+		 * "... Note that every AMS is Interruptible until the first
+		 * Message in the sequence has been successfully sent (GoodCRC
+		 * Message received)."
+		 */
+		if (support_ams(port) && port->ams != NONE_AMS)
+			port->in_ams = true;
+		break;
 	case TCPC_TX_DISCARDED:
-		return -EAGAIN;
+		ret = -EAGAIN;
+		break;
 	case TCPC_TX_FAILED:
 	default:
-		return -EIO;
+		ret = -EIO;
+		break;
 	}
+
+	/* Some AMS don't expect responses. Finish them here. */
+	if (port->ams == ATTENTION || port->ams == SOURCE_ALERT)
+		tcpm_ams_finish(port);
+
+	return ret;
 }
 
 void tcpm_pd_transmit_complete(struct tcpm_port *port,
@@ -891,17 +994,20 @@ static void tcpm_set_state(struct tcpm_port *port, enum tcpm_state state,
 			   unsigned int delay_ms)
 {
 	if (delay_ms) {
-		tcpm_log(port, "pending state change %s -> %s @ %u ms",
-			 tcpm_states[port->state], tcpm_states[state],
-			 delay_ms);
+		tcpm_log(port, "pending state change %s -> %s @ %u ms%s%s",
+			 tcpm_states[port->state], tcpm_states[state], delay_ms,
+			 support_ams(port) ? " in AMS " : "",
+			 support_ams(port) ? tcpm_ams_str[port->ams] : "");
 		port->delayed_state = state;
 		mod_delayed_work(port->wq, &port->state_machine,
 				 msecs_to_jiffies(delay_ms));
 		port->delayed_runtime = jiffies + msecs_to_jiffies(delay_ms);
 		port->delay_ms = delay_ms;
 	} else {
-		tcpm_log(port, "state change %s -> %s",
-			 tcpm_states[port->state], tcpm_states[state]);
+		tcpm_log(port, "state change %s -> %s%s%s",
+			 tcpm_states[port->state], tcpm_states[state],
+			 support_ams(port) ? " in AMS " : "",
+			 support_ams(port) ? tcpm_ams_str[port->ams] : "");
 		port->delayed_state = INVALID_STATE;
 		port->prev_state = port->state;
 		port->state = state;
@@ -923,10 +1029,12 @@ static void tcpm_set_state_cond(struct tcpm_port *port, enum tcpm_state state,
 		tcpm_set_state(port, state, delay_ms);
 	else
 		tcpm_log(port,
-			 "skipped %sstate change %s -> %s [%u ms], context state %s",
+			 "skipped %sstate change %s -> %s [%u ms], context state %s%s%s",
 			 delay_ms ? "delayed " : "",
 			 tcpm_states[port->state], tcpm_states[state],
-			 delay_ms, tcpm_states[port->enter_state]);
+			 delay_ms, tcpm_states[port->enter_state],
+			 support_ams(port) ? " in AMS " : "",
+			 support_ams(port) ? tcpm_ams_str[port->ams] : "");
 }
 
 static void tcpm_queue_message(struct tcpm_port *port,
@@ -936,6 +1044,139 @@ static void tcpm_queue_message(struct tcpm_port *port,
 	mod_delayed_work(port->wq, &port->state_machine, 0);
 }
 
+static bool tcpm_vdm_ams(struct tcpm_port *port)
+{
+	switch (port->ams) {
+	case DISCOVER_IDENTITY:
+	case SOURCE_STARTUP_CABLE_PLUG_DISCOVER_IDENTITY:
+	case DISCOVER_SVIDS:
+	case DISCOVER_MODES:
+	case DFP_TO_UFP_ENTER_MODE:
+	case DFP_TO_UFP_EXIT_MODE:
+	case DFP_TO_CABLE_PLUG_ENTER_MODE:
+	case DFP_TO_CABLE_PLUG_EXIT_MODE:
+	case ATTENTION:
+	case UNSTRUCTURED_VDMS:
+	case STRUCTURED_VDMS:
+		break;
+	default:
+		return false;
+	}
+
+	return true;
+}
+
+static bool tcpm_ams_interruptible(struct tcpm_port *port)
+{
+	switch (port->ams) {
+	/* Interruptible AMS */
+	case NONE_AMS:
+	case SECURITY:
+	case FIRMWARE_UPDATE:
+	case DISCOVER_IDENTITY:
+	case SOURCE_STARTUP_CABLE_PLUG_DISCOVER_IDENTITY:
+	case DISCOVER_SVIDS:
+	case DISCOVER_MODES:
+	case DFP_TO_UFP_ENTER_MODE:
+	case DFP_TO_UFP_EXIT_MODE:
+	case DFP_TO_CABLE_PLUG_ENTER_MODE:
+	case DFP_TO_CABLE_PLUG_EXIT_MODE:
+	case UNSTRUCTURED_VDMS:
+	case STRUCTURED_VDMS:
+	case COUNTRY_INFO:
+	case COUNTRY_CODES:
+		break;
+	/* Non-Interruptible AMS */
+	default:
+		if (port->in_ams)
+			return false;
+		break;
+	}
+
+	return true;
+}
+
+static int tcpm_ams_start(struct tcpm_port *port, enum tcpm_ams ams)
+{
+	int ret = 0;
+
+	if (!support_ams(port)) {
+		port->upcoming_state = INVALID_STATE;
+		return -EOPNOTSUPP;
+	}
+
+	tcpm_log(port, "AMS %s start", tcpm_ams_str[ams]);
+
+	if (!tcpm_ams_interruptible(port) && ams != HARD_RESET) {
+		port->upcoming_state = INVALID_STATE;
+		tcpm_log(port, "AMS %s not interruptible, aborting",
+			 tcpm_ams_str[port->ams]);
+		return -EAGAIN;
+	}
+
+	if (port->pwr_role == TYPEC_SOURCE) {
+		enum typec_cc_status cc_req;
+
+		port->ams = ams;
+
+		if (ams == HARD_RESET) {
+			tcpm_set_cc(port, tcpm_rp_cc(port));
+			tcpm_pd_transmit(port, TCPC_TX_HARD_RESET, NULL);
+			tcpm_set_state(port, HARD_RESET_START, 0);
+			return ret;
+		} else if (ams == SOFT_RESET_AMS) {
+			if (!port->explicit_contract) {
+				port->upcoming_state = INVALID_STATE;
+				tcpm_set_cc(port, tcpm_rp_cc(port));
+				return ret;
+			}
+		} else if (tcpm_vdm_ams(port)) {
+			/* tSinkTx is enforced in vdm_run_state_machine */
+			tcpm_set_cc(port, SINK_TX_NG);
+			return ret;
+		}
+
+		cc_req = port->cc_req;
+		tcpm_set_cc(port, SINK_TX_NG);
+		if (port->state == SRC_READY ||
+		    port->state == SRC_STARTUP ||
+		    port->state == SRC_SOFT_RESET_WAIT_SNK_TX ||
+		    port->state == SOFT_RESET ||
+		    port->state == SOFT_RESET_SEND)
+			tcpm_set_state(port, AMS_START, cc_req == SINK_TX_OK ?
+				       PD_T_SINK_TX : 0);
+		else
+			tcpm_set_state(port, SRC_READY, cc_req == SINK_TX_OK ?
+				       PD_T_SINK_TX : 0);
+	} else {
+		if (!tcpm_sink_tx_ok(port) &&
+		    ams != SOFT_RESET_AMS &&
+		    ams != HARD_RESET) {
+			port->upcoming_state = INVALID_STATE;
+			tcpm_log(port, "Sink TX No Go");
+			return -EAGAIN;
+		}
+
+		port->ams = ams;
+
+		if (ams == HARD_RESET) {
+			tcpm_pd_transmit(port, TCPC_TX_HARD_RESET, NULL);
+			tcpm_set_state(port, HARD_RESET_START, 0);
+			return ret;
+		} else if (tcpm_vdm_ams(port)) {
+			return ret;
+		}
+
+		if (port->state == SNK_READY ||
+		    port->state == SNK_SOFT_RESET)
+			tcpm_set_state(port, AMS_START, 0);
+		else
+			tcpm_set_state(port, SNK_READY, 0);
+	}
+
+	return ret;
+}
+
 /*
  * VDM/VDO handling functions
  */
@@ -1179,6 +1420,7 @@ static int tcpm_pd_svdm(struct tcpm_port *port, const __le32 *payload, int cnt,
 		default:
 			break;
 		}
+		tcpm_ams_finish(port);
 		break;
 	case CMDT_RSP_NAK:
 		switch (cmd) {
@@ -1192,6 +1434,7 @@ static int tcpm_pd_svdm(struct tcpm_port *port, const __le32 *payload, int cnt,
 		default:
 			break;
 		}
+		tcpm_ams_finish(port);
 		break;
 	default:
 		break;
@@ -1278,7 +1521,7 @@ static unsigned int vdm_ready_timeout(u32 vdm_hdr)
 static void vdm_run_state_machine(struct tcpm_port *port)
 {
 	struct pd_message msg;
-	int i, res;
+	int i, res = 0;
 
 	switch (port->vdm_state) {
 	case VDM_STATE_READY:
@@ -1295,27 +1538,43 @@ static void vdm_run_state_machine(struct tcpm_port *port)
 		if (port->state != SRC_READY && port->state != SNK_READY)
 			break;
 
-		/* Prepare and send VDM */
-		memset(&msg, 0, sizeof(msg));
-		msg.header = PD_HEADER_LE(PD_DATA_VENDOR_DEF,
-					  port->pwr_role,
-					  port->data_role,
-					  port->negotiated_rev,
-					  port->message_id, port->vdo_count);
-		for (i = 0; i < port->vdo_count; i++)
-			msg.payload[i] = cpu_to_le32(port->vdo_data[i]);
-		res = tcpm_pd_transmit(port, TCPC_TX_SOP, &msg);
-		if (res < 0) {
-			port->vdm_state = VDM_STATE_ERR_SEND;
-		} else {
-			unsigned long timeout;
+		if (PD_VDO_CMDT(port->vdo_data[0]) == CMDT_INIT) {
+			switch (PD_VDO_CMD(port->vdo_data[0])) {
+			case CMD_DISCOVER_IDENT:
+				res = tcpm_ams_start(port, DISCOVER_IDENTITY);
+				break;
+			case CMD_DISCOVER_SVID:
+				res = tcpm_ams_start(port, DISCOVER_SVIDS);
+				break;
+			case CMD_DISCOVER_MODES:
+				res = tcpm_ams_start(port, DISCOVER_MODES);
+				break;
+			case CMD_ENTER_MODE:
+				res = tcpm_ams_start(port,
+						     DFP_TO_UFP_ENTER_MODE);
+				break;
+			case CMD_EXIT_MODE:
+				res = tcpm_ams_start(port,
+						     DFP_TO_UFP_EXIT_MODE);
+				break;
+			case CMD_ATTENTION:
+				res = tcpm_ams_start(port, ATTENTION);
+				break;
+			default:
+				res = -EOPNOTSUPP;
+				break;
+			}
 
-			port->vdm_retries = 0;
-			port->vdm_state = VDM_STATE_BUSY;
-			timeout = vdm_ready_timeout(port->vdo_data[0]);
-			mod_delayed_work(port->wq, &port->vdm_state_machine,
-					 timeout);
+			if (res == -EAGAIN)
+				return;
 		}
+
+		port->vdm_state = VDM_STATE_SEND_MESSAGE;
+		mod_delayed_work(port->wq, &port->vdm_state_machine,
+				 (res != -EOPNOTSUPP) &&
+				 (port->pwr_role == TYPEC_SOURCE) &&
+				 (PD_VDO_CMDT(port->vdo_data[0]) == CMDT_INIT) ?
+				 PD_T_SINK_TX : 0);
 		break;
 	case VDM_STATE_WAIT_RSP_BUSY:
 		port->vdo_data[0] = port->vdo_retry;
@@ -1324,6 +1583,8 @@ static void vdm_run_state_machine(struct tcpm_port *port)
 		break;
 	case VDM_STATE_BUSY:
 		port->vdm_state = VDM_STATE_ERR_TMOUT;
+		if (port->ams != NONE_AMS)
+			tcpm_ams_finish(port);
 		break;
 	case VDM_STATE_ERR_SEND:
 		/*
@@ -1336,6 +1597,30 @@ static void vdm_run_state_machine(struct tcpm_port *port)
 			tcpm_log(port, "VDM Tx error, retry");
 			port->vdm_retries++;
 			port->vdm_state = VDM_STATE_READY;
+			tcpm_ams_finish(port);
+		}
+		break;
+	case VDM_STATE_SEND_MESSAGE:
+		/* Prepare and send VDM */
+		memset(&msg, 0, sizeof(msg));
+		msg.header = PD_HEADER_LE(PD_DATA_VENDOR_DEF,
+					  port->pwr_role,
+					  port->data_role,
+					  port->negotiated_rev,
+					  port->message_id, port->vdo_count);
+		for (i = 0; i < port->vdo_count; i++)
+			msg.payload[i] = cpu_to_le32(port->vdo_data[i]);
+		res = tcpm_pd_transmit(port, TCPC_TX_SOP, &msg);
+		if (res < 0) {
+			port->vdm_state = VDM_STATE_ERR_SEND;
+		} else {
+			unsigned long timeout;
+
+			port->vdm_retries = 0;
+			port->vdm_state = VDM_STATE_BUSY;
+			timeout = vdm_ready_timeout(port->vdo_data[0]);
+			mod_delayed_work(port->wq, &port->vdm_state_machine,
+					 timeout);
 		}
 		break;
 	default:
@@ -1359,7 +1644,8 @@ static void vdm_state_machine_work(struct work_struct *work)
 		prev_state = port->vdm_state;
 		vdm_run_state_machine(port);
 	} while (port->vdm_state != prev_state &&
-		 port->vdm_state != VDM_STATE_BUSY);
+		 port->vdm_state != VDM_STATE_BUSY &&
+		 port->vdm_state != VDM_STATE_SEND_MESSAGE);
 
 	mutex_unlock(&port->lock);
 }
@@ -1689,6 +1975,7 @@ static void tcpm_pd_ctrl_request(struct tcpm_port *port,
 {
 	enum pd_ctrl_msg_type type = pd_header_type_le(msg->header);
 	enum tcpm_state next_state;
+	int ret = 0;
 
 	switch (type) {
 	case PD_CTRL_GOOD_CRC:
@@ -1803,11 +2090,18 @@ static void tcpm_pd_ctrl_request(struct tcpm_port *port,
 		case SOFT_RESET_SEND:
 			port->message_id = 0;
 			port->rx_msgid = -1;
-			if (port->pwr_role == TYPEC_SOURCE)
-				next_state = SRC_SEND_CAPABILITIES;
-			else
-				next_state = SNK_WAIT_CAPABILITIES;
-			tcpm_set_state(port, next_state, 0);
+			if (port->ams == SOFT_RESET_AMS)
+				tcpm_ams_finish(port);
+			if (port->pwr_role == TYPEC_SOURCE) {
+				port->upcoming_state = SRC_SEND_CAPABILITIES;
+				ret = tcpm_ams_start(port, POWER_NEGOTIATION);
+				if (ret == -EOPNOTSUPP)
+					tcpm_set_state(port,
+						       SRC_SEND_CAPABILITIES,
+						       0);
+			} else {
+				tcpm_set_state(port, SNK_WAIT_CAPABILITIES, 0);
+			}
 			break;
 		case DR_SWAP_SEND:
 			tcpm_set_state(port, DR_SWAP_CHANGE_DR, 0);
@@ -2569,13 +2863,6 @@ static bool tcpm_start_toggling(struct tcpm_port *port, enum typec_cc_status cc)
 	return ret == 0;
 }
 
-static void tcpm_set_cc(struct tcpm_port *port, enum typec_cc_status cc)
-{
-	tcpm_log(port, "cc:=%d", cc);
-	port->cc_req = cc;
-	port->tcpc->set_cc(port->tcpc, cc);
-}
-
 static int tcpm_init_vbus(struct tcpm_port *port)
 {
 	int ret;
@@ -2694,6 +2981,8 @@ static void tcpm_unregister_altmodes(struct tcpm_port *port)
 
 static void tcpm_reset_port(struct tcpm_port *port)
 {
+	port->in_ams = false;
+	port->ams = NONE_AMS;
 	tcpm_unregister_altmodes(port);
 	tcpm_typec_disconnect(port);
 	port->attached = false;
@@ -2857,6 +3146,7 @@ static void run_state_machine(struct tcpm_port *port)
 	int ret;
 	enum typec_pwr_opmode opmode;
 	unsigned int msecs;
+	enum tcpm_state upcoming_state;
 
 	port->enter_state = port->state;
 	switch (port->state) {
@@ -2959,7 +3249,14 @@ static void run_state_machine(struct tcpm_port *port)
 		port->message_id = 0;
 		port->rx_msgid = -1;
 		port->explicit_contract = false;
-		tcpm_set_state(port, SRC_SEND_CAPABILITIES, 0);
+		/* SNK -> SRC POWER/FAST_ROLE_SWAP finished */
+		if (port->ams == POWER_ROLE_SWAP ||
+		    port->ams == FAST_ROLE_SWAP)
+			tcpm_ams_finish(port);
+		port->upcoming_state = SRC_SEND_CAPABILITIES;
+		ret = tcpm_ams_start(port, POWER_NEGOTIATION);
+		if (ret == -EOPNOTSUPP)
+			tcpm_set_state(port, SRC_SEND_CAPABILITIES, 0);
 		break;
 	case SRC_SEND_CAPABILITIES:
 		port->caps_count++;
@@ -3041,6 +3338,19 @@ static void run_state_machine(struct tcpm_port *port)
 		tcpm_swap_complete(port, 0);
 		tcpm_typec_connect(port);
 
+		if (port->ams != NONE_AMS)
+			tcpm_ams_finish(port);
+		/*
+		 * If previous AMS is interrupted, switch to the upcoming
+		 * state.
+		 */
+		upcoming_state = port->upcoming_state;
+		if (port->upcoming_state != INVALID_STATE) {
+			port->upcoming_state = INVALID_STATE;
+			tcpm_set_state(port, upcoming_state, 0);
+			break;
+		}
+
 		tcpm_check_send_discover(port);
 		/*
 		 * 6.3.5
@@ -3158,6 +3468,12 @@ static void run_state_machine(struct tcpm_port *port)
 		port->message_id = 0;
 		port->rx_msgid = -1;
 		port->explicit_contract = false;
+
+		if (port->ams == POWER_ROLE_SWAP ||
+		    port->ams == FAST_ROLE_SWAP)
+			/* SRC -> SNK POWER/FAST_ROLE_SWAP finished */
+			tcpm_ams_finish(port);
+
 		tcpm_set_state(port, SNK_DISCOVERY, 0);
 		break;
 	case SNK_DISCOVERY:
@@ -3207,7 +3523,7 @@ static void run_state_machine(struct tcpm_port *port)
 		 */
 		if (port->vbus_never_low) {
 			port->vbus_never_low = false;
-			tcpm_set_state(port, SOFT_RESET_SEND,
+			tcpm_set_state(port, SNK_SOFT_RESET,
 				       PD_T_SINK_WAIT_CAP);
 		} else {
 			tcpm_set_state(port, hard_reset_state(port),
@@ -3260,9 +3576,22 @@ static void run_state_machine(struct tcpm_port *port)
 
 		tcpm_swap_complete(port, 0);
 		tcpm_typec_connect(port);
-		tcpm_check_send_discover(port);
 		tcpm_pps_complete(port, port->pps_status);
 
+		if (port->ams != NONE_AMS)
+			tcpm_ams_finish(port);
+		/*
+		 * If previous AMS is interrupted, switch to the upcoming
+		 * state.
+		 */
+		upcoming_state = port->upcoming_state;
+		if (port->upcoming_state != INVALID_STATE) {
+			port->upcoming_state = INVALID_STATE;
+			tcpm_set_state(port, upcoming_state, 0);
+			break;
+		}
+
+		tcpm_check_send_discover(port);
 		power_supply_changed(port->psy);
 
 		break;
@@ -3284,8 +3613,18 @@ static void run_state_machine(struct tcpm_port *port)
 
 	/* Hard_Reset states */
 	case HARD_RESET_SEND:
-		tcpm_pd_transmit(port, TCPC_TX_HARD_RESET, NULL);
-		tcpm_set_state(port, HARD_RESET_START, 0);
+		if (port->ams != NONE_AMS)
+			tcpm_ams_finish(port);
+		/*
+		 * State machine will be directed to HARD_RESET_START,
+		 * thus set upcoming_state to INVALID_STATE.
+		 */
+		port->upcoming_state = INVALID_STATE;
+		ret = tcpm_ams_start(port, HARD_RESET);
+		if (ret == -EOPNOTSUPP) {
+			tcpm_pd_transmit(port, TCPC_TX_HARD_RESET, NULL);
+			tcpm_set_state(port, HARD_RESET_START, 0);
+		}
 		break;
 	case HARD_RESET_START:
 		port->hard_reset_count++;
@@ -3307,6 +3646,8 @@ static void run_state_machine(struct tcpm_port *port)
 		break;
 	case SRC_HARD_RESET_VBUS_ON:
 		tcpm_set_vbus(port, true);
+		if (port->ams == HARD_RESET)
+			tcpm_ams_finish(port);
 		port->tcpc->set_pd_rx(port->tcpc, true);
 		tcpm_set_attached_state(port, true);
 		tcpm_set_state(port, SRC_UNATTACHED, PD_T_PS_SOURCE_ON);
@@ -3326,6 +3667,8 @@ static void run_state_machine(struct tcpm_port *port)
 		tcpm_set_state(port, SNK_HARD_RESET_SINK_ON, PD_T_SAFE_0V);
 		break;
 	case SNK_HARD_RESET_WAIT_VBUS:
+		if (port->ams == HARD_RESET)
+			tcpm_ams_finish(port);
 		/* Assume we're disconnected if VBUS doesn't come back. */
 		tcpm_set_state(port, SNK_UNATTACHED,
 			       PD_T_SRC_RECOVER_MAX + PD_T_SRC_TURN_ON);
@@ -3353,6 +3696,8 @@ static void run_state_machine(struct tcpm_port *port)
 					       5000);
 			tcpm_set_charge(port, true);
 		}
+		if (port->ams == HARD_RESET)
+			tcpm_ams_finish(port);
 		tcpm_set_attached_state(port, true);
 		tcpm_set_state(port, SNK_STARTUP, 0);
 		break;
@@ -3362,10 +3707,23 @@ static void run_state_machine(struct tcpm_port *port)
 		port->message_id = 0;
 		port->rx_msgid = -1;
 		tcpm_pd_send_control(port, PD_CTRL_ACCEPT);
-		if (port->pwr_role == TYPEC_SOURCE)
-			tcpm_set_state(port, SRC_SEND_CAPABILITIES, 0);
-		else
+		if (port->pwr_role == TYPEC_SOURCE) {
+			port->upcoming_state = SRC_SEND_CAPABILITIES;
+			ret = tcpm_ams_start(port, POWER_NEGOTIATION);
+			if (ret == -EOPNOTSUPP)
+				tcpm_set_state(port, SRC_SEND_CAPABILITIES, 0);
+		} else {
 			tcpm_set_state(port, SNK_WAIT_CAPABILITIES, 0);
+		}
+		break;
+	case SRC_SOFT_RESET_WAIT_SNK_TX:
+	case SNK_SOFT_RESET:
+		if (port->ams != NONE_AMS)
+			tcpm_ams_finish(port);
+		port->upcoming_state = SOFT_RESET_SEND;
+		ret = tcpm_ams_start(port, SOFT_RESET_AMS);
+		if (ret == -EOPNOTSUPP)
+			tcpm_set_state(port, SOFT_RESET_SEND, 0);
 		break;
 	case SOFT_RESET_SEND:
 		port->message_id = 0;
@@ -3571,6 +3929,19 @@ static void run_state_machine(struct tcpm_port *port)
 			       tcpm_default_state(port),
 			       port->vbus_present ? PD_T_PS_SOURCE_OFF : 0);
 		break;
+
+	/* Collision Avoidance state */
+	case AMS_START:
+		if (port->upcoming_state == INVALID_STATE) {
+			tcpm_set_state(port, port->pwr_role == TYPEC_SOURCE ?
+				       SRC_READY : SNK_READY, 0);
+			break;
+		}
+
+		upcoming_state = port->upcoming_state;
+		port->upcoming_state = INVALID_STATE;
+		tcpm_set_state(port, upcoming_state, 0);
+		break;
 	default:
 		WARN(1, "Unexpected port state %d\n", port->state);
 		break;
@@ -3897,6 +4268,8 @@ static void _tcpm_pd_vbus_off(struct tcpm_port *port)
 static void _tcpm_pd_hard_reset(struct tcpm_port *port)
 {
 	tcpm_log_force(port, "Received hard reset");
+	if (port->ams != NONE_AMS)
+		port->ams = NONE_AMS;
 	/*
 	 * If we keep receiving hard reset requests, executing the hard reset
 	 * must have failed. Revert to error recovery if that happens.
@@ -4014,7 +4387,15 @@ static int tcpm_dr_set(const struct typec_capability *cap,
 		port->non_pd_role_swap = true;
 		tcpm_set_state(port, PORT_RESET, 0);
 	} else {
-		tcpm_set_state(port, DR_SWAP_SEND, 0);
+		port->upcoming_state = DR_SWAP_SEND;
+		ret = tcpm_ams_start(port, DATA_ROLE_SWAP);
+		if (ret == -EAGAIN) {
+			port->upcoming_state = INVALID_STATE;
+			goto port_unlock;
+		} else if (ret == -EOPNOTSUPP) {
+			port->upcoming_state = INVALID_STATE;
+			tcpm_set_state(port, DR_SWAP_SEND, 0);
+		}
 	}
 
 	port->swap_status = 0;
@@ -4061,10 +4442,19 @@ static int tcpm_pr_set(const struct typec_capability *cap,
 		goto port_unlock;
 	}
 
+	port->upcoming_state = PR_SWAP_SEND;
+	ret = tcpm_ams_start(port, POWER_ROLE_SWAP);
+	if (ret == -EAGAIN) {
+		port->upcoming_state = INVALID_STATE;
+		goto port_unlock;
+	} else if (ret == -EOPNOTSUPP) {
+		port->upcoming_state = INVALID_STATE;
+		tcpm_set_state(port, PR_SWAP_SEND, 0);
+	}
+
 	port->swap_status = 0;
 	port->swap_pending = true;
 	reinit_completion(&port->swap_complete);
-	tcpm_set_state(port, PR_SWAP_SEND, 0);
 	mutex_unlock(&port->lock);
 
 	if (!wait_for_completion_timeout(&port->swap_complete,
@@ -4101,10 +4491,19 @@ static int tcpm_vconn_set(const struct typec_capability *cap,
 		goto port_unlock;
 	}
 
+	port->upcoming_state = VCONN_SWAP_SEND;
+	ret = tcpm_ams_start(port, VCONN_SWAP);
+	if (ret == -EAGAIN) {
+		port->upcoming_state = INVALID_STATE;
+		goto port_unlock;
+	} else if (ret == -EOPNOTSUPP) {
+		port->upcoming_state = INVALID_STATE;
+		tcpm_set_state(port, VCONN_SWAP_SEND, 0);
+	}
+
 	port->swap_status = 0;
 	port->swap_pending = true;
 	reinit_completion(&port->swap_complete);
-	tcpm_set_state(port, VCONN_SWAP_SEND, 0);
 	mutex_unlock(&port->lock);
 
 	if (!wait_for_completion_timeout(&port->swap_complete,
@@ -4169,6 +4568,13 @@ static int tcpm_pps_set_op_curr(struct tcpm_port *port, u16 op_curr)
 		goto port_unlock;
 	}
 
+	port->upcoming_state = SNK_NEGOTIATE_PPS_CAPABILITIES;
+	ret = tcpm_ams_start(port, POWER_NEGOTIATION);
+	if (ret == -EAGAIN || ret == -EOPNOTSUPP) {
+		port->upcoming_state = INVALID_STATE;
+		goto port_unlock;
+	}
+
 	/* Round down operating current to align with PPS valid steps */
 	op_curr = op_curr - (op_curr % RDO_PROG_CURR_MA_STEP);
 
@@ -4176,7 +4582,6 @@ static int tcpm_pps_set_op_curr(struct tcpm_port *port, u16 op_curr)
 	port->pps_data.op_curr = op_curr;
 	port->pps_status = 0;
 	port->pps_pending = true;
-	tcpm_set_state(port, SNK_NEGOTIATE_PPS_CAPABILITIES, 0);
 	mutex_unlock(&port->lock);
 
 	if (!wait_for_completion_timeout(&port->pps_complete,
@@ -4225,6 +4630,13 @@ static int tcpm_pps_set_out_volt(struct tcpm_port *port, u16 out_volt)
 		goto port_unlock;
 	}
 
+	port->upcoming_state = SNK_NEGOTIATE_PPS_CAPABILITIES;
+	ret = tcpm_ams_start(port, POWER_NEGOTIATION);
+	if (ret == -EAGAIN || ret == -EOPNOTSUPP) {
+		port->upcoming_state = INVALID_STATE;
+		goto port_unlock;
+	}
+
 	/* Round down output voltage to align with PPS valid steps */
 	out_volt = out_volt - (out_volt % RDO_PROG_VOLT_MV_STEP);
 
@@ -4232,7 +4644,6 @@ static int tcpm_pps_set_out_volt(struct tcpm_port *port, u16 out_volt)
 	port->pps_data.out_volt = out_volt;
 	port->pps_status = 0;
 	port->pps_pending = true;
-	tcpm_set_state(port, SNK_NEGOTIATE_PPS_CAPABILITIES, 0);
 	mutex_unlock(&port->lock);
 
 	if (!wait_for_completion_timeout(&port->pps_complete,
@@ -4272,6 +4683,16 @@ static int tcpm_pps_activate(struct tcpm_port *port, bool activate)
 		goto port_unlock;
 	}
 
+	if (activate)
+		port->upcoming_state = SNK_NEGOTIATE_PPS_CAPABILITIES;
+	else
+		port->upcoming_state = SNK_NEGOTIATE_CAPABILITIES;
+	ret = tcpm_ams_start(port, POWER_NEGOTIATION);
+	if (ret == -EAGAIN || ret == -EOPNOTSUPP) {
+		port->upcoming_state = INVALID_STATE;
+		goto port_unlock;
+	}
+
 	reinit_completion(&port->pps_complete);
 	port->pps_status = 0;
 	port->pps_pending = true;
@@ -4280,9 +4701,6 @@ static int tcpm_pps_activate(struct tcpm_port *port, bool activate)
 	if (activate) {
 		port->pps_data.out_volt = port->supply_voltage;
 		port->pps_data.op_curr = port->current_limit;
-		tcpm_set_state(port, SNK_NEGOTIATE_PPS_CAPABILITIES, 0);
-	} else {
-		tcpm_set_state(port, SNK_NEGOTIATE_CAPABILITIES, 0);
 	}
 	mutex_unlock(&port->lock);
 
diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
index 145c38e351c2..e4343e656470 100644
--- a/include/linux/usb/pd.h
+++ b/include/linux/usb/pd.h
@@ -442,6 +442,7 @@ static inline unsigned int rdo_max_power(u32 rdo)
 #define PD_T_ERROR_RECOVERY	100	/* minimum 25 is insufficient */
 #define PD_T_SRCSWAPSTDBY      625     /* Maximum of 650ms */
 #define PD_T_NEWSRC            250     /* Maximum of 275ms */
+#define PD_T_SINK_TX		16	/* 16 - 20 ms */
 
 #define PD_T_DRP_TRY		100	/* 75 - 150 ms */
 #define PD_T_DRP_TRYWAIT	600	/* 400 - 800 ms */
diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
index f516955a0cf4..53afd61fe003 100644
--- a/include/linux/usb/tcpm.h
+++ b/include/linux/usb/tcpm.h
@@ -19,6 +19,10 @@ enum typec_cc_status {
 	TYPEC_CC_RP_3_0,
 };
 
+/* Collision Avoidance */
+#define SINK_TX_NG	TYPEC_CC_RP_1_5
+#define SINK_TX_OK	TYPEC_CC_RP_3_0
+
 enum typec_cc_polarity {
 	TYPEC_POLARITY_CC1,
 	TYPEC_POLARITY_CC2,
-- 
2.23.0.351.gc4317032e6-goog

[PATCH v3 2/2] usb: typec: tcpm: AMS for PD2.0

[Kyle Tso]

AMS is defined in PD2.0 as well. Remove the filter in tcpm_ams_start
and change the CC for Collision Avoidance only if the negotiated
revision is higher than PD2.0.

Signed-off-by: Kyle Tso <kyletso@google.com>
---
Changelog since v2:
- N/A; This is the first version.

 drivers/usb/typec/tcpm/tcpm.c | 129 +++++++++++++++-------------------
 1 file changed, 57 insertions(+), 72 deletions(-)

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 7d1c30c33097..aca1c5bbe870 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -391,6 +391,12 @@ struct pd_rx_event {
 	struct pd_message msg;
 };
 
+static const char * const pd_rev[] = {
+	[PD_REV10]		= "rev1",
+	[PD_REV20]		= "rev2",
+	[PD_REV30]		= "rev3",
+};
+
 #define tcpm_cc_is_sink(cc) \
 	((cc) == TYPEC_CC_RP_DEF || (cc) == TYPEC_CC_RP_1_5 || \
 	 (cc) == TYPEC_CC_RP_3_0)
@@ -431,8 +437,6 @@ struct pd_rx_event {
 	(tcpm_port_is_sink(port) && \
 	((port)->cc1 == TYPEC_CC_RP_3_0 || (port)->cc2 == TYPEC_CC_RP_3_0))
 
-#define support_ams(port)       ((port)->negotiated_rev >= PD_REV30)
-
 static enum tcpm_state tcpm_default_state(struct tcpm_port *port)
 {
 	if (port->port_type == TYPEC_PORT_DRP) {
@@ -679,14 +683,9 @@ static int tcpm_ams_finish(struct tcpm_port *port)
 {
 	int ret = 0;
 
-	if (!support_ams(port)) {
-		port->upcoming_state = INVALID_STATE;
-		return -EOPNOTSUPP;
-	}
-
 	tcpm_log(port, "AMS %s finished", tcpm_ams_str[port->ams]);
 
-	if (port->pwr_role == TYPEC_SOURCE)
+	if (port->negotiated_rev >= PD_REV30 && port->pwr_role == TYPEC_SOURCE)
 		tcpm_set_cc(port, SINK_TX_OK);
 
 	port->in_ams = false;
@@ -723,12 +722,13 @@ static int tcpm_pd_transmit(struct tcpm_port *port,
 	case TCPC_TX_SUCCESS:
 		port->message_id = (port->message_id + 1) & PD_HEADER_ID_MASK;
 		/*
+		 * USB PD rev 2.0, 8.3.2.2.1:
 		 * USB PD rev 3.0, 8.3.2.1.3:
 		 * "... Note that every AMS is Interruptible until the first
 		 * Message in the sequence has been successfully sent (GoodCRC
 		 * Message received)."
 		 */
-		if (support_ams(port) && port->ams != NONE_AMS)
+		if (port->ams != NONE_AMS)
 			port->in_ams = true;
 		break;
 	case TCPC_TX_DISCARDED:
@@ -994,20 +994,18 @@ static void tcpm_set_state(struct tcpm_port *port, enum tcpm_state state,
 			   unsigned int delay_ms)
 {
 	if (delay_ms) {
-		tcpm_log(port, "pending state change %s -> %s @ %u ms%s%s",
+		tcpm_log(port, "pending state change %s -> %s @ %u ms [%s %s]",
 			 tcpm_states[port->state], tcpm_states[state], delay_ms,
-			 support_ams(port) ? " in AMS " : "",
-			 support_ams(port) ? tcpm_ams_str[port->ams] : "");
+			 pd_rev[port->negotiated_rev], tcpm_ams_str[port->ams]);
 		port->delayed_state = state;
 		mod_delayed_work(port->wq, &port->state_machine,
 				 msecs_to_jiffies(delay_ms));
 		port->delayed_runtime = jiffies + msecs_to_jiffies(delay_ms);
 		port->delay_ms = delay_ms;
 	} else {
-		tcpm_log(port, "state change %s -> %s%s%s",
+		tcpm_log(port, "state change %s -> %s [%s %s]",
 			 tcpm_states[port->state], tcpm_states[state],
-			 support_ams(port) ? " in AMS " : "",
-			 support_ams(port) ? tcpm_ams_str[port->ams] : "");
+			 pd_rev[port->negotiated_rev], tcpm_ams_str[port->ams]);
 		port->delayed_state = INVALID_STATE;
 		port->prev_state = port->state;
 		port->state = state;
@@ -1029,12 +1027,11 @@ static void tcpm_set_state_cond(struct tcpm_port *port, enum tcpm_state state,
 		tcpm_set_state(port, state, delay_ms);
 	else
 		tcpm_log(port,
-			 "skipped %sstate change %s -> %s [%u ms], context state %s%s%s",
+			 "skipped %sstate change %s -> %s [%u ms], context state %s [%s %s]",
 			 delay_ms ? "delayed " : "",
 			 tcpm_states[port->state], tcpm_states[state],
 			 delay_ms, tcpm_states[port->enter_state],
-			 support_ams(port) ? " in AMS " : "",
-			 support_ams(port) ? tcpm_ams_str[port->ams] : "");
+			 pd_rev[port->negotiated_rev], tcpm_ams_str[port->ams]);
 }
 
 static void tcpm_queue_message(struct tcpm_port *port,
@@ -1100,11 +1097,6 @@ static int tcpm_ams_start(struct tcpm_port *port, enum tcpm_ams ams)
 {
 	int ret = 0;
 
-	if (!support_ams(port)) {
-		port->upcoming_state = INVALID_STATE;
-		return -EOPNOTSUPP;
-	}
-
 	tcpm_log(port, "AMS %s start", tcpm_ams_str[ams]);
 
 	if (!tcpm_ams_interruptible(port) && ams != HARD_RESET) {
@@ -1132,24 +1124,41 @@ static int tcpm_ams_start(struct tcpm_port *port, enum tcpm_ams ams)
 			}
 		} else if (tcpm_vdm_ams(port)) {
 			/* tSinkTx is enforced in vdm_run_state_machine */
-			tcpm_set_cc(port, SINK_TX_NG);
+			if (port->negotiated_rev >= PD_REV30)
+				tcpm_set_cc(port, SINK_TX_NG);
 			return ret;
 		}
 
-		cc_req = port->cc_req;
-		tcpm_set_cc(port, SINK_TX_NG);
-		if (port->state == SRC_READY ||
-		    port->state == SRC_STARTUP ||
-		    port->state == SRC_SOFT_RESET_WAIT_SNK_TX ||
-		    port->state == SOFT_RESET ||
-		    port->state == SOFT_RESET_SEND)
-			tcpm_set_state(port, AMS_START, cc_req == SINK_TX_OK ?
-				       PD_T_SINK_TX : 0);
-		else
-			tcpm_set_state(port, SRC_READY, cc_req == SINK_TX_OK ?
-				       PD_T_SINK_TX : 0);
+		if (port->negotiated_rev >= PD_REV30) {
+			cc_req = port->cc_req;
+			tcpm_set_cc(port, SINK_TX_NG);
+		}
+
+		switch (port->state) {
+		case SRC_READY:
+		case SRC_STARTUP:
+		case SRC_SOFT_RESET_WAIT_SNK_TX:
+		case SOFT_RESET:
+		case SOFT_RESET_SEND:
+			if (port->negotiated_rev >= PD_REV30)
+				tcpm_set_state(port, AMS_START,
+					       cc_req == SINK_TX_OK ?
+					       PD_T_SINK_TX : 0);
+			else
+				tcpm_set_state(port, AMS_START, 0);
+			break;
+		default:
+			if (port->negotiated_rev >= PD_REV30)
+				tcpm_set_state(port, SRC_READY,
+					       cc_req == SINK_TX_OK ?
+					       PD_T_SINK_TX : 0);
+			else
+				tcpm_set_state(port, SRC_READY, 0);
+			break;
+		}
 	} else {
-		if (!tcpm_sink_tx_ok(port) &&
+		if (port->negotiated_rev >= PD_REV30 &&
+		    !tcpm_sink_tx_ok(port) &&
 		    ams != SOFT_RESET_AMS &&
 		    ams != HARD_RESET) {
 			port->upcoming_state = INVALID_STATE;
@@ -1565,13 +1574,13 @@ static void vdm_run_state_machine(struct tcpm_port *port)
 				break;
 			}
 
-			if (res == -EAGAIN)
+			if (res < 0)
 				return;
 		}
 
 		port->vdm_state = VDM_STATE_SEND_MESSAGE;
 		mod_delayed_work(port->wq, &port->vdm_state_machine,
-				 (res != -EOPNOTSUPP) &&
+				 (port->negotiated_rev >= PD_REV30) &&
 				 (port->pwr_role == TYPEC_SOURCE) &&
 				 (PD_VDO_CMDT(port->vdo_data[0]) == CMDT_INIT) ?
 				 PD_T_SINK_TX : 0);
@@ -1975,7 +1984,6 @@ static void tcpm_pd_ctrl_request(struct tcpm_port *port,
 {
 	enum pd_ctrl_msg_type type = pd_header_type_le(msg->header);
 	enum tcpm_state next_state;
-	int ret = 0;
 
 	switch (type) {
 	case PD_CTRL_GOOD_CRC:
@@ -2094,11 +2102,7 @@ static void tcpm_pd_ctrl_request(struct tcpm_port *port,
 				tcpm_ams_finish(port);
 			if (port->pwr_role == TYPEC_SOURCE) {
 				port->upcoming_state = SRC_SEND_CAPABILITIES;
-				ret = tcpm_ams_start(port, POWER_NEGOTIATION);
-				if (ret == -EOPNOTSUPP)
-					tcpm_set_state(port,
-						       SRC_SEND_CAPABILITIES,
-						       0);
+				tcpm_ams_start(port, POWER_NEGOTIATION);
 			} else {
 				tcpm_set_state(port, SNK_WAIT_CAPABILITIES, 0);
 			}
@@ -3254,9 +3258,7 @@ static void run_state_machine(struct tcpm_port *port)
 		    port->ams == FAST_ROLE_SWAP)
 			tcpm_ams_finish(port);
 		port->upcoming_state = SRC_SEND_CAPABILITIES;
-		ret = tcpm_ams_start(port, POWER_NEGOTIATION);
-		if (ret == -EOPNOTSUPP)
-			tcpm_set_state(port, SRC_SEND_CAPABILITIES, 0);
+		tcpm_ams_start(port, POWER_NEGOTIATION);
 		break;
 	case SRC_SEND_CAPABILITIES:
 		port->caps_count++;
@@ -3620,11 +3622,7 @@ static void run_state_machine(struct tcpm_port *port)
 		 * thus set upcoming_state to INVALID_STATE.
 		 */
 		port->upcoming_state = INVALID_STATE;
-		ret = tcpm_ams_start(port, HARD_RESET);
-		if (ret == -EOPNOTSUPP) {
-			tcpm_pd_transmit(port, TCPC_TX_HARD_RESET, NULL);
-			tcpm_set_state(port, HARD_RESET_START, 0);
-		}
+		tcpm_ams_start(port, HARD_RESET);
 		break;
 	case HARD_RESET_START:
 		port->hard_reset_count++;
@@ -3709,9 +3707,7 @@ static void run_state_machine(struct tcpm_port *port)
 		tcpm_pd_send_control(port, PD_CTRL_ACCEPT);
 		if (port->pwr_role == TYPEC_SOURCE) {
 			port->upcoming_state = SRC_SEND_CAPABILITIES;
-			ret = tcpm_ams_start(port, POWER_NEGOTIATION);
-			if (ret == -EOPNOTSUPP)
-				tcpm_set_state(port, SRC_SEND_CAPABILITIES, 0);
+			tcpm_ams_start(port, POWER_NEGOTIATION);
 		} else {
 			tcpm_set_state(port, SNK_WAIT_CAPABILITIES, 0);
 		}
@@ -3721,9 +3717,7 @@ static void run_state_machine(struct tcpm_port *port)
 		if (port->ams != NONE_AMS)
 			tcpm_ams_finish(port);
 		port->upcoming_state = SOFT_RESET_SEND;
-		ret = tcpm_ams_start(port, SOFT_RESET_AMS);
-		if (ret == -EOPNOTSUPP)
-			tcpm_set_state(port, SOFT_RESET_SEND, 0);
+		tcpm_ams_start(port, SOFT_RESET_AMS);
 		break;
 	case SOFT_RESET_SEND:
 		port->message_id = 0;
@@ -3930,7 +3924,7 @@ static void run_state_machine(struct tcpm_port *port)
 			       port->vbus_present ? PD_T_PS_SOURCE_OFF : 0);
 		break;
 
-	/* Collision Avoidance state */
+	/* AMS intermediate state */
 	case AMS_START:
 		if (port->upcoming_state == INVALID_STATE) {
 			tcpm_set_state(port, port->pwr_role == TYPEC_SOURCE ?
@@ -4392,9 +4386,6 @@ static int tcpm_dr_set(const struct typec_capability *cap,
 		if (ret == -EAGAIN) {
 			port->upcoming_state = INVALID_STATE;
 			goto port_unlock;
-		} else if (ret == -EOPNOTSUPP) {
-			port->upcoming_state = INVALID_STATE;
-			tcpm_set_state(port, DR_SWAP_SEND, 0);
 		}
 	}
 
@@ -4447,9 +4438,6 @@ static int tcpm_pr_set(const struct typec_capability *cap,
 	if (ret == -EAGAIN) {
 		port->upcoming_state = INVALID_STATE;
 		goto port_unlock;
-	} else if (ret == -EOPNOTSUPP) {
-		port->upcoming_state = INVALID_STATE;
-		tcpm_set_state(port, PR_SWAP_SEND, 0);
 	}
 
 	port->swap_status = 0;
@@ -4496,9 +4484,6 @@ static int tcpm_vconn_set(const struct typec_capability *cap,
 	if (ret == -EAGAIN) {
 		port->upcoming_state = INVALID_STATE;
 		goto port_unlock;
-	} else if (ret == -EOPNOTSUPP) {
-		port->upcoming_state = INVALID_STATE;
-		tcpm_set_state(port, VCONN_SWAP_SEND, 0);
 	}
 
 	port->swap_status = 0;
@@ -4570,7 +4555,7 @@ static int tcpm_pps_set_op_curr(struct tcpm_port *port, u16 op_curr)
 
 	port->upcoming_state = SNK_NEGOTIATE_PPS_CAPABILITIES;
 	ret = tcpm_ams_start(port, POWER_NEGOTIATION);
-	if (ret == -EAGAIN || ret == -EOPNOTSUPP) {
+	if (ret == -EAGAIN) {
 		port->upcoming_state = INVALID_STATE;
 		goto port_unlock;
 	}
@@ -4632,7 +4617,7 @@ static int tcpm_pps_set_out_volt(struct tcpm_port *port, u16 out_volt)
 
 	port->upcoming_state = SNK_NEGOTIATE_PPS_CAPABILITIES;
 	ret = tcpm_ams_start(port, POWER_NEGOTIATION);
-	if (ret == -EAGAIN || ret == -EOPNOTSUPP) {
+	if (ret == -EAGAIN) {
 		port->upcoming_state = INVALID_STATE;
 		goto port_unlock;
 	}
@@ -4688,7 +4673,7 @@ static int tcpm_pps_activate(struct tcpm_port *port, bool activate)
 	else
 		port->upcoming_state = SNK_NEGOTIATE_CAPABILITIES;
 	ret = tcpm_ams_start(port, POWER_NEGOTIATION);
-	if (ret == -EAGAIN || ret == -EOPNOTSUPP) {
+	if (ret == -EAGAIN) {
 		port->upcoming_state = INVALID_STATE;
 		goto port_unlock;
 	}
-- 
2.23.0.351.gc4317032e6-goog

Re: [PATCH v3 0/2] tcpm: AMS and Collision Avoidance

[Hans de Goede]

Hi Kyle,

On 20-09-2019 05:24, Kyle Tso wrote:
> *** BLURB HERE ***
> 
> Kyle Tso (2):
>    usb: typec: tcpm: AMS and Collision Avoidance
>    usb: typec: tcpm: AMS for PD2.0

May I ask how and on which hardware you have tested this?

And specifically if you have tested this in combination with pwr-role swapping?

Regards,

Hans

Re: [PATCH v3 0/2] tcpm: AMS and Collision Avoidance

[Kyle Tso]

Hi Hans,

I have tested these on an Android device (ARM64).
All the swap operations work fine (Power Role/Data Role/Vconn Swap).
(except for Fast Role Swap because it is still not supported in TCPM)

Regards,
Kyle Tso


On Fri, Sep 20, 2019 at 4:02 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi Kyle,
>
> On 20-09-2019 05:24, Kyle Tso wrote:
> > *** BLURB HERE ***
> >
> > Kyle Tso (2):
> >    usb: typec: tcpm: AMS and Collision Avoidance
> >    usb: typec: tcpm: AMS for PD2.0
>
> May I ask how and on which hardware you have tested this?
>
> And specifically if you have tested this in combination with pwr-role swapping?
>
> Regards,
>
> Hans
>

Re: [PATCH v3 0/2] tcpm: AMS and Collision Avoidance

[Hans de Goede]

Hi Kyle,

On 20-09-2019 10:19, Kyle Tso wrote:
> Hi Hans,
> 
> I have tested these on an Android device (ARM64).
> All the swap operations work fine (Power Role/Data Role/Vconn Swap).
> (except for Fast Role Swap because it is still not supported in TCPM)

May I ask which type-c controller are these devices using?

Regards,

Hans



> 
> Regards,
> Kyle Tso
> 
> 
> On Fri, Sep 20, 2019 at 4:02 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi Kyle,
>>
>> On 20-09-2019 05:24, Kyle Tso wrote:
>>> *** BLURB HERE ***
>>>
>>> Kyle Tso (2):
>>>     usb: typec: tcpm: AMS and Collision Avoidance
>>>     usb: typec: tcpm: AMS for PD2.0
>>
>> May I ask how and on which hardware you have tested this?
>>
>> And specifically if you have tested this in combination with pwr-role swapping?
>>
>> Regards,
>>
>> Hans
>>

Re: [PATCH v3 0/2] tcpm: AMS and Collision Avoidance

[Kyle Tso]

On Fri, Sep 20, 2019 at 4:25 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi Kyle,
>
> On 20-09-2019 10:19, Kyle Tso wrote:
> > Hi Hans,
> >
> > I have tested these on an Android device (ARM64).
> > All the swap operations work fine (Power Role/Data Role/Vconn Swap).
> > (except for Fast Role Swap because it is still not supported in TCPM)
>
> May I ask which type-c controller are these devices using?
>
> Regards,
>
> Hans
>

It's a Type-C/PD controller embedded in Qualcomm PMIC PM8150.

Regards,
Kyle Tso

Re: [PATCH v3 2/2] usb: typec: tcpm: AMS for PD2.0

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 8099 bytes --]

Hi Kyle,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[cannot apply to v5.3 next-20190919]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Kyle-Tso/usb-typec-tcpm-AMS-and-Collision-Avoidance/20190920-112652
config: i386-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.4.0-13) 7.4.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 
:::::: branch date: 7 hours ago
:::::: commit date: 7 hours ago

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   drivers/usb/typec/tcpm/tcpm.c: In function 'tcpm_ams_start':
>> drivers/usb/typec/tcpm/tcpm.c:1144:5: warning: 'cc_req' may be used uninitialized in this function [-Wmaybe-uninitialized]
        tcpm_set_state(port, AMS_START,
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                cc_req == SINK_TX_OK ?
                ~~~~~~~~~~~~~~~~~~~~~~
                PD_T_SINK_TX : 0);
                ~~~~~~~~~~~~~~~~~

# https://github.com/0day-ci/linux/commit/59e8594ebb63b2f2f6d8ec5a9c9c914f6c476cae
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 59e8594ebb63b2f2f6d8ec5a9c9c914f6c476cae
vim +/cc_req +1144 drivers/usb/typec/tcpm/tcpm.c

0ea47e4d06fe89 Kyle Tso 2019-09-20  1095  
0ea47e4d06fe89 Kyle Tso 2019-09-20  1096  static int tcpm_ams_start(struct tcpm_port *port, enum tcpm_ams ams)
0ea47e4d06fe89 Kyle Tso 2019-09-20  1097  {
0ea47e4d06fe89 Kyle Tso 2019-09-20  1098  	int ret = 0;
0ea47e4d06fe89 Kyle Tso 2019-09-20  1099  
0ea47e4d06fe89 Kyle Tso 2019-09-20  1100  	tcpm_log(port, "AMS %s start", tcpm_ams_str[ams]);
0ea47e4d06fe89 Kyle Tso 2019-09-20  1101  
0ea47e4d06fe89 Kyle Tso 2019-09-20  1102  	if (!tcpm_ams_interruptible(port) && ams != HARD_RESET) {
0ea47e4d06fe89 Kyle Tso 2019-09-20  1103  		port->upcoming_state = INVALID_STATE;
0ea47e4d06fe89 Kyle Tso 2019-09-20  1104  		tcpm_log(port, "AMS %s not interruptible, aborting",
0ea47e4d06fe89 Kyle Tso 2019-09-20  1105  			 tcpm_ams_str[port->ams]);
0ea47e4d06fe89 Kyle Tso 2019-09-20  1106  		return -EAGAIN;
0ea47e4d06fe89 Kyle Tso 2019-09-20  1107  	}
0ea47e4d06fe89 Kyle Tso 2019-09-20  1108  
0ea47e4d06fe89 Kyle Tso 2019-09-20  1109  	if (port->pwr_role == TYPEC_SOURCE) {
0ea47e4d06fe89 Kyle Tso 2019-09-20  1110  		enum typec_cc_status cc_req;
0ea47e4d06fe89 Kyle Tso 2019-09-20  1111  
0ea47e4d06fe89 Kyle Tso 2019-09-20  1112  		port->ams = ams;
0ea47e4d06fe89 Kyle Tso 2019-09-20  1113  
0ea47e4d06fe89 Kyle Tso 2019-09-20  1114  		if (ams == HARD_RESET) {
0ea47e4d06fe89 Kyle Tso 2019-09-20  1115  			tcpm_set_cc(port, tcpm_rp_cc(port));
0ea47e4d06fe89 Kyle Tso 2019-09-20  1116  			tcpm_pd_transmit(port, TCPC_TX_HARD_RESET, NULL);
0ea47e4d06fe89 Kyle Tso 2019-09-20  1117  			tcpm_set_state(port, HARD_RESET_START, 0);
0ea47e4d06fe89 Kyle Tso 2019-09-20  1118  			return ret;
0ea47e4d06fe89 Kyle Tso 2019-09-20  1119  		} else if (ams == SOFT_RESET_AMS) {
0ea47e4d06fe89 Kyle Tso 2019-09-20  1120  			if (!port->explicit_contract) {
0ea47e4d06fe89 Kyle Tso 2019-09-20  1121  				port->upcoming_state = INVALID_STATE;
0ea47e4d06fe89 Kyle Tso 2019-09-20  1122  				tcpm_set_cc(port, tcpm_rp_cc(port));
0ea47e4d06fe89 Kyle Tso 2019-09-20  1123  				return ret;
0ea47e4d06fe89 Kyle Tso 2019-09-20  1124  			}
0ea47e4d06fe89 Kyle Tso 2019-09-20  1125  		} else if (tcpm_vdm_ams(port)) {
0ea47e4d06fe89 Kyle Tso 2019-09-20  1126  			/* tSinkTx is enforced in vdm_run_state_machine */
59e8594ebb63b2 Kyle Tso 2019-09-20  1127  			if (port->negotiated_rev >= PD_REV30)
0ea47e4d06fe89 Kyle Tso 2019-09-20  1128  				tcpm_set_cc(port, SINK_TX_NG);
0ea47e4d06fe89 Kyle Tso 2019-09-20  1129  			return ret;
0ea47e4d06fe89 Kyle Tso 2019-09-20  1130  		}
0ea47e4d06fe89 Kyle Tso 2019-09-20  1131  
59e8594ebb63b2 Kyle Tso 2019-09-20  1132  		if (port->negotiated_rev >= PD_REV30) {
0ea47e4d06fe89 Kyle Tso 2019-09-20  1133  			cc_req = port->cc_req;
0ea47e4d06fe89 Kyle Tso 2019-09-20  1134  			tcpm_set_cc(port, SINK_TX_NG);
59e8594ebb63b2 Kyle Tso 2019-09-20  1135  		}
59e8594ebb63b2 Kyle Tso 2019-09-20  1136  
59e8594ebb63b2 Kyle Tso 2019-09-20  1137  		switch (port->state) {
59e8594ebb63b2 Kyle Tso 2019-09-20  1138  		case SRC_READY:
59e8594ebb63b2 Kyle Tso 2019-09-20  1139  		case SRC_STARTUP:
59e8594ebb63b2 Kyle Tso 2019-09-20  1140  		case SRC_SOFT_RESET_WAIT_SNK_TX:
59e8594ebb63b2 Kyle Tso 2019-09-20  1141  		case SOFT_RESET:
59e8594ebb63b2 Kyle Tso 2019-09-20  1142  		case SOFT_RESET_SEND:
59e8594ebb63b2 Kyle Tso 2019-09-20  1143  			if (port->negotiated_rev >= PD_REV30)
59e8594ebb63b2 Kyle Tso 2019-09-20 @1144  				tcpm_set_state(port, AMS_START,
59e8594ebb63b2 Kyle Tso 2019-09-20  1145  					       cc_req == SINK_TX_OK ?
0ea47e4d06fe89 Kyle Tso 2019-09-20  1146  					       PD_T_SINK_TX : 0);
0ea47e4d06fe89 Kyle Tso 2019-09-20  1147  			else
59e8594ebb63b2 Kyle Tso 2019-09-20  1148  				tcpm_set_state(port, AMS_START, 0);
59e8594ebb63b2 Kyle Tso 2019-09-20  1149  			break;
59e8594ebb63b2 Kyle Tso 2019-09-20  1150  		default:
59e8594ebb63b2 Kyle Tso 2019-09-20  1151  			if (port->negotiated_rev >= PD_REV30)
59e8594ebb63b2 Kyle Tso 2019-09-20  1152  				tcpm_set_state(port, SRC_READY,
59e8594ebb63b2 Kyle Tso 2019-09-20  1153  					       cc_req == SINK_TX_OK ?
0ea47e4d06fe89 Kyle Tso 2019-09-20  1154  					       PD_T_SINK_TX : 0);
59e8594ebb63b2 Kyle Tso 2019-09-20  1155  			else
59e8594ebb63b2 Kyle Tso 2019-09-20  1156  				tcpm_set_state(port, SRC_READY, 0);
59e8594ebb63b2 Kyle Tso 2019-09-20  1157  			break;
59e8594ebb63b2 Kyle Tso 2019-09-20  1158  		}
0ea47e4d06fe89 Kyle Tso 2019-09-20  1159  	} else {
59e8594ebb63b2 Kyle Tso 2019-09-20  1160  		if (port->negotiated_rev >= PD_REV30 &&
59e8594ebb63b2 Kyle Tso 2019-09-20  1161  		    !tcpm_sink_tx_ok(port) &&
0ea47e4d06fe89 Kyle Tso 2019-09-20  1162  		    ams != SOFT_RESET_AMS &&
0ea47e4d06fe89 Kyle Tso 2019-09-20  1163  		    ams != HARD_RESET) {
0ea47e4d06fe89 Kyle Tso 2019-09-20  1164  			port->upcoming_state = INVALID_STATE;
0ea47e4d06fe89 Kyle Tso 2019-09-20  1165  			tcpm_log(port, "Sink TX No Go");
0ea47e4d06fe89 Kyle Tso 2019-09-20  1166  			return -EAGAIN;
0ea47e4d06fe89 Kyle Tso 2019-09-20  1167  		}
0ea47e4d06fe89 Kyle Tso 2019-09-20  1168  
0ea47e4d06fe89 Kyle Tso 2019-09-20  1169  		port->ams = ams;
0ea47e4d06fe89 Kyle Tso 2019-09-20  1170  
0ea47e4d06fe89 Kyle Tso 2019-09-20  1171  		if (ams == HARD_RESET) {
0ea47e4d06fe89 Kyle Tso 2019-09-20  1172  			tcpm_pd_transmit(port, TCPC_TX_HARD_RESET, NULL);
0ea47e4d06fe89 Kyle Tso 2019-09-20  1173  			tcpm_set_state(port, HARD_RESET_START, 0);
0ea47e4d06fe89 Kyle Tso 2019-09-20  1174  			return ret;
0ea47e4d06fe89 Kyle Tso 2019-09-20  1175  		} else if (tcpm_vdm_ams(port)) {
0ea47e4d06fe89 Kyle Tso 2019-09-20  1176  			return ret;
0ea47e4d06fe89 Kyle Tso 2019-09-20  1177  		}
0ea47e4d06fe89 Kyle Tso 2019-09-20  1178  
0ea47e4d06fe89 Kyle Tso 2019-09-20  1179  		if (port->state == SNK_READY ||
0ea47e4d06fe89 Kyle Tso 2019-09-20  1180  		    port->state == SNK_SOFT_RESET)
0ea47e4d06fe89 Kyle Tso 2019-09-20  1181  			tcpm_set_state(port, AMS_START, 0);
0ea47e4d06fe89 Kyle Tso 2019-09-20  1182  		else
0ea47e4d06fe89 Kyle Tso 2019-09-20  1183  			tcpm_set_state(port, SNK_READY, 0);
0ea47e4d06fe89 Kyle Tso 2019-09-20  1184  	}
0ea47e4d06fe89 Kyle Tso 2019-09-20  1185  
0ea47e4d06fe89 Kyle Tso 2019-09-20  1186  	return ret;
0ea47e4d06fe89 Kyle Tso 2019-09-20  1187  }
0ea47e4d06fe89 Kyle Tso 2019-09-20  1188  

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 69956 bytes --]

Re: [PATCH v3 0/2] tcpm: AMS and Collision Avoidance

[Hans de Goede]

Hi Kyle,

On 20-09-2019 05:24, Kyle Tso wrote:
> *** BLURB HERE ***
> 
> Kyle Tso (2):
>    usb: typec: tcpm: AMS and Collision Avoidance
>    usb: typec: tcpm: AMS for PD2.0

I've finally gotten a chance to test this on one of my own devices
which uses the tcpm framework for its Type-c port.

I am afraid that this series breaks DP altmode support,
specifically, the dp_altmode_configure() function no longer
seems to get called, leading to no pin-assignment being
selected by default and DP thus not working.

So sorry, but I have to NACK this series since it causes
regressions.

It might be easiest if you can get yourself some hardware
which supports DP altmode and uses the fusb302 Type-C
controller (which unlike your controller is actually
supported by the mainline kernel).

2 devices which have this are the original (version 1)
of the GPD win and the GPD pocket. Since the version
is not always clearly marked, make sure you get one which
has a X7-Z8750 CPU, those are the version 1 models, you
can still get these e.g. here:

https://www.geekbuying.com/item/GPD-Pocket-7-Inch-Tablet-PC-Intel-Atom-X7-Z8750-8GB-128GB-375711.html
https://www.geekbuying.com/item/GPD-Win-Intel-Z8750-Windows-10-4GB-64GB-Gamepad-Tablet-PC---Black-378018.html

These 2 devices still need 2 minor patches to hookup the DP
support, I have just finished these 2 patches up and I'm
submitting them upstream today, I will Cc you on these.

If you combine these with one of the many DP-charging pass-through
+ USB-3 out + HDMI out dongles, e.g.:
https://www.aliexpress.com/item/32953320909.html

And then after plugging in do:

cat /sys/class/typec/port0-partner/port0-partner.0/displayport/pin_assignment

This should print:

C [D]

But when I add your patches into the mix it prints just:

C D

And these debug pr_err calls never happen:

diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
index 7845df030b72..d14f94078dd9 100644
--- a/drivers/usb/typec/altmodes/displayport.c
+++ b/drivers/usb/typec/altmodes/displayport.c
@@ -106,6 +106,7 @@ static int dp_altmode_configure(struct dp_altmode *dp, u8 con)
  		break;
  	}

+	pr_err("dp_altmode_configure pin_assign %08x conf %08x\n", pin_assign, DP_CONF_GET_PIN_ASSIGN(dp->data.conf));
  	/* Determining the initial pin assignment. */
  	if (!DP_CONF_GET_PIN_ASSIGN(dp->data.conf)) {
  		/* Is USB together with DP preferred */
@@ -115,6 +116,8 @@ static int dp_altmode_configure(struct dp_altmode *dp, u8 con)
  		else if (pin_assign & DP_PIN_ASSIGN_DP_ONLY_MASK)
  			pin_assign &= DP_PIN_ASSIGN_DP_ONLY_MASK;

+		pr_err("dp_altmode_configure masked pin_assign %08x\n", pin_assign);
+
  		if (!pin_assign)
  			return -EINVAL;


Regards,

Hans

Re: [PATCH v3 0/2] tcpm: AMS and Collision Avoidance

[Kyle Tso]

Hi Hans

Could you append the TCPM log?

On Thu, Oct 3, 2019 at 5:47 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi Kyle,
>
> On 20-09-2019 05:24, Kyle Tso wrote:
> > *** BLURB HERE ***
> >
> > Kyle Tso (2):
> >    usb: typec: tcpm: AMS and Collision Avoidance
> >    usb: typec: tcpm: AMS for PD2.0
>
> I've finally gotten a chance to test this on one of my own devices
> which uses the tcpm framework for its Type-c port.
>
> I am afraid that this series breaks DP altmode support,
> specifically, the dp_altmode_configure() function no longer
> seems to get called, leading to no pin-assignment being
> selected by default and DP thus not working.
>
> So sorry, but I have to NACK this series since it causes
> regressions.
>
> It might be easiest if you can get yourself some hardware
> which supports DP altmode and uses the fusb302 Type-C
> controller (which unlike your controller is actually
> supported by the mainline kernel).
>
> 2 devices which have this are the original (version 1)
> of the GPD win and the GPD pocket. Since the version
> is not always clearly marked, make sure you get one which
> has a X7-Z8750 CPU, those are the version 1 models, you
> can still get these e.g. here:
>
> https://www.geekbuying.com/item/GPD-Pocket-7-Inch-Tablet-PC-Intel-Atom-X7-Z8750-8GB-128GB-375711.html
> https://www.geekbuying.com/item/GPD-Win-Intel-Z8750-Windows-10-4GB-64GB-Gamepad-Tablet-PC---Black-378018.html
>
> These 2 devices still need 2 minor patches to hookup the DP
> support, I have just finished these 2 patches up and I'm
> submitting them upstream today, I will Cc you on these.
>
> If you combine these with one of the many DP-charging pass-through
> + USB-3 out + HDMI out dongles, e.g.:
> https://www.aliexpress.com/item/32953320909.html
>
> And then after plugging in do:
>
> cat /sys/class/typec/port0-partner/port0-partner.0/displayport/pin_assignment
>
> This should print:
>
> C [D]
>
> But when I add your patches into the mix it prints just:
>
> C D
>
> And these debug pr_err calls never happen:
>
> diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
> index 7845df030b72..d14f94078dd9 100644
> --- a/drivers/usb/typec/altmodes/displayport.c
> +++ b/drivers/usb/typec/altmodes/displayport.c
> @@ -106,6 +106,7 @@ static int dp_altmode_configure(struct dp_altmode *dp, u8 con)
>                 break;
>         }
>
> +       pr_err("dp_altmode_configure pin_assign %08x conf %08x\n", pin_assign, DP_CONF_GET_PIN_ASSIGN(dp->data.conf));
>         /* Determining the initial pin assignment. */
>         if (!DP_CONF_GET_PIN_ASSIGN(dp->data.conf)) {
>                 /* Is USB together with DP preferred */
> @@ -115,6 +116,8 @@ static int dp_altmode_configure(struct dp_altmode *dp, u8 con)
>                 else if (pin_assign & DP_PIN_ASSIGN_DP_ONLY_MASK)
>                         pin_assign &= DP_PIN_ASSIGN_DP_ONLY_MASK;
>
> +               pr_err("dp_altmode_configure masked pin_assign %08x\n", pin_assign);
> +
>                 if (!pin_assign)
>                         return -EINVAL;
>
>
> Regards,
>
> Hans
>

Re: [PATCH v3 0/2] tcpm: AMS and Collision Avoidance

[Hans de Goede]

[-- Attachment #1: Type: text/plain, Size: 4183 bytes --]

Hi,

On 03-10-2019 12:04, Kyle Tso wrote:
> Hi Hans
> 
> Could you append the TCPM log?

I've attached both good and bad logs, both start with plugging in
one of these PD charging pass-through + USB-3 + HDMI out dongles.

at a quick glance the problem
seems to be that with the 2 AMS patches added we stop transmitting
after:

[  137.751964] Rx VDM cmd 0xff018144 type 1 cmd 4 len 1

Where as the good logs still transmits (and receives) a couple of
packets extra after this:

[ 4475.965108] Rx VDM cmd 0xff018144 type 1 cmd 4 len 1
[ 4475.965224] PD TX, header: 0x2f6f
[ 4475.968979] PD TX complete, status: 0
[ 4475.980811] PD RX, header: 0x2a4f [1]
[ 4475.980816] Rx VDM cmd 0xff018150 type 1 cmd 16 len 2
[ 4475.980929] PD TX, header: 0x216f
[ 4475.984093] PD TX complete, status: 0
[ 4475.996798] PD RX, header: 0x1c4f [1]
[ 4475.996803] Rx VDM cmd 0xff018151 type 1 cmd 17 len 1

Regards,

Hans


> 
> On Thu, Oct 3, 2019 at 5:47 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi Kyle,
>>
>> On 20-09-2019 05:24, Kyle Tso wrote:
>>> *** BLURB HERE ***
>>>
>>> Kyle Tso (2):
>>>     usb: typec: tcpm: AMS and Collision Avoidance
>>>     usb: typec: tcpm: AMS for PD2.0
>>
>> I've finally gotten a chance to test this on one of my own devices
>> which uses the tcpm framework for its Type-c port.
>>
>> I am afraid that this series breaks DP altmode support,
>> specifically, the dp_altmode_configure() function no longer
>> seems to get called, leading to no pin-assignment being
>> selected by default and DP thus not working.
>>
>> So sorry, but I have to NACK this series since it causes
>> regressions.
>>
>> It might be easiest if you can get yourself some hardware
>> which supports DP altmode and uses the fusb302 Type-C
>> controller (which unlike your controller is actually
>> supported by the mainline kernel).
>>
>> 2 devices which have this are the original (version 1)
>> of the GPD win and the GPD pocket. Since the version
>> is not always clearly marked, make sure you get one which
>> has a X7-Z8750 CPU, those are the version 1 models, you
>> can still get these e.g. here:
>>
>> https://www.geekbuying.com/item/GPD-Pocket-7-Inch-Tablet-PC-Intel-Atom-X7-Z8750-8GB-128GB-375711.html
>> https://www.geekbuying.com/item/GPD-Win-Intel-Z8750-Windows-10-4GB-64GB-Gamepad-Tablet-PC---Black-378018.html
>>
>> These 2 devices still need 2 minor patches to hookup the DP
>> support, I have just finished these 2 patches up and I'm
>> submitting them upstream today, I will Cc you on these.
>>
>> If you combine these with one of the many DP-charging pass-through
>> + USB-3 out + HDMI out dongles, e.g.:
>> https://www.aliexpress.com/item/32953320909.html
>>
>> And then after plugging in do:
>>
>> cat /sys/class/typec/port0-partner/port0-partner.0/displayport/pin_assignment
>>
>> This should print:
>>
>> C [D]
>>
>> But when I add your patches into the mix it prints just:
>>
>> C D
>>
>> And these debug pr_err calls never happen:
>>
>> diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
>> index 7845df030b72..d14f94078dd9 100644
>> --- a/drivers/usb/typec/altmodes/displayport.c
>> +++ b/drivers/usb/typec/altmodes/displayport.c
>> @@ -106,6 +106,7 @@ static int dp_altmode_configure(struct dp_altmode *dp, u8 con)
>>                  break;
>>          }
>>
>> +       pr_err("dp_altmode_configure pin_assign %08x conf %08x\n", pin_assign, DP_CONF_GET_PIN_ASSIGN(dp->data.conf));
>>          /* Determining the initial pin assignment. */
>>          if (!DP_CONF_GET_PIN_ASSIGN(dp->data.conf)) {
>>                  /* Is USB together with DP preferred */
>> @@ -115,6 +116,8 @@ static int dp_altmode_configure(struct dp_altmode *dp, u8 con)
>>                  else if (pin_assign & DP_PIN_ASSIGN_DP_ONLY_MASK)
>>                          pin_assign &= DP_PIN_ASSIGN_DP_ONLY_MASK;
>>
>> +               pr_err("dp_altmode_configure masked pin_assign %08x\n", pin_assign);
>> +
>>                  if (!pin_assign)
>>                          return -EINVAL;
>>
>>
>> Regards,
>>
>> Hans
>>

[-- Attachment #2: tcpm-bad.log --]
[-- Type: text/x-log, Size: 5063 bytes --]

[   45.033737] Setting voltage/current limit 0 mV 0 mA
[   45.034062] polarity 0
[   45.034066] Requesting mux state 0, usb-role 0, orientation 0
[   45.039791] state change INVALID_STATE -> SNK_UNATTACHED [rev1 NONE_AMS]
[   45.040579] CC1: 0 -> 0, CC2: 0 -> 0 [state SNK_UNATTACHED, polarity 0, disconnected]
[   45.040603] i2c-fusb302: registered
[   45.043532] Setting voltage/current limit 0 mV 0 mA
[   45.043543] polarity 0
[   45.043546] Requesting mux state 0, usb-role 0, orientation 0
[   45.045309] cc:=0
[   45.047407] pending state change PORT_RESET -> PORT_RESET_WAIT_OFF @ 100 ms [rev1 NONE_AMS]
[   45.153486] state change PORT_RESET -> PORT_RESET_WAIT_OFF [delayed 100 ms]
[   45.153501] state change PORT_RESET_WAIT_OFF -> SNK_UNATTACHED [rev1 NONE_AMS]
[   45.153510] Start toggling
[  136.458043] CC1: 0 -> 2, CC2: 0 -> 1 [state TOGGLING, polarity 0, connected]
[  136.458056] state change TOGGLING -> SRC_ATTACH_WAIT [rev1 NONE_AMS]
[  136.458124] pending state change SRC_ATTACH_WAIT -> SNK_TRY @ 200 ms [rev1 NONE_AMS]
[  136.659629] state change SRC_ATTACH_WAIT -> SNK_TRY [delayed 200 ms]
[  136.659647] cc:=2
[  136.662964] pending state change SNK_TRY -> SNK_TRY_WAIT @ 100 ms [rev1 NONE_AMS]
[  136.771185] state change SNK_TRY -> SNK_TRY_WAIT [delayed 100 ms]
[  136.771200] state change SNK_TRY_WAIT -> SRC_TRYWAIT [rev1 NONE_AMS]
[  136.771207] cc:=4
[  136.774216] pending state change SRC_TRYWAIT -> SRC_TRYWAIT_UNATTACHED @ 100 ms [rev1 NONE_AMS]
[  136.874416] state change SRC_TRYWAIT -> SRC_TRYWAIT_UNATTACHED [delayed 100 ms]
[  136.874427] state change SRC_TRYWAIT_UNATTACHED -> SNK_UNATTACHED [rev1 NONE_AMS]
[  136.874434] Start toggling
[  136.876604] state change SNK_UNATTACHED -> TOGGLING [rev1 NONE_AMS]
[  136.935349] CC1: 2 -> 2, CC2: 1 -> 1 [state TOGGLING, polarity 0, connected]
[  136.935356] state change TOGGLING -> SRC_ATTACH_WAIT [rev1 NONE_AMS]
[  136.935376] pending state change SRC_ATTACH_WAIT -> SRC_ATTACHED @ 200 ms [rev1 NONE_AMS]
[  137.138416] state change SRC_ATTACH_WAIT -> SRC_ATTACHED [delayed 200 ms]
[  137.138431] polarity 0
[  137.138438] Requesting mux state 1, usb-role 1, orientation 1
[  137.400456] vconn:=1
[  137.401212] vbus:=1 charge=0
[  137.414633] pending state change SRC_ATTACHED -> SRC_UNATTACHED @ 480 ms [rev1 NONE_AMS]
[  137.434650] VBUS on
[  137.434656] state change SRC_ATTACHED -> SRC_STARTUP [rev1 NONE_AMS]
[  137.435506] AMS POWER_NEGOTIATION start
[  137.435513] cc:=4
[  137.440580] state change SRC_STARTUP -> AMS_START [rev3 POWER_NEGOTIATION]
[  137.440591] state change AMS_START -> SRC_SEND_CAPABILITIES [rev3 POWER_NEGOTIATION]
[  137.440596] PD TX, header: 0x11a1
[  137.449240] PD TX complete, status: 2
[  137.451075] pending state change SRC_SEND_CAPABILITIES -> SRC_SEND_CAPABILITIES @ 150 ms [rev3 POWER_NEGOTIATION]
[  137.603369] state change SRC_SEND_CAPABILITIES -> SRC_SEND_CAPABILITIES [delayed 150 ms]
[  137.603380] PD TX, header: 0x11a1
[  137.606492] PD TX complete, status: 0
[  137.606544] pending state change SRC_SEND_CAPABILITIES -> SRC_SEND_CAPABILITIES_TIMEOUT @ 150 ms [rev3 POWER_NEGOTIATION]
[  137.612978] PD RX, header: 0x1042 [1]
[  137.613001] state change SRC_SEND_CAPABILITIES -> SRC_NEGOTIATE_CAPABILITIES [rev2 POWER_NEGOTIATION]
[  137.613171] Requested 5000 mV, 1500 mA for 1500 / 1500 mA
[  137.613180] PD TX, header: 0x363
[  137.617198] PD TX complete, status: 0
[  137.617237] pending state change SRC_NEGOTIATE_CAPABILITIES -> SRC_TRANSITION_SUPPLY @ 35 ms [rev2 POWER_NEGOTIATION]
[  137.652433] state change SRC_NEGOTIATE_CAPABILITIES -> SRC_TRANSITION_SUPPLY [delayed 35 ms]
[  137.652444] PD TX, header: 0x566
[  137.655955] PD TX complete, status: 0
[  137.656164] state change SRC_TRANSITION_SUPPLY -> SRC_READY [rev2 POWER_NEGOTIATION]
[  137.656540] AMS POWER_NEGOTIATION finished
[  137.656973] AMS DISCOVER_IDENTITY start
[  137.657101] PD TX, header: 0x176f
[  137.660691] PD TX complete, status: 0
[  137.676290] PD RX, header: 0x524f [1]
[  137.676298] Rx VDM cmd 0xff008041 type 1 cmd 1 len 5
[  137.677650] Identity: 177a:1120.0001
[  137.677660] AMS DISCOVER_IDENTITY finished
[  137.677694] AMS DISCOVER_SVIDS start
[  137.677706] PD TX, header: 0x196f
[  137.680918] PD TX complete, status: 0
[  137.690755] PD RX, header: 0x244f [1]
[  137.690767] Rx VDM cmd 0xff008042 type 1 cmd 2 len 2
[  137.690773] SVID 1: 0xff01
[  137.690777] AMS DISCOVER_SVIDS finished
[  137.690799] AMS DISCOVER_MODES start
[  137.690813] PD TX, header: 0x1b6f
[  137.694841] PD TX complete, status: 0
[  137.706629] PD RX, header: 0x264f [1]
[  137.706638] Rx VDM cmd 0xff018043 type 1 cmd 3 len 2
[  137.706644]  Alternate mode 0: SVID 0xff01, VDO 1: 0x00000c05
[  137.707886] AMS DISCOVER_MODES finished
[  137.739303] AMS DFP_TO_UFP_ENTER_MODE start
[  137.740205] PD TX, header: 0x1d6f
[  137.743047] PD TX complete, status: 0
[  137.751952] PD RX, header: 0x184f [1]
[  137.751964] Rx VDM cmd 0xff018144 type 1 cmd 4 len 1

[-- Attachment #3: tcpm-good.log --]
[-- Type: text/x-log, Size: 3781 bytes --]

[ 4474.691171] CC1: 0 -> 2, CC2: 0 -> 1 [state TOGGLING, polarity 0, connected]
[ 4474.691176] state change TOGGLING -> SRC_ATTACH_WAIT
[ 4474.691192] pending state change SRC_ATTACH_WAIT -> SNK_TRY @ 200 ms
[ 4474.896231] state change SRC_ATTACH_WAIT -> SNK_TRY [delayed 200 ms]
[ 4474.896249] cc:=2
[ 4474.899518] pending state change SNK_TRY -> SNK_TRY_WAIT @ 100 ms
[ 4475.008034] state change SNK_TRY -> SNK_TRY_WAIT [delayed 100 ms]
[ 4475.008058] state change SNK_TRY_WAIT -> SRC_TRYWAIT
[ 4475.008070] cc:=4
[ 4475.011131] pending state change SRC_TRYWAIT -> SRC_TRYWAIT_UNATTACHED @ 100 ms
[ 4475.119009] state change SRC_TRYWAIT -> SRC_TRYWAIT_UNATTACHED [delayed 100 ms]
[ 4475.119039] state change SRC_TRYWAIT_UNATTACHED -> SNK_UNATTACHED
[ 4475.119051] Start toggling
[ 4475.122693] state change SNK_UNATTACHED -> TOGGLING
[ 4475.181832] CC1: 2 -> 2, CC2: 1 -> 1 [state TOGGLING, polarity 0, connected]
[ 4475.181843] state change TOGGLING -> SRC_ATTACH_WAIT
[ 4475.181879] pending state change SRC_ATTACH_WAIT -> SRC_ATTACHED @ 200 ms
[ 4475.383256] state change SRC_ATTACH_WAIT -> SRC_ATTACHED [delayed 200 ms]
[ 4475.383268] polarity 0
[ 4475.383272] Requesting mux state 1, usb-role 1, orientation 1
[ 4475.647963] vconn:=1
[ 4475.648307] vbus:=1 charge=0
[ 4475.666560] pending state change SRC_ATTACHED -> SRC_UNATTACHED @ 480 ms
[ 4475.685693] VBUS on
[ 4475.685697] state change SRC_ATTACHED -> SRC_STARTUP
[ 4475.685822] state change SRC_STARTUP -> SRC_SEND_CAPABILITIES
[ 4475.685826] PD TX, header: 0x11a1
[ 4475.693636] PD TX complete, status: 2
[ 4475.693663] pending state change SRC_SEND_CAPABILITIES -> SRC_SEND_CAPABILITIES @ 150 ms
[ 4475.846995] state change SRC_SEND_CAPABILITIES -> SRC_SEND_CAPABILITIES [delayed 150 ms]
[ 4475.847008] PD TX, header: 0x11a1
[ 4475.851029] PD TX complete, status: 0
[ 4475.851195] pending state change SRC_SEND_CAPABILITIES -> SRC_SEND_CAPABILITIES_TIMEOUT @ 150 ms
[ 4475.857173] PD RX, header: 0x1042 [1]
[ 4475.857186] state change SRC_SEND_CAPABILITIES -> SRC_NEGOTIATE_CAPABILITIES
[ 4475.857212] Requested 5000 mV, 1500 mA for 1500 / 1500 mA
[ 4475.857220] PD TX, header: 0x363
[ 4475.860901] PD TX complete, status: 0
[ 4475.860970] pending state change SRC_NEGOTIATE_CAPABILITIES -> SRC_TRANSITION_SUPPLY @ 35 ms
[ 4475.897261] state change SRC_NEGOTIATE_CAPABILITIES -> SRC_TRANSITION_SUPPLY [delayed 35 ms]
[ 4475.897290] PD TX, header: 0x566
[ 4475.901901] PD TX complete, status: 0
[ 4475.902200] state change SRC_TRANSITION_SUPPLY -> SRC_READY
[ 4475.902549] PD TX, header: 0x176f
[ 4475.906360] PD TX complete, status: 0
[ 4475.921523] PD RX, header: 0x524f [1]
[ 4475.921530] Rx VDM cmd 0xff008041 type 1 cmd 1 len 5
[ 4475.921555] Identity: 177a:1120.0001
[ 4475.921570] PD TX, header: 0x196f
[ 4475.925036] PD TX complete, status: 0
[ 4475.935315] PD RX, header: 0x244f [1]
[ 4475.935322] Rx VDM cmd 0xff008042 type 1 cmd 2 len 2
[ 4475.935326] SVID 1: 0xff01
[ 4475.935343] PD TX, header: 0x1b6f
[ 4475.939463] PD TX complete, status: 0
[ 4475.950648] PD RX, header: 0x264f [1]
[ 4475.950661] Rx VDM cmd 0xff018043 type 1 cmd 3 len 2
[ 4475.950674]  Alternate mode 0: SVID 0xff01, VDO 1: 0x00000c05
[ 4475.951938] PD TX, header: 0x1d6f
[ 4475.955930] PD TX complete, status: 0
[ 4475.965103] PD RX, header: 0x184f [1]
[ 4475.965108] Rx VDM cmd 0xff018144 type 1 cmd 4 len 1
[ 4475.965224] PD TX, header: 0x2f6f
[ 4475.968979] PD TX complete, status: 0
[ 4475.980811] PD RX, header: 0x2a4f [1]
[ 4475.980816] Rx VDM cmd 0xff018150 type 1 cmd 16 len 2
[ 4475.980929] PD TX, header: 0x216f
[ 4475.984093] PD TX complete, status: 0
[ 4475.996798] PD RX, header: 0x1c4f [1]
[ 4475.996803] Rx VDM cmd 0xff018151 type 1 cmd 17 len 1

Re: [PATCH v3 0/2] tcpm: AMS and Collision Avoidance

[Kyle Tso]

Hi Hans,

It seems that there is a bug in the patch. Thank you for catching this!
I will revise the patch and upload it again.





On Thu, Oct 3, 2019 at 6:38 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 03-10-2019 12:04, Kyle Tso wrote:
> > Hi Hans
> >
> > Could you append the TCPM log?
>
> I've attached both good and bad logs, both start with plugging in
> one of these PD charging pass-through + USB-3 + HDMI out dongles.
>
> at a quick glance the problem
> seems to be that with the 2 AMS patches added we stop transmitting
> after:
>
> [  137.751964] Rx VDM cmd 0xff018144 type 1 cmd 4 len 1
>
> Where as the good logs still transmits (and receives) a couple of
> packets extra after this:
>
> [ 4475.965108] Rx VDM cmd 0xff018144 type 1 cmd 4 len 1
> [ 4475.965224] PD TX, header: 0x2f6f
> [ 4475.968979] PD TX complete, status: 0
> [ 4475.980811] PD RX, header: 0x2a4f [1]
> [ 4475.980816] Rx VDM cmd 0xff018150 type 1 cmd 16 len 2
> [ 4475.980929] PD TX, header: 0x216f
> [ 4475.984093] PD TX complete, status: 0
> [ 4475.996798] PD RX, header: 0x1c4f [1]
> [ 4475.996803] Rx VDM cmd 0xff018151 type 1 cmd 17 len 1
>
> Regards,
>
> Hans
>
>
> >
> > On Thu, Oct 3, 2019 at 5:47 PM Hans de Goede <hdegoede@redhat.com> wrote:
> >>
> >> Hi Kyle,
> >>
> >> On 20-09-2019 05:24, Kyle Tso wrote:
> >>> *** BLURB HERE ***
> >>>
> >>> Kyle Tso (2):
> >>>     usb: typec: tcpm: AMS and Collision Avoidance
> >>>     usb: typec: tcpm: AMS for PD2.0
> >>
> >> I've finally gotten a chance to test this on one of my own devices
> >> which uses the tcpm framework for its Type-c port.
> >>
> >> I am afraid that this series breaks DP altmode support,
> >> specifically, the dp_altmode_configure() function no longer
> >> seems to get called, leading to no pin-assignment being
> >> selected by default and DP thus not working.
> >>
> >> So sorry, but I have to NACK this series since it causes
> >> regressions.
> >>
> >> It might be easiest if you can get yourself some hardware
> >> which supports DP altmode and uses the fusb302 Type-C
> >> controller (which unlike your controller is actually
> >> supported by the mainline kernel).
> >>
> >> 2 devices which have this are the original (version 1)
> >> of the GPD win and the GPD pocket. Since the version
> >> is not always clearly marked, make sure you get one which
> >> has a X7-Z8750 CPU, those are the version 1 models, you
> >> can still get these e.g. here:
> >>
> >> https://www.geekbuying.com/item/GPD-Pocket-7-Inch-Tablet-PC-Intel-Atom-X7-Z8750-8GB-128GB-375711.html
> >> https://www.geekbuying.com/item/GPD-Win-Intel-Z8750-Windows-10-4GB-64GB-Gamepad-Tablet-PC---Black-378018.html
> >>
> >> These 2 devices still need 2 minor patches to hookup the DP
> >> support, I have just finished these 2 patches up and I'm
> >> submitting them upstream today, I will Cc you on these.
> >>
> >> If you combine these with one of the many DP-charging pass-through
> >> + USB-3 out + HDMI out dongles, e.g.:
> >> https://www.aliexpress.com/item/32953320909.html
> >>
> >> And then after plugging in do:
> >>
> >> cat /sys/class/typec/port0-partner/port0-partner.0/displayport/pin_assignment
> >>
> >> This should print:
> >>
> >> C [D]
> >>
> >> But when I add your patches into the mix it prints just:
> >>
> >> C D
> >>
> >> And these debug pr_err calls never happen:
> >>
> >> diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
> >> index 7845df030b72..d14f94078dd9 100644
> >> --- a/drivers/usb/typec/altmodes/displayport.c
> >> +++ b/drivers/usb/typec/altmodes/displayport.c
> >> @@ -106,6 +106,7 @@ static int dp_altmode_configure(struct dp_altmode *dp, u8 con)
> >>                  break;
> >>          }
> >>
> >> +       pr_err("dp_altmode_configure pin_assign %08x conf %08x\n", pin_assign, DP_CONF_GET_PIN_ASSIGN(dp->data.conf));
> >>          /* Determining the initial pin assignment. */
> >>          if (!DP_CONF_GET_PIN_ASSIGN(dp->data.conf)) {
> >>                  /* Is USB together with DP preferred */
> >> @@ -115,6 +116,8 @@ static int dp_altmode_configure(struct dp_altmode *dp, u8 con)
> >>                  else if (pin_assign & DP_PIN_ASSIGN_DP_ONLY_MASK)
> >>                          pin_assign &= DP_PIN_ASSIGN_DP_ONLY_MASK;
> >>
> >> +               pr_err("dp_altmode_configure masked pin_assign %08x\n", pin_assign);
> >> +
> >>                  if (!pin_assign)
> >>                          return -EINVAL;
> >>
> >>
> >> Regards,
> >>
> >> Hans
> >>