On Tue, 14 Jan 2014, Sarah Sharp wrote: > Alan, I believe your analysis of the code was incorrect when you asked > Xenia to create commit 60e102ac73cd40069d077014c93c86dc7205cb68 in > August: > > http://marc.info/?l=linux-usb&m=137780837109921&w=2 > > You asked her to set lpm_capable because you thought it simply wasn't > being set at all. You overlooked the xHCI driver setting lpm_capable in > drivers/usb/host/xhci-pci.c. So I did. > The xHCI driver only sets lpm_capable for xHCI host controllers that it > knows how to calculate the U1/U2 timeout values for. Since those > timeout values are highly dependent on how the xHCI host hardware > scheduling works, I've only enabled USB 3.0 Link PM for Intel hosts: > > static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci) > { > ... > if (pdev->vendor == PCI_VENDOR_ID_INTEL) { > xhci->quirks |= XHCI_LPM_SUPPORT; > xhci->quirks |= XHCI_INTEL_HOST; > } > > static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id) > { > ... > /* We know the LPM timeout algorithms for this host, let the USB core > * enable and disable LPM for devices under the USB 3.0 roothub. > */ > if (xhci->quirks & XHCI_LPM_SUPPORT) > hcd_to_bus(xhci->shared_hcd)->root_hub->lpm_capable = 1; > > lpm_capable shouldn't be set for hosts where we don't know how to > calculate the U1/U2 timeout values, so commit > 60e102ac73cd40069d077014c93c86dc7205cb68 should just be reverted. Allow me to propose an alternative. Let's do our best to treat root hubs the same as external hubs. This means that udev->lpm_capable should be set the same way in hub_port_init() and register_root_hub(): by calling usb_device_supports_lpm(), which gets the information from the BOS extended capabilities. In particular, xhci-hcd shouldn't go messing around with the innards of the root hub structure. Instead of setting lpm_capable when it knows how to calculate the timeout values, it should pass the necessary information through the ss_cap descriptor, just like an external hub would. Either don't provide that descriptor at all if you don't know the timeout values, or else set those fields in the descriptor to something invalid (and make usb_device_supports_lpm() test for it). And while we're at it, let's change the comment in usb_device_supports_lpm() that says "All USB 3.0 must support LPM". Not only is it ungrammatical, but as you point out, it is wrong for some root hubs. 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
