diff mbox series

[v1,1/1] usb: typec: tcpm: avoid resets for missing source capability messages

Message ID 20240523171806.223727-1-sebastian.reichel@collabora.com
State New
Headers show
Series [v1,1/1] usb: typec: tcpm: avoid resets for missing source capability messages | expand

Commit Message

Sebastian Reichel May 23, 2024, 5:17 p.m. UTC
When the Linux Type-C controller drivers probe, they requests a soft
reset, which should result in the source restarting to send Source
Capability messages again independently of the previous state.
Unfortunately some USB PD sources do not follow the specification and
do not send them after a soft reset when they already negotiated a
specific contract before. The current way (and what is described in the
specificiation) to resolve this problem is triggering a hard reset.

But a hard reset is fatal on batteryless platforms powered via USB-C PD,
since that removes VBUS for some time. Since this is triggered at boot
time, the system will be stuck in a boot loop. Examples for platforms
affected by this are the Radxa Rock 5B or the Libre Computer Renegade
Elite ROC-RK3399-PC.

Instead of directly trying a hard reset when no Source Capability
message is send by the USB-PD source automatically, this changes the
state machine to try explicitly asking for the capabilities by sending
a Get Source Capability control message.

For me this solves issues with 2 different USB-PD sources - a RAVPower
powerbank and a Lemorele USB-C dock. Every other PD source I own
follows the specification and automatically sends the Source Capability
message after a soft reset, which works with or without this change.

I decided against making this extra step limited to devices not having
the self_powered flag set, since I don't see any huge drawbacks in this
approach and it keeps the logic simpler. The worst case scenario would
be a power source, which is really stuck. In that case the hard reset
is delayed by another 310ms.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/usb/typec/tcpm/tcpm.c | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

Comments

Heikki Krogerus June 3, 2024, 10:13 a.m. UTC | #1
On Thu, May 23, 2024 at 07:17:52PM +0200, Sebastian Reichel wrote:
> When the Linux Type-C controller drivers probe, they requests a soft
> reset, which should result in the source restarting to send Source
> Capability messages again independently of the previous state.
> Unfortunately some USB PD sources do not follow the specification and
> do not send them after a soft reset when they already negotiated a
> specific contract before. The current way (and what is described in the
> specificiation) to resolve this problem is triggering a hard reset.
> 
> But a hard reset is fatal on batteryless platforms powered via USB-C PD,
> since that removes VBUS for some time. Since this is triggered at boot
> time, the system will be stuck in a boot loop. Examples for platforms
> affected by this are the Radxa Rock 5B or the Libre Computer Renegade
> Elite ROC-RK3399-PC.
> 
> Instead of directly trying a hard reset when no Source Capability
> message is send by the USB-PD source automatically, this changes the
> state machine to try explicitly asking for the capabilities by sending
> a Get Source Capability control message.
> 
> For me this solves issues with 2 different USB-PD sources - a RAVPower
> powerbank and a Lemorele USB-C dock. Every other PD source I own
> follows the specification and automatically sends the Source Capability
> message after a soft reset, which works with or without this change.
> 
> I decided against making this extra step limited to devices not having
> the self_powered flag set, since I don't see any huge drawbacks in this
> approach and it keeps the logic simpler. The worst case scenario would
> be a power source, which is really stuck. In that case the hard reset
> is delayed by another 310ms.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>

Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
>  drivers/usb/typec/tcpm/tcpm.c | 27 +++++++++++++++++++++++++--
>  1 file changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 375bc84d14a2..bac6866617c8 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -57,6 +57,7 @@
>  	S(SNK_DISCOVERY_DEBOUNCE),		\
>  	S(SNK_DISCOVERY_DEBOUNCE_DONE),		\
>  	S(SNK_WAIT_CAPABILITIES),		\
> +	S(SNK_WAIT_CAPABILITIES_TIMEOUT),	\
>  	S(SNK_NEGOTIATE_CAPABILITIES),		\
>  	S(SNK_NEGOTIATE_PPS_CAPABILITIES),	\
>  	S(SNK_TRANSITION_SINK),			\
> @@ -3108,7 +3109,8 @@ static void tcpm_pd_data_request(struct tcpm_port *port,
>  						   PD_MSG_CTRL_REJECT :
>  						   PD_MSG_CTRL_NOT_SUPP,
>  						   NONE_AMS);
> -		} else if (port->state == SNK_WAIT_CAPABILITIES) {
> +		} else if (port->state == SNK_WAIT_CAPABILITIES ||
> +			   port->state == SNK_WAIT_CAPABILITIES_TIMEOUT) {
>  		/*
>  		 * This message may be received even if VBUS is not
>  		 * present. This is quite unexpected; see USB PD
> @@ -5039,10 +5041,31 @@ static void run_state_machine(struct tcpm_port *port)
>  			tcpm_set_state(port, SNK_SOFT_RESET,
>  				       PD_T_SINK_WAIT_CAP);
>  		} else {
> -			tcpm_set_state(port, hard_reset_state(port),
> +			tcpm_set_state(port, SNK_WAIT_CAPABILITIES_TIMEOUT,
>  				       PD_T_SINK_WAIT_CAP);
>  		}
>  		break;
> +	case SNK_WAIT_CAPABILITIES_TIMEOUT:
> +		/*
> +		 * There are some USB PD sources in the field, which do not
> +		 * properly implement the specification and fail to start
> +		 * sending Source Capability messages after a soft reset. The
> +		 * specification suggests to do a hard reset when no Source
> +		 * capability message is received within PD_T_SINK_WAIT_CAP,
> +		 * but that might effectively kil the machine's power source.
> +		 *
> +		 * This slightly diverges from the specification and tries to
> +		 * recover from this by explicitly asking for the capabilities
> +		 * using the Get_Source_Cap control message before falling back
> +		 * to a hard reset. The control message should also be supported
> +		 * and handled by all USB PD source and dual role devices
> +		 * according to the specification.
> +		 */
> +		if (tcpm_pd_send_control(port, PD_CTRL_GET_SOURCE_CAP, TCPC_TX_SOP))
> +			tcpm_set_state_cond(port, hard_reset_state(port), 0);
> +		else
> +			tcpm_set_state(port, hard_reset_state(port), PD_T_SINK_WAIT_CAP);
> +		break;
>  	case SNK_NEGOTIATE_CAPABILITIES:
>  		port->pd_capable = true;
>  		tcpm_set_partner_usb_comm_capable(port,
> -- 
> 2.43.0
diff mbox series

Patch

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 375bc84d14a2..bac6866617c8 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -57,6 +57,7 @@ 
 	S(SNK_DISCOVERY_DEBOUNCE),		\
 	S(SNK_DISCOVERY_DEBOUNCE_DONE),		\
 	S(SNK_WAIT_CAPABILITIES),		\
+	S(SNK_WAIT_CAPABILITIES_TIMEOUT),	\
 	S(SNK_NEGOTIATE_CAPABILITIES),		\
 	S(SNK_NEGOTIATE_PPS_CAPABILITIES),	\
 	S(SNK_TRANSITION_SINK),			\
@@ -3108,7 +3109,8 @@  static void tcpm_pd_data_request(struct tcpm_port *port,
 						   PD_MSG_CTRL_REJECT :
 						   PD_MSG_CTRL_NOT_SUPP,
 						   NONE_AMS);
-		} else if (port->state == SNK_WAIT_CAPABILITIES) {
+		} else if (port->state == SNK_WAIT_CAPABILITIES ||
+			   port->state == SNK_WAIT_CAPABILITIES_TIMEOUT) {
 		/*
 		 * This message may be received even if VBUS is not
 		 * present. This is quite unexpected; see USB PD
@@ -5039,10 +5041,31 @@  static void run_state_machine(struct tcpm_port *port)
 			tcpm_set_state(port, SNK_SOFT_RESET,
 				       PD_T_SINK_WAIT_CAP);
 		} else {
-			tcpm_set_state(port, hard_reset_state(port),
+			tcpm_set_state(port, SNK_WAIT_CAPABILITIES_TIMEOUT,
 				       PD_T_SINK_WAIT_CAP);
 		}
 		break;
+	case SNK_WAIT_CAPABILITIES_TIMEOUT:
+		/*
+		 * There are some USB PD sources in the field, which do not
+		 * properly implement the specification and fail to start
+		 * sending Source Capability messages after a soft reset. The
+		 * specification suggests to do a hard reset when no Source
+		 * capability message is received within PD_T_SINK_WAIT_CAP,
+		 * but that might effectively kil the machine's power source.
+		 *
+		 * This slightly diverges from the specification and tries to
+		 * recover from this by explicitly asking for the capabilities
+		 * using the Get_Source_Cap control message before falling back
+		 * to a hard reset. The control message should also be supported
+		 * and handled by all USB PD source and dual role devices
+		 * according to the specification.
+		 */
+		if (tcpm_pd_send_control(port, PD_CTRL_GET_SOURCE_CAP, TCPC_TX_SOP))
+			tcpm_set_state_cond(port, hard_reset_state(port), 0);
+		else
+			tcpm_set_state(port, hard_reset_state(port), PD_T_SINK_WAIT_CAP);
+		break;
 	case SNK_NEGOTIATE_CAPABILITIES:
 		port->pd_capable = true;
 		tcpm_set_partner_usb_comm_capable(port,