On Fri, Jun 19, 2015 at 6:06 PM, Bjorn Helgaas <bhelgaas@xxxxxxxxxx> wrote:
> [-cc Dexuan, Matthew (bounced)]
> [+cc Mathias, since MAINTAINERS mentions USB, Intel, and you in one entry :)]
>
> On Fri, Jun 19, 2015 at 5:42 PM, Bjorn Helgaas <bhelgaas@xxxxxxxxxx> wrote:
>> We checked for "dev->class == PCI_CLASS_SERIAL_USB", but dev->class
>> contains the entire three-byte base class/sub-class/interface, while
>> PCI_CLASS_SERIAL_USB is only the two-byte base class/sub-class.
>>
>> This error meant that we used the Intel device-specific reset on devices
>> with class code 0x000c03 instead of those with class code 0x0c03xx.
>> 0x000c03 is a reserved value in the 0x00 backwards compatibility base class
>> and shouldn't match any devices, so I think reset_intel_generic_dev()
>> always failed.
>>
>> Shift dev->class to discard the interface byte before comparing with
>> PCI_CLASS_SERIAL_USB.
>>
>> Fixes: aeb30016fec3 ("PCI: add Intel USB specific reset method")
>> Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
>> CC: Dexuan Cui <dexuan.cui@xxxxxxxxx>
>> CC: Yu Zhao <yu.zhao@xxxxxxxxx>
>> ---
>> drivers/pci/quirks.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
>> index 8bc60c2..356a401 100644
>> --- a/drivers/pci/quirks.c
>> +++ b/drivers/pci/quirks.c
>> @@ -3358,7 +3358,7 @@ static int reset_intel_generic_dev(struct pci_dev *dev, int probe)
>> int pos;
>>
>> /* only implement PCI_CLASS_SERIAL_USB at present */
>> - if (dev->class == PCI_CLASS_SERIAL_USB) {
>> + if ((dev->class >> 8) == PCI_CLASS_SERIAL_USB) {
>
> I'm a little bit concerned about this change because it *looks* like
> this has always been broken, but I assume it was tested and somebody
> would have noticed.
Unless we get confirmation that somebody cares about
reset_intel_generic_dev() and it has been tested, I'm inclined to
revert aeb30016fec3 ("PCI: add Intel USB specific reset method")
instead of applying this fix.
I think reset_intel_generic_dev() always returns -ENOTTY except for
devices with "class == 0x000c03", which is an invalid class code. If
I "fix" it by putting in the shift, it will start poking at config
space on Intel USB devices. I have no way of testing that, and I
haven't seen a problem report related to this, so a change is just as
likely to break something as to fix something.
So the safest thing looks like just dropping the quirk altogether,
which shouldn't change behavior at all.
>> pos = pci_find_capability(dev, PCI_CAP_ID_VNDR);
>> if (!pos)
>> return -ENOTTY;
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
