Re: usb3 hdd docking station doesn't work with recent kernels

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jan 14, 2014 at 02:43:40PM -0500, Alan Stern wrote:
> 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.

I agree your alternative is much cleaner, and I agree we should try to
treat roothubs like external hubs.  I've made a patch (to be sent
shortly) that fixes this.

However, I would like to see the original bad commit ("usbcore: set
lpm_capable field for LPM capable root hubs") reverted from the stable
kernels, since it's been backported to 3.10, 3.11, and 3.12.  I would
rather go for the simple revert, since that's less likely to cause
issues than new code.

Sarah Sharp
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]