On Mon, 9 Jan 2012, Sarah Sharp wrote: > > > XXX: We could clear PLC in hub_activate() when hub_activate() is called for a > > > hub resume only (HUB_RESUME). I'm not sure about the port state machine here. > > > Alan? > > > > Why would you want to clear it there? As far as I can see, > > C_LINK_STATE on a USB-3 hub is more or less equivalent to C_SUSPEND on > > a USB-2 hub. Therefore the HUB_RESUME case should treat them the same. > > Or have I misunderstood something? > > Oh, sorry, that was an old comment I left in the patch. You're right > that the link state change is basically like a suspend change. However, > a link state change can also mean that something bad happened after we > enabled the USB 3.0 link power management. But I think we should cross > that bridge when the USB 3.0 LPM code gets added. Sure. > > > + if (portchange & USB_PORT_STAT_C_LINK_STATE) > > > need_debounce_delay = true; > > > - clear_port_feature(hub->hdev, port1, > > > - USB_PORT_FEAT_C_PORT_LINK_STATE); > > > - } > > > > Is the debounce delay > > really needed in this case? Maybe the whole test can be removed. > > There's no comparable test for C_SUSPEND. > > I wasn't sure if the debounce delay was necessary. I'll remove it and > see if khubd still notices the port has resumed. As far as I can recall, the debounce delay is intended only for situations where there may have been a connect change. It's basically just a small optimization: debounce all the changed ports at once instead of doing them one at a time. > > This is ugly. Instead, have two separate tests: one for !superspeed && > > C_SUSPEND and the other for superspeed && C_LINK_STATE. Sort of like > > we have in usb_port_resume(). The common parts of the code that > > follows can be moved into a subroutine. > > Ok, sure. The whole of hub_events() is pretty ugly, but I'm not sure > how to make it better. The xHCI hub handler is also pretty ugly. Maybe > there's something unique to port status handling that leads to long, > ugly code (possibly the sheer amount of status bit cases?). Heh, the hub_control routines tend to be pretty ugly too. Alan Stern -- 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
