On Wed, Nov 17, 2010 at 11:14:36AM -0500, Alan Stern wrote:
> On Tue, 16 Nov 2010, Sarah Sharp wrote:
> > At this point, the device has migrated from port 1 (a SuperSpeed port)
> > to port 3 (a High Speed port):
>
> Surely this is the real problem. Once the device has changed speeds
> like this, what's the right way to get it to change back to SuperSpeed?
> Reset the high-speed port?
Yes, the High Speed port needs to get reset. Then the USB device will
go searching for SuperSpeed terminations. If it doesn't find those,
it's supposed to drop back to high speed, but apparently this device
loses its mind and stops responding to descriptor requests.
> > xhci_hcd 0000:04:00.0: get port status, actual port 0 status = 0x2a0
> > xhci_hcd 0000:04:00.0: Get port status returned 0x100
> > xhci_hcd 0000:04:00.0: get port status, actual port 1 status = 0x2a0
> > xhci_hcd 0000:04:00.0: Get port status returned 0x100
> > xhci_hcd 0000:04:00.0: get port status, actual port 2 status = 0x2a0
> > xhci_hcd 0000:04:00.0: Get port status returned 0x100
> > xhci_hcd 0000:04:00.0: get port status, actual port 3 status = 0x202e1
> > xhci_hcd 0000:04:00.0: Get port status returned 0x10101
> >
> > The USB core is trying to do something sneaky and persist USB devices
> > across suspend. But since the original port (port 1) reports a
> > disconnect, it assumes the device isn't there and disables the port:
> >
> > xhci_hcd 0000:04:00.0: disable port, actual port 1 status = 0x2a0
>
> There used to be code to prevent xHCI ports from being disabled. What
> happened to it? Does it need to be added back in?
There were places in the USB core that were prevented from calling
hub_port_disable(), but no big patch to make that function just return
when it was called with a SuperSpeed port.
You might be thinking of John Youn's USB 3.0 hub patch. There was a bit
in there to make hub_port_disable() return if it was called with a USB
3.0 hub:
+static inline int hub_is_superspeed(struct usb_device *hdev)
+{
+ return (hdev->descriptor.bDeviceProtocol == 3);
+}
@@ -607,7 +632,7 @@ static int hub_port_disable(struct usb_hub *hub, int port1, int set_state)
if (hdev->children[port1-1] && set_state)
usb_set_device_state(hdev->children[port1-1],
USB_STATE_NOTATTACHED);
- if (!hub->error)
+ if (!hub->error && !hub_is_superspeed(hub->hdev))
ret = clear_port_feature(hdev, port1, USB_PORT_FEAT_ENABLE);
if (ret)
dev_err(hub->intfdev, "cannot disable port %d (err = %d)\n",
But the problem with that patch and the current state of the xHCI driver
is then the USB core is disallowed from disabling all the roothub ports
(including HS/FS/LS) under an xHCI host. Ideally, we would just prevent
it from disabling SS ports.
> We had a discussion about this issue some time back, and as I recall,
> the outcome was that we decided there aren't any situations where
> usbcore should need to disable a SuperSpeed port. I don't remember if
> we decided anything about non-SuperSpeed xHCI ports.
Yeah, I don't think anything was decided. I'm still not sure why the
persist code disables a port if the device disappears from under it.
> > Only then do we get a device disconnect on port 1:
> >
> > xhci_hcd 0000:04:00.0: get port status, actual port 1 status = 0x280
> > xhci_hcd 0000:04:00.0: Get port status returned 0x100
> > usb 6-2: USB disconnect, address 3
>
> This may be where the disconnect occurs, but the driver already
> reported that the port was disconnected when usbcore polled the status
> of all four ports just above.
True.
> > The USB core tries to enumerate the device as a High Speed device, but
> > the device is unresponsive:
> > usb 6-4: device descriptor read/8, error -61
> >
> > The end result is that the USB 3.0 terminations are permanently
> > disabled, and the device is stuck at High Speed. That would be
> > consistent with your observation that a system reboot is required to get
> > the device to connect at SuperSpeed again. I'm not sure if simply
> > unloading the xHCI driver and reloading it would help.
> >
> >
> > The real fix for this is to separate the xHCI roothub into two hubs: a
> > USB 3.0 hub and a USB 2.0 hub. This is how an external USB 3.0 hub
> > looks like to the system (two separate devices). Then on resume, we can
> > look at the USB 2.0 hub ports first, reset any of the ports that report
> > a connection, and then deal with USB 3.0 devices that migrated over from
> > High Speed to SuperSpeed.
>
> But the ports get reset anyway, as part of the persist handling. How
> would breaking the root hub into two help?
If there were two root hubs (one for SuperSpeed ports only, and the
other for HS/FS/LS), then John's patch to prevent port disabling would
work fine. But since the xHCI driver makes a single roothub with all
device speeds, there's no easy way for the USB core to figure out when
it needs to prevent port disabling. (It can't look at the port speed,
as that's not valid until a device is plugged in.)
So the two extra patches I send Don dig around in the xHCI Extended
Capabilities registers and look for the registers that tell the driver
which speeds the ports are supposed to be. Then the xHCI driver simply
refuses to disable SS ports. But it feels like that code should really
be in the USB core instead.
I'm also wondering if the roothub splitting patches will help deal with
the other issue Don is having. Ideally, the USB core would look at the
HS/FS/LS ports first, reset any connected devices under them, and then
go process the SS ports. Then any SS devices that originally connected
as HS would migrate over during the reset.
I'm not sure how to do this with the current combined roothub and the
current USB core code. But if the roothub were split instead, couldn't
we make the SS roothub wait for the HS roothub? How does the current
resume code handle companion controllers? Doesn't UHCI or OHCI have to
get resumed first, before EHCI?
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
