LinuxLists.cc - [PATCH] usb: typec: tcpm: Fix SINK_DISCOVERY current limit for Rp-default

2021-05-10 21:21:11

Subject: [PATCH] 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(-)

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index c4fdc00a3bc8..a73299a08ef7 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -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);
--
2.31.1.607.g51e8a6a459-goog

2021-05-10 22:08:35

by Guenter Roeck

[permalink] [raw]

Subject: Re: [PATCH] usb: typec: tcpm: Fix SINK_DISCOVERY current limit for Rp-default

On 5/10/21 2:17 PM, Badhri Jagan Sridharan wrote:
> 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]>

Reviewed-by: Guenter Roeck <[email protected]>

> ---
> 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 c4fdc00a3bc8..a73299a08ef7 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -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);
>

2021-05-11 07:22:40

by Heikki Krogerus

[permalink] [raw]

Subject: Re: [PATCH] usb: typec: tcpm: Fix SINK_DISCOVERY current limit for Rp-default

On Mon, May 10, 2021 at 02:17:56PM -0700, Badhri Jagan Sridharan wrote:
> 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]>

Acked-by: Heikki Krogerus <[email protected]>

> ---
> 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 c4fdc00a3bc8..a73299a08ef7 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -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);
> --
> 2.31.1.607.g51e8a6a459-goog

--
heikki

2021-05-13 19:37:19

by Greg Kroah-Hartman

[permalink] [raw]

Subject: Re: [PATCH] usb: typec: tcpm: Fix SINK_DISCOVERY current limit for Rp-default

2021-05-13 23:01:57

by Badhri Jagan Sridharan

[permalink] [raw]

Subject: Re: [PATCH] usb: typec: tcpm: Fix SINK_DISCOVERY current limit for Rp-default

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

Thanks for fixing it up !
-Badhri

On Thu, May 13, 2021 at 6:01 AM Greg Kroah-Hartman
<[email protected]> wrote:
>
> On Mon, May 10, 2021 at 02:17:56PM -0700, Badhri Jagan Sridharan wrote:
> > 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")
>
> 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