Linux-USB Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] usb: typec: tcpm: reset hard_reset_count for any disconnect
@ 2020-10-12 11:03 Li Jun
  2020-10-12 13:14 ` Guenter Roeck
  2020-10-19 13:56 ` Heikki Krogerus
  0 siblings, 2 replies; 3+ messages in thread
From: Li Jun @ 2020-10-12 11:03 UTC (permalink / raw)
  To: linux, gregkh, heikki.krogerus; +Cc: cy_huang, u0084500, jun.li, linux-usb

Current tcpm_detach() only reset hard_reset_count if port->attached
is true, this may cause this counter clear is missed if the CC
disconnect event is generated after tcpm_port_reset() is done
by other events, e.g. VBUS off comes first before CC disconect for
a power sink, in that case the first tcpm_detach() will only clear
port->attached flag but leave hard_reset_count there because
tcpm_port_is_disconnected() is still false, then later tcpm_detach()
by CC disconnect will directly return due to port->attached is cleared,
finally this will result tcpm will not try hard reset or error recovery
for later attach.

ChiYuan reported this issue on his platform with below tcpm trace:
After power sink session setup after hard reset 2 times, detach
from the power source and then attach:
[ 4848.046358] VBUS off
[ 4848.046384] state change SNK_READY -> SNK_UNATTACHED
[ 4848.050908] Setting voltage/current limit 0 mV 0 mA
[ 4848.050936] polarity 0
[ 4848.052593] Requesting mux state 0, usb-role 0, orientation 0
[ 4848.053222] Start toggling
[ 4848.086500] state change SNK_UNATTACHED -> TOGGLING
[ 4848.089983] CC1: 0 -> 0, CC2: 3 -> 3 [state TOGGLING, polarity 0, connected]
[ 4848.089993] state change TOGGLING -> SNK_ATTACH_WAIT
[ 4848.090031] pending state change SNK_ATTACH_WAIT -> SNK_DEBOUNCED @200 ms
[ 4848.141162] CC1: 0 -> 0, CC2: 3 -> 0 [state SNK_ATTACH_WAIT, polarity 0, disconnected]
[ 4848.141170] state change SNK_ATTACH_WAIT -> SNK_ATTACH_WAIT
[ 4848.141184] pending state change SNK_ATTACH_WAIT -> SNK_UNATTACHED @20 ms
[ 4848.163156] state change SNK_ATTACH_WAIT -> SNK_UNATTACHED [delayed 20 ms]
[ 4848.163162] Start toggling
[ 4848.216918] CC1: 0 -> 0, CC2: 0 -> 3 [state TOGGLING, polarity 0, connected]
[ 4848.216954] state change TOGGLING -> SNK_ATTACH_WAIT
[ 4848.217080] pending state change SNK_ATTACH_WAIT -> SNK_DEBOUNCED @200 ms
[ 4848.231771] CC1: 0 -> 0, CC2: 3 -> 0 [state SNK_ATTACH_WAIT, polarity 0, disconnected]
[ 4848.231800] state change SNK_ATTACH_WAIT -> SNK_ATTACH_WAIT
[ 4848.231857] pending state change SNK_ATTACH_WAIT -> SNK_UNATTACHED @20 ms
[ 4848.256022] state change SNK_ATTACH_WAIT -> SNK_UNATTACHED [delayed20 ms]
[ 4848.256049] Start toggling
[ 4848.871148] VBUS on
[ 4848.885324] CC1: 0 -> 0, CC2: 0 -> 3 [state TOGGLING, polarity 0, connected]
[ 4848.885372] state change TOGGLING -> SNK_ATTACH_WAIT
[ 4848.885548] pending state change SNK_ATTACH_WAIT -> SNK_DEBOUNCED @200 ms
[ 4849.088240] state change SNK_ATTACH_WAIT -> SNK_DEBOUNCED [delayed200 ms]
[ 4849.088284] state change SNK_DEBOUNCED -> SNK_ATTACHED
[ 4849.088291] polarity 1
[ 4849.088769] Requesting mux state 1, usb-role 2, orientation 2
[ 4849.088895] state change SNK_ATTACHED -> SNK_STARTUP
[ 4849.088907] state change SNK_STARTUP -> SNK_DISCOVERY
[ 4849.088915] Setting voltage/current limit 5000 mV 0 mA
[ 4849.088927] vbus=0 charge:=1
[ 4849.090505] state change SNK_DISCOVERY -> SNK_WAIT_CAPABILITIES
[ 4849.090828] pending state change SNK_WAIT_CAPABILITIES -> SNK_READY @240 ms
[ 4849.335878] state change SNK_WAIT_CAPABILITIES -> SNK_READY [delayed240 ms]

this patch fix this issue by clear hard_reset_count at any cases
of cc disconnect, í.e. don't check port->attached flag.

Cc: stable@vger.kernel.org
Fixes: 4b4e02c83167 ("typec: tcpm: Move out of staging")
Reported-and-tested-by: ChiYuan Huang <cy_huang@richtek.com>
Signed-off-by: Li Jun <jun.li@nxp.com>
---
 drivers/usb/typec/tcpm/tcpm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 55535c4..a6fae1f 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -2890,6 +2890,9 @@ static void tcpm_reset_port(struct tcpm_port *port)
 
 static void tcpm_detach(struct tcpm_port *port)
 {
+	if (tcpm_port_is_disconnected(port))
+		port->hard_reset_count = 0;
+
 	if (!port->attached)
 		return;
 
@@ -2898,9 +2901,6 @@ static void tcpm_detach(struct tcpm_port *port)
 		port->tcpc->set_bist_data(port->tcpc, false);
 	}
 
-	if (tcpm_port_is_disconnected(port))
-		port->hard_reset_count = 0;
-
 	tcpm_reset_port(port);
 }
 
-- 
2.7.4


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] usb: typec: tcpm: reset hard_reset_count for any disconnect
  2020-10-12 11:03 [PATCH] usb: typec: tcpm: reset hard_reset_count for any disconnect Li Jun
@ 2020-10-12 13:14 ` Guenter Roeck
  2020-10-19 13:56 ` Heikki Krogerus
  1 sibling, 0 replies; 3+ messages in thread
From: Guenter Roeck @ 2020-10-12 13:14 UTC (permalink / raw)
  To: Li Jun; +Cc: gregkh, heikki.krogerus, cy_huang, u0084500, linux-usb

On Mon, Oct 12, 2020 at 07:03:12PM +0800, Li Jun wrote:
> Current tcpm_detach() only reset hard_reset_count if port->attached
> is true, this may cause this counter clear is missed if the CC
> disconnect event is generated after tcpm_port_reset() is done
> by other events, e.g. VBUS off comes first before CC disconect for
> a power sink, in that case the first tcpm_detach() will only clear
> port->attached flag but leave hard_reset_count there because
> tcpm_port_is_disconnected() is still false, then later tcpm_detach()
> by CC disconnect will directly return due to port->attached is cleared,
> finally this will result tcpm will not try hard reset or error recovery
> for later attach.
> 
> ChiYuan reported this issue on his platform with below tcpm trace:
> After power sink session setup after hard reset 2 times, detach
> from the power source and then attach:
> [ 4848.046358] VBUS off
> [ 4848.046384] state change SNK_READY -> SNK_UNATTACHED
> [ 4848.050908] Setting voltage/current limit 0 mV 0 mA
> [ 4848.050936] polarity 0
> [ 4848.052593] Requesting mux state 0, usb-role 0, orientation 0
> [ 4848.053222] Start toggling
> [ 4848.086500] state change SNK_UNATTACHED -> TOGGLING
> [ 4848.089983] CC1: 0 -> 0, CC2: 3 -> 3 [state TOGGLING, polarity 0, connected]
> [ 4848.089993] state change TOGGLING -> SNK_ATTACH_WAIT
> [ 4848.090031] pending state change SNK_ATTACH_WAIT -> SNK_DEBOUNCED @200 ms
> [ 4848.141162] CC1: 0 -> 0, CC2: 3 -> 0 [state SNK_ATTACH_WAIT, polarity 0, disconnected]
> [ 4848.141170] state change SNK_ATTACH_WAIT -> SNK_ATTACH_WAIT
> [ 4848.141184] pending state change SNK_ATTACH_WAIT -> SNK_UNATTACHED @20 ms
> [ 4848.163156] state change SNK_ATTACH_WAIT -> SNK_UNATTACHED [delayed 20 ms]
> [ 4848.163162] Start toggling
> [ 4848.216918] CC1: 0 -> 0, CC2: 0 -> 3 [state TOGGLING, polarity 0, connected]
> [ 4848.216954] state change TOGGLING -> SNK_ATTACH_WAIT
> [ 4848.217080] pending state change SNK_ATTACH_WAIT -> SNK_DEBOUNCED @200 ms
> [ 4848.231771] CC1: 0 -> 0, CC2: 3 -> 0 [state SNK_ATTACH_WAIT, polarity 0, disconnected]
> [ 4848.231800] state change SNK_ATTACH_WAIT -> SNK_ATTACH_WAIT
> [ 4848.231857] pending state change SNK_ATTACH_WAIT -> SNK_UNATTACHED @20 ms
> [ 4848.256022] state change SNK_ATTACH_WAIT -> SNK_UNATTACHED [delayed20 ms]
> [ 4848.256049] Start toggling
> [ 4848.871148] VBUS on
> [ 4848.885324] CC1: 0 -> 0, CC2: 0 -> 3 [state TOGGLING, polarity 0, connected]
> [ 4848.885372] state change TOGGLING -> SNK_ATTACH_WAIT
> [ 4848.885548] pending state change SNK_ATTACH_WAIT -> SNK_DEBOUNCED @200 ms
> [ 4849.088240] state change SNK_ATTACH_WAIT -> SNK_DEBOUNCED [delayed200 ms]
> [ 4849.088284] state change SNK_DEBOUNCED -> SNK_ATTACHED
> [ 4849.088291] polarity 1
> [ 4849.088769] Requesting mux state 1, usb-role 2, orientation 2
> [ 4849.088895] state change SNK_ATTACHED -> SNK_STARTUP
> [ 4849.088907] state change SNK_STARTUP -> SNK_DISCOVERY
> [ 4849.088915] Setting voltage/current limit 5000 mV 0 mA
> [ 4849.088927] vbus=0 charge:=1
> [ 4849.090505] state change SNK_DISCOVERY -> SNK_WAIT_CAPABILITIES
> [ 4849.090828] pending state change SNK_WAIT_CAPABILITIES -> SNK_READY @240 ms
> [ 4849.335878] state change SNK_WAIT_CAPABILITIES -> SNK_READY [delayed240 ms]
> 
> this patch fix this issue by clear hard_reset_count at any cases
> of cc disconnect, í.e. don't check port->attached flag.
> 
> Cc: stable@vger.kernel.org
> Fixes: 4b4e02c83167 ("typec: tcpm: Move out of staging")
> Reported-and-tested-by: ChiYuan Huang <cy_huang@richtek.com>
> Signed-off-by: Li Jun <jun.li@nxp.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/usb/typec/tcpm/tcpm.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 55535c4..a6fae1f 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -2890,6 +2890,9 @@ static void tcpm_reset_port(struct tcpm_port *port)
>  
>  static void tcpm_detach(struct tcpm_port *port)
>  {
> +	if (tcpm_port_is_disconnected(port))
> +		port->hard_reset_count = 0;
> +
>  	if (!port->attached)
>  		return;
>  
> @@ -2898,9 +2901,6 @@ static void tcpm_detach(struct tcpm_port *port)
>  		port->tcpc->set_bist_data(port->tcpc, false);
>  	}
>  
> -	if (tcpm_port_is_disconnected(port))
> -		port->hard_reset_count = 0;
> -
>  	tcpm_reset_port(port);
>  }
>  
> -- 
> 2.7.4
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] usb: typec: tcpm: reset hard_reset_count for any disconnect
  2020-10-12 11:03 [PATCH] usb: typec: tcpm: reset hard_reset_count for any disconnect Li Jun
  2020-10-12 13:14 ` Guenter Roeck
@ 2020-10-19 13:56 ` Heikki Krogerus
  1 sibling, 0 replies; 3+ messages in thread
From: Heikki Krogerus @ 2020-10-19 13:56 UTC (permalink / raw)
  To: Li Jun; +Cc: linux, gregkh, cy_huang, u0084500, linux-usb

On Mon, Oct 12, 2020 at 07:03:12PM +0800, Li Jun wrote:
> Current tcpm_detach() only reset hard_reset_count if port->attached
> is true, this may cause this counter clear is missed if the CC
> disconnect event is generated after tcpm_port_reset() is done
> by other events, e.g. VBUS off comes first before CC disconect for
> a power sink, in that case the first tcpm_detach() will only clear
> port->attached flag but leave hard_reset_count there because
> tcpm_port_is_disconnected() is still false, then later tcpm_detach()
> by CC disconnect will directly return due to port->attached is cleared,
> finally this will result tcpm will not try hard reset or error recovery
> for later attach.
> 
> ChiYuan reported this issue on his platform with below tcpm trace:
> After power sink session setup after hard reset 2 times, detach
> from the power source and then attach:
> [ 4848.046358] VBUS off
> [ 4848.046384] state change SNK_READY -> SNK_UNATTACHED
> [ 4848.050908] Setting voltage/current limit 0 mV 0 mA
> [ 4848.050936] polarity 0
> [ 4848.052593] Requesting mux state 0, usb-role 0, orientation 0
> [ 4848.053222] Start toggling
> [ 4848.086500] state change SNK_UNATTACHED -> TOGGLING
> [ 4848.089983] CC1: 0 -> 0, CC2: 3 -> 3 [state TOGGLING, polarity 0, connected]
> [ 4848.089993] state change TOGGLING -> SNK_ATTACH_WAIT
> [ 4848.090031] pending state change SNK_ATTACH_WAIT -> SNK_DEBOUNCED @200 ms
> [ 4848.141162] CC1: 0 -> 0, CC2: 3 -> 0 [state SNK_ATTACH_WAIT, polarity 0, disconnected]
> [ 4848.141170] state change SNK_ATTACH_WAIT -> SNK_ATTACH_WAIT
> [ 4848.141184] pending state change SNK_ATTACH_WAIT -> SNK_UNATTACHED @20 ms
> [ 4848.163156] state change SNK_ATTACH_WAIT -> SNK_UNATTACHED [delayed 20 ms]
> [ 4848.163162] Start toggling
> [ 4848.216918] CC1: 0 -> 0, CC2: 0 -> 3 [state TOGGLING, polarity 0, connected]
> [ 4848.216954] state change TOGGLING -> SNK_ATTACH_WAIT
> [ 4848.217080] pending state change SNK_ATTACH_WAIT -> SNK_DEBOUNCED @200 ms
> [ 4848.231771] CC1: 0 -> 0, CC2: 3 -> 0 [state SNK_ATTACH_WAIT, polarity 0, disconnected]
> [ 4848.231800] state change SNK_ATTACH_WAIT -> SNK_ATTACH_WAIT
> [ 4848.231857] pending state change SNK_ATTACH_WAIT -> SNK_UNATTACHED @20 ms
> [ 4848.256022] state change SNK_ATTACH_WAIT -> SNK_UNATTACHED [delayed20 ms]
> [ 4848.256049] Start toggling
> [ 4848.871148] VBUS on
> [ 4848.885324] CC1: 0 -> 0, CC2: 0 -> 3 [state TOGGLING, polarity 0, connected]
> [ 4848.885372] state change TOGGLING -> SNK_ATTACH_WAIT
> [ 4848.885548] pending state change SNK_ATTACH_WAIT -> SNK_DEBOUNCED @200 ms
> [ 4849.088240] state change SNK_ATTACH_WAIT -> SNK_DEBOUNCED [delayed200 ms]
> [ 4849.088284] state change SNK_DEBOUNCED -> SNK_ATTACHED
> [ 4849.088291] polarity 1
> [ 4849.088769] Requesting mux state 1, usb-role 2, orientation 2
> [ 4849.088895] state change SNK_ATTACHED -> SNK_STARTUP
> [ 4849.088907] state change SNK_STARTUP -> SNK_DISCOVERY
> [ 4849.088915] Setting voltage/current limit 5000 mV 0 mA
> [ 4849.088927] vbus=0 charge:=1
> [ 4849.090505] state change SNK_DISCOVERY -> SNK_WAIT_CAPABILITIES
> [ 4849.090828] pending state change SNK_WAIT_CAPABILITIES -> SNK_READY @240 ms
> [ 4849.335878] state change SNK_WAIT_CAPABILITIES -> SNK_READY [delayed240 ms]
> 
> this patch fix this issue by clear hard_reset_count at any cases
> of cc disconnect, í.e. don't check port->attached flag.
> 
> Cc: stable@vger.kernel.org
> Fixes: 4b4e02c83167 ("typec: tcpm: Move out of staging")
> Reported-and-tested-by: ChiYuan Huang <cy_huang@richtek.com>
> Signed-off-by: Li Jun <jun.li@nxp.com>

FWIW:

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

> ---
>  drivers/usb/typec/tcpm/tcpm.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 55535c4..a6fae1f 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -2890,6 +2890,9 @@ static void tcpm_reset_port(struct tcpm_port *port)
>  
>  static void tcpm_detach(struct tcpm_port *port)
>  {
> +	if (tcpm_port_is_disconnected(port))
> +		port->hard_reset_count = 0;
> +
>  	if (!port->attached)
>  		return;
>  
> @@ -2898,9 +2901,6 @@ static void tcpm_detach(struct tcpm_port *port)
>  		port->tcpc->set_bist_data(port->tcpc, false);
>  	}
>  
> -	if (tcpm_port_is_disconnected(port))
> -		port->hard_reset_count = 0;
> -
>  	tcpm_reset_port(port);
>  }

thanks,

-- 
heikki

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-12 11:03 [PATCH] usb: typec: tcpm: reset hard_reset_count for any disconnect Li Jun
2020-10-12 13:14 ` Guenter Roeck
2020-10-19 13:56 ` Heikki Krogerus

Linux-USB Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-usb/0 linux-usb/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-usb linux-usb/ https://lore.kernel.org/linux-usb \
		linux-usb@vger.kernel.org
	public-inbox-index linux-usb

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-usb


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git