On Mon, 17 Jun 2013, Sarah Sharp wrote: > > Correct me if I'm wrong here. The original idea was to switch > > everything over to xHCI during some early stage (like when the xHCI > > controller is first passed to the pci-quirks.c code) and switch back to > > EHCI at shutdown. As a refinement, you want to switch over to xHCI > > again following system resume, in case the BIOS messed things up. Is > > there anything else? > > Yes, that was the idea. > > > Why does the old code need to be changed? The only problem I see is > > that it may be a little too elaborate. For example, why bother trying > > to do the switch when the EHCI controller resumes? Won't everything > > work okay if you wait until the xHCI controller resumes? > > The code needs to be changed because we don't want to keep adding new > PCI IDs for all the new Intel hosts. Okay. That's simple enough; just remove the product ID checks. > Mathias also didn't like that we > inefficiently walked the PCI bus, looking for the xHCI host in the EHCI > resume routine, so he stored the xHCI pointer. Perhaps this would be > more clear if these two goals were broken up into two patches? Why not just remove all that stuff from the EHCI resume routine? It would be a lot simpler. See below. > > Why does the code check so hard to see whether or not there are any > > switchable ports? Why not just always try to set the switch on any > > Intel controller? > > Not all ports may be exposed on the platform (because there are > different skews). If we don't check which ports to switchover, some > BIOSes freak out on either shutdown, or suspend. I don't remember > which. So we need to pay attention to the port switchover masks. Sorry, I wasn't clear. Yes, by all means, do check the switchover masks. But there's no need to check the product ID; checking just the vendor ID should be good enough. > > Why is it necessary to search through all the PCI devices? Why is it > > necessary to store any sort of list of switchable controllers? > > We don't know which host will resume from suspend first. > > Assume we have a broken BIOS that switches ports back to EHCI on resume. > We don't want the USB core to notice EHCI connects and start servicing > them before xHCI resumes. Therefore, we need to have both the xHCI and > the EHCI resume functions attempt to switch the ports over to xHCI. The USB core won't notice anything, because the hub driver doesn't start running again until after all the devices (including host controllers) have been resumed. In other words, the khubd thread is freezable. (This is not by some random chance; it was an explicit decision that we don't want the hub driver mucking things up while we're in the middle of a big transition like system suspend or resume. You can rely on it.) So it won't matter if a device gets switched over to EHCI for a while. When the xHCI controller resumes, the device will get switched back to it, and usbcore will see only that there was a disconnect/reconnect during suspend. > That means for EHCI, we either need to search the PCI tree for the xHCI > host on resume (because that's where the port switchover mechanism > registers are), or we need to store the xHCI PCI host pointer somewhere. > Mathias chose to store the pointer. The reasoning above shows that there's no need either to search the entire PCI tree or to store any host controller pointers. > > Why should hot-unplug be an issue? Or multiple xHCI controllers? > > This is not a problem right now, I'm perhaps being paranoid. :) For all > current Intel systems, AFAIK, there will only be one xHCI host > controller, and it will be a part of the PCH, so you won't see any PCI > hot unplug events unless someone removes the entire PCH (which I don't > know if we support?). Even if someone in the future decides to put an Intel xHCI controller on a PCIe card, there shouldn't be any trouble if you handle things as decribed above. Agreed? 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
