Skip to contentSkip to conversation

staging: typec: tcpm: Constify alternate modes

PATCH New Guenter Roeck 1 participant 7 messages Aug 11, 2017 Staging

lore
Share & tools
lore

Guenter Roeck

author
#0 lore raw

PATCH 1/7 staging: typec: tcpm: Constify alternate modes

Constify alternate mode configuration data which won't be touched by the driver.

Signed-off-by Guenter Roeck <[email protected]>

---
This series has been in my queue and kind of got lost. Sorry for the delay.

 drivers/staging/typec/tcpm.c | 2 +-
 drivers/staging/typec/tcpm.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
drivers/staging/typec/tcpm.c +1 -1
index 20eb4ebcf8c3..06c49873df8e 100644
@@ -3485,7 +3485,7 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
 	}
 
 	if (tcpc->config->alt_modes) {
-		struct typec_altmode_desc *paltmode = tcpc->config->alt_modes;
+		const struct typec_altmode_desc *paltmode = tcpc->config->alt_modes;
 
 		i = 0;
 		while (paltmode->svid && i < ARRAY_SIZE(port->port_altmode)) {
drivers/staging/typec/tcpm.h +1 -1
index 19c307d31a5a..4c6b38cb2c8a 100644
@@ -72,7 +72,7 @@ struct tcpc_config {
 	enum typec_role default_role;
 	bool try_role_hw;	/* try.{src,snk} implemented in hardware */
 
-	struct typec_altmode_desc *alt_modes;
+	const struct typec_altmode_desc *alt_modes;
 };
 
 enum tcpc_usb_switch {
Signature · 1 line
2.7.4
author
#1 lore raw

PATCH 2/7 staging: typec: tcpm: Report role swap complete after entering READY state

Role swap requests fail unless the current role is either SRC_READY or SNK_READY. This works fine for VCONN and data role swaps, where we immediately enter READY state after reporting a successful role swap to user space. However, on power role changes, the role swap is currently reported as successful while power negotiation is still in process. User space does not know this, and may request another role swap immediately after a power role swap is reported to be complete. This second role swap will fail with -EAGAIN.

To fix the problem, report role swap completion after power negotiation is complete and the state machine enters SRC_READY or SNK_READY state. This is better anyway since it captures errors due to failed power negotiations. It also simplifies the code since the number of calls needed to report successful role swaps is reduced.

Reported-by Howard Yen <[email protected]>
Signed-off-by Guenter Roeck <[email protected]>
---
 drivers/staging/typec/tcpm.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)
drivers/staging/typec/tcpm.c +2 -7
index 06c49873df8e..be2a91b52d69 100644
@@ -2262,8 +2262,8 @@ static void run_state_machine(struct tcpm_port *port)
 #endif
 		port->try_src_count = 0;
 
+		tcpm_swap_complete(port, 0);
 		tcpm_typec_connect(port);
-
 		tcpm_check_send_discover(port);
 		/*
 		 * 6.3.5

@@ -2445,8 +2445,8 @@ static void run_state_machine(struct tcpm_port *port)
 		typec_set_pwr_opmode(port->typec_port, TYPEC_PWR_MODE_PD);
 		port->pwr_opmode = TYPEC_PWR_MODE_PD;
 
+		tcpm_swap_complete(port, 0);
 		tcpm_typec_connect(port);
-
 		tcpm_check_send_discover(port);
 		break;
 

@@ -2574,7 +2574,6 @@ static void run_state_machine(struct tcpm_port *port)
 				       TYPEC_HOST);
 			port->send_discover = true;
 		}
-		tcpm_swap_complete(port, 0);
 		tcpm_set_state(port, ready_state(port), 0);
 		break;
 

@@ -2622,7 +2621,6 @@ static void run_state_machine(struct tcpm_port *port)
 		tcpm_set_state_cond(port, SNK_UNATTACHED, PD_T_PS_SOURCE_ON);
 		break;
 	case PR_SWAP_SRC_SNK_SINK_ON:
-		tcpm_swap_complete(port, 0);
 		tcpm_set_state(port, SNK_STARTUP, 0);
 		break;
 	case PR_SWAP_SNK_SRC_SINK_OFF:

@@ -2642,7 +2640,6 @@ static void run_state_machine(struct tcpm_port *port)
 		 */
 		tcpm_set_pwr_role(port, TYPEC_SOURCE);
 		tcpm_pd_send_control(port, PD_CTRL_PS_RDY);
-		tcpm_swap_complete(port, 0);
 		tcpm_set_state(port, SRC_STARTUP, 0);
 		break;
 

@@ -2672,12 +2669,10 @@ static void run_state_machine(struct tcpm_port *port)
 	case VCONN_SWAP_TURN_ON_VCONN:
 		tcpm_set_vconn(port, true);
 		tcpm_pd_send_control(port, PD_CTRL_PS_RDY);
-		tcpm_swap_complete(port, 0);
 		tcpm_set_state(port, ready_state(port), 0);
 		break;
 	case VCONN_SWAP_TURN_OFF_VCONN:
 		tcpm_set_vconn(port, false);
-		tcpm_swap_complete(port, 0);
 		tcpm_set_state(port, ready_state(port), 0);
 		break;
Signature · 1 line
2.7.4
author
#2 lore raw

PATCH 3/7 staging: typec: tcpm: Set default state after error recovery based on port type

In reply to Guenter Roeck

From: Badhri Jagan Sridharan <[email protected]>

While exiting ERROR_RECOVERY, choose default state based on the port type instead of current power role.

Quoting from specification:

4.5.2.2.2 ErrorRecovery State
This state appears in Figure 4-12, Figure 4-13, Figure 4-14, Figure 4-15, Figure 4-16 and Figure 4-17.
The ErrorRecovery state is where the port removes the terminations from the CC1 and CC2 pins for tErrorRecovery followed by transitioning to the appropriate Unattached.SNK or Unattached.SRC state based on port type. This is the equivalent of forcing a detach event and looking for a new attach.

Signed-off-by Badhri Jagan Sridharan <[email protected]>
Signed-off-by Guenter Roeck <[email protected]>
---
 drivers/staging/typec/tcpm.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
drivers/staging/typec/tcpm.c +1 -4
index be2a91b52d69..e7873286b5ea 100644
@@ -3038,10 +3038,7 @@ static void _tcpm_pd_vbus_off(struct tcpm_port *port)
 		break;
 
 	case ERROR_RECOVERY_WAIT_OFF:
-		tcpm_set_state(port,
-			       port->pwr_role == TYPEC_SOURCE ?
-					SRC_UNATTACHED : SNK_UNATTACHED,
-			       0);
+		tcpm_set_state(port, tcpm_default_state(port), 0);
 		break;
 
 	default:
Signature · 1 line
2.7.4
author
#3 lore raw

PATCH 4/7 staging: typec: tcpm: Select default state based on port type

In reply to Guenter Roeck

From: Badhri Jagan Sridharan <[email protected]>

tcpm_default_state wasn't considering the port type when determining the default role. This change makes tcpm_default_state to consider port type as well.

tcpm_default_state would return the following based on the port type:
TYPEC_PORT_UFP - SNK_UNATTACHED
TYPEC_PORT_DFP - SRC_UNATTACHED
TYPEC_PORT_DRP - based on the preferred_role setting

Signed-off-by Badhri Jagan Sridharan <[email protected]>
[groeck: Reworded description; minor formatting changes]
Signed-off-by Guenter Roeck <[email protected]>
---
 drivers/staging/typec/tcpm.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)
drivers/staging/typec/tcpm.c +10 -5
index e7873286b5ea..cf498c68c4af 100644
@@ -332,12 +332,17 @@ struct pd_rx_event {
 
 static enum tcpm_state tcpm_default_state(struct tcpm_port *port)
 {
-	if (port->try_role == TYPEC_SINK)
-		return SNK_UNATTACHED;
-	else if (port->try_role == TYPEC_SOURCE)
-		return SRC_UNATTACHED;
-	else if (port->tcpc->config->default_role == TYPEC_SINK)
+	if (port->typec_caps.type == TYPEC_PORT_DRP) {
+		if (port->try_role == TYPEC_SINK)
+			return SNK_UNATTACHED;
+		else if (port->try_role == TYPEC_SOURCE)
+			return SRC_UNATTACHED;
+		else if (port->tcpc->config->default_role == TYPEC_SINK)
+			return SNK_UNATTACHED;
+		/* Fall through to return SRC_UNATTACHED */
+	} else if (port->typec_caps.type == TYPEC_PORT_UFP) {
 		return SNK_UNATTACHED;
+	}
 	return SRC_UNATTACHED;
 }
Signature · 1 line
2.7.4
author
#4 lore raw

PATCH 5/7 staging: typec: tcpm: Add timeout when waiting for role swap completion

In reply to Guenter Roeck

The Type-C protocol manager state machine could fail, which might result in role swap requests from user space to hang forever. Add a generous timeout when waiting for role swaps to complete to avoid this situation.

Originally-from: Badhri Jagan Sridharan <[email protected]>

Signed-off-by Guenter Roeck <[email protected]>
---
 drivers/staging/typec/tcpm.c | 21 +++++++++++++++------
 drivers/staging/typec/tcpm.h |  3 ++-
 2 files changed, 17 insertions(+), 7 deletions(-)
drivers/staging/typec/tcpm.c +15 -6
index cf498c68c4af..515bec1807d7 100644
@@ -3166,9 +3166,12 @@ static int tcpm_dr_set(const struct typec_capability *cap,
 	tcpm_set_state(port, DR_SWAP_SEND, 0);
 	mutex_unlock(&port->lock);
 
-	wait_for_completion(&port->swap_complete);
+	if (!wait_for_completion_timeout(&port->swap_complete,
+				msecs_to_jiffies(PD_ROLE_SWAP_TIMEOUT)))
+		ret = -ETIMEDOUT;
+	else
+		ret = port->swap_status;
 
-	ret = port->swap_status;
 	goto swap_unlock;
 
 port_unlock:

@@ -3223,9 +3226,12 @@ static int tcpm_pr_set(const struct typec_capability *cap,
 	tcpm_set_state(port, PR_SWAP_SEND, 0);
 	mutex_unlock(&port->lock);
 
-	wait_for_completion(&port->swap_complete);
+	if (!wait_for_completion_timeout(&port->swap_complete,
+				msecs_to_jiffies(PD_ROLE_SWAP_TIMEOUT)))
+		ret = -ETIMEDOUT;
+	else
+		ret = port->swap_status;
 
-	ret = port->swap_status;
 	goto swap_unlock;
 
 port_unlock:

@@ -3260,9 +3266,12 @@ static int tcpm_vconn_set(const struct typec_capability *cap,
 	tcpm_set_state(port, VCONN_SWAP_SEND, 0);
 	mutex_unlock(&port->lock);
 
-	wait_for_completion(&port->swap_complete);
+	if (!wait_for_completion_timeout(&port->swap_complete,
+				msecs_to_jiffies(PD_ROLE_SWAP_TIMEOUT)))
+		ret = -ETIMEDOUT;
+	else
+		ret = port->swap_status;
 
-	ret = port->swap_status;
 	goto swap_unlock;
 
 port_unlock:
drivers/staging/typec/tcpm.h +2 -1
index 4c6b38cb2c8a..374cea44a84a 100644
@@ -34,7 +34,8 @@ enum typec_cc_polarity {
 };
 
 /* Time to wait for TCPC to complete transmit */
-#define PD_T_TCPC_TX_TIMEOUT  100
+#define PD_T_TCPC_TX_TIMEOUT	100		/* in ms	*/
+#define PD_ROLE_SWAP_TIMEOUT	(MSEC_PER_SEC * 10)
 
 enum tcpm_transmit_status {
 	TCPC_TX_SUCCESS = 0,
Signature · 1 line
2.7.4
author
#5 lore raw

PATCH 6/7 staging: typec: tcpm: Improve role swap with non PD capable partners

In reply to Guenter Roeck

If the partner is not PD capable, we can not use a power role set request to swap roles. Use the data role set request instead.

Also, if a partner is not PD capable, it does not really make sense to send a PD message to trigger a role swap. On top of that, we should really wait for the attempted role change to complete. Otherwise, it may well be that user space requests another role change immediately afterwards which will fail because the port is not yet in ready state.

Trigger the role swap from data role change requests and introduce new state PORT_RESET and use it to solve the problem. This new state is mostly identical to ERROR_RECOVERY, only it does not cause a pending role change to fail. Use this new state also when initializing the driver. Rename ERROR_RECOVERY_WAIT_OFF to PORT_RESET_WAIT_OFF to better reflect its new meaning.

Signed-off-by Guenter Roeck <[email protected]>
---
 drivers/staging/typec/tcpm.c | 60 +++++++++++++++++++++++++-------------------
 1 file changed, 34 insertions(+), 26 deletions(-)
drivers/staging/typec/tcpm.c +34 -26
index 515bec1807d7..e0f71aeb4ed3 100644
@@ -115,7 +115,8 @@
 	S(BIST_RX),				\
 						\
 	S(ERROR_RECOVERY),			\
-	S(ERROR_RECOVERY_WAIT_OFF)
+	S(PORT_RESET),				\
+	S(PORT_RESET_WAIT_OFF)
 
 #define GENERATE_ENUM(e)	e
 #define GENERATE_STRING(s)	#s

@@ -230,6 +231,7 @@ struct tcpm_port {
 
 	struct mutex swap_lock;		/* swap command lock */
 	bool swap_pending;
+	bool non_pd_role_swap;
 	struct completion swap_complete;
 	int swap_status;
 

@@ -2124,6 +2126,7 @@ static void tcpm_swap_complete(struct tcpm_port *port, int result)
 	if (port->swap_pending) {
 		port->swap_status = result;
 		port->swap_pending = false;
+		port->non_pd_role_swap = false;
 		complete(&port->swap_complete);
 	}
 }

@@ -2138,7 +2141,8 @@ static void run_state_machine(struct tcpm_port *port)
 		break;
 	/* SRC states */
 	case SRC_UNATTACHED:
-		tcpm_swap_complete(port, -ENOTCONN);
+		if (!port->non_pd_role_swap)
+			tcpm_swap_complete(port, -ENOTCONN);
 		tcpm_src_detach(port);
 		if (tcpm_start_drp_toggling(port)) {
 			tcpm_set_state(port, DRP_TOGGLING, 0);

@@ -2293,7 +2297,8 @@ static void run_state_machine(struct tcpm_port *port)
 
 	/* SNK states */
 	case SNK_UNATTACHED:
-		tcpm_swap_complete(port, -ENOTCONN);
+		if (!port->non_pd_role_swap)
+			tcpm_swap_complete(port, -ENOTCONN);
 		tcpm_snk_detach(port);
 		if (tcpm_start_drp_toggling(port)) {
 			tcpm_set_state(port, DRP_TOGGLING, 0);

@@ -2704,13 +2709,15 @@ static void run_state_machine(struct tcpm_port *port)
 		break;
 	case ERROR_RECOVERY:
 		tcpm_swap_complete(port, -EPROTO);
+		tcpm_set_state(port, PORT_RESET, 0);
+		break;
+	case PORT_RESET:
 		tcpm_reset_port(port);
-
 		tcpm_set_cc(port, TYPEC_CC_OPEN);
-		tcpm_set_state(port, ERROR_RECOVERY_WAIT_OFF,
+		tcpm_set_state(port, PORT_RESET_WAIT_OFF,
 			       PD_T_ERROR_RECOVERY);
 		break;
-	case ERROR_RECOVERY_WAIT_OFF:
+	case PORT_RESET_WAIT_OFF:
 		tcpm_set_state(port,
 			       tcpm_default_state(port),
 			       port->vbus_present ? PD_T_PS_SOURCE_OFF : 0);

@@ -3042,7 +3049,7 @@ static void _tcpm_pd_vbus_off(struct tcpm_port *port)
 		/* Do nothing, expected */
 		break;
 
-	case ERROR_RECOVERY_WAIT_OFF:
+	case PORT_RESET_WAIT_OFF:
 		tcpm_set_state(port, tcpm_default_state(port), 0);
 		break;
 

@@ -3139,7 +3146,7 @@ static int tcpm_dr_set(const struct typec_capability *cap,
 	mutex_lock(&port->swap_lock);
 	mutex_lock(&port->lock);
 
-	if (port->typec_caps.type != TYPEC_PORT_DRP || !port->pd_capable) {
+	if (port->typec_caps.type != TYPEC_PORT_DRP) {
 		ret = -EINVAL;
 		goto port_unlock;
 	}

@@ -3160,10 +3167,26 @@ static int tcpm_dr_set(const struct typec_capability *cap,
 	 * Reject data role swap request in this case.
 	 */
 
+	if (!port->pd_capable) {
+		/*
+		 * If the partner is not PD capable, reset the port to
+		 * trigger a role change. This can only work if a preferred
+		 * role is configured, and if it matches the requested role.
+		 */
+		if (port->try_role == TYPEC_NO_PREFERRED_ROLE ||
+		    port->try_role == port->pwr_role) {
+			ret = -EINVAL;
+			goto port_unlock;
+		}
+		port->non_pd_role_swap = true;
+		tcpm_set_state(port, PORT_RESET, 0);
+	} else {
+		tcpm_set_state(port, DR_SWAP_SEND, 0);
+	}
+
 	port->swap_status = 0;
 	port->swap_pending = true;
 	reinit_completion(&port->swap_complete);
-	tcpm_set_state(port, DR_SWAP_SEND, 0);
 	mutex_unlock(&port->lock);
 
 	if (!wait_for_completion_timeout(&port->swap_complete,

@@ -3172,6 +3195,7 @@ static int tcpm_dr_set(const struct typec_capability *cap,
 	else
 		ret = port->swap_status;
 
+	port->non_pd_role_swap = false;
 	goto swap_unlock;
 
 port_unlock:

@@ -3204,22 +3228,6 @@ static int tcpm_pr_set(const struct typec_capability *cap,
 		goto port_unlock;
 	}
 
-	if (!port->pd_capable) {
-		/*
-		 * If the partner is not PD capable, reset the port to
-		 * trigger a role change. This can only work if a preferred
-		 * role is configured, and if it matches the requested role.
-		 */
-		if (port->try_role == TYPEC_NO_PREFERRED_ROLE ||
-		    port->try_role == port->pwr_role) {
-			ret = -EINVAL;
-			goto port_unlock;
-		}
-		tcpm_set_state(port, HARD_RESET_SEND, 0);
-		ret = 0;
-		goto port_unlock;
-	}
-
 	port->swap_status = 0;
 	port->swap_pending = true;
 	reinit_completion(&port->swap_complete);

@@ -3325,7 +3333,7 @@ static void tcpm_init(struct tcpm_port *port)
 	 * Some adapters need a clean slate at startup, and won't recover
 	 * otherwise. So do not try to be fancy and force a clean disconnect.
 	 */
-	tcpm_set_state(port, ERROR_RECOVERY, 0);
+	tcpm_set_state(port, PORT_RESET, 0);
 }
 
 void tcpm_tcpc_reset(struct tcpm_port *port)
Signature · 1 line
2.7.4
author
#6 lore raw

PATCH 7/7 staging: typec: tcpm: Check cc status before entering SRC_TRY_DEBOUCE

In reply to Guenter Roeck

From: Badhri Jagan Sridharan <[email protected]>

[  130.893355] state change SNK_DEBOUNCED -> SRC_TRY
[  130.893363] cc:=3
[  130.893490] pending state change SRC_TRY -> SNK_TRYWAIT @ 100 ms
[  130.895602] CC1: 3 -> 0, CC2: 0 -> 0 [state SRC_TRY, polarity 0, disconnected]
[  130.895613] state change SRC_TRY -> SRC_TRY_DEBOUNCE
[  130.895621] pending state change SRC_TRY_DEBOUNCE -> SRC_ATTACHED @ 20 ms
[  130.916843] state change SRC_TRY_DEBOUNCE -> SRC_ATTACHED [delayed 20 ms]

Although the CC state was changing to TYPEC_CC_OPEN, the port entered SRC_TRY_DEBOUNCE from SRC_TRY. The port must enter SRC_TRY_DEBOUNCE only if the CC state is TYPEC_CC_RD.

Signed-off-by Badhri Jagan Sridharan <[email protected]>
[groeck: Wording]
Signed-off-by Guenter Roeck <[email protected]>
---
 drivers/staging/typec/tcpm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/staging/typec/tcpm.c +2 -1
index e0f71aeb4ed3..e72286464164 100644
@@ -2898,7 +2898,8 @@ static void _tcpm_cc_change(struct tcpm_port *port, enum typec_cc_status cc1,
 		break;
 
 	case SRC_TRY:
-		tcpm_set_state(port, SRC_TRY_DEBOUNCE, 0);
+		if (tcpm_port_is_source(port))
+			tcpm_set_state(port, SRC_TRY_DEBOUNCE, 0);
 		break;
 	case SRC_TRY_DEBOUNCE:
 		tcpm_set_state(port, SRC_TRY, 0);
Signature · 1 line
2.7.4