On Fri, 21 Feb 2014, Mathias Nyman wrote:
> >> @@ -4753,6 +4753,8 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
> >> * companion controller.
> >> */
> >> hcd->has_tt = 1;
> >> + /* prevent USB2 runtime pm until USB3 parts are initialized */
> >> + pm_runtime_get_noresume(hcd->self.controller);
> >> } else {
> >> /* xHCI private pointer was set in xhci_pci_probe for the second
> >> * registered roothub.
> >> @@ -4765,6 +4767,9 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
> >> if (xhci->hci_version < 0x100)
> >> hcd->self.no_sg_constraint = 1;
> >>
> >> + /* USB3 initialized far enough to allow runtime pm suspend */
> >> + pm_runtime_put_noidle(hcd->self.controller);
> >> +
> >
> > Why the noresume and noidle versions? Shouldn't these be the _sync versions?
> >
>
> As I understood it the _sync versions may actually run pm_runtime_resume
> / pm_runtime_idle. Which is not what I'm looking for here.
The _sync versions won't run those routines if the usage counter isn't
0.
> Noresume and noidle will just increase / decrease the usage count and
> that way prevent usb2 roothub register from calling xhci_suspend.
>
> 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?
> 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.
> 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.
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
