[RFC PATCH] usb, PCI: split quirk for usb host controller to three

[RFC PATCH] usb, PCI: split quirk for usb host controller to three

[Yinghai Lu]

so we avoid checking class again and again in that quirk.

need to be applied after pci/linux-next and usb/usb-next

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 drivers/usb/host/pci-quirks.c |   42 ++++++++++++++++++++++++++++--------------
 1 file changed, 28 insertions(+), 14 deletions(-)

Index: linux-2.6/drivers/usb/host/pci-quirks.c
===================================================================
--- linux-2.6.orig/drivers/usb/host/pci-quirks.c
+++ linux-2.6/drivers/usb/host/pci-quirks.c
@@ -884,33 +884,47 @@ static void __devinit quirk_usb_handoff_
 	iounmap(base);
 }
 
-static void __devinit quirk_usb_early_handoff(struct pci_dev *pdev)
+static void __devinit quirk_usb_early_handoff_uhci(struct pci_dev *pdev)
+{
+	quirk_usb_handoff_uhci(pdev);
+}
+DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
+		PCI_CLASS_SERIAL_USB_UHCI, 0, quirk_usb_early_handoff_uhci);
+
+static void __devinit quirk_usb_early_handoff_ohci(struct pci_dev *pdev)
 {
 	/* Skip Netlogic mips SoC's internal PCI USB controller.
 	 * This device does not need/support EHCI/OHCI handoff
 	 */
 	if (pdev->vendor == 0x184e)	/* vendor Netlogic */
 		return;
-	if (pdev->class != PCI_CLASS_SERIAL_USB_UHCI &&
-			pdev->class != PCI_CLASS_SERIAL_USB_OHCI &&
-			pdev->class != PCI_CLASS_SERIAL_USB_EHCI &&
-			pdev->class != PCI_CLASS_SERIAL_USB_XHCI)
+
+	quirk_usb_handoff_ohci(pdev);
+}
+DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
+		PCI_CLASS_SERIAL_USB_OHCI, 0, quirk_usb_early_handoff_ohci);
+
+static void __devinit quirk_usb_early_handoff_ehci(struct pci_dev *pdev)
+{
+	if (pdev->vendor == 0x184e)	/* vendor Netlogic */
 		return;
 
+	quirk_usb_handoff_ehci(pdev);
+}
+DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
+		PCI_CLASS_SERIAL_USB_EHCI, 0, quirk_usb_early_handoff_ehci);
+
+static void __devinit quirk_usb_early_handoff_xhci(struct pci_dev *pdev)
+{
 	if (pci_enable_device(pdev) < 0) {
 		dev_warn(&pdev->dev, "Can't enable PCI device, "
 				"BIOS handoff failed.\n");
 		return;
 	}
-	if (pdev->class == PCI_CLASS_SERIAL_USB_UHCI)
-		quirk_usb_handoff_uhci(pdev);
-	else if (pdev->class == PCI_CLASS_SERIAL_USB_OHCI)
-		quirk_usb_handoff_ohci(pdev);
-	else if (pdev->class == PCI_CLASS_SERIAL_USB_EHCI)
-		quirk_usb_handoff_ehci(pdev);
-	else if (pdev->class == PCI_CLASS_SERIAL_USB_XHCI)
-		quirk_usb_handoff_xhci(pdev);
+
+	quirk_usb_handoff_xhci(pdev);
+
 	pci_disable_device(pdev);
 }
 DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
-			PCI_CLASS_SERIAL_USB, 8, quirk_usb_early_handoff);
+		PCI_CLASS_SERIAL_USB_XHCI, 0, quirk_usb_early_handoff_xhci);

Re: [RFC PATCH] usb, PCI: split quirk for usb host controller to three

[Greg Kroah-Hartman]

On Thu, Mar 01, 2012 at 09:17:07AM -0800, Yinghai Lu wrote:
> so we avoid checking class again and again in that quirk.
> 
> need to be applied after pci/linux-next and usb/usb-next

As those are two independant trees, this needs to wait until after
3.4-rc1 is out.

> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> 
> ---
>  drivers/usb/host/pci-quirks.c |   42 ++++++++++++++++++++++++++++--------------
>  1 file changed, 28 insertions(+), 14 deletions(-)

Is this really helping anything here?  You added code overall :(

greg k-h

Re: [RFC PATCH] usb, PCI: split quirk for usb host controller to three

[Yinghai Lu]

On Thu, Mar 1, 2012 at 9:24 AM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Thu, Mar 01, 2012 at 09:17:07AM -0800, Yinghai Lu wrote:
>> so we avoid checking class again and again in that quirk.
>>
>> need to be applied after pci/linux-next and usb/usb-next
>
> As those are two independant trees, this needs to wait until after
> 3.4-rc1 is out.

ok.

>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>>
>> ---
>>  drivers/usb/host/pci-quirks.c |   42 ++++++++++++++++++++++++++++--------------
>>  1 file changed, 28 insertions(+), 14 deletions(-)
>
> Is this really helping anything here?  You added code overall :(

just make the code looks simple by removing

        if (pdev->class != PCI_CLASS_SERIAL_USB_UHCI &&
                        pdev->class != PCI_CLASS_SERIAL_USB_OHCI &&
                        pdev->class != PCI_CLASS_SERIAL_USB_EHCI &&
                        pdev->class != PCI_CLASS_SERIAL_USB_XHCI)
                return;
...
        if (pdev->class == PCI_CLASS_SERIAL_USB_UHCI)
   ...
        else if (pdev->class == PCI_CLASS_SERIAL_USB_OHCI)
   ...
        else if (pdev->class == PCI_CLASS_SERIAL_USB_EHCI)
    ...
        else if (pdev->class == PCI_CLASS_SERIAL_USB_XHCI)
    ...


also skip disable/enable device on non xhci controller.

Thanks

Yinghai

Re: [RFC PATCH] usb, PCI: split quirk for usb host controller to three

[Bjorn Helgaas]

On Thu, Mar 1, 2012 at 10:45 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Thu, Mar 1, 2012 at 9:24 AM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
>> On Thu, Mar 01, 2012 at 09:17:07AM -0800, Yinghai Lu wrote:
>>> so we avoid checking class again and again in that quirk.
>>>
>>> need to be applied after pci/linux-next and usb/usb-next
>>
>> As those are two independant trees, this needs to wait until after
>> 3.4-rc1 is out.
>
> ok.
>
>>
>>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>>>
>>> ---
>>>  drivers/usb/host/pci-quirks.c |   42 ++++++++++++++++++++++++++++--------------
>>>  1 file changed, 28 insertions(+), 14 deletions(-)
>>
>> Is this really helping anything here?  You added code overall :(
>
> just make the code looks simple by removing
>
>        if (pdev->class != PCI_CLASS_SERIAL_USB_UHCI &&
>                        pdev->class != PCI_CLASS_SERIAL_USB_OHCI &&
>                        pdev->class != PCI_CLASS_SERIAL_USB_EHCI &&
>                        pdev->class != PCI_CLASS_SERIAL_USB_XHCI)
>                return;
> ...
>        if (pdev->class == PCI_CLASS_SERIAL_USB_UHCI)
>   ...
>        else if (pdev->class == PCI_CLASS_SERIAL_USB_OHCI)
>   ...
>        else if (pdev->class == PCI_CLASS_SERIAL_USB_EHCI)
>    ...
>        else if (pdev->class == PCI_CLASS_SERIAL_USB_XHCI)
>    ...
>
>
> also skip disable/enable device on non xhci controller.

Hmm...  From the description, I thought this patch only took advantage
of DECLARE_PCI_FIXUP_CLASS_FINAL to remove the "if (pdev->class ==
XXX)" tests.

If it *also* skips a disable/enable sequence on non-xhci controllers,
please make that a separate patch so it's clear what's happening.

It's very important to be clear about these things for people who are
deciding whether to backport patches into distros.  All the zillions
of PCI changes we're doing make their lives hard enough without having
subtle behavior changes hidden in patches that seem to be "code
restructure only, no behavior change."

Bjorn

Re: [RFC PATCH] usb, PCI: split quirk for usb host controller to three

[Yinghai Lu]

On Thu, Mar 1, 2012 at 10:20 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:

> Hmm...  From the description, I thought this patch only took advantage
> of DECLARE_PCI_FIXUP_CLASS_FINAL to remove the "if (pdev->class ==
> XXX)" tests.
>
> If it *also* skips a disable/enable sequence on non-xhci controllers,
> please make that a separate patch so it's clear what's happening.
>
> It's very important to be clear about these things for people who are
> deciding whether to backport patches into distros.  All the zillions
> of PCI changes we're doing make their lives hard enough without having
> subtle behavior changes hidden in patches that seem to be "code
> restructure only, no behavior change."

sure, will separate the patch to small ones.

Thanks

Yinghai

Re: [RFC PATCH] usb, PCI: split quirk for usb host controller to three

[Sarah Sharp]

On Thu, Mar 01, 2012 at 09:24:35AM -0800, Greg Kroah-Hartman wrote:
> On Thu, Mar 01, 2012 at 09:17:07AM -0800, Yinghai Lu wrote:
> > so we avoid checking class again and again in that quirk.
> > 
> > need to be applied after pci/linux-next and usb/usb-next
> 
> As those are two independant trees, this needs to wait until after
> 3.4-rc1 is out.
> 
> > Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> > 
> > ---
> >  drivers/usb/host/pci-quirks.c |   42 ++++++++++++++++++++++++++++--------------
> >  1 file changed, 28 insertions(+), 14 deletions(-)
> 
> Is this really helping anything here?  You added code overall :(

I agree with Greg.  Why change this?  Does it shave off any boot time?
Please show hard numbers for what improvements this makes.  I really
don't want to change PCI init and break people's systems.

Sarah Sharp

Re: [RFC PATCH] usb, PCI: split quirk for usb host controller to three

[Yinghai Lu]

On Thu, Mar 1, 2012 at 11:03 AM, Sarah Sharp
<sarah.a.sharp@linux.intel.com> wrote:
> On Thu, Mar 01, 2012 at 09:24:35AM -0800, Greg Kroah-Hartman wrote:
>> On Thu, Mar 01, 2012 at 09:17:07AM -0800, Yinghai Lu wrote:
>> > so we avoid checking class again and again in that quirk.
>> >
>> > need to be applied after pci/linux-next and usb/usb-next
>>
>> As those are two independant trees, this needs to wait until after
>> 3.4-rc1 is out.
>>
>> > Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>> >
>> > ---
>> >  drivers/usb/host/pci-quirks.c |   42 ++++++++++++++++++++++++++++--------------
>> >  1 file changed, 28 insertions(+), 14 deletions(-)
>>
>> Is this really helping anything here?  You added code overall :(
>
> I agree with Greg.  Why change this?  Does it shave off any boot time?
> Please show hard numbers for what improvements this makes.  I really
> don't want to change PCI init and break people's systems.

but your code make pci init change to call disable/enable for non-xhci
for fixing xhci problem.

| commit cab928ee1f221c9cc48d6615070fefe2e444384a
| Author: Sarah Sharp <sarah.a.sharp@linux.intel.com>
| Date:   Tue Feb 7 15:11:46 2012 -0800
|
|    USB: Fix handoff when BIOS disables host PCI device.

Yinghai

Re: [RFC PATCH] usb, PCI: split quirk for usb host controller to three

[Sarah Sharp]

On Thu, Mar 01, 2012 at 11:19:06AM -0800, Yinghai Lu wrote:
> On Thu, Mar 1, 2012 at 11:03 AM, Sarah Sharp
> <sarah.a.sharp@linux.intel.com> wrote:
> > On Thu, Mar 01, 2012 at 09:24:35AM -0800, Greg Kroah-Hartman wrote:
> >> On Thu, Mar 01, 2012 at 09:17:07AM -0800, Yinghai Lu wrote:
> >> > so we avoid checking class again and again in that quirk.
> >> >
> >> > need to be applied after pci/linux-next and usb/usb-next
> >>
> >> As those are two independant trees, this needs to wait until after
> >> 3.4-rc1 is out.
> >>
> >> > Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> >> >
> >> > ---
> >> >  drivers/usb/host/pci-quirks.c |   42 ++++++++++++++++++++++++++++--------------
> >> >  1 file changed, 28 insertions(+), 14 deletions(-)
> >>
> >> Is this really helping anything here?  You added code overall :(
> >
> > I agree with Greg.  Why change this?  Does it shave off any boot time?
> > Please show hard numbers for what improvements this makes.  I really
> > don't want to change PCI init and break people's systems.
> 
> but your code make pci init change to call disable/enable for non-xhci
> for fixing xhci problem.

We discussed this, and agreed it was safe to do so for all USB host
controllers.  It's possible some other BIOS will disable the PCI host
controller before handing it off to the OS, so we have to handle that
case.  So it's fine that we enable/disable the device for all USB host
controllers.  I tried to make sure we weren't enabling/disabling for
non-USB PCI devices.

Sarah Sharp