On Tue, Oct 16, 2012 at 01:38:06PM -0400, Alan Stern wrote:
> On Tue, 16 Oct 2012, Sarah Sharp wrote:
> 
> > > The kernel support we have now for USB-2 LPM doesn't make any sense.  
> > > usb_port_suspend() disables LPM before changing the link state out of
> > > L0, and usb_port_resume() enables LPM after changing the state back to
> > > L0.
> > > 
> > > Thus LPM is disabled precisely at the times when it might be useful!
> > 
> > One thing to understand is that the USB 2.1 LPM code is designed to only
> > take advantage of host controllers that can do hardware-controlled LPM.
> 
> Not so.  At least, not in the two documents that are part of the USB-2 
> zip archive currently available from www.usb.org.  They make no 
> mention of automatic power-level changes; everything has to be 
> initiated by the host OS (except for device-initiated wakeup).

Right, that's in the xHCI spec instead.

> > It also only works for devices directly attached to the roothub.  The
> > code expects that once LPM is enabled, the host controller will
> > automatically put the idle link into L1 after the port L1 timeout
> > expires.
> 
> Where is this hardware-controlled LPM documented?  And the port L1 
> timeout?  Are you referring to section 4.23.5.1.1.1 in the xHCI spec?  
> It doesn't apply to USB-2 hosts or hubs.

The xHCI 1.0 spec added support for hardware-controlled LPM.  xHCI 0.96
hosts won't have this feature.  It's documented in section 4.23.5.1.1.1.

> > > In addition to this, I see that ehci-hcd includes some basic support
> > > for LPM but it doesn't implement the set_usb2_hw_lpm HCD method.  It's
> > > a sort of do-it-yourself approach (and it includes a bunch of bugs).
> > 
> > Yeah, I think Jacob Pan and another OTC person worked on the USB 2.1 LPM
> > for an Intel SoC board, but they never found very many USB 2.1 devices,
> > so they couldn't test it very well.
> 
> Maybe that's an indication we shouldn't bother to support it.  What do 
> you think?  I could take it out of ehci-hcd easily enough.

My instinct is to just take out.  But Alex and Jacob should probably
have some input on that.

> Okay.  But that still leaves a question: Is it reasonable to call
> xhci-hcd's bus_suspend method without first putting all the devices on
> the bus into U3?  If it is then generic_suspend() can avoid calling
> usb_port_suspend() for the FREEZE and PRETHAW events, even on a USB-3
> bus.

I think we really do need to place all the devices into suspend during
the FREEZE or PRETHAW.  The xHCI specification says that before a host
can be suspended, the driver shall "Stop all USB activity by issuing
Stop Endpoint Commands for each endpoint in the Running state."  I
assume that's because the endpoint rings need to be stopped, or the host
may attempt to do DMA to get the next TRB for a periodic endpoint.

Currently, the xHCI driver kind of hacks around this.  If bus_suspend is
called and one of the rootports is not in U3, then it issues a Stop
Endpoint command with the 'suspend' bit set, and places the root port
link into U3.  But that only stops the endpoints on devices attached to
the roothub, not ones that are deeper in the tree.  So the xHCI host
could, for example, go fetch a TRB from a periodic endpoint of a device
on the second hub tier, in the middle of the hibernate operation.

Since the xHCI spec really wants us to suspend all devices, I would
rather the hibernate actually do that for both USB 2.0 and USB 3.0
devices.

Sarah Sharp
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to