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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 1 Mar 2012 11:20:12 -0700
From:	Bjorn Helgaas <bhelgaas@...gle.com>
To:	Yinghai Lu <yinghai@...nel.org>
Cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Sarah Sharp <sarah.a.sharp@...ux.intel.com>,
	Jesse Barnes <jbarnes@...tuousgeek.org>,
	linux-usb@...r.kernel.org, linux-pci@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH] usb, PCI: split quirk for usb host controller to three

On Thu, Mar 1, 2012 at 10:45 AM, Yinghai Lu <yinghai@...nel.org> wrote:
> On Thu, Mar 1, 2012 at 9:24 AM, Greg Kroah-Hartman
> <gregkh@...uxfoundation.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@...nel.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/