usb: typec: tcpm: Wait for vbus discharge to VSAFE0V before toggling
Subsystems (4)
Badhri Jagan Sridharan
maintainerPATCH v2 1/1 usb: typec: tcpm: Wait for vbus discharge to VSAFE0V before toggling
When vbus auto discharge is enabled, TCPM can sometimes be faster than the TCPC i.e. TCPM can go ahead and move the port to unattached state (involves disabling vbus auto discharge) before TCPC could effectively discharge vbus to VSAFE0V. This leaves vbus with residual charge and increases the decay time which prevents tsafe0v from being met. This change introduces a new state VBUS_DISCHARGE where the TCPM waits for a maximum of tSafe0V(max) for vbus to discharge to VSAFE0V before transitioning to unattached state and re-enable toggling. If vbus discharges to vsafe0v sooner, then, transition to unattached state happens right away.
Also, while in SNK_READY, when auto discharge is enabled, drive disconnect based on vbus turning off instead of Rp disappearing on CC pins. Rp disappearing on CC pins is almost instanteous compared to vbus decay.
Discharge disconnect support")
---
Changes since V1:
- Add Fixes tag
--- drivers/usb/typec/tcpm/tcpm.c | 60 +++++++++++++++++++++++++++++++---- 1 file changed, 53 insertions(+), 7 deletions(-)
drivers/usb/typec/tcpm/tcpm.c +53 -7
@@ -62,6 +62,8 @@S (SNK_TRANSITION_SINK_VBUS ), \S (SNK_READY ), \ \ +S (VBUS_DISCHARGE ), \ + \S (ACC_UNATTACHED ), \S (DEBUG_ACC_ATTACHED ), \ S(AUDIO_ACC_ATTACHED), \
@@ -438,6 +440,9 @@ struct tcpm_port {enum tcpm_ams next_ams;bool in_ams; +/* Auto vbus discharge state */ +bool auto_vbus_discharge_enabled; +#ifdef CONFIG_DEBUG_FS struct dentry *dentry;struct mutex logbuffer_lock;/* log buffer access lock */
@@ -3413,6 +3418,8 @@ static int tcpm_src_attach(struct tcpm_port *port)if (port->tcpc->enable_auto_vbus_discharge) { ret = port->tcpc->enable_auto_vbus_discharge (port->tcpc, true);tcpm_log_force (port,"enable vbus discharge ret:%d" , ret); +if (!ret) + port->auto_vbus_discharge_enabled = true; } ret =tcpm_set_roles (port, true,TYPEC_SOURCE ,tcpm_data_role_for_source (port));
@@ -3495,6 +3502,8 @@ static void tcpm_reset_port(struct tcpm_port *port)if (port->tcpc->enable_auto_vbus_discharge) { ret = port->tcpc->enable_auto_vbus_discharge (port->tcpc, false);tcpm_log_force (port,"Disable vbus discharge ret:%d" , ret); +if (!ret) + port->auto_vbus_discharge_enabled = false; } port->in_ams = false; port->ams =NONE_AMS ;
@@ -3568,6 +3577,8 @@ static int tcpm_snk_attach(struct tcpm_port *port)tcpm_set_auto_vbus_discharge_threshold (port,TYPEC_PWR_MODE_USB , false,VSAFE5V ); ret = port->tcpc->enable_auto_vbus_discharge (port->tcpc, true);tcpm_log_force (port,"enable vbus discharge ret:%d" , ret); +if (!ret) + port->auto_vbus_discharge_enabled = true; } ret =tcpm_set_roles (port, true,TYPEC_SINK ,tcpm_data_role_for_sink (port));
@@ -3684,6 +3695,12 @@ static void run_state_machine(struct tcpm_port *port)switch (port->state) {case TOGGLING :break ; +case VBUS_DISCHARGE : +if (port->port_type ==TYPEC_PORT_SRC ) +tcpm_set_state (port,SRC_UNATTACHED ,PD_T_SAFE_0V ); +else +tcpm_set_state (port,SNK_UNATTACHED ,PD_T_SAFE_0V ); +break ;/* SRC states */ case SRC_UNATTACHED :if (!port->non_pd_role_swap)
@@ -4669,7 +4686,9 @@ static void _tcpm_cc_change(struct tcpm_port *port, enum typec_cc_status cc1,case SRC_READY :if (tcpm_port_is_disconnected (port) || !tcpm_port_is_source (port)) { -if (port->port_type ==TYPEC_PORT_SRC ) +if (port->auto_vbus_discharge_enabled && !port->vbus_vsafe0v) +tcpm_set_state (port,VBUS_DISCHARGE ,0 ); +else if (port->port_type ==TYPEC_PORT_SRC )tcpm_set_state (port,SRC_UNATTACHED ,0 );else tcpm_set_state (port,SNK_UNATTACHED ,0 );
@@ -4703,7 +4722,18 @@ static void _tcpm_cc_change(struct tcpm_port *port, enum typec_cc_status cc1,tcpm_set_state (port,SNK_DEBOUNCED ,0 );break ;case SNK_READY : -if (tcpm_port_is_disconnected (port)) +/* +* When set_auto_vbus_discharge_threshold is enabled, CC pins go +* away before vbus decays to disconnect threshold. Allow +* disconnect to be driven by vbus disconnect when auto vbus +* discharge is enabled. +* +* EXIT condition is based primarily on vbus disconnect and CC is secondary. +* "A port that has entered into USB PD communications with the Source and +* has seen the CC voltage exceed vRd-USB may monitor the CC pin to detect +* cable disconnect in addition to monitoring VBUS. +*/ +if (!port->auto_vbus_discharge_enabled &&tcpm_port_is_disconnected (port))tcpm_set_state (port,unattached_state (port),0 );else if (!port->pd_capable && (cc1 != old_cc1 || cc2 != old_cc2))
@@ -4803,9 +4833,16 @@ static void _tcpm_cc_change(struct tcpm_port *port, enum typec_cc_status cc1,*/ break ; +case VBUS_DISCHARGE : +/* Do nothing. Waiting for vsafe0v signal */ +break ;default : -if (tcpm_port_is_disconnected (port)) -tcpm_set_state (port,unattached_state (port),0 ); +if (tcpm_port_is_disconnected (port)) { +if (port->auto_vbus_discharge_enabled && !port->vbus_vsafe0v) +tcpm_set_state (port,VBUS_DISCHARGE ,0 ); +else +tcpm_set_state (port,unattached_state (port),0 ); + }break ; } }
@@ -4988,9 +5025,12 @@ static void _tcpm_pd_vbus_off(struct tcpm_port *port)break ;default : -if (port->pwr_role ==TYPEC_SINK && - port->attached) -tcpm_set_state (port,SNK_UNATTACHED ,0 ); +if (port->pwr_role ==TYPEC_SINK && port->attached) { +if (port->auto_vbus_discharge_enabled && !port->vbus_vsafe0v) +tcpm_set_state (port,VBUS_DISCHARGE ,0 ); +else +tcpm_set_state (port,SNK_UNATTACHED ,0 ); + }break ; } }
@@ -5012,6 +5052,12 @@ static void _tcpm_pd_vbus_vsafe0v(struct tcpm_port *port)tcpm_set_state (port,tcpm_try_snk (port) ?SNK_TRY :SRC_ATTACHED ,PD_T_CC_DEBOUNCE );break ; +case VBUS_DISCHARGE : +if (port->port_type ==TYPEC_PORT_SRC ) +tcpm_set_state (port,SRC_UNATTACHED ,0 ); +else +tcpm_set_state (port,SNK_UNATTACHED ,0 ); +break ;default :break ; }
Signature · 1 line
2.30.0.617.g56c4b15f3c-goog
Quoted 90 lines · Badhri Jagan Sridharan
When vbus auto discharge is enabled, TCPM can sometimes be faster than the TCPC i.e. TCPM can go ahead and move the port to unattached state (involves disabling vbus auto discharge) before TCPC could effectively discharge vbus to VSAFE0V. This leaves vbus with residual charge and increases the decay time which prevents tsafe0v from being met. This change introduces a new state
VBUS_DISCHARGEwhere the TCPM waits for a maximum of tSafe0V(max) for vbus to discharge to VSAFE0V before transitioning to unattached state and re-enable toggling. If vbus discharges to vsafe0v sooner, then, transition to unattached state happens right away.Also, while in
SNK_READY, when auto discharge is enabled, drive disconnect based on vbus turning off instead of Rp disappearing on CC pins. Rp disappearing on CC pins is almost instanteous compared to vbus decay.Fixes f321a02caebd ("usb: typec: tcpm: Implement enabling AutoDischarge disconnect support")
Signed-off-by Badhri Jagan Sridharan <[email protected]>---
Changes since V1:
- Add Fixes tag--- drivers/usb/typec/tcpm/tcpm.c | 60 +++++++++++++++++++++++++++++++---- 1 file changed, 53 insertions(+), 7 deletions(-)drivers/usb/typec/tcpm/tcpm.c +17
-0@@ -62,6 +62,8 @@S (SNK_TRANSITION_SINK_VBUS ), \S (SNK_READY ), \ \ +S (VBUS_DISCHARGE ), \ + \S (ACC_UNATTACHED ), \S (DEBUG_ACC_ATTACHED ), \ S(AUDIO_ACC_ATTACHED), \@@ -438,6 +440,9 @@ struct tcpm_port {enum tcpm_ams next_ams;bool in_ams; +/* Auto vbus discharge state */ +bool auto_vbus_discharge_enabled; +#ifdef CONFIG_DEBUG_FS struct dentry *dentry;struct mutex logbuffer_lock;/* log buffer access lock */ @@ -3413,6 +3418,8 @@ static int tcpm_src_attach(struct tcpm_port *port)if (port->tcpc->enable_auto_vbus_discharge) { ret = port->tcpc->enable_auto_vbus_discharge (port->tcpc, true);tcpm_log_force (port,"enable vbus discharge ret:%d" , ret); +if (!ret) + port->auto_vbus_discharge_enabled = true; } ret =tcpm_set_roles (port, true,TYPEC_SOURCE ,tcpm_data_role_for_source (port));@@ -3495,6 +3502,8 @@ static void tcpm_reset_port(struct tcpm_port *port)if (port->tcpc->enable_auto_vbus_discharge) { ret = port->tcpc->enable_auto_vbus_discharge (port->tcpc, false);tcpm_log_force (port,"Disable vbus discharge ret:%d" , ret); +if (!ret) + port->auto_vbus_discharge_enabled = false; } port->in_ams = false; port->ams =NONE_AMS ;@@ -3568,6 +3577,8 @@ static int tcpm_snk_attach(struct tcpm_port *port)tcpm_set_auto_vbus_discharge_threshold (port,TYPEC_PWR_MODE_USB , false,VSAFE5V ); ret = port->tcpc->enable_auto_vbus_discharge (port->tcpc, true);tcpm_log_force (port,"enable vbus discharge ret:%d" , ret); +if (!ret) + port->auto_vbus_discharge_enabled = true; } ret =tcpm_set_roles (port, true,TYPEC_SINK ,tcpm_data_role_for_sink (port));@@ -3684,6 +3695,12 @@ static void run_state_machine(struct tcpm_port *port)switch (port->state) {case TOGGLING :break ; +case VBUS_DISCHARGE : +if (port->port_type ==TYPEC_PORT_SRC ) +tcpm_set_state (port,SRC_UNATTACHED ,PD_T_SAFE_0V ); +else +tcpm_set_state (port,SNK_UNATTACHED ,PD_T_SAFE_0V ); +break ;/* SRC states */ case SRC_UNATTACHED :if (!port->non_pd_role_swap)
drivers/usb/typec/tcpm/tcpm.c static void _tcpm_cc_change(struct tcpm_port *port, enum typec_cc_status cc1, · line 4694
Quoted 10 lines
drivers/usb/typec/tcpm/tcpm.c +3
-1@@ -4669,7 +4686,9 @@ static void _tcpm_cc_change(struct tcpm_port *port, enum typec_cc_status cc1,case SRC_READY :if (tcpm_port_is_disconnected (port) || !tcpm_port_is_source (port)) { -if (port->port_type ==TYPEC_PORT_SRC ) +if (port->auto_vbus_discharge_enabled && !port->vbus_vsafe0v) +tcpm_set_state (port,VBUS_DISCHARGE ,0 ); +else if (port->port_type ==TYPEC_PORT_SRC )tcpm_set_state (port,SRC_UNATTACHED ,0 );else tcpm_set_state (port,SNK_UNATTACHED ,0 );
Unless I am missing something, the new state is only used to set the PD_T_SAFE_0V timeout. Is it really necessary/useful to add a new state just for that, while keeping the rest of if/else statements ? Personally I would prefer something like
timeout = (port->auto_vbus_discharge_enabled && !port->vbus_vsafe0v) ? PD_T_SAFE_0V : 0; if (port->port_type == TYPEC_PORT_SRC) tcpm_set_state(port, SRC_UNATTACHED, timeout); else tcpm_set_state(port, SNK_UNATTACHED, timeout);
In this context, any idea why port_type==TYPEC_PORT_DRP results in SNK_UNATTACHED state ? That seems a bit odd.
Guenter
Quoted 69 lines
drivers/usb/typec/tcpm/tcpm.c +33
-6@@ -4703,7 +4722,18 @@ static void _tcpm_cc_change(struct tcpm_port *port, enum typec_cc_status cc1,tcpm_set_state (port,SNK_DEBOUNCED ,0 );break ;case SNK_READY : -if (tcpm_port_is_disconnected (port)) +/* +* When set_auto_vbus_discharge_threshold is enabled, CC pins go +* away before vbus decays to disconnect threshold. Allow +* disconnect to be driven by vbus disconnect when auto vbus +* discharge is enabled. +* +* EXIT condition is based primarily on vbus disconnect and CC is secondary. +* "A port that has entered into USB PD communications with the Source and +* has seen the CC voltage exceed vRd-USB may monitor the CC pin to detect +* cable disconnect in addition to monitoring VBUS. +*/ +if (!port->auto_vbus_discharge_enabled &&tcpm_port_is_disconnected (port))tcpm_set_state (port,unattached_state (port),0 );else if (!port->pd_capable && (cc1 != old_cc1 || cc2 != old_cc2))@@ -4803,9 +4833,16 @@ static void _tcpm_cc_change(struct tcpm_port *port, enum typec_cc_status cc1,*/ break ; +case VBUS_DISCHARGE : +/* Do nothing. Waiting for vsafe0v signal */ +break ;default : -if (tcpm_port_is_disconnected (port)) -tcpm_set_state (port,unattached_state (port),0 ); +if (tcpm_port_is_disconnected (port)) { +if (port->auto_vbus_discharge_enabled && !port->vbus_vsafe0v) +tcpm_set_state (port,VBUS_DISCHARGE ,0 ); +else +tcpm_set_state (port,unattached_state (port),0 ); + }break ; } }@@ -4988,9 +5025,12 @@ static void _tcpm_pd_vbus_off(struct tcpm_port *port)break ;default : -if (port->pwr_role ==TYPEC_SINK && - port->attached) -tcpm_set_state (port,SNK_UNATTACHED ,0 ); +if (port->pwr_role ==TYPEC_SINK && port->attached) { +if (port->auto_vbus_discharge_enabled && !port->vbus_vsafe0v) +tcpm_set_state (port,VBUS_DISCHARGE ,0 ); +else +tcpm_set_state (port,SNK_UNATTACHED ,0 ); + }break ; } }@@ -5012,6 +5052,12 @@ static void _tcpm_pd_vbus_vsafe0v(struct tcpm_port *port)tcpm_set_state (port,tcpm_try_snk (port) ?SNK_TRY :SRC_ATTACHED ,PD_T_CC_DEBOUNCE );break ; +case VBUS_DISCHARGE : +if (port->port_type ==TYPEC_PORT_SRC ) +tcpm_set_state (port,SRC_UNATTACHED ,0 ); +else +tcpm_set_state (port,SNK_UNATTACHED ,0 ); +break ;default :break ; }
Badhri Jagan Sridharan
maintainerQuoted 107 lines · Guenter Roeck
Quoted 90 lines · Badhri Jagan Sridharan
When vbus auto discharge is enabled, TCPM can sometimes be faster than the TCPC i.e. TCPM can go ahead and move the port to unattached state (involves disabling vbus auto discharge) before TCPC could effectively discharge vbus to VSAFE0V. This leaves vbus with residual charge and increases the decay time which prevents tsafe0v from being met. This change introduces a new state
VBUS_DISCHARGEwhere the TCPM waits for a maximum of tSafe0V(max) for vbus to discharge to VSAFE0V before transitioning to unattached state and re-enable toggling. If vbus discharges to vsafe0v sooner, then, transition to unattached state happens right away.Also, while in
SNK_READY, when auto discharge is enabled, drive disconnect based on vbus turning off instead of Rp disappearing on CC pins. Rp disappearing on CC pins is almost instanteous compared to vbus decay.Fixes f321a02caebd ("usb: typec: tcpm: Implement enabling AutoDischarge disconnect support")
Signed-off-by Badhri Jagan Sridharan <[email protected]>---
Changes since V1:
- Add Fixes tag--- drivers/usb/typec/tcpm/tcpm.c | 60 +++++++++++++++++++++++++++++++---- 1 file changed, 53 insertions(+), 7 deletions(-)drivers/usb/typec/tcpm/tcpm.c +17
-0@@ -62,6 +62,8 @@S (SNK_TRANSITION_SINK_VBUS ), \S (SNK_READY ), \ \ +S (VBUS_DISCHARGE ), \ + \S (ACC_UNATTACHED ), \S (DEBUG_ACC_ATTACHED ), \ S(AUDIO_ACC_ATTACHED), \@@ -438,6 +440,9 @@ struct tcpm_port {enum tcpm_ams next_ams;bool in_ams; +/* Auto vbus discharge state */ +bool auto_vbus_discharge_enabled; +#ifdef CONFIG_DEBUG_FS struct dentry *dentry;struct mutex logbuffer_lock;/* log buffer access lock */ @@ -3413,6 +3418,8 @@ static int tcpm_src_attach(struct tcpm_port *port)if (port->tcpc->enable_auto_vbus_discharge) { ret = port->tcpc->enable_auto_vbus_discharge (port->tcpc, true);tcpm_log_force (port,"enable vbus discharge ret:%d" , ret); +if (!ret) + port->auto_vbus_discharge_enabled = true; } ret =tcpm_set_roles (port, true,TYPEC_SOURCE ,tcpm_data_role_for_source (port));@@ -3495,6 +3502,8 @@ static void tcpm_reset_port(struct tcpm_port *port)if (port->tcpc->enable_auto_vbus_discharge) { ret = port->tcpc->enable_auto_vbus_discharge (port->tcpc, false);tcpm_log_force (port,"Disable vbus discharge ret:%d" , ret); +if (!ret) + port->auto_vbus_discharge_enabled = false; } port->in_ams = false; port->ams =NONE_AMS ;@@ -3568,6 +3577,8 @@ static int tcpm_snk_attach(struct tcpm_port *port)tcpm_set_auto_vbus_discharge_threshold (port,TYPEC_PWR_MODE_USB , false,VSAFE5V ); ret = port->tcpc->enable_auto_vbus_discharge (port->tcpc, true);tcpm_log_force (port,"enable vbus discharge ret:%d" , ret); +if (!ret) + port->auto_vbus_discharge_enabled = true; } ret =tcpm_set_roles (port, true,TYPEC_SINK ,tcpm_data_role_for_sink (port));@@ -3684,6 +3695,12 @@ static void run_state_machine(struct tcpm_port *port)switch (port->state) {case TOGGLING :break ; +case VBUS_DISCHARGE : +if (port->port_type ==TYPEC_PORT_SRC ) +tcpm_set_state (port,SRC_UNATTACHED ,PD_T_SAFE_0V ); +else +tcpm_set_state (port,SNK_UNATTACHED ,PD_T_SAFE_0V ); +break ;/* SRC states */ case SRC_UNATTACHED :if (!port->non_pd_role_swap)drivers/usb/typec/tcpm/tcpm.c static void _tcpm_cc_change(struct tcpm_port *port, enum typec_cc_status cc1, · line 4694
Quoted 10 lines
drivers/usb/typec/tcpm/tcpm.c +3
-1@@ -4669,7 +4686,9 @@ static void _tcpm_cc_change(struct tcpm_port *port, enum typec_cc_status cc1,case SRC_READY :if (tcpm_port_is_disconnected (port) || !tcpm_port_is_source (port)) { -if (port->port_type ==TYPEC_PORT_SRC ) +if (port->auto_vbus_discharge_enabled && !port->vbus_vsafe0v) +tcpm_set_state (port,VBUS_DISCHARGE ,0 ); +else if (port->port_type ==TYPEC_PORT_SRC )tcpm_set_state (port,SRC_UNATTACHED ,0 );else tcpm_set_state (port,SNK_UNATTACHED ,0 );Unless I am missing something, the new state is only used to set the
PD_T_SAFE_0Vtimeout. Is it really necessary/useful to add a new state just for that, while keeping the rest of if/else statements ? Personally I would prefer something like
Quoted 6 lines
timeout = (port->auto_vbus_discharge_enabled && !port->vbus_vsafe0v) ? PD_T_SAFE_0V : 0;
if (port->port_type == TYPEC_PORT_SRC)
tcpm_set_state(port, SRC_UNATTACHED, timeout);
else
tcpm_set_state(port, SNK_UNATTACHED, timeout);Yes this should be OK as well. I was thinking it would be more clearer during debug if there
was a separate state altogether, but, looks like we should be fine. Implementing/Validating it now. Will send a follow up version today.
Quoted 2 lines
In this context, any idea why port_type==TYPEC_PORT_DRP results in
SNK_UNATTACHEDstate ? That seems a bit odd.
This comes from the patch here:
https://lore.kernel.org/r/[email protected]
Looks reasonable to me as tcpm_*_detach functions call tcpm_detach so teardown should
happen anyways.
static void tcpm_snk_detach(struct tcpm_port *port)
{
tcpm_detach(port);
}static void tcpm_src_detach(struct tcpm_port *port)
{
tcpm_detach(port);
}Thanks,
Badhri
Quoted 73 lines
Guenter
Quoted 69 lines
drivers/usb/typec/tcpm/tcpm.c +33
-6@@ -4703,7 +4722,18 @@ static void _tcpm_cc_change(struct tcpm_port *port, enum typec_cc_status cc1,tcpm_set_state (port,SNK_DEBOUNCED ,0 );break ;case SNK_READY : -if (tcpm_port_is_disconnected (port)) +/* +* When set_auto_vbus_discharge_threshold is enabled, CC pins go +* away before vbus decays to disconnect threshold. Allow +* disconnect to be driven by vbus disconnect when auto vbus +* discharge is enabled. +* +* EXIT condition is based primarily on vbus disconnect and CC is secondary. +* "A port that has entered into USB PD communications with the Source and +* has seen the CC voltage exceed vRd-USB may monitor the CC pin to detect +* cable disconnect in addition to monitoring VBUS. +*/ +if (!port->auto_vbus_discharge_enabled &&tcpm_port_is_disconnected (port))tcpm_set_state (port,unattached_state (port),0 );else if (!port->pd_capable && (cc1 != old_cc1 || cc2 != old_cc2))@@ -4803,9 +4833,16 @@ static void _tcpm_cc_change(struct tcpm_port *port, enum typec_cc_status cc1,*/ break ; +case VBUS_DISCHARGE : +/* Do nothing. Waiting for vsafe0v signal */ +break ;default : -if (tcpm_port_is_disconnected (port)) -tcpm_set_state (port,unattached_state (port),0 ); +if (tcpm_port_is_disconnected (port)) { +if (port->auto_vbus_discharge_enabled && !port->vbus_vsafe0v) +tcpm_set_state (port,VBUS_DISCHARGE ,0 ); +else +tcpm_set_state (port,unattached_state (port),0 ); + }break ; } }@@ -4988,9 +5025,12 @@ static void _tcpm_pd_vbus_off(struct tcpm_port *port)break ;default : -if (port->pwr_role ==TYPEC_SINK && - port->attached) -tcpm_set_state (port,SNK_UNATTACHED ,0 ); +if (port->pwr_role ==TYPEC_SINK && port->attached) { +if (port->auto_vbus_discharge_enabled && !port->vbus_vsafe0v) +tcpm_set_state (port,VBUS_DISCHARGE ,0 ); +else +tcpm_set_state (port,SNK_UNATTACHED ,0 ); + }break ; } }@@ -5012,6 +5052,12 @@ static void _tcpm_pd_vbus_vsafe0v(struct tcpm_port *port)tcpm_set_state (port,tcpm_try_snk (port) ?SNK_TRY :SRC_ATTACHED ,PD_T_CC_DEBOUNCE );break ; +case VBUS_DISCHARGE : +if (port->port_type ==TYPEC_PORT_SRC ) +tcpm_set_state (port,SRC_UNATTACHED ,0 ); +else +tcpm_set_state (port,SNK_UNATTACHED ,0 ); +break ;default :break ; }
Badhri Jagan Sridharan
maintainerQuoted 115 lines · Badhri Jagan Sridharan
Quoted 107 lines · Guenter Roeck
Quoted 90 lines · Badhri Jagan Sridharan
When vbus auto discharge is enabled, TCPM can sometimes be faster than the TCPC i.e. TCPM can go ahead and move the port to unattached state (involves disabling vbus auto discharge) before TCPC could effectively discharge vbus to VSAFE0V. This leaves vbus with residual charge and increases the decay time which prevents tsafe0v from being met. This change introduces a new state
VBUS_DISCHARGEwhere the TCPM waits for a maximum of tSafe0V(max) for vbus to discharge to VSAFE0V before transitioning to unattached state and re-enable toggling. If vbus discharges to vsafe0v sooner, then, transition to unattached state happens right away.Also, while in
SNK_READY, when auto discharge is enabled, drive disconnect based on vbus turning off instead of Rp disappearing on CC pins. Rp disappearing on CC pins is almost instanteous compared to vbus decay.Fixes f321a02caebd ("usb: typec: tcpm: Implement enabling AutoDischarge disconnect support")
Signed-off-by Badhri Jagan Sridharan <[email protected]>---
Changes since V1:
- Add Fixes tag--- drivers/usb/typec/tcpm/tcpm.c | 60 +++++++++++++++++++++++++++++++---- 1 file changed, 53 insertions(+), 7 deletions(-)drivers/usb/typec/tcpm/tcpm.c +17
-0@@ -62,6 +62,8 @@S (SNK_TRANSITION_SINK_VBUS ), \S (SNK_READY ), \ \ +S (VBUS_DISCHARGE ), \ + \S (ACC_UNATTACHED ), \S (DEBUG_ACC_ATTACHED ), \ S(AUDIO_ACC_ATTACHED), \@@ -438,6 +440,9 @@ struct tcpm_port {enum tcpm_ams next_ams;bool in_ams; +/* Auto vbus discharge state */ +bool auto_vbus_discharge_enabled; +#ifdef CONFIG_DEBUG_FS struct dentry *dentry;struct mutex logbuffer_lock;/* log buffer access lock */ @@ -3413,6 +3418,8 @@ static int tcpm_src_attach(struct tcpm_port *port)if (port->tcpc->enable_auto_vbus_discharge) { ret = port->tcpc->enable_auto_vbus_discharge (port->tcpc, true);tcpm_log_force (port,"enable vbus discharge ret:%d" , ret); +if (!ret) + port->auto_vbus_discharge_enabled = true; } ret =tcpm_set_roles (port, true,TYPEC_SOURCE ,tcpm_data_role_for_source (port));@@ -3495,6 +3502,8 @@ static void tcpm_reset_port(struct tcpm_port *port)if (port->tcpc->enable_auto_vbus_discharge) { ret = port->tcpc->enable_auto_vbus_discharge (port->tcpc, false);tcpm_log_force (port,"Disable vbus discharge ret:%d" , ret); +if (!ret) + port->auto_vbus_discharge_enabled = false; } port->in_ams = false; port->ams =NONE_AMS ;@@ -3568,6 +3577,8 @@ static int tcpm_snk_attach(struct tcpm_port *port)tcpm_set_auto_vbus_discharge_threshold (port,TYPEC_PWR_MODE_USB , false,VSAFE5V ); ret = port->tcpc->enable_auto_vbus_discharge (port->tcpc, true);tcpm_log_force (port,"enable vbus discharge ret:%d" , ret); +if (!ret) + port->auto_vbus_discharge_enabled = true; } ret =tcpm_set_roles (port, true,TYPEC_SINK ,tcpm_data_role_for_sink (port));@@ -3684,6 +3695,12 @@ static void run_state_machine(struct tcpm_port *port)switch (port->state) {case TOGGLING :break ; +case VBUS_DISCHARGE : +if (port->port_type ==TYPEC_PORT_SRC ) +tcpm_set_state (port,SRC_UNATTACHED ,PD_T_SAFE_0V ); +else +tcpm_set_state (port,SNK_UNATTACHED ,PD_T_SAFE_0V ); +break ;/* SRC states */ case SRC_UNATTACHED :if (!port->non_pd_role_swap)drivers/usb/typec/tcpm/tcpm.c static void _tcpm_cc_change(struct tcpm_port *port, enum typec_cc_status cc1, · line 4694
Quoted 10 lines
drivers/usb/typec/tcpm/tcpm.c +3
-1@@ -4669,7 +4686,9 @@ static void _tcpm_cc_change(struct tcpm_port *port, enum typec_cc_status cc1,case SRC_READY :if (tcpm_port_is_disconnected (port) || !tcpm_port_is_source (port)) { -if (port->port_type ==TYPEC_PORT_SRC ) +if (port->auto_vbus_discharge_enabled && !port->vbus_vsafe0v) +tcpm_set_state (port,VBUS_DISCHARGE ,0 ); +else if (port->port_type ==TYPEC_PORT_SRC )tcpm_set_state (port,SRC_UNATTACHED ,0 );else tcpm_set_state (port,SNK_UNATTACHED ,0 );Unless I am missing something, the new state is only used to set the
PD_T_SAFE_0Vtimeout. Is it really necessary/useful to add a new state just for that, while keeping the rest of if/else statements ? Personally I would prefer something likeQuoted 6 lines
timeout = (port->auto_vbus_discharge_enabled && !port->vbus_vsafe0v) ? PD_T_SAFE_0V : 0; if (port->port_type == TYPEC_PORT_SRC) tcpm_set_state(port, SRC_UNATTACHED, timeout); else tcpm_set_state(port, SNK_UNATTACHED, timeout);
Quoted 4 lines
Yes this should be OK as well. I was thinking it would be more clearer during debug if there
was a separate state altogether, but, looks like we should be fine. Implementing/Validating it now. Will send a follow up version today.
Just sent out the V3 version of the patch.
Quoted 98 lines
Quoted 3 lines
In this context, any idea why port_type==TYPEC_PORT_DRP results in
SNK_UNATTACHEDstate ? That seems a bit odd.This comes from the patch here:
https://lore.kernel.org/r/[email protected]Looks reasonable to me as tcpm_*_detach functions call
tcpm_detachso teardown should
happen anyways.static void
tcpm_snk_detach(struct tcpm_port *port){ tcpm_detach(port); }static void
tcpm_src_detach(struct tcpm_port *port){ tcpm_detach(port); }Thanks,
BadhriQuoted 73 lines
Guenter
Quoted 69 lines
drivers/usb/typec/tcpm/tcpm.c +33
-6@@ -4703,7 +4722,18 @@ static void _tcpm_cc_change(struct tcpm_port *port, enum typec_cc_status cc1,tcpm_set_state (port,SNK_DEBOUNCED ,0 );break ;case SNK_READY : -if (tcpm_port_is_disconnected (port)) +/* +* When set_auto_vbus_discharge_threshold is enabled, CC pins go +* away before vbus decays to disconnect threshold. Allow +* disconnect to be driven by vbus disconnect when auto vbus +* discharge is enabled. +* +* EXIT condition is based primarily on vbus disconnect and CC is secondary. +* "A port that has entered into USB PD communications with the Source and +* has seen the CC voltage exceed vRd-USB may monitor the CC pin to detect +* cable disconnect in addition to monitoring VBUS. +*/ +if (!port->auto_vbus_discharge_enabled &&tcpm_port_is_disconnected (port))tcpm_set_state (port,unattached_state (port),0 );else if (!port->pd_capable && (cc1 != old_cc1 || cc2 != old_cc2))@@ -4803,9 +4833,16 @@ static void _tcpm_cc_change(struct tcpm_port *port, enum typec_cc_status cc1,*/ break ; +case VBUS_DISCHARGE : +/* Do nothing. Waiting for vsafe0v signal */ +break ;default : -if (tcpm_port_is_disconnected (port)) -tcpm_set_state (port,unattached_state (port),0 ); +if (tcpm_port_is_disconnected (port)) { +if (port->auto_vbus_discharge_enabled && !port->vbus_vsafe0v) +tcpm_set_state (port,VBUS_DISCHARGE ,0 ); +else +tcpm_set_state (port,unattached_state (port),0 ); + }break ; } }@@ -4988,9 +5025,12 @@ static void _tcpm_pd_vbus_off(struct tcpm_port *port)break ;default : -if (port->pwr_role ==TYPEC_SINK && - port->attached) -tcpm_set_state (port,SNK_UNATTACHED ,0 ); +if (port->pwr_role ==TYPEC_SINK && port->attached) { +if (port->auto_vbus_discharge_enabled && !port->vbus_vsafe0v) +tcpm_set_state (port,VBUS_DISCHARGE ,0 ); +else +tcpm_set_state (port,SNK_UNATTACHED ,0 ); + }break ; } }@@ -5012,6 +5052,12 @@ static void _tcpm_pd_vbus_vsafe0v(struct tcpm_port *port)tcpm_set_state (port,tcpm_try_snk (port) ?SNK_TRY :SRC_ATTACHED ,PD_T_CC_DEBOUNCE );break ; +case VBUS_DISCHARGE : +if (port->port_type ==TYPEC_PORT_SRC ) +tcpm_set_state (port,SRC_UNATTACHED ,0 ); +else +tcpm_set_state (port,SNK_UNATTACHED ,0 ); +break ;default :break ; }