On Thu, Jun 20, 2013 at 01:36:17PM -0400, Alan Stern wrote:
> On Thu, 20 Jun 2013, Mathias Nyman wrote:
>
> > Make the Linux xHCI driver automatically try to switchover the EHCI ports to
> > xHCI when an Intel xHCI host is detected, and it also finds an Intel EHCI host.
> >
> > This means we will no longer have to add Intel xHCI hosts to a quirks list when
> > the PCI device IDs change. Simply continuing to add new Intel xHCI PCI device
> > IDs to the quirks list is not sustainable.
> >
> > During suspend ports may be swicthed back to EHCI by BIOS and not properly
> > restored to xHCI at resume. Previously both EHCI and xHCI resume functions
> > switched ports back to XHCI, but it's enough to do it in xHCI only
> > because the hub driver doesn't start running again until after both hosts are resumed.
> >
> > Signed-off-by: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx>
>
> Looks good, and it removes a lot more code than it adds, which is
> always beneficial.
Yes, I like that too. :)
> > index 595d210..6bd299e 100644
> > --- a/drivers/usb/host/ehci-pci.c
> > +++ b/drivers/usb/host/ehci-pci.c
> > @@ -315,53 +315,11 @@ done:
> > * Also they depend on separate root hub suspend/resume.
> > */
> >
> > -static bool usb_is_intel_switchable_ehci(struct pci_dev *pdev)
> > @@ -921,8 +895,16 @@ static void quirk_usb_handoff_xhci(struct pci_dev *pdev)
> > writel(val, base + ext_cap_offset + XHCI_LEGACY_CONTROL_OFFSET);
> >
> > hc_init:
> > - if (usb_is_intel_switchable_xhci(pdev))
> > - usb_enable_xhci_ports(pdev);
> > + if (pdev->vendor == PCI_VENDOR_ID_INTEL) {
> > + struct pci_dev *companion = NULL;
> > + for_each_pci_dev(companion) {
> > + if (companion->class == PCI_CLASS_SERIAL_USB_EHCI &&
> > + companion->vendor == PCI_VENDOR_ID_INTEL) {
> > + usb_enable_intel_xhci_ports(pdev);
> > + break;
> > + }
> > + }
> > + }
>
> Maybe this loop isn't needed. Would it be okay to call
> usb_enable_intel_xhci_ports() even if there are no EHCI controllers?
Probably not. If there are no EHCI controllers, the xHCI host is
unlikely to have the port switchover registers in its PCI config space.
That means we would be reading and writing to registers we have no idea
what they do.
The code at the end of this patch also needs to check for Intel EHCI
controllers on xHCI resume:
- if (usb_is_intel_switchable_xhci(pdev))
- usb_enable_xhci_ports(pdev);
+
+ if (pdev->vendor == PCI_VENDOR_ID_INTEL)
+ usb_enable_intel_xhci_ports(pdev);
> For that matter, what happens if there aren't any "companion" EHCI
> controllers for the xHCI controller in question, but there are separate
> EHCI controllers on an add-on PCI card?
They wouldn't be Intel EHCI controllers. We don't make discrete host
controllers. If that changes, we can revisit this, but I don't think
it's an issue.
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
