[Cbe-oss-dev] [PATCH 1/5] usb: Fix PS3 EHCI suspend

[Alan Stern]

On Wed, 23 Nov 2011, Geoff Levand wrote:

> Hi Alan,
> 
> On Fri, 2011-11-18 at 11:19 -0500, Alan Stern wrote:
> > On Thu, 17 Nov 2011, Geoff Levand wrote:
> > > I don't think the frame index should stop incrementing after the second port is
> > > suspended, and I think the cause is the reason the ehci_quiesce handshake
> > > fails.
> > 
> > Yes indeed.  So it appears the root hub really does get stopped when 
> > all the ports are suspended.  Can you add a call to dbg_status() in 
> > _ps3_frame_index_check()?  I'd like to know if the STS_HALT bit is set.
> 
> Here's the output:
> 
> ps3-ehci-driver sb_06: port 1 SUSPEND
> ehci_hub_control:976: frame_index: ok
> ps3-ehci-driver sb_06: ps3_frame_index_check status 8009 Async FLR INT
> ehci_hub_control:1004: frame_index: ok
> ps3-ehci-driver sb_06: ps3_frame_index_check status 8009 Async FLR INT
> ps3-ehci-driver sb_06: port 2 SUSPEND
> ehci_hub_control:976: frame_index: ok
> ps3-ehci-driver sb_06: ps3_frame_index_check status 8008 Async FLR
> ....................
> ehci_hub_control:1004: frame_index: stopped
> ps3-ehci-driver sb_06: ps3_frame_index_check status 8008 Async FLR

Aha!  The controller stops the root hub but leaves the STS_HLT bit
clear, so the host computer will think the root hub is still running.  
How thoughtful of it...  :-(

> usb usb1: bus auto-suspend, wakeup 1
> ps3-ehci-driver sb_06: suspend root hub
> ps3-ehci-driver sb_06: force halt; handshake d000080080020014 0000c000 00000000 -> -110

And then we pay the price.  Since the root hub isn't running, it 
doesn't want to clear the STS_ASS bit.

> > > > Does the controller do this if only one port is suspended and the other 
> > > > is not enabled?  What if neither port is enabled?
> > > 
> > > I have not tested these.
> > 
> > It would be nice to know.  Is there any way you can test it?  If those
> > hubs are internal and permanently connected then I guess you can't.  Do 
> > the controllers really have only two ports?
> 
> They are internal, and only two ports.  The PS3 has an additional
> USB hub inside its case to provide 4 external ports.

So in theory you could leave the port that's not attached to the 
internal hub unconnected, and wait for the internal hub to be 
suspended.  At that point one port would be suspended and the other 
disabled.  Does the root hub get stopped then?

Actually, this doesn't really matter.  We'll have to deal with the
problem either way.  I'm just curious to know the answer.

Here's one possible approach.  In handshake_on_error_set_halt(), read
the frame counter before the call to handshake().  If handshake()  
returns an error then read the frame counter again.  If the value
hasn't changed, assume the root hub is stopped and return 0.  And of
course, add some comments explaining why the PS3 controller needs all
this.

Admittedly, this will make handshake_on_error_set_halt() inefficient on
the PS3 (it will have to wait for the full timeout period).  This may
be unavoidable, given that the driver has no good way to tell when the
root hub is stopped other than checking the frame counter.

> > > > >   kernel BUG at drivers/usb/host/ehci-mem.c:74!
> > > > 
> > > > This means some qh was still linked (or something similar) when it was 
> > > > removed.  I don't see how these are connected to the root hub stopping.
> > > 
> > > I think it is because when the HC frame index stops incrementing the qh
> > > processing does not finish up, so it is still linked.
> > 
> > I would expect the IAA watchdog timer to catch something like that.
> 
> I'll do some more investigation into this error and give a more
> detailed report.

Okay.

Alan Stern