Re: [PATCH 3/3] usb: dwc3: add a quirk for device disconnection issue in Synopsis dwc3 core

[Felipe Balbi <balbi@ti.com>]

[-- Attachment #1: Type: text/plain, Size: 3537 bytes --]

Hi,

On Sun, Jan 11, 2015 at 11:19:55PM +0800, Sneeker Yeh wrote:
> > > > enable the quirk only for you. Isn't there a better way of enabling the
> > > > quirk based off of revision detection couple with a look on GHWPARAMS*
> > > > registers ?
> > > >
> > > > What's tricking me is this claim that only config-free PHYs would be
> > > > affected, why ?
> > > >
> > >
> > > i'm still struggling now to try to get more information about this.
> > > some security policy inside Fujitsu make me unable to access full
> > > information of this errata today.
> > >
> > > Someday after i get enough information,
> > > i shall take your suggestion here that seems better to incur quirk
> > > dynamically via GHWPARAMS,
> > > and then send it here again.
> >
> > ok, hopefully you'll find a way ;-)
> >
> > I got some update information here finally~
> in case i express unclearly i also put a pdf:
> https://drive.google.com/file/d/0B18MmcvvKjNNbDF6eEdHSzZCazA/view
> 
> This issue is defined by a two-way race at disconnect, between
> 1) class driver interrupt endpoint resheduling attempts if the ISR gave an
> ep error event due to device detach (it would try 3 times)
> 2) Disconnect interrupt on PORTSC_CSC, which is cleared by hub thread
> asynchronously
> 3) The hardware IP was configured in silicon with
>        - DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1 (this is an IP configuration

yeah, aparently this is another configuration which is not exposed on
HWPARAMS registers. Paul, can you confirm for us ? I couldn't find it on
Databook on any of the HWPARAMS registers.

> port whose state cannot be checked from software)
>        - Synopsys IP version is < 3.00a

heh, so pretty much everybody :-)

>        The IP will auto-suspend itself on device detach with some
> phy-specific interval after CSC is cleared by 2)
> 
> If 2) and 3) complete before 1), the interrupts it expects will not be
> generated by the autosuspended IP, leading to a deadlock. Even later
> disconnection procedure would detect that corresponding urb is still
> in-progress and issue a ep stop command, auto-suspended IP still won't
> respond to that command.
> 
> this defect would result in this when device detached:
> -------------------
> [   99.603544] usb 4-1: USB disconnect, device number 2
> [  104.615254] xhci-hcd xhci-hcd.0.auto: xHCI host not responding to stop
> endpoint command.
> [  104.623362] xhci-hcd xhci-hcd.0.auto: Assuming host is dying, halting
> host.
> [  104.653261] xhci-hcd xhci-hcd.0.auto: Host not halted after 16000
> microseconds.
> [  104.660584] xhci-hcd xhci-hcd.0.auto: Non-responsive xHCI host is not
> halting.
> [  104.667817] xhci-hcd xhci-hcd.0.auto: Completing active URBs anyway.
> [  104.674198] xhci-hcd xhci-hcd.0.auto: HC died; cleaning up
> --------------------
> As a result, when device detached, we desired to postpone "PORTCSC clear"
> behind "disable slot". it's found that all executed ep command related to
> disconnetion, are executed before "disable slot".

Now this is all great information and they should all be part of your
commit log and probably a big comment should be added to code as well.

Thanks for going after all these details, now let's figure out a way to
pass dwc3 revision to xhci, or maybe we pass just a flag for the quirk,
something like:

	if (dwc->revision < 3.00a && dwc->has_suspend_on_disconnect)
		xhci_pdata.delay_portcsc_clear = true;

or something similar to that.

cheers

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]