On Fri, 21 Feb 2014, Sarah Sharp wrote: > > Why would this trigger xhci_suspend? Isn't we still running in the > > context of the probe routine? The controller won't be suspended until > > the probe call returns. > > Maybe it helps to look at the context of the bug report Mathias is > trying to fix? > > http://marc.info/?l=linux-usb&m=138914518219334&w=2 > http://marc.info/?l=linux-kernel&m=138919609832200&w=2 > > It's pretty clear from the oops that xhci_suspend gets called before the > second roothub is registered. From the group it came from, I suspect > they have lots of random patches applied on top of their 3.10 kernel, > but let's assume that's not the issue and explore whether xhci_suspend > can be called before probe completes. > > The xHCI driver registers its own PCI probe function, and then it calls > usb_hcd_pci_probe(), and then registers the second roothub. So if the > USB core is somehow preventing PCI suspend in usb_hcd_pci_probe(), it > won't help for our special init flow. If the PCI core is supposed to > prevent PCI suspend until probe completes, then yes, xhci_suspend > shouldn't ever be called until both buses are registered. Ah, I see the problem. After calling usb_hcd_pci_probe to register the primary bus, the driver registers the secondary bus by hand. The thing is, usb_hcd_pci_probe does a pm_runtime_put_noidle just before returning. Therefore a runtime suspend can occur before the driver is fully ready. However, Mathias's patch is incorrect because it ignores the possibility of errors during probing. An error would leave the controller with an elevated usage counter. Instead of calling the pm_runtime routines for hcd->self.controller, it should use &hcd->self.root_hub->dev in the "if" clause and &hcd->primary_hcd->self.root_hub->dev in the "else" clause. Then errors won't matter, because the USB-2 root-hub structure will be destroyed if anything goes wrong. 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
