linux-kernel - Re: [PATCH] usb: typec: ucsi: Wait 20ms before retrying reset

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAB2FV=4Z1W1HSba50KaB3rR4=Ussb5RWPwUArr0_=3pFwxpAhA@mail.gmail.com>
Date: Tue, 26 Mar 2024 16:34:44 -0700
From: Pavan Holla <pholla@...omium.org>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>, 
	Heikki Krogerus <heikki.krogerus@...ux.intel.com>
Cc: linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Abhishek Pandit-Subedi <abhishekpandit@...omium.org>
Subject: Re: [PATCH] usb: typec: ucsi: Wait 20ms before retrying reset

On Tue, Mar 26, 2024 at 1:29 AM Greg Kroah-Hartman
<gregkh@...uxfoundation.org> wrote:
>
> On Mon, Mar 25, 2024 at 09:19:43PM +0000, Pavan Holla wrote:
> > The PPM might take time to process reset. Allow 20ms for the reset to
> > complete before issuing another reset.
> What commit id does this fix?  Does it need to go to older kernels?

This does not fix any commit. However, the time taken by a CCI read is
insufficient for a ChromeOS EC and PDC to perform a reset.

> > There is a 20ms delay for a reset retry to complete. However, the first
> > reset attempt is expected to complete immediately after an async write
> > of the reset command. This patch adds 20ms between the async write and
> > the CCI read that expects the reset to be complete. The additional delay
> > also allows the PPM to settle after the first reset, which seems to be
> > the intention behind the original 20ms delay ( kernel v4.14 has a comment
> > regarding the same )
>
> Why was the comment removed in newer kernels?

The comment was removed when the old UCSI API was removed in
2ede55468ca8cc236da66579359c2c406d4c1cba

> Where does the magic 20ms number come from?  What about systems that do
> not need that time delay, did things just slow down for them?

I am not sure how 20ms was decided upon. However, UCSI v1.2 has
MIN_TIME_TO_RESPOND_WITH_BUSY=10ms. So, we need to provide at least
10ms for the PPM to respond with CCI busy. Indeed, this patch slows down other
implementations by 20ms. UCSIv3 also defines a 200ms timeout for PPM_RESET.

Hi Heikki,

Do you think the right thing to do here is:
1) poll CCI ( poll duration TBD) for 20ms until busy is set or reset
is complete.
2) poll CCI ( poll duration TBD) for 180ms until reset is complete if
busy was set.
3) If either 1) or 2) timeout, retry the reset.

If you agree with the approach, please advise a poll duration as well ( 20ms? )

Thanks,
Pavan