2021-06-13 15:16:15

by Kyle Tso

[permalink] [raw]
Subject: [PATCH] usb: typec: tcpm: Relax disconnect threshold during power negotiation

If the voltage is being decreased in power negotiation, the Source will
set the power supply to operate at the new voltage level before sending
PS_RDY. Relax the disconnect threshold for Sink after receiving Accept
Message to ensure the relaxed setting is enabled before the voltage
collapse. And the real threshold will be set after Sink receives PS_RDY
Message.

Fixes: f321a02caebd ("usb: typec: tcpm: Implement enabling Auto Discharge disconnect support")
Cc: Badhri Jagan Sridharan <[email protected]>
Signed-off-by: Kyle Tso <[email protected]>
---
drivers/usb/typec/tcpm/tcpm.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 0db685d5d9c0..9f3f37da71b6 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -2646,6 +2646,8 @@ static void tcpm_pd_ctrl_request(struct tcpm_port *port,
switch (port->state) {
case SNK_NEGOTIATE_CAPABILITIES:
port->pps_data.active = false;
+ /* Voltage is going to be at new level. Relax the threshold here. */
+ tcpm_set_auto_vbus_discharge_threshold(port, TYPEC_PWR_MODE_USB, false, 0);
tcpm_set_state(port, SNK_TRANSITION_SINK, 0);
break;
case SNK_NEGOTIATE_PPS_CAPABILITIES:
@@ -2656,6 +2658,8 @@ static void tcpm_pd_ctrl_request(struct tcpm_port *port,
port->req_supply_voltage = port->pps_data.req_out_volt;
port->req_current_limit = port->pps_data.req_op_curr;
power_supply_changed(port->psy);
+ /* Voltage is going to be at new level. Relax the threshold here. */
+ tcpm_set_auto_vbus_discharge_threshold(port, TYPEC_PWR_MODE_USB, false, 0);
tcpm_set_state(port, SNK_TRANSITION_SINK, 0);
break;
case SOFT_RESET_SEND:
--
2.32.0.272.g935e593368-goog


2021-06-14 16:54:09

by Badhri Jagan Sridharan

[permalink] [raw]
Subject: Re: [PATCH] usb: typec: tcpm: Relax disconnect threshold during power negotiation

Hi Kyle,

The change doesn't seem to handle the cases where the partner does not
accept the request i.e.
"case PD_CTRL_REJECT:
case PD_CTRL_WAIT:
case PD_CTRL_NOT_SUPP:"
We should fall back to the disconnect threshold based on the
previously negotiated voltage levels in those cases.

Regards,
Badhri


On Sun, Jun 13, 2021 at 8:13 AM Kyle Tso <[email protected]> wrote:
>
> If the voltage is being decreased in power negotiation, the Source will
> set the power supply to operate at the new voltage level before sending
> PS_RDY. Relax the disconnect threshold for Sink after receiving Accept
> Message to ensure the relaxed setting is enabled before the voltage
> collapse. And the real threshold will be set after Sink receives PS_RDY
> Message.
>
> Fixes: f321a02caebd ("usb: typec: tcpm: Implement enabling Auto Discharge disconnect support")
> Cc: Badhri Jagan Sridharan <[email protected]>
> Signed-off-by: Kyle Tso <[email protected]>
> ---
> drivers/usb/typec/tcpm/tcpm.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 0db685d5d9c0..9f3f37da71b6 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -2646,6 +2646,8 @@ static void tcpm_pd_ctrl_request(struct tcpm_port *port,
> switch (port->state) {
> case SNK_NEGOTIATE_CAPABILITIES:
> port->pps_data.active = false;
> + /* Voltage is going to be at new level. Relax the threshold here. */
> + tcpm_set_auto_vbus_discharge_threshold(port, TYPEC_PWR_MODE_USB, false, 0);
> tcpm_set_state(port, SNK_TRANSITION_SINK, 0);
> break;
> case SNK_NEGOTIATE_PPS_CAPABILITIES:
> @@ -2656,6 +2658,8 @@ static void tcpm_pd_ctrl_request(struct tcpm_port *port,
> port->req_supply_voltage = port->pps_data.req_out_volt;
> port->req_current_limit = port->pps_data.req_op_curr;
> power_supply_changed(port->psy);
> + /* Voltage is going to be at new level. Relax the threshold here. */
> + tcpm_set_auto_vbus_discharge_threshold(port, TYPEC_PWR_MODE_USB, false, 0);
> tcpm_set_state(port, SNK_TRANSITION_SINK, 0);
> break;
> case SOFT_RESET_SEND:
> --
> 2.32.0.272.g935e593368-goog
>

2021-06-14 16:56:43

by Kyle Tso

[permalink] [raw]
Subject: Re: [PATCH] usb: typec: tcpm: Relax disconnect threshold during power negotiation

On Tue, Jun 15, 2021 at 12:52 AM Badhri Jagan Sridharan
<[email protected]> wrote:
>
> Hi Kyle,
>
> The change doesn't seem to handle the cases where the partner does not
> accept the request i.e.
> "case PD_CTRL_REJECT:
> case PD_CTRL_WAIT:
> case PD_CTRL_NOT_SUPP:"
> We should fall back to the disconnect threshold based on the
> previously negotiated voltage levels in those cases.
>
> Regards,
> Badhri
>
>
> On Sun, Jun 13, 2021 at 8:13 AM Kyle Tso <[email protected]> wrote:
> >
> > If the voltage is being decreased in power negotiation, the Source will
> > set the power supply to operate at the new voltage level before sending
> > PS_RDY. Relax the disconnect threshold for Sink after receiving Accept
> > Message to ensure the relaxed setting is enabled before the voltage
> > collapse. And the real threshold will be set after Sink receives PS_RDY
> > Message.
> >
> > Fixes: f321a02caebd ("usb: typec: tcpm: Implement enabling Auto Discharge disconnect support")
> > Cc: Badhri Jagan Sridharan <[email protected]>
> > Signed-off-by: Kyle Tso <[email protected]>
> > ---
> > drivers/usb/typec/tcpm/tcpm.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> > index 0db685d5d9c0..9f3f37da71b6 100644
> > --- a/drivers/usb/typec/tcpm/tcpm.c
> > +++ b/drivers/usb/typec/tcpm/tcpm.c
> > @@ -2646,6 +2646,8 @@ static void tcpm_pd_ctrl_request(struct tcpm_port *port,
> > switch (port->state) {
> > case SNK_NEGOTIATE_CAPABILITIES:
> > port->pps_data.active = false;
> > + /* Voltage is going to be at new level. Relax the threshold here. */
> > + tcpm_set_auto_vbus_discharge_threshold(port, TYPEC_PWR_MODE_USB, false, 0);
> > tcpm_set_state(port, SNK_TRANSITION_SINK, 0);
> > break;
> > case SNK_NEGOTIATE_PPS_CAPABILITIES:
> > @@ -2656,6 +2658,8 @@ static void tcpm_pd_ctrl_request(struct tcpm_port *port,
> > port->req_supply_voltage = port->pps_data.req_out_volt;
> > port->req_current_limit = port->pps_data.req_op_curr;
> > power_supply_changed(port->psy);
> > + /* Voltage is going to be at new level. Relax the threshold here. */
> > + tcpm_set_auto_vbus_discharge_threshold(port, TYPEC_PWR_MODE_USB, false, 0);
> > tcpm_set_state(port, SNK_TRANSITION_SINK, 0);
> > break;

Yes, Badhri you are right.

And it's still too late for PPS power negotiation. The Source begins
to lower it's Vbus level just after Accept Message.
I will send patch v2 to fix these problems.

thanks,
Kyle

> > case SOFT_RESET_SEND:
> > --
> > 2.32.0.272.g935e593368-goog
> >