usb: typec: tcpm: Fix SINK_DISCOVERY current limit for Rp-default

PATCH In review Badhri Jagan Sridharan 4 participants 5 messages May 10, 2021 – May 13, 2021

Subsystems (3)

USB USB Type-C class USB Type-C port controller drivers

lore

Share & tools

lore

Badhri Jagan Sridharan

author maintainer

2021-05-10 21:18 UTC #0 lore raw

PATCH 1/1 usb: typec: tcpm: Fix SINK_DISCOVERY current limit for Rp-default

This is a regression introduced by <1373fefc6243cc96b3565f0ffffadfac4ccfb977> "Allow slow charging loops to comply to pSnkStby".

When Source advertises Rp-default, tcpm would request 500mA when in SINK_DISCOVERY, Type-C spec advises the sink to follow BC1.2 current limits when Rp-default is advertised.

[12750.503381] Requesting mux state 1, usb-role 2, orientation 1
[12750.503837] state change SNK_ATTACHED -> SNK_STARTUP [rev3 NONE_AMS]
[12751.003891] state change SNK_STARTUP -> SNK_DISCOVERY
[12751.003900] Setting voltage/current limit 5000 mV 500 mA

This patch restores the behavior where the tcpm would request 0mA when Rp-default is advertised by the source.
[ 73.174252] Requesting mux state 1, usb-role 2, orientation 1 [ 73.174749] state change SNK_ATTACHED -> SNK_STARTUP [rev3 NONE_AMS] [ 73.674800] state change SNK_STARTUP -> SNK_DISCOVERY
[ 73.674808] Setting voltage/current limit 5000 mV 0 mA

During SNK_DISCOVERY, Cap the current limit to PD_P_SNK_STDBY_MW / 5 only for slow_charger_loop case.

Fixes 1373fefc6243 ("Allow slow charging loops to comply to pSnkStby")

Signed-off-by Badhri Jagan Sridharan <[email protected]>

---
 drivers/usb/typec/tcpm/tcpm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/usb/typec/tcpm/tcpm.c +1 -1

index c4fdc00a3bc8..a73299a08ef7 100644
@@ -4055,7 +4055,7 @@ static void run_state_machine(struct tcpm_port *port)
 		if (port->vbus_present) {
 			u32 current_lim = tcpm_get_current_limit(port);
 
-			if (port->slow_charger_loop || (current_lim > PD_P_SNK_STDBY_MW / 5))
+			if (port->slow_charger_loop && (current_lim > PD_P_SNK_STDBY_MW / 5))
 				current_lim = PD_P_SNK_STDBY_MW / 5;
 			tcpm_set_current_limit(port, current_lim, 5000);
 			tcpm_set_charge(port, true);

Signature · 1 line

2.31.1.607.g51e8a6a459-goog

Guenter Roeck

Reviewed-by on v1 1/1

2021-05-10 22:07 UTC #1 lore raw

Quoted 19 lines · Badhri Jagan Sridharan

This is a regression introduced by <1373fefc6243cc96b3565f0ffffadfac4ccfb977> "Allow slow charging loops to comply to pSnkStby".
When Source advertises Rp-default, tcpm would request 500mA when in SINK_DISCOVERY, Type-C spec advises the sink to follow BC1.2 current limits when Rp-default is advertised.
[12750.503381] Requesting mux state 1, usb-role 2, orientation 1
[12750.503837] state change SNK_ATTACHED -> SNK_STARTUP [rev3 NONE_AMS]
[12751.003891] state change SNK_STARTUP -> SNK_DISCOVERY
[12751.003900] Setting voltage/current limit 5000 mV 500 mA
This patch restores the behavior where the tcpm would request 0mA when Rp-default is advertised by the source.
[ 73.174252] Requesting mux state 1, usb-role 2, orientation 1 [ 73.174749] state change SNK_ATTACHED -> SNK_STARTUP [rev3 NONE_AMS] [ 73.674800] state change SNK_STARTUP -> SNK_DISCOVERY
[ 73.674808] Setting voltage/current limit 5000 mV 0 mA

Quoted 5 lines

During SNK_DISCOVERY, Cap the current limit to PD_P_SNK_STDBY_MW / 5 only for slow_charger_loop case.
Fixes 1373fefc6243 ("Allow slow charging loops to comply to pSnkStby")
Signed-off-by Badhri Jagan Sridharan <[email protected]>

Reviewed-by Guenter Roeck <[email protected]>

Quoted 18 lines

---
  drivers/usb/typec/tcpm/tcpm.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

drivers/usb/typec/tcpm/tcpm.c +1 -1

index c4fdc00a3bc8..a73299a08ef7 100644
@@ -4055,7 +4055,7 @@ static void run_state_machine(struct tcpm_port *port)
  		if (port->vbus_present) {
  			u32 current_lim = tcpm_get_current_limit(port);
 
-			if (port->slow_charger_loop || (current_lim > PD_P_SNK_STDBY_MW / 5))
+			if (port->slow_charger_loop && (current_lim > PD_P_SNK_STDBY_MW / 5))
  				current_lim = PD_P_SNK_STDBY_MW / 5;
  			tcpm_set_current_limit(port, current_lim, 5000);
  			tcpm_set_charge(port, true);

Heikki Krogerus

maintainer Acked-by on v1 1/1

2021-05-11 07:20 UTC #2 lore raw

In reply to Badhri Jagan Sridharan

Quoted 19 lines · Badhri Jagan Sridharan

This is a regression introduced by <1373fefc6243cc96b3565f0ffffadfac4ccfb977> "Allow slow charging loops to comply to pSnkStby".
When Source advertises Rp-default, tcpm would request 500mA when in SINK_DISCOVERY, Type-C spec advises the sink to follow BC1.2 current limits when Rp-default is advertised.
[12750.503381] Requesting mux state 1, usb-role 2, orientation 1
[12750.503837] state change SNK_ATTACHED -> SNK_STARTUP [rev3 NONE_AMS]
[12751.003891] state change SNK_STARTUP -> SNK_DISCOVERY
[12751.003900] Setting voltage/current limit 5000 mV 500 mA
This patch restores the behavior where the tcpm would request 0mA when Rp-default is advertised by the source.
[ 73.174252] Requesting mux state 1, usb-role 2, orientation 1 [ 73.174749] state change SNK_ATTACHED -> SNK_STARTUP [rev3 NONE_AMS] [ 73.674800] state change SNK_STARTUP -> SNK_DISCOVERY
[ 73.674808] Setting voltage/current limit 5000 mV 0 mA

Quoted 5 lines

During SNK_DISCOVERY, Cap the current limit to PD_P_SNK_STDBY_MW / 5 only for slow_charger_loop case.
Fixes 1373fefc6243 ("Allow slow charging loops to comply to pSnkStby")
Signed-off-by Badhri Jagan Sridharan <[email protected]>

Acked-by Heikki Krogerus <[email protected]>

Quoted 19 lines

---
 drivers/usb/typec/tcpm/tcpm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/usb/typec/tcpm/tcpm.c +1 -1

index c4fdc00a3bc8..a73299a08ef7 100644
@@ -4055,7 +4055,7 @@ static void run_state_machine(struct tcpm_port *port)
 		if (port->vbus_present) {
 			u32 current_lim = tcpm_get_current_limit(port);
 
-			if (port->slow_charger_loop || (current_lim > PD_P_SNK_STDBY_MW / 5))
+			if (port->slow_charger_loop && (current_lim > PD_P_SNK_STDBY_MW / 5))
 				current_lim = PD_P_SNK_STDBY_MW / 5;
 			tcpm_set_current_limit(port, current_lim, 5000);
 			tcpm_set_charge(port, true);

Signature · 1 line

2.31.1.607.g51e8a6a459-goog

Signature · 1 line

heikki

Greg Kroah-Hartman

maintainer

2021-05-13 13:01 UTC #3 lore raw

In reply to Badhri Jagan Sridharan

Quoted 19 lines · Badhri Jagan Sridharan

This is a regression introduced by <1373fefc6243cc96b3565f0ffffadfac4ccfb977> "Allow slow charging loops to comply to pSnkStby".
When Source advertises Rp-default, tcpm would request 500mA when in SINK_DISCOVERY, Type-C spec advises the sink to follow BC1.2 current limits when Rp-default is advertised.
[12750.503381] Requesting mux state 1, usb-role 2, orientation 1
[12750.503837] state change SNK_ATTACHED -> SNK_STARTUP [rev3 NONE_AMS]
[12751.003891] state change SNK_STARTUP -> SNK_DISCOVERY
[12751.003900] Setting voltage/current limit 5000 mV 500 mA
This patch restores the behavior where the tcpm would request 0mA when Rp-default is advertised by the source.
[ 73.174252] Requesting mux state 1, usb-role 2, orientation 1 [ 73.174749] state change SNK_ATTACHED -> SNK_STARTUP [rev3 NONE_AMS] [ 73.674800] state change SNK_STARTUP -> SNK_DISCOVERY
[ 73.674808] Setting voltage/current limit 5000 mV 0 mA

Quoted 4 lines

During SNK_DISCOVERY, Cap the current limit to PD_P_SNK_STDBY_MW / 5 only for slow_charger_loop case.
Fixes 1373fefc6243 ("Allow slow charging loops to comply to pSnkStby")

Your string here was incorrect, I'll fix it up this time, but please be more careful in the future as it will get caught by our scripts.

thanks,

greg k-h

Badhri Jagan Sridharan

author maintainer

2021-05-13 18:00 UTC #4 lore raw

Apologies Greg ! Will pay more attention to the "fixes" string next time.

Thanks for fixing it up !
-Badhri

Quoted 32 lines · Greg Kroah-Hartman

Quoted 19 lines · Badhri Jagan Sridharan
This is a regression introduced by <1373fefc6243cc96b3565f0ffffadfac4ccfb977> "Allow slow charging loops to comply to pSnkStby".
When Source advertises Rp-default, tcpm would request 500mA when in SINK_DISCOVERY, Type-C spec advises the sink to follow BC1.2 current limits when Rp-default is advertised.
[12750.503381] Requesting mux state 1, usb-role 2, orientation 1
[12750.503837] state change SNK_ATTACHED -> SNK_STARTUP [rev3 NONE_AMS]
[12751.003891] state change SNK_STARTUP -> SNK_DISCOVERY
[12751.003900] Setting voltage/current limit 5000 mV 500 mA
This patch restores the behavior where the tcpm would request 0mA when Rp-default is advertised by the source.
[   73.174252] Requesting mux state 1, usb-role 2, orientation 1
[   73.174749] state change SNK_ATTACHED -> SNK_STARTUP [rev3 NONE_AMS]
[   73.674800] state change SNK_STARTUP -> SNK_DISCOVERY
[   73.674808] Setting voltage/current limit 5000 mV 0 mA
Quoted 4 lines
During SNK_DISCOVERY, Cap the current limit to PD_P_SNK_STDBY_MW / 5 only for slow_charger_loop case.
Fixes 1373fefc6243 ("Allow slow charging loops to comply to pSnkStby")
Your string here was incorrect, I'll fix it up this time, but please be more careful in the future as it will get caught by our scripts.
thanks,
greg k-h