Re: [RFC 5/7] USB/xHCI: USB 3.0 link PM change bit means port resume.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jan 12, 2012 at 11:48:10AM -0500, Alan Stern wrote:
> On Wed, 11 Jan 2012, Sarah Sharp wrote:
> 
> > Sigh.  So it turns out the behavior of USB 3.0 external hubs and the
> > xHCI roothub port link state change bit is different.
> > 
> > The USB 3.0 bus spec, section 10.14.2.6.2 says the port link state
> > change bit will be set when the port goes from U3 to U0 when the host
> > initiated the resume, but the bit will *not* be set if the device has
> > signaled remote wakeup.  But the xHCI spec says that the link state
> > change bit will be set on a remote wakeup.
> 
> The spec is a little confusing.  9.2.5.2 talks about a Function Wake 
> Notification and refers to section 7.4.9, which doesn't exist.  Maybe 
> it means 8.5.6.4.
> 
> > I think the idea is that software is supposed to know to resume a device
> > by looking at the device notification the device sends (and the xHCI
> > host passes on in an event) when the roothub port finishes resuming.
> > Only the xHCI host doesn't send device notifications itself, so it uses
> > the link state change.
> 
> What does xhci-hcd do with the device notifications?

Absolutely nothing right now.  I couldn't figure out why we needed them
since function suspend wasn't implemented, so the xHCI event code just
handles the event and ignores it.

I'm thinking that if we get a device wake notification, the xHCI driver
should find the parent hub of that device and somehow kick khubd for that
hub.  But I'd have to look and see if we could do that without
interfering with the URB completion for the parent hub's interrupt URB.

> > The xHCI driver needs a way to tell khubd which device caused the remote
> > wakeup, because khubd won't be able to tell by looking at the link state
> > change bits.  It could possibly tell, if it internally kept track of
> > which link state each USB 3.0 device should be in, and checked which
> > ports changed link states to U0.
> > 
> > I'll go look at the hub code more closely to see what it currently does,
> > but I thought I'd pass my finding on to you and see if you have any
> > thoughts.
> 
> For SuperSpeed hubs, the wakeup code will probably have to look at the
> link state status bits instead of the link state change bits.  I guess 
> we could do the same thing with non-SuperSpeed hubs -- look for ports 
> whose suspend status bit isn't set but where the child device is marked 
> as being in USB_STATE_SUSPENDED.

Ok, that's what I was thinking of doing.

Also, it turns out that the port link status change bit also indicates
when a USB 3.0 port moves to SS.Inactive (even on the roothub), so the
usb_handle_remote_wake() code needs to handle that case.  I got a lot of
-19 errors from usb_autoresume() because udev was NULL when I first
plugged in a USB 3.0 hub that didn't link train with the host controller
properly.

The USB core tried to disable the port, which doesn't work for USB 3.0
ports, and so it turned into a loop of trying to auto-resume, failing
because udev was NULL, attempting to disable the port, noticing the
SS.Inactive state and issuing a warm reset, clearing the link and reset
change bits, waiting for the debounce delay, device slips back into
SS.Inactive in the meantime, and then we try to auto-resume, etc.

I don't really see a way to make that loop end, since we can't disable
USB 3.0 ports, so I guess the user will just see the failed attempts
filling up their dmesg.  Good news is that this is the first time I've
seen such a loop, and the USB 3.0 hub is still only a prototype that
probably isn't electrically compliant yet.

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


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux