On Fri, Feb 21, 2014 at 01:57:25PM -0500, Alan Stern wrote: > On Fri, 21 Feb 2014, Mathias Nyman wrote: > > This is how I gather it works: > > > > Round 1: add usb2 hcd > > > > usb_add_hcd() // primary usb2 hcd > > hcd->self.root_hub = usb_alloc_dev() > > hcd->driver->reset(hcd) -> xhci_gen_setup() > > if(primary_hcd) // true > > pm_runtime_get_noresume(hcd->self.controller) // +1 usage > > At this point, we know the controller's usage counter is already > 0. > Therefore it doesn't matter whether the _sync or _noresume version is > used. > > But is this really what you want to do? That is, do you want to > prevent the controller from being suspended, or do you want to prevent > the root hub from being suspended? Either one would work. Mathias is just trying to prevent xhci_suspend from being called before the xHCI driver finishes registering both roothubs. It could be better to prevent the root hub from suspending, in case there's other issues with bus suspend we haven't found yet. > > register_root_hub(hcd) -> usb_new_device(usb_dev) > > pm_runtime_set_active(&udev->dev); > > pm_runtime_get_noresume(&udev->dev); > > pm_runtime_use_autosuspend(&udev->dev); > > pm_runtime_enable(&udev->dev); > > ... > > pm_runtime_put_sync_autosuspend(&udev->dev); this would trigger > > xhci_suspend if all usages were 0. (for both roothub and controller) > > which oopses as usb3 stuff is not yet initialized > > 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. What about the xHCI platform case? xhci_plat_suspend is registered as a system sleep PM operation, and it calls xhci_suspend: static int xhci_plat_suspend(struct device *dev) { struct usb_hcd *hcd = dev_get_drvdata(dev); struct xhci_hcd *xhci = hcd_to_xhci(hcd); return xhci_suspend(xhci); } static int xhci_plat_resume(struct device *dev) { struct usb_hcd *hcd = dev_get_drvdata(dev); struct xhci_hcd *xhci = hcd_to_xhci(hcd); return xhci_resume(xhci, 0); } static const struct dev_pm_ops xhci_plat_pm_ops = { SET_SYSTEM_SLEEP_PM_OPS(xhci_plat_suspend, xhci_plat_resume) }; So could we have a system sleep event in the middle of platform probe? > > Round 2: add usb 3 hcd > > > > usb_add_hcd() // secondary usb3 hcd > > hcd->self.root_hub = usb_alloc_dev() > > hcd->driver->reset(hcd) -> xhci_gen_setup() > > if(primary_hcd) // false > > .. > > else > > pm_runtime_put_noidle(hcd->self.controller) // -1 usage, * > > Again, is this really what you want? I thought you were trying to > prevent runtime suspends until the USB-3 root hub was registered. In > that case, you shouldn't do the _put until after the register_root_hub > call. > > > register_root_hub(hcd) -> usb_new_device(usb_dev) > > pm_runtime_set_active(&udev->dev); > > pm_runtime_get_noresume(&udev->dev); > > pm_runtime_use_autosuspend(&udev->dev); > > pm_runtime_enable(&udev->dev); > > ... > > pm_runtime_put_sync_autosuspend(&udev->dev); // ok to trigger would > > trigger xhci_suspend if all usages were 0. > > > > *) unnecessary to trigger any suspend as we're just about to register > > the usb3 roothub. but we need to decrement the usage counter. 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
