On Thu, Jan 12, 2012 at 02:55:31PM -0500, Alan Stern wrote: > On Thu, 12 Jan 2012, Sarah Sharp wrote: > > > > > 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. > > Hmmm. As far as I can see, when a child of non-suspended non-root hub > requests a wakeup, the hub doesn't send any special signal to the host. > In particular, it doesn't set any port-status-change bits. Which means > we _must_ use the notification somehow. > > The notification packet contains only a device address; xhci-hcd will > have to translate that to either a device number or (ideally) a struct > usb_device. The xHCI host translates that device address into a slot ID and puts that into the Device Notification Event TRB that it passes up to the xHCI driver. We can use the slot ID to map to the struct usb_device. > We'll create a hub->wakeup_bits bitflag array, and hub.c > can export a function for xhci-hcd to call that will set the > appropriate bitflag and kick khubd. Ok, sounds good. > > > 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, > > usb_autoresume_device() should never be called with a NULL argument. > > > 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. > > If the link fails during the debounce delay, we'll never enumerate the > device. So how come we then try to autoresume it? Shouldn't the > device get unregistered (or fail to be registered in the first place) > when enumeration fails? What happened was that the code to check for the SS.Inactive state was after the code I added to handle the link state change bits. I thought (before I read the USB 3.0 bus spec more closely) that the link change bit was only set on a resume or remote wakeup, so the function I added, hub_handle_remote_wakeup(), would check the link change bit, clear it, and then return -ENODEV and try to disable the port because there was no udev allocated for that port. Then the hub events code would continue on its way, notice the SS.Inactive state, and attempt to issue a warm port reset. > > 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. > > I don't understand how that loop could get started at all. Part of the > problem may be that I don't have a copy of your tree (and > git.kernel.org is currently unavailable). Huh, it seems to be up for me now, but of course I'm using ssh. Anyway, if you don't have access to my usb3-hub-suspend branch, you can just look at the proposed refactoring patch I sent you to see how things could get stuck in a loop. 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
