diff mbox series

usb: typec: tcpm: avoid sink goto SNK_UNATTACHED state if not received source capability message

Message ID 20240802064156.1846768-1-xu.yang_2@nxp.com
State New
Headers show
Series usb: typec: tcpm: avoid sink goto SNK_UNATTACHED state if not received source capability message | expand

Commit Message

Xu Yang Aug. 2, 2024, 6:41 a.m. UTC 
Since commit (122968f8dda8 usb: typec: tcpm: avoid resets for missing
source capability messages), state will change from SNK_WAIT_CAPABILITIES
to SNK_WAIT_CAPABILITIES_TIMEOUT. We need to change SNK_WAIT_CAPABILITIES
-> SNK_READY path to SNK_WAIT_CAPABILITIES_TIMEOUT -> SNK_READY
accordingly. Otherwise, the sink port will never change to SNK_READY state
if the source does't have PD capability.

[  503.547183] pending state change SNK_WAIT_CAPABILITIES -> SNK_WAIT_CAPABILITIES_TIMEOUT @ 310 ms [rev3 NONE_AMS]
[  503.857239] state change SNK_WAIT_CAPABILITIES -> SNK_WAIT_CAPABILITIES_TIMEOUT [delayed 310 ms]
[  503.857254] PD TX, header: 0x87
[  503.862440] PD TX complete, status: 2
[  503.862484] state change SNK_WAIT_CAPABILITIES_TIMEOUT -> SNK_UNATTACHED [rev3 NONE_AMS]

Fixes: 122968f8dda8 ("usb: typec: tcpm: avoid resets for missing source capability messages")
Cc: stable@vger.kernel.org
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
---
 drivers/usb/typec/tcpm/tcpm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Sebastian Reichel Aug. 2, 2024, 3:43 p.m. UTC | #1 
Hi,

On Fri, Aug 02, 2024 at 02:41:56PM GMT, Xu Yang wrote:
> Since commit (122968f8dda8 usb: typec: tcpm: avoid resets for missing
> source capability messages), state will change from SNK_WAIT_CAPABILITIES
> to SNK_WAIT_CAPABILITIES_TIMEOUT. We need to change SNK_WAIT_CAPABILITIES
> -> SNK_READY path to SNK_WAIT_CAPABILITIES_TIMEOUT -> SNK_READY
> accordingly. Otherwise, the sink port will never change to SNK_READY state
> if the source does't have PD capability.
> 
> [  503.547183] pending state change SNK_WAIT_CAPABILITIES -> SNK_WAIT_CAPABILITIES_TIMEOUT @ 310 ms [rev3 NONE_AMS]
> [  503.857239] state change SNK_WAIT_CAPABILITIES -> SNK_WAIT_CAPABILITIES_TIMEOUT [delayed 310 ms]
> [  503.857254] PD TX, header: 0x87
> [  503.862440] PD TX complete, status: 2
> [  503.862484] state change SNK_WAIT_CAPABILITIES_TIMEOUT -> SNK_UNATTACHED [rev3 NONE_AMS]
> 
> Fixes: 122968f8dda8 ("usb: typec: tcpm: avoid resets for missing source capability messages")
> Cc: stable@vger.kernel.org
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> ---

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>

-- Sebastian

>  drivers/usb/typec/tcpm/tcpm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 26f9006e95e1..cce39818e99a 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -4515,7 +4515,7 @@ static inline enum tcpm_state hard_reset_state(struct tcpm_port *port)
>  		return ERROR_RECOVERY;
>  	if (port->pwr_role == TYPEC_SOURCE)
>  		return SRC_UNATTACHED;
> -	if (port->state == SNK_WAIT_CAPABILITIES)
> +	if (port->state == SNK_WAIT_CAPABILITIES_TIMEOUT)
>  		return SNK_READY;
>  	return SNK_UNATTACHED;
>  }
> -- 
> 2.34.1
>
Badhri Jagan Sridharan Aug. 4, 2024, 5:01 p.m. UTC | #2 
On Fri, Aug 2, 2024 at 8:43 AM Sebastian Reichel
<sebastian.reichel@collabora.com> wrote:
>
> Hi,
>
> On Fri, Aug 02, 2024 at 02:41:56PM GMT, Xu Yang wrote:
> > Since commit (122968f8dda8 usb: typec: tcpm: avoid resets for missing
> > source capability messages), state will change from SNK_WAIT_CAPABILITIES
> > to SNK_WAIT_CAPABILITIES_TIMEOUT. We need to change SNK_WAIT_CAPABILITIES
> > -> SNK_READY path to SNK_WAIT_CAPABILITIES_TIMEOUT -> SNK_READY
> > accordingly. Otherwise, the sink port will never change to SNK_READY state
> > if the source does't have PD capability.
> >
> > [  503.547183] pending state change SNK_WAIT_CAPABILITIES -> SNK_WAIT_CAPABILITIES_TIMEOUT @ 310 ms [rev3 NONE_AMS]
> > [  503.857239] state change SNK_WAIT_CAPABILITIES -> SNK_WAIT_CAPABILITIES_TIMEOUT [delayed 310 ms]
> > [  503.857254] PD TX, header: 0x87
> > [  503.862440] PD TX complete, status: 2
> > [  503.862484] state change SNK_WAIT_CAPABILITIES_TIMEOUT -> SNK_UNATTACHED [rev3 NONE_AMS]
> >
> > Fixes: 122968f8dda8 ("usb: typec: tcpm: avoid resets for missing source capability messages")

This patch looks OK. However, the original patch 122968f8dda8 might be
regressing compliance
(https://www.usb.org/document-library/usb-power-delivery-compliance-test-specification-0)
behavior.
Will send a follow up patch if we notice any.

> > Cc: stable@vger.kernel.org
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > ---
>
> Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>

Reviewed-by: Badhri Jagan Sridharan <badhri@google.com>

>
> -- Sebastian
>
> >  drivers/usb/typec/tcpm/tcpm.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> > index 26f9006e95e1..cce39818e99a 100644
> > --- a/drivers/usb/typec/tcpm/tcpm.c
> > +++ b/drivers/usb/typec/tcpm/tcpm.c
> > @@ -4515,7 +4515,7 @@ static inline enum tcpm_state hard_reset_state(struct tcpm_port *port)
> >               return ERROR_RECOVERY;
> >       if (port->pwr_role == TYPEC_SOURCE)
> >               return SRC_UNATTACHED;
> > -     if (port->state == SNK_WAIT_CAPABILITIES)
> > +     if (port->state == SNK_WAIT_CAPABILITIES_TIMEOUT)
> >               return SNK_READY;
> >       return SNK_UNATTACHED;
> >  }
> > --
> > 2.34.1
> >
Heikki Krogerus Aug. 5, 2024, 8:08 a.m. UTC | #3 
On Fri, Aug 02, 2024 at 02:41:56PM +0800, Xu Yang wrote:
> Since commit (122968f8dda8 usb: typec: tcpm: avoid resets for missing
> source capability messages), state will change from SNK_WAIT_CAPABILITIES
> to SNK_WAIT_CAPABILITIES_TIMEOUT. We need to change SNK_WAIT_CAPABILITIES
> -> SNK_READY path to SNK_WAIT_CAPABILITIES_TIMEOUT -> SNK_READY
> accordingly. Otherwise, the sink port will never change to SNK_READY state
> if the source does't have PD capability.
> 
> [  503.547183] pending state change SNK_WAIT_CAPABILITIES -> SNK_WAIT_CAPABILITIES_TIMEOUT @ 310 ms [rev3 NONE_AMS]
> [  503.857239] state change SNK_WAIT_CAPABILITIES -> SNK_WAIT_CAPABILITIES_TIMEOUT [delayed 310 ms]
> [  503.857254] PD TX, header: 0x87
> [  503.862440] PD TX complete, status: 2
> [  503.862484] state change SNK_WAIT_CAPABILITIES_TIMEOUT -> SNK_UNATTACHED [rev3 NONE_AMS]
> 
> Fixes: 122968f8dda8 ("usb: typec: tcpm: avoid resets for missing source capability messages")
> Cc: stable@vger.kernel.org
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

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

> ---
>  drivers/usb/typec/tcpm/tcpm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 26f9006e95e1..cce39818e99a 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -4515,7 +4515,7 @@ static inline enum tcpm_state hard_reset_state(struct tcpm_port *port)
>  		return ERROR_RECOVERY;
>  	if (port->pwr_role == TYPEC_SOURCE)
>  		return SRC_UNATTACHED;
> -	if (port->state == SNK_WAIT_CAPABILITIES)
> +	if (port->state == SNK_WAIT_CAPABILITIES_TIMEOUT)
>  		return SNK_READY;
>  	return SNK_UNATTACHED;
>  }
> -- 
> 2.34.1
diff mbox series

Patch

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 26f9006e95e1..cce39818e99a 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -4515,7 +4515,7 @@  static inline enum tcpm_state hard_reset_state(struct tcpm_port *port)
 		return ERROR_RECOVERY;
 	if (port->pwr_role == TYPEC_SOURCE)
 		return SRC_UNATTACHED;
-	if (port->state == SNK_WAIT_CAPABILITIES)
+	if (port->state == SNK_WAIT_CAPABILITIES_TIMEOUT)
 		return SNK_READY;
 	return SNK_UNATTACHED;
 }