* [PATCH] usb: typec: tcpm: Prevent Hard_Reset if Vbus was never low @ 2024-10-22 17:28 Yanik Fuchs 2024-10-23 9:05 ` Heikki Krogerus 2024-10-24 3:13 ` Amit Sunil Dhamne 0 siblings, 2 replies; 5+ messages in thread From: Yanik Fuchs @ 2024-10-22 17:28 UTC (permalink / raw) To: heikki.krogerus, gregkh, rdbabiera, linux, kyletso, badhri, xu.yang_2, amitsd Cc: linux-usb, linux-kernel Good Evening Here is a Patch to resolve an issue with TCPM if Vbus was never low. Please consider that this is one of my first kernel pull requests, I may have missunderstood the process. Freundliche Grüsse Best regards Yanik Fuchs --- From 604b97b6394b5643394bc63d4ac691c098c99c40 Mon Sep 17 00:00:00 2001 From: yfu <yanikfuchs@me.com> Date: Tue, 22 Oct 2024 18:23:18 +0200 Subject: [PATCH] usb: typec: tcpm: Prevent Hard_Reset if Vbus was never low Before this patch, tcpm went into SOFT_RESET state, if Vbus was never low resulting in Hard_Reset, if power supply does not support USB_PD Soft_Reset. In order to prevent this, I remove the Vbus check completely, so that we go as well into the SNK_WAIT_CAPABILITIES_TIMEOUT state. There, we send PD_CTRL_GET_SOURCE_CAP which many power supply do support. (122968f8dda8 usb: typec: tcpm: avoid resets for missing source capability messages) Additionally, I added SOFT_RESET (instead of Hard_Reset) as Fallback solution if we still not have gotten any capabilities. Hard_Reset is now only done, if PD_CTRL_GET_SOURCE_CAP and SOFT_RESET fail to get capabilities. --- drivers/usb/typec/tcpm/tcpm.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c index fc619478200f..c7a59c9f78ee 100644 --- a/drivers/usb/typec/tcpm/tcpm.c +++ b/drivers/usb/typec/tcpm/tcpm.c @@ -5038,14 +5038,8 @@ static void run_state_machine(struct tcpm_port *port) * were already in a stable contract before this boot. * Do this only once. */ - if (port->vbus_never_low) { - port->vbus_never_low = false; - tcpm_set_state(port, SNK_SOFT_RESET, + tcpm_set_state(port, SNK_WAIT_CAPABILITIES_TIMEOUT, PD_T_SINK_WAIT_CAP); - } else { - tcpm_set_state(port, SNK_WAIT_CAPABILITIES_TIMEOUT, - PD_T_SINK_WAIT_CAP); - } break; case SNK_WAIT_CAPABILITIES_TIMEOUT: /* @@ -5064,7 +5058,7 @@ static void run_state_machine(struct tcpm_port *port) * according to the specification. */ if (tcpm_pd_send_control(port, PD_CTRL_GET_SOURCE_CAP, TCPC_TX_SOP)) - tcpm_set_state_cond(port, hard_reset_state(port), 0); + tcpm_set_state_cond(port, SNK_SOFT_RESET, 0); else tcpm_set_state(port, hard_reset_state(port), PD_T_SINK_WAIT_CAP); break; -- 2.34.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] usb: typec: tcpm: Prevent Hard_Reset if Vbus was never low 2024-10-22 17:28 [PATCH] usb: typec: tcpm: Prevent Hard_Reset if Vbus was never low Yanik Fuchs @ 2024-10-23 9:05 ` Heikki Krogerus 2024-10-24 3:13 ` Amit Sunil Dhamne 1 sibling, 0 replies; 5+ messages in thread From: Heikki Krogerus @ 2024-10-23 9:05 UTC (permalink / raw) To: Yanik Fuchs Cc: gregkh, rdbabiera, linux, kyletso, badhri, xu.yang_2, amitsd, linux-usb, linux-kernel Hi Yanik, On Tue, Oct 22, 2024 at 05:28:51PM +0000, Yanik Fuchs wrote: > Good Evening > > Here is a Patch to resolve an issue with TCPM if Vbus was never low. > Please consider that this is one of my first kernel pull requests, I may have missunderstood the process. Welcome aboard :) Thank you for the patch. Unfortunately it is not properly formatted. As this is a patch, you can't really comment it like this here. Instead you should put any additional comments... > Freundliche Grüsse > Best regards > > > Yanik Fuchs > > --- > > >From 604b97b6394b5643394bc63d4ac691c098c99c40 Mon Sep 17 00:00:00 2001 > From: yfu <yanikfuchs@me.com> > Date: Tue, 22 Oct 2024 18:23:18 +0200 > Subject: [PATCH] usb: typec: tcpm: Prevent Hard_Reset if Vbus was never low > > Before this patch, tcpm went into SOFT_RESET state, if Vbus was never low > resulting in Hard_Reset, if power supply does not support USB_PD Soft_Reset. > > In order to prevent this, I remove the Vbus check completely, so that > we go as well into the SNK_WAIT_CAPABILITIES_TIMEOUT state. There, we send > PD_CTRL_GET_SOURCE_CAP which many power supply do support. > (122968f8dda8 usb: typec: tcpm: avoid resets for missing source capability messages) > > Additionally, I added SOFT_RESET (instead of Hard_Reset) as Fallback solution > if we still not have gotten any capabilities. Hard_Reset is now only done, > if PD_CTRL_GET_SOURCE_CAP and SOFT_RESET fail to get capabilities. > --- ... here after those three lines. The proper format, and the whole development process is documented here: https://docs.kernel.org/process/development-process.html You have also not signed your patch with a Signed-off-by tag. The importance of the signature in patches is explained in the fifth section of the development process documentation, here: https://docs.kernel.org/process/5.Posting.html > drivers/usb/typec/tcpm/tcpm.c | 10 ++-------- > 1 file changed, 2 insertions(+), 8 deletions(-) > > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c > index fc619478200f..c7a59c9f78ee 100644 > --- a/drivers/usb/typec/tcpm/tcpm.c > +++ b/drivers/usb/typec/tcpm/tcpm.c > @@ -5038,14 +5038,8 @@ static void run_state_machine(struct tcpm_port *port) > * were already in a stable contract before this boot. > * Do this only once. > */ > - if (port->vbus_never_low) { > - port->vbus_never_low = false; > - tcpm_set_state(port, SNK_SOFT_RESET, > + tcpm_set_state(port, SNK_WAIT_CAPABILITIES_TIMEOUT, > PD_T_SINK_WAIT_CAP); Here you should fix the alignment of the code so it matches the parentheses. You can use the scripts/checkpatch.pl script, which is part of the kernel source, to detect this kind of issues in your code by supplying your patch to it. > - } else { > - tcpm_set_state(port, SNK_WAIT_CAPABILITIES_TIMEOUT, > - PD_T_SINK_WAIT_CAP); > - } > break; > case SNK_WAIT_CAPABILITIES_TIMEOUT: > /* > @@ -5064,7 +5058,7 @@ static void run_state_machine(struct tcpm_port *port) > * according to the specification. > */ > if (tcpm_pd_send_control(port, PD_CTRL_GET_SOURCE_CAP, TCPC_TX_SOP)) > - tcpm_set_state_cond(port, hard_reset_state(port), 0); > + tcpm_set_state_cond(port, SNK_SOFT_RESET, 0); > else > tcpm_set_state(port, hard_reset_state(port), PD_T_SINK_WAIT_CAP); > break; > -- > 2.34.1 Otherwise the code looks very good to me, but I can't yet say if the change is appropriate. Let's fix the patch format first. Br, -- heikki ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] usb: typec: tcpm: Prevent Hard_Reset if Vbus was never low 2024-10-22 17:28 [PATCH] usb: typec: tcpm: Prevent Hard_Reset if Vbus was never low Yanik Fuchs 2024-10-23 9:05 ` Heikki Krogerus @ 2024-10-24 3:13 ` Amit Sunil Dhamne 2024-10-24 8:54 ` AW: " Yanik Fuchs 1 sibling, 1 reply; 5+ messages in thread From: Amit Sunil Dhamne @ 2024-10-24 3:13 UTC (permalink / raw) To: Yanik Fuchs, heikki.krogerus, gregkh, rdbabiera, linux, kyletso, badhri, xu.yang_2, sebastian.reichel Cc: linux-usb, linux-kernel Hi Yanik, On 10/22/24 10:28 AM, Yanik Fuchs wrote: > Good Evening > > Here is a Patch to resolve an issue with TCPM if Vbus was never low. > Please consider that this is one of my first kernel pull requests, I may have missunderstood the process. > > Freundliche Grüsse > Best regards > > > Yanik Fuchs > > --- > > From 604b97b6394b5643394bc63d4ac691c098c99c40 Mon Sep 17 00:00:00 2001 > From: yfu <yanikfuchs@me.com> > Date: Tue, 22 Oct 2024 18:23:18 +0200 > Subject: [PATCH] usb: typec: tcpm: Prevent Hard_Reset if Vbus was never low > > Before this patch, tcpm went into SOFT_RESET state, if Vbus was never low > resulting in Hard_Reset, if power supply does not support USB_PD Soft_Reset. > > In order to prevent this, I remove the Vbus check completely, so that > we go as well into the SNK_WAIT_CAPABILITIES_TIMEOUT state. There, we send > PD_CTRL_GET_SOURCE_CAP which many power supply do support. > (122968f8dda8 usb: typec: tcpm: avoid resets for missing source capability messages) Please refer to https://lore.kernel.org/all/20241024022233.3276995-1-amitsd@google.com/ as 122968f8dda8 is causing USB Type-C PD compliance failures. > > Additionally, I added SOFT_RESET (instead of Hard_Reset) as Fallback solution > if we still not have gotten any capabilities. Hard_Reset is now only done, > if PD_CTRL_GET_SOURCE_CAP and SOFT_RESET fail to get capabilities. > --- > drivers/usb/typec/tcpm/tcpm.c | 10 ++-------- > 1 file changed, 2 insertions(+), 8 deletions(-) > > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c > index fc619478200f..c7a59c9f78ee 100644 > --- a/drivers/usb/typec/tcpm/tcpm.c > +++ b/drivers/usb/typec/tcpm/tcpm.c > @@ -5038,14 +5038,8 @@ static void run_state_machine(struct tcpm_port *port) > * were already in a stable contract before this boot. > * Do this only once. > */ > - if (port->vbus_never_low) { > - port->vbus_never_low = false; > - tcpm_set_state(port, SNK_SOFT_RESET, > + tcpm_set_state(port, SNK_WAIT_CAPABILITIES_TIMEOUT, > PD_T_SINK_WAIT_CAP); > - } else { > - tcpm_set_state(port, SNK_WAIT_CAPABILITIES_TIMEOUT, > - PD_T_SINK_WAIT_CAP); > - } Instead of deleting code, please restrict this behavior to non self powered battery case as this most likely break compliance and may break actual use-cases for other users as a result. If you want you can move stuff around after https://lore.kernel.org/all/20241024022233.3276995-1-amitsd@google.com/ gets accepted in the following way: ``` if (!port->self_powered) { tcpm_set_state(port, SNK_WAIT_CAPABILITIES_TIMEOUT, PD_T_SINK_WAIT_CAP); break; } if (port->vbus_never_low) { tcpm_set_state(port, SNK_SOFT_RESET, PD_T_SINK_WAIT_CAP); } else { tcpm_set_state(port, hard_reset_state(..), PD_T_SINK_WAIT_CAP); } ``` This way you don't have to execute the SNK_SOFT_RESET flow for non self powered use-case. Thanks, Amit > break; > case SNK_WAIT_CAPABILITIES_TIMEOUT: > /* > @@ -5064,7 +5058,7 @@ static void run_state_machine(struct tcpm_port *port) > * according to the specification. > */ > if (tcpm_pd_send_control(port, PD_CTRL_GET_SOURCE_CAP, TCPC_TX_SOP)) > - tcpm_set_state_cond(port, hard_reset_state(port), 0); > + tcpm_set_state_cond(port, SNK_SOFT_RESET, 0); > else > tcpm_set_state(port, hard_reset_state(port), PD_T_SINK_WAIT_CAP); > break; ^ permalink raw reply [flat|nested] 5+ messages in thread
* AW: [PATCH] usb: typec: tcpm: Prevent Hard_Reset if Vbus was never low 2024-10-24 3:13 ` Amit Sunil Dhamne @ 2024-10-24 8:54 ` Yanik Fuchs 2024-10-24 9:20 ` Yanik Fuchs 0 siblings, 1 reply; 5+ messages in thread From: Yanik Fuchs @ 2024-10-24 8:54 UTC (permalink / raw) To: Amit Sunil Dhamne, heikki.krogerus, gregkh, rdbabiera, linux, kyletso, badhri, xu.yang_2, sebastian.reichel Cc: linux-usb, linux-kernel Hi Amit If the setting "bus-powered" is not set, there is only one way, that Vbus does not go down in init (where we force a Hard_Reset, which sets CC to Open if not bus-powerded). The case where Vbus never goes down and setting Bus-powered is not set is only with a legacy cable/adabter. In that case, we do not have PD compatibility anyway, and have to end up in SNK_READY eventually. So we do the SOFT_RESET if Vbus was never down out of historical reasons and is not needed anymore since tcpm is sending Get_Capabilities (the only reason we send SOFT_RESET if Vbus was never down, is to get capabilities of Source, and that we still do, if Get_Capabilities does not work). As you correctly have pointed out, I have to fix a issue I spotted yesterday as well, that with this Patch, after two hard_resets and then after 3 SoftReset we seem not to end up in SNK_READY, but in SNK_UNNCONECTED -> I have to find a way to check if we try hard reset in SOFT_RESET state after leaving the WAIT_SINK_CAPABILITIES_TIMOUT and in that case send it to SNK_CONNECTED (same as if we would be in state WAIT_SINK_CAPABILITIES or WAIT_SINK_CAPABILITIES_TIMOUT). Maybe we have to count softresets after requesing capabilities and reset it as soon as we are in SNK_READY BTW: I have a patch which I use in the 5.15 version of the driver, where I can get ptn5110 to init correctly without forcing the Hardreset and the manual CC_Change trigger (I changed default state of sink to SNK_DEBOUNCED (where Vbus is checked and if not true sends us to SNK_UNCONNECTED) and added a delay of one second to be shure that IRQ is running when state machine starts) Freundliche Grüsse Best regards Freundliche Grüsse Best regards Freundliche Grüsse Best regards ________________________________________ Von: Amit Sunil Dhamne <amitsd@google.com> Gesendet: Donnerstag, 24. Oktober 2024 05:13 An: Yanik Fuchs <Yanik.fuchs@mbv.ch>; heikki.krogerus@linux.intel.com <heikki.krogerus@linux.intel.com>; gregkh@linuxfoundation.org <gregkh@linuxfoundation.org>; rdbabiera@google.com <rdbabiera@google.com>; linux@roeck-us.net <linux@roeck-us.net>; kyletso@google.com <kyletso@google.com>; badhri@google.com <badhri@google.com>; xu.yang_2@nxp.com <xu.yang_2@nxp.com>; sebastian.reichel@collabora.com <sebastian.reichel@collabora.com> Cc: linux-usb@vger.kernel.org <linux-usb@vger.kernel.org>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org> Betreff: Re: [PATCH] usb: typec: tcpm: Prevent Hard_Reset if Vbus was never low ACHTUNG: Dieses E-Mail stammt von einem externen Absender. Prüfe Anhänge oder Links sehr genau! Hi Yanik, On 10/22/24 10:28 AM, Yanik Fuchs wrote: > Good Evening > > Here is a Patch to resolve an issue with TCPM if Vbus was never low. > Please consider that this is one of my first kernel pull requests, I may have missunderstood the process. > > Freundliche Grüsse > Best regards > > > Yanik Fuchs > > --- > > From 604b97b6394b5643394bc63d4ac691c098c99c40 Mon Sep 17 00:00:00 2001 > From: yfu <yanikfuchs@me.com> > Date: Tue, 22 Oct 2024 18:23:18 +0200 > Subject: [PATCH] usb: typec: tcpm: Prevent Hard_Reset if Vbus was never low > > Before this patch, tcpm went into SOFT_RESET state, if Vbus was never low > resulting in Hard_Reset, if power supply does not support USB_PD Soft_Reset. > > In order to prevent this, I remove the Vbus check completely, so that > we go as well into the SNK_WAIT_CAPABILITIES_TIMEOUT state. There, we send > PD_CTRL_GET_SOURCE_CAP which many power supply do support. > (122968f8dda8 usb: typec: tcpm: avoid resets for missing source capability messages) Please refer to https://lore.kernel.org/all/20241024022233.3276995-1-amitsd@google.com/ as 122968f8dda8 is causing USB Type-C PD compliance failures. > > Additionally, I added SOFT_RESET (instead of Hard_Reset) as Fallback solution > if we still not have gotten any capabilities. Hard_Reset is now only done, > if PD_CTRL_GET_SOURCE_CAP and SOFT_RESET fail to get capabilities. > --- > drivers/usb/typec/tcpm/tcpm.c | 10 ++-------- > 1 file changed, 2 insertions(+), 8 deletions(-) > > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c > index fc619478200f..c7a59c9f78ee 100644 > --- a/drivers/usb/typec/tcpm/tcpm.c > +++ b/drivers/usb/typec/tcpm/tcpm.c > @@ -5038,14 +5038,8 @@ static void run_state_machine(struct tcpm_port *port) > * were already in a stable contract before this boot. > * Do this only once. > */ > - if (port->vbus_never_low) { > - port->vbus_never_low = false; > - tcpm_set_state(port, SNK_SOFT_RESET, > + tcpm_set_state(port, SNK_WAIT_CAPABILITIES_TIMEOUT, > PD_T_SINK_WAIT_CAP); > - } else { > - tcpm_set_state(port, SNK_WAIT_CAPABILITIES_TIMEOUT, > - PD_T_SINK_WAIT_CAP); > - } Instead of deleting code, please restrict this behavior to non self powered battery case as this most likely break compliance and may break actual use-cases for other users as a result. If you want you can move stuff around after https://lore.kernel.org/all/20241024022233.3276995-1-amitsd@google.com/ gets accepted in the following way: ``` if (!port->self_powered) { tcpm_set_state(port, SNK_WAIT_CAPABILITIES_TIMEOUT, PD_T_SINK_WAIT_CAP); break; } if (port->vbus_never_low) { tcpm_set_state(port, SNK_SOFT_RESET, PD_T_SINK_WAIT_CAP); } else { tcpm_set_state(port, hard_reset_state(..), PD_T_SINK_WAIT_CAP); } ``` This way you don't have to execute the SNK_SOFT_RESET flow for non self powered use-case. Thanks, Amit > break; > case SNK_WAIT_CAPABILITIES_TIMEOUT: > /* > @@ -5064,7 +5058,7 @@ static void run_state_machine(struct tcpm_port *port) > * according to the specification. > */ > if (tcpm_pd_send_control(port, PD_CTRL_GET_SOURCE_CAP, TCPC_TX_SOP)) > - tcpm_set_state_cond(port, hard_reset_state(port), 0); > + tcpm_set_state_cond(port, SNK_SOFT_RESET, 0); > else > tcpm_set_state(port, hard_reset_state(port), PD_T_SINK_WAIT_CAP); > break; ^ permalink raw reply [flat|nested] 5+ messages in thread
* AW: [PATCH] usb: typec: tcpm: Prevent Hard_Reset if Vbus was never low 2024-10-24 8:54 ` AW: " Yanik Fuchs @ 2024-10-24 9:20 ` Yanik Fuchs 0 siblings, 0 replies; 5+ messages in thread From: Yanik Fuchs @ 2024-10-24 9:20 UTC (permalink / raw) To: Amit Sunil Dhamne, heikki.krogerus, gregkh, rdbabiera, linux, kyletso, badhri, xu.yang_2, sebastian.reichel Cc: linux-usb, linux-kernel Hey all Does anybody see a problem, if we in the function hard_reset_state() not check for the state it was send, but check if Vbus is present. If Vbus, we go to SNK_READY and if no Vbus we go to SNK_UNCONNECTED? Freundliche Grüsse Best regards ________________________________________ Von: Yanik Fuchs <Yanik.fuchs@mbv.ch> Gesendet: Donnerstag, 24. Oktober 2024 10:54 An: Amit Sunil Dhamne <amitsd@google.com>; heikki.krogerus@linux.intel.com <heikki.krogerus@linux.intel.com>; gregkh@linuxfoundation.org <gregkh@linuxfoundation.org>; rdbabiera@google.com <rdbabiera@google.com>; linux@roeck-us.net <linux@roeck-us.net>; kyletso@google.com <kyletso@google.com>; badhri@google.com <badhri@google.com>; xu.yang_2@nxp.com <xu.yang_2@nxp.com>; sebastian.reichel@collabora.com <sebastian.reichel@collabora.com> Cc: linux-usb@vger.kernel.org <linux-usb@vger.kernel.org>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org> Betreff: AW: [PATCH] usb: typec: tcpm: Prevent Hard_Reset if Vbus was never low Hi Amit If the setting "bus-powered" is not set, there is only one way, that Vbus does not go down in init (where we force a Hard_Reset, which sets CC to Open if not bus-powerded). The case where Vbus never goes down and setting Bus-powered is not set is only with a legacy cable/adabter. In that case, we do not have PD compatibility anyway, and have to end up in SNK_READY eventually. So we do the SOFT_RESET if Vbus was never down out of historical reasons and is not needed anymore since tcpm is sending Get_Capabilities (the only reason we send SOFT_RESET if Vbus was never down, is to get capabilities of Source, and that we still do, if Get_Capabilities does not work). As you correctly have pointed out, I have to fix a issue I spotted yesterday as well, that with this Patch, after two hard_resets and then after 3 SoftReset we seem not to end up in SNK_READY, but in SNK_UNNCONECTED -> I have to find a way to check if we try hard reset in SOFT_RESET state after leaving the WAIT_SINK_CAPABILITIES_TIMOUT and in that case send it to SNK_CONNECTED (same as if we would be in state WAIT_SINK_CAPABILITIES or WAIT_SINK_CAPABILITIES_TIMOUT). Maybe we have to count softresets after requesing capabilities and reset it as soon as we are in SNK_READY BTW: I have a patch which I use in the 5.15 version of the driver, where I can get ptn5110 to init correctly without forcing the Hardreset and the manual CC_Change trigger (I changed default state of sink to SNK_DEBOUNCED (where Vbus is checked and if not true sends us to SNK_UNCONNECTED) and added a delay of one second to be shure that IRQ is running when state machine starts) Freundliche Grüsse Best regards Freundliche Grüsse Best regards Freundliche Grüsse Best regards ________________________________________ Von: Amit Sunil Dhamne <amitsd@google.com> Gesendet: Donnerstag, 24. Oktober 2024 05:13 An: Yanik Fuchs <Yanik.fuchs@mbv.ch>; heikki.krogerus@linux.intel.com <heikki.krogerus@linux.intel.com>; gregkh@linuxfoundation.org <gregkh@linuxfoundation.org>; rdbabiera@google.com <rdbabiera@google.com>; linux@roeck-us.net <linux@roeck-us.net>; kyletso@google.com <kyletso@google.com>; badhri@google.com <badhri@google.com>; xu.yang_2@nxp.com <xu.yang_2@nxp.com>; sebastian.reichel@collabora.com <sebastian.reichel@collabora.com> Cc: linux-usb@vger.kernel.org <linux-usb@vger.kernel.org>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org> Betreff: Re: [PATCH] usb: typec: tcpm: Prevent Hard_Reset if Vbus was never low ACHTUNG: Dieses E-Mail stammt von einem externen Absender. Prüfe Anhänge oder Links sehr genau! Hi Yanik, On 10/22/24 10:28 AM, Yanik Fuchs wrote: > Good Evening > > Here is a Patch to resolve an issue with TCPM if Vbus was never low. > Please consider that this is one of my first kernel pull requests, I may have missunderstood the process. > > Freundliche Grüsse > Best regards > > > Yanik Fuchs > > --- > > From 604b97b6394b5643394bc63d4ac691c098c99c40 Mon Sep 17 00:00:00 2001 > From: yfu <yanikfuchs@me.com> > Date: Tue, 22 Oct 2024 18:23:18 +0200 > Subject: [PATCH] usb: typec: tcpm: Prevent Hard_Reset if Vbus was never low > > Before this patch, tcpm went into SOFT_RESET state, if Vbus was never low > resulting in Hard_Reset, if power supply does not support USB_PD Soft_Reset. > > In order to prevent this, I remove the Vbus check completely, so that > we go as well into the SNK_WAIT_CAPABILITIES_TIMEOUT state. There, we send > PD_CTRL_GET_SOURCE_CAP which many power supply do support. > (122968f8dda8 usb: typec: tcpm: avoid resets for missing source capability messages) Please refer to https://lore.kernel.org/all/20241024022233.3276995-1-amitsd@google.com/ as 122968f8dda8 is causing USB Type-C PD compliance failures. > > Additionally, I added SOFT_RESET (instead of Hard_Reset) as Fallback solution > if we still not have gotten any capabilities. Hard_Reset is now only done, > if PD_CTRL_GET_SOURCE_CAP and SOFT_RESET fail to get capabilities. > --- > drivers/usb/typec/tcpm/tcpm.c | 10 ++-------- > 1 file changed, 2 insertions(+), 8 deletions(-) > > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c > index fc619478200f..c7a59c9f78ee 100644 > --- a/drivers/usb/typec/tcpm/tcpm.c > +++ b/drivers/usb/typec/tcpm/tcpm.c > @@ -5038,14 +5038,8 @@ static void run_state_machine(struct tcpm_port *port) > * were already in a stable contract before this boot. > * Do this only once. > */ > - if (port->vbus_never_low) { > - port->vbus_never_low = false; > - tcpm_set_state(port, SNK_SOFT_RESET, > + tcpm_set_state(port, SNK_WAIT_CAPABILITIES_TIMEOUT, > PD_T_SINK_WAIT_CAP); > - } else { > - tcpm_set_state(port, SNK_WAIT_CAPABILITIES_TIMEOUT, > - PD_T_SINK_WAIT_CAP); > - } Instead of deleting code, please restrict this behavior to non self powered battery case as this most likely break compliance and may break actual use-cases for other users as a result. If you want you can move stuff around after https://lore.kernel.org/all/20241024022233.3276995-1-amitsd@google.com/ gets accepted in the following way: ``` if (!port->self_powered) { tcpm_set_state(port, SNK_WAIT_CAPABILITIES_TIMEOUT, PD_T_SINK_WAIT_CAP); break; } if (port->vbus_never_low) { tcpm_set_state(port, SNK_SOFT_RESET, PD_T_SINK_WAIT_CAP); } else { tcpm_set_state(port, hard_reset_state(..), PD_T_SINK_WAIT_CAP); } ``` This way you don't have to execute the SNK_SOFT_RESET flow for non self powered use-case. Thanks, Amit > break; > case SNK_WAIT_CAPABILITIES_TIMEOUT: > /* > @@ -5064,7 +5058,7 @@ static void run_state_machine(struct tcpm_port *port) > * according to the specification. > */ > if (tcpm_pd_send_control(port, PD_CTRL_GET_SOURCE_CAP, TCPC_TX_SOP)) > - tcpm_set_state_cond(port, hard_reset_state(port), 0); > + tcpm_set_state_cond(port, SNK_SOFT_RESET, 0); > else > tcpm_set_state(port, hard_reset_state(port), PD_T_SINK_WAIT_CAP); > break; ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-10-24 9:20 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-10-22 17:28 [PATCH] usb: typec: tcpm: Prevent Hard_Reset if Vbus was never low Yanik Fuchs 2024-10-23 9:05 ` Heikki Krogerus 2024-10-24 3:13 ` Amit Sunil Dhamne 2024-10-24 8:54 ` AW: " Yanik Fuchs 2024-10-24 9:20 ` Yanik Fuchs
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®