[v1] usb: typec: tcpm: apply vbus before data bringup in tcpm_src_attach

[PATCH v1] usb: typec: tcpm: apply vbus before data bringup in tcpm_src_attach

Posted by RD Babiera 9 months, 2 weeks ago

This patch fixes Type-C compliance test TD 4.7.6 - Try.SNK DRP Connect
SNKAS.

tVbusON has a limit of 275ms when entering SRC_ATTACHED. Compliance
testers can interpret the TryWait.Src to Attached.Src transition after
Try.Snk as being in Attached.Src the entire time, so ~170ms is lost
to the debounce timer.

Setting the data role can be a costly operation in host mode, and when
completed after 100ms can cause Type-C compliance test check TD 4.7.5.V.4
to fail.

Turn VBUS on before tcpm_set_roles to meet timing requirement.

Fixes: f0690a25a140 ("staging: typec: USB Type-C Port Manager (tcpm)")
Cc: stable@vger.kernel.org
Signed-off-by: RD Babiera <rdbabiera@google.com>
Reviewed-by: Badhri Jagan Sridharan <badhri@google.com>
---
 drivers/usb/typec/tcpm/tcpm.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 784fa23102f9..e099a3c4428d 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -4355,17 +4355,6 @@ static int tcpm_src_attach(struct tcpm_port *port)
 
 	tcpm_enable_auto_vbus_discharge(port, true);
 
-	ret = tcpm_set_roles(port, true, TYPEC_STATE_USB,
-			     TYPEC_SOURCE, tcpm_data_role_for_source(port));
-	if (ret < 0)
-		return ret;
-
-	if (port->pd_supported) {
-		ret = port->tcpc->set_pd_rx(port->tcpc, true);
-		if (ret < 0)
-			goto out_disable_mux;
-	}
-
 	/*
 	 * USB Type-C specification, version 1.2,
 	 * chapter 4.5.2.2.8.1 (Attached.SRC Requirements)
@@ -4375,13 +4364,24 @@ static int tcpm_src_attach(struct tcpm_port *port)
 	    (polarity == TYPEC_POLARITY_CC2 && port->cc1 == TYPEC_CC_RA)) {
 		ret = tcpm_set_vconn(port, true);
 		if (ret < 0)
-			goto out_disable_pd;
+			return ret;
 	}
 
 	ret = tcpm_set_vbus(port, true);
 	if (ret < 0)
 		goto out_disable_vconn;
 
+	ret = tcpm_set_roles(port, true, TYPEC_STATE_USB, TYPEC_SOURCE,
+			     tcpm_data_role_for_source(port));
+	if (ret < 0)
+		goto out_disable_vbus;
+
+	if (port->pd_supported) {
+		ret = port->tcpc->set_pd_rx(port->tcpc, true);
+		if (ret < 0)
+			goto out_disable_mux;
+	}
+
 	port->pd_capable = false;
 
 	port->partner = NULL;
@@ -4392,14 +4392,14 @@ static int tcpm_src_attach(struct tcpm_port *port)
 
 	return 0;
 
-out_disable_vconn:
-	tcpm_set_vconn(port, false);
-out_disable_pd:
-	if (port->pd_supported)
-		port->tcpc->set_pd_rx(port->tcpc, false);
 out_disable_mux:
 	tcpm_mux_set(port, TYPEC_STATE_SAFE, USB_ROLE_NONE,
 		     TYPEC_ORIENTATION_NONE);
+out_disable_vbus:
+	tcpm_set_vbus(port, false);
+out_disable_vconn:
+	tcpm_set_vconn(port, false);
+
 	return ret;
 }
 

base-commit: 615dca38c2eae55aff80050275931c87a812b48c
-- 
2.49.0.967.g6a0df3ecc3-goog

Re: [PATCH v1] usb: typec: tcpm: apply vbus before data bringup in tcpm_src_attach

Posted by Greg KH 9 months, 2 weeks ago

On Tue, Apr 29, 2025 at 11:47:42PM +0000, RD Babiera wrote:
> This patch fixes Type-C compliance test TD 4.7.6 - Try.SNK DRP Connect
> SNKAS.
> 
> tVbusON has a limit of 275ms when entering SRC_ATTACHED. Compliance
> testers can interpret the TryWait.Src to Attached.Src transition after
> Try.Snk as being in Attached.Src the entire time, so ~170ms is lost
> to the debounce timer.
> 
> Setting the data role can be a costly operation in host mode, and when
> completed after 100ms can cause Type-C compliance test check TD 4.7.5.V.4
> to fail.
> 
> Turn VBUS on before tcpm_set_roles to meet timing requirement.
> 
> Fixes: f0690a25a140 ("staging: typec: USB Type-C Port Manager (tcpm)")
> Cc: stable@vger.kernel.org
> Signed-off-by: RD Babiera <rdbabiera@google.com>
> Reviewed-by: Badhri Jagan Sridharan <badhri@google.com>
> ---
>  drivers/usb/typec/tcpm/tcpm.c | 34 +++++++++++++++++-----------------
>  1 file changed, 17 insertions(+), 17 deletions(-)

Does not apply to my tree, can you rebase against usb-next and resend?

thanks,

greg k-h

Re: [PATCH v1] usb: typec: tcpm: apply vbus before data bringup in tcpm_src_attach

Posted by RD Babiera 9 months, 1 week ago

On Thu, May 1, 2025 at 8:41 AM Greg KH <gregkh@linuxfoundation.org> wrote:

> Does not apply to my tree, can you rebase against usb-next and resend?

This patch is rebased against usb-next/usb-next, but I think I do need to rebase
against usb-linus. commit 8a50da849151e7e12b43c1d8fe7ad302223aef6b is
present in usb-next but not usb-linus, and my patch as it is now is
dependent on it.

Would you prefer that I rebase against usb-linus and resubmit given
I'm submitting
as a stable fix? It looks like the conflicting patch would be up for
the 6.16 merge
window.

Thanks in advance,
RD

Re: [PATCH v1] usb: typec: tcpm: apply vbus before data bringup in tcpm_src_attach

Posted by Greg KH 9 months ago

On Tue, May 06, 2025 at 10:57:10AM -0700, RD Babiera wrote:
> On Thu, May 1, 2025 at 8:41 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> 
> > Does not apply to my tree, can you rebase against usb-next and resend?
> 
> This patch is rebased against usb-next/usb-next, but I think I do need to rebase
> against usb-linus. commit 8a50da849151e7e12b43c1d8fe7ad302223aef6b is
> present in usb-next but not usb-linus, and my patch as it is now is
> dependent on it.
> 
> Would you prefer that I rebase against usb-linus and resubmit given
> I'm submitting
> as a stable fix? It looks like the conflicting patch would be up for
> the 6.16 merge
> window.

It depends on when you want it merged, for 6.15-final, or 6.16-final.
Your choice.

thanks,

greg k-h

Re: [PATCH v1] usb: typec: tcpm: apply vbus before data bringup in tcpm_src_attach

Posted by Heikki Krogerus 9 months, 2 weeks ago

On Tue, Apr 29, 2025 at 11:47:42PM +0000, RD Babiera wrote:
> This patch fixes Type-C compliance test TD 4.7.6 - Try.SNK DRP Connect
> SNKAS.
> 
> tVbusON has a limit of 275ms when entering SRC_ATTACHED. Compliance
> testers can interpret the TryWait.Src to Attached.Src transition after
> Try.Snk as being in Attached.Src the entire time, so ~170ms is lost
> to the debounce timer.
> 
> Setting the data role can be a costly operation in host mode, and when
> completed after 100ms can cause Type-C compliance test check TD 4.7.5.V.4
> to fail.
> 
> Turn VBUS on before tcpm_set_roles to meet timing requirement.
> 
> Fixes: f0690a25a140 ("staging: typec: USB Type-C Port Manager (tcpm)")
> Cc: stable@vger.kernel.org
> Signed-off-by: RD Babiera <rdbabiera@google.com>
> Reviewed-by: Badhri Jagan Sridharan <badhri@google.com>

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

> ---
>  drivers/usb/typec/tcpm/tcpm.c | 34 +++++++++++++++++-----------------
>  1 file changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 784fa23102f9..e099a3c4428d 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -4355,17 +4355,6 @@ static int tcpm_src_attach(struct tcpm_port *port)
>  
>  	tcpm_enable_auto_vbus_discharge(port, true);
>  
> -	ret = tcpm_set_roles(port, true, TYPEC_STATE_USB,
> -			     TYPEC_SOURCE, tcpm_data_role_for_source(port));
> -	if (ret < 0)
> -		return ret;
> -
> -	if (port->pd_supported) {
> -		ret = port->tcpc->set_pd_rx(port->tcpc, true);
> -		if (ret < 0)
> -			goto out_disable_mux;
> -	}
> -
>  	/*
>  	 * USB Type-C specification, version 1.2,
>  	 * chapter 4.5.2.2.8.1 (Attached.SRC Requirements)
> @@ -4375,13 +4364,24 @@ static int tcpm_src_attach(struct tcpm_port *port)
>  	    (polarity == TYPEC_POLARITY_CC2 && port->cc1 == TYPEC_CC_RA)) {
>  		ret = tcpm_set_vconn(port, true);
>  		if (ret < 0)
> -			goto out_disable_pd;
> +			return ret;
>  	}
>  
>  	ret = tcpm_set_vbus(port, true);
>  	if (ret < 0)
>  		goto out_disable_vconn;
>  
> +	ret = tcpm_set_roles(port, true, TYPEC_STATE_USB, TYPEC_SOURCE,
> +			     tcpm_data_role_for_source(port));
> +	if (ret < 0)
> +		goto out_disable_vbus;
> +
> +	if (port->pd_supported) {
> +		ret = port->tcpc->set_pd_rx(port->tcpc, true);
> +		if (ret < 0)
> +			goto out_disable_mux;
> +	}
> +
>  	port->pd_capable = false;
>  
>  	port->partner = NULL;
> @@ -4392,14 +4392,14 @@ static int tcpm_src_attach(struct tcpm_port *port)
>  
>  	return 0;
>  
> -out_disable_vconn:
> -	tcpm_set_vconn(port, false);
> -out_disable_pd:
> -	if (port->pd_supported)
> -		port->tcpc->set_pd_rx(port->tcpc, false);
>  out_disable_mux:
>  	tcpm_mux_set(port, TYPEC_STATE_SAFE, USB_ROLE_NONE,
>  		     TYPEC_ORIENTATION_NONE);
> +out_disable_vbus:
> +	tcpm_set_vbus(port, false);
> +out_disable_vconn:
> +	tcpm_set_vconn(port, false);
> +
>  	return ret;
>  }
>  
> 
> base-commit: 615dca38c2eae55aff80050275931c87a812b48c
> -- 
> 2.49.0.967.g6a0df3ecc3-goog

-- 
heikki