Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751321AbdH0RVa (ORCPT ); Sun, 27 Aug 2017 13:21:30 -0400 Received: from bh-25.webhostbox.net ([208.91.199.152]:46347 "EHLO bh-25.webhostbox.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751182AbdH0RV3 (ORCPT ); Sun, 27 Aug 2017 13:21:29 -0400 Date: Sun, 27 Aug 2017 10:21:27 -0700 From: Guenter Roeck To: Badhri Jagan Sridharan Cc: Greg Kroah-Hartman , devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 3/6] staging: usb: tcpm: usb: typec: tcpm: Prevent TCPM from looping in SRC_TRYWAIT Message-ID: <20170827172127.GM22819@roeck-us.net> References: <20170827052431.12064-1-Badhri@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170827052431.12064-1-Badhri@google.com> User-Agent: Mutt/1.5.24 (2015-08-30) X-Authenticated_sender: guenter@roeck-us.net X-OutGoing-Spam-Status: No, score=-1.0 X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - bh-25.webhostbox.net X-AntiAbuse: Original Domain - vger.kernel.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - roeck-us.net X-Get-Message-Sender-Via: bh-25.webhostbox.net: authenticated_id: guenter@roeck-us.net X-Authenticated-Sender: bh-25.webhostbox.net: guenter@roeck-us.net X-Source: X-Source-Args: X-Source-Dir: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6336 Lines: 159 On Sat, Aug 26, 2017 at 10:24:31PM -0700, Badhri Jagan Sridharan wrote: > According to the spec the following is the condition > for exiting TryWait.SRC: > > "The port shall transition to Attached.SRC when V BUS is at vSafe0V > and the SRC.Rd state is detected on exactly one of the CC pins for at > least tCCDebounce. The port shall transition to Unattached.SNK after > tDRPTry if neither of the CC1 or CC2 pins are in the SRC.Rd state" > > TCPM at present keeps re-entering the SRC_TRYWAIT and keeps restarting > tDRPTry if the CC presents Rp and disconnects within tCCDebounce. > > For example: > [ 447.164308] pending state change SRC_TRYWAIT -> SRC_ATTACHED @ 200 ms > [ 447.164386] CC1: 2 -> 0, CC2: 0 -> 0 [state SRC_TRYWAIT, polarity 0, disconnected] > [ 447.164406] state change SRC_TRYWAIT -> SRC_TRYWAIT > [ 447.164573] cc:=3 > [ 447.191408] pending state change SRC_TRYWAIT -> SRC_TRYWAIT_UNATTACHED @ 100 ms > [ 447.191478] CC1: 0 -> 0, CC2: 0 -> 0 [state SRC_TRYWAIT, polarity 0, disconnected] > [ 447.207261] CC1: 0 -> 2, CC2: 0 -> 0 [state SRC_TRYWAIT, polarity 0, connected] > [ 447.207306] state change SRC_TRYWAIT -> SRC_TRYWAIT > [ 447.207485] cc:=3 > [ 447.237283] pending state change SRC_TRYWAIT -> SRC_ATTACHED @ 200 ms > [ 447.237357] CC1: 2 -> 0, CC2: 0 -> 0 [state SRC_TRYWAIT, polarity 0, disconnected] > [ 447.237379] state change SRC_TRYWAIT -> SRC_TRYWAIT > [ 447.237532] cc:=3 > [ 447.263219] pending state change SRC_TRYWAIT -> SRC_TRYWAIT_UNATTACHED @ 100 ms > [ 447.263289] CC1: 0 -> 0, CC2: 0 -> 0 [state SRC_TRYWAIT, polarity 0, disconnected] > [ 447.280926] CC1: 0 -> 2, CC2: 0 -> 0 [state SRC_TRYWAIT, polarity 0, connected] > [ 447.280970] state change SRC_TRYWAIT -> SRC_TRYWAIT > [ 447.281158] cc:=3 > [ 447.307767] pending state change SRC_TRYWAIT -> SRC_ATTACHED @ 200 ms > [ 447.307838] CC1: 2 -> 0, CC2: 0 -> 0 [state SRC_TRYWAIT, polarity 0, disconnected] > [ 447.307858] state change SRC_TRYWAIT -> SRC_TRYWAIT > > In TCPM, tDRPTry is set tp 100ms (min 75ms and max 150ms) > and tCCdebounce is set to 200ms (min 100ms and max 200ms). > To overcome the issue, record the time at which the port > enters TryWait.SRC(SRC_TRYWAIT) and re-enter SRC_TRYWAIT > only when CC keeps debouncing within tDRPTry. > > Signed-off-by: Badhri Jagan Sridharan Reviewed-by: Guenter Roeck > --- > drivers/staging/typec/tcpm.c | 45 ++++++++++++++++++++++++++++---------------- > 1 file changed, 29 insertions(+), 16 deletions(-) > > diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c > index 1219e3bc13ef..d45ffa8f2cfd 100644 > --- a/drivers/staging/typec/tcpm.c > +++ b/drivers/staging/typec/tcpm.c > @@ -17,6 +17,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -105,6 +106,7 @@ > S(SNK_TRY), \ > S(SNK_TRY_WAIT), \ > S(SRC_TRYWAIT), \ > + S(SRC_TRYWAIT_DEBOUNCE), \ > S(SRC_TRYWAIT_UNATTACHED), \ > \ > S(SRC_TRY), \ > @@ -284,6 +286,9 @@ struct tcpm_port { > struct typec_altmode *partner_altmode[SVID_DISCOVERY_MAX]; > struct typec_altmode *port_altmode[SVID_DISCOVERY_MAX]; > > + /* Deadline in jiffies to exit src_try_wait state */ > + unsigned long max_wait; > + > #ifdef CONFIG_DEBUG_FS > struct dentry *dentry; > struct mutex logbuffer_lock; /* log buffer access lock */ > @@ -2204,6 +2209,7 @@ static void run_state_machine(struct tcpm_port *port) > if (!tcpm_port_is_sink(port)) { > tcpm_set_state(port, SRC_TRYWAIT, > PD_T_PD_DEBOUNCE); > + port->max_wait = 0; > break; > } > /* No vbus, cc state is sink or open */ > @@ -2211,11 +2217,22 @@ static void run_state_machine(struct tcpm_port *port) > break; > case SRC_TRYWAIT: > tcpm_set_cc(port, tcpm_rp_cc(port)); > - if (!port->vbus_present && tcpm_port_is_source(port)) > - tcpm_set_state(port, SRC_ATTACHED, PD_T_CC_DEBOUNCE); > - else > + if (port->max_wait == 0) { > + port->max_wait = jiffies + > + msecs_to_jiffies(PD_T_DRP_TRY); > tcpm_set_state(port, SRC_TRYWAIT_UNATTACHED, > PD_T_DRP_TRY); > + } else { > + if (time_is_after_jiffies(port->max_wait)) > + tcpm_set_state(port, SRC_TRYWAIT_UNATTACHED, > + jiffies_to_msecs(port->max_wait - > + jiffies)); > + else > + tcpm_set_state(port, SNK_UNATTACHED, 0); > + } > + break; > + case SRC_TRYWAIT_DEBOUNCE: > + tcpm_set_state(port, SRC_ATTACHED, PD_T_CC_DEBOUNCE); > break; > case SRC_TRYWAIT_UNATTACHED: > tcpm_set_state(port, SNK_UNATTACHED, 0); > @@ -2898,11 +2915,10 @@ static void _tcpm_cc_change(struct tcpm_port *port, enum typec_cc_status cc1, > case SRC_TRYWAIT: > /* Hand over to state machine if needed */ > if (!port->vbus_present && tcpm_port_is_source(port)) > - new_state = SRC_ATTACHED; > - else > - new_state = SRC_TRYWAIT_UNATTACHED; > - > - if (new_state != port->delayed_state) > + tcpm_set_state(port, SRC_TRYWAIT_DEBOUNCE, 0); > + break; > + case SRC_TRYWAIT_DEBOUNCE: > + if (port->vbus_present || !tcpm_port_is_source(port)) > tcpm_set_state(port, SRC_TRYWAIT, 0); > break; > case SNK_TRY_WAIT: > @@ -2990,9 +3006,10 @@ static void _tcpm_pd_vbus_on(struct tcpm_port *port) > /* Do nothing, waiting for timeout */ > break; > case SRC_TRYWAIT: > - /* Hand over to state machine if needed */ > - if (port->delayed_state != SRC_TRYWAIT_UNATTACHED) > - tcpm_set_state(port, SRC_TRYWAIT, 0); > + /* Do nothing, Waiting for Rd to be detected */ > + break; > + case SRC_TRYWAIT_DEBOUNCE: > + tcpm_set_state(port, SRC_TRYWAIT, 0); > break; > case SNK_TRY_WAIT: > if (tcpm_port_is_sink(port)) { > @@ -3039,11 +3056,7 @@ static void _tcpm_pd_vbus_off(struct tcpm_port *port) > case SRC_TRYWAIT: > /* Hand over to state machine if needed */ > if (tcpm_port_is_source(port)) > - new_state = SRC_ATTACHED; > - else > - new_state = SRC_TRYWAIT_UNATTACHED; > - if (new_state != port->delayed_state) > - tcpm_set_state(port, SRC_TRYWAIT, 0); > + tcpm_set_state(port, SRC_TRYWAIT_DEBOUNCE, 0); > break; > case SNK_TRY_WAIT: > if (!tcpm_port_is_sink(port)) > -- > 2.14.1.342.g6490525c54-goog >