From: Guenter Roeck <linux@roeck-us.net>
To: ChiYuan Huang <u0084500@gmail.com>
Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>,
Greg KH <gregkh@linuxfoundation.org>,
linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
cy_huang <cy_huang@richtek.com>
Subject: Re: [PATCH] usb: typec: tcpm: Fix if vbus before cc, hard_reset_count not reset issue
Date: Sat, 5 Sep 2020 08:51:18 -0700 [thread overview]
Message-ID: <63c7f5e4-eff2-1420-30a5-a0b98a7815e0@roeck-us.net> (raw)
In-Reply-To: <CADiBU3_vYAmHDCONrExzyM+1CTfqJx_eS1hYG8aHkNWFzTcwfg@mail.gmail.com>
On 9/4/20 6:24 PM, ChiYuan Huang wrote:
> Guenter Roeck <linux@roeck-us.net> 於 2020年9月5日 週六 上午3:41寫道:
>>
>> On 9/3/20 9:21 AM, ChiYuan Huang wrote:
>>> Guenter Roeck <linux@roeck-us.net> 於 2020年9月3日 週四 上午12:57寫道:
>>>>
>>>> On Wed, Sep 02, 2020 at 11:35:33PM +0800, cy_huang wrote:
>>>>> From: ChiYuan Huang <cy_huang@richtek.com>
>>>>>
>>>>> Fix: If vbus event is before cc_event trigger, hard_reset_count
>>>>> won't bt reset for some case.
>>>>>
>>>>> Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
>>>>> ---
>>>>> Below's the flow.
>>>>>
>>>>> _tcpm_pd_vbus_off() -> run_state_machine to change state to SNK_UNATTACHED
>>>>> call tcpm_snk_detach() -> tcpm_snk_detach() -> tcpm_detach()
>>>>> tcpm_port_is_disconnected() will be called.
>>>>> But port->attached is still true and port->cc1=open and port->cc2=open
>>>>>
>>>>> It cause tcpm_port_is_disconnected return false, then hard_reset_count won't be reset.
>>>>> After that, tcpm_reset_port() is called.
>>>>> port->attached become false.
>>>>>
>>>>> After that, cc now trigger cc_change event, the hard_reset_count will be kept.
>>>>> Even tcpm_detach will be called, due to port->attached is false, tcpm_detach()
>>>>> will directly return.
>>>>>
>>>>> CC_EVENT will only trigger drp toggling again.
>>>>> ---
>>>>> drivers/usb/typec/tcpm/tcpm.c | 3 +--
>>>>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
>>>>> index a48e3f90..5c73e1d 100644
>>>>> --- a/drivers/usb/typec/tcpm/tcpm.c
>>>>> +++ b/drivers/usb/typec/tcpm/tcpm.c
>>>>> @@ -2797,8 +2797,7 @@ static void tcpm_detach(struct tcpm_port *port)
>>>>> port->tcpc->set_bist_data(port->tcpc, false);
>>>>> }
>>>>>
>>>>> - if (tcpm_port_is_disconnected(port))
>>>>> - port->hard_reset_count = 0;
>>>>> + port->hard_reset_count = 0;
>>>>>
>>>>
>>>> Doesn't that mean that the state machine will never enter
>>>> error recovery ?
>>>>
>>> I think it does't affect the error recovery.
>>> All error recovery seems to check pd_capable flag.
>>>
>>> >From my below case, it's A to C cable only. There is no USBPD contract
>>> will be estabilished.
>>>
>>> This case occurred following by the below test condition
>>> Cable -> A to C (default Rp bind to vbus) connected to PC.
>>> 1. first time plugged in the cable with PC
>>> It will make HARD_RESET_COUNT to be equal 2
>>> 2. And then plug out. At that time HARD_RESET_COUNT is till 2.
>>> 3. next time plugged in again.
>>> Due to hard_reset_count is still 2 , after wait_cap_timeout, the state
>>> eventually changed to SNK_READY.
>>> But during the state transition, no hard_reset be sent.
>>>
>>> Defined in the USBPD policy engine, typec transition to USBPD, all
>>> variables must be reset included hard_reset_count.
>>> So it expected SNK must send hard_reset again.
>>>
>>> The original code defined hard_reset_count must be reset only when
>>> tcpm_port_is_disconnected.
>>>
>>> It doesn't make sense that it only occurred in some scenario.
>>> If tcpm_detach is called, hard_reset count must be reset also.
>>>
>>
>> If a hard reset fails, the state machine may cycle through states
>> HARD_RESET_SEND, HARD_RESET_START, SRC_HARD_RESET_VBUS_OFF,
>> SRC_HARD_RESET_VBUS_ON back to SRC_UNATTACHED. In this state,
>> tcpm_src_detach() and with it tcpm_detach() is called. The hard
>> reset counter is incremented in HARD_RESET_SEND. If tcpm_detach()
>> resets the counter, the state machine will keep cycling through hard
>> resets without ever entering the error recovery state. I am not
>> entirely sure where the counter should be reset, but tcpm_detach()
>> seems to be the wrong place.
>
> This case you specified means locally error occurred.
It could be a local error (with the local hardware), or with the
remote partner not accepting the reset. We only know that an error
occurred.
> It intended to re-run the state machine from typec to USBPD.
>>From my understanding, hard_reset_count to be reset is reasonable.
>
> The normal stare from the state transition you specified is
> HARD_RESET_SEND, HARD_RESET_START -> SRC_HARD_RESET_VBUS_OFF,
> SRC_HARD_RESET_VBUS_ON -> received VBUS_EVENT then go to SRC_STARTUP.
>
The operational word is "normal". Error recovery is expected to handle
situations which are not normal.
I don't question the need to reset the counter. The only question
is where and when to reset it.
Guenter
next prev parent reply other threads:[~2020-09-05 15:52 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-02 15:35 [PATCH] usb: typec: tcpm: Fix if vbus before cc, hard_reset_count not reset issue cy_huang
2020-09-02 16:57 ` Guenter Roeck
2020-09-03 16:21 ` ChiYuan Huang
2020-09-04 19:41 ` Guenter Roeck
2020-09-05 1:24 ` ChiYuan Huang
2020-09-05 15:51 ` Guenter Roeck [this message]
2020-09-06 15:22 ` ChiYuan Huang
2020-09-15 3:07 ` ChiYuan Huang
2020-10-02 13:31 ` Greg KH
2020-10-02 14:26 ` Guenter Roeck
2020-10-05 11:08 ` Greg KH
2020-10-05 15:30 ` Guenter Roeck
2020-10-06 4:37 ` ChiYuan Huang
2020-10-06 16:52 ` Jun Li
2020-10-06 17:39 ` ChiYuan Huang
2020-10-07 10:13 ` ChiYuan Huang
2020-10-09 6:12 ` Jun Li
2020-10-09 16:06 ` ChiYuan Huang
2020-10-10 11:21 ` Jun Li
2020-10-10 19:31 ` Guenter Roeck
2020-10-12 6:22 ` ChiYuan Huang
2020-10-12 9:25 ` Jun Li
2020-10-12 8:58 ` Jun Li
2020-10-10 15:46 ` Guenter Roeck
2020-10-09 2:58 ` Jun Li
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=63c7f5e4-eff2-1420-30a5-a0b98a7815e0@roeck-us.net \
--to=linux@roeck-us.net \
--cc=cy_huang@richtek.com \
--cc=gregkh@linuxfoundation.org \
--cc=heikki.krogerus@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=u0084500@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).