Patchwork [v2,2/2] PCI: Override Synopsys USB 3.x HAPS device class

login
register
mail settings
Submitter Thinh Nguyen
Date Feb. 4, 2019, 11:18 p.m.
Message ID <30102591E157244384E984126FC3CB4F639C0B41@us01wembx1.internal.synopsys.com>
Download mbox | patch
Permalink /patch/717937/
State New
Headers show

Comments

Thinh Nguyen - Feb. 4, 2019, 11:18 p.m.
Hi Trent,

Trent Piepho wrote:
> On Mon, 2018-12-10 at 14:08 -0800, Thinh Nguyen wrote:
>> Synopsys USB 3.x host HAPS platform has a class code of
>> PCI_CLASS_SERIAL_USB_XHCI, and xhci driver can claim it. However, these
>> devices should use dwc3-haps driver. Change these devices' class code to
>> PCI_CLASS_SERIAL_USB_DEVICE to prevent the xhci-pci driver from claiming
>> them.
>>
>> +
>> +	switch (pdev->device) {
>> +	case PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3:
>> +	case PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3_AXI:
>> +	case PCI_DEVICE_ID_SYNOPSYS_HAPSUSB31:
>> +		pdev->class = PCI_CLASS_SERIAL_USB_DEVICE;
>> +		pci_info(pdev, "PCI class overridden (%#08x -> %#08x) so dwc3 driver can claim this instead of xhci\n",
>> +			 class, pdev->class);
>> +		break;
>> +	default:
>> +		return;
>> +	}
>> +}
>> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SYNOPSYS, PCI_ANY_ID,
>> +			 quirk_synopsys_haps);
>> +
> This change breaks my IMX7d based device.  This SoC has a PCIe bus
> based on the Synopsys Designware host controller.  This has a root
> bridge that shows up as:
>
> 00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01) (prog-if 00 [Normal decode])                                 
> 00:00.0 0604: 16c3:abcd (rev 01) (prog-if 00 [Normal decode])                                                        
>
> Which is to say, class 0x0604, vendor PCI_VENDOR_ID_SYNOPSYS, and the
> device ID 0xabcd.
>
> It looks like there has been a bit of sloppy allocation of PCI device
> codes, as PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3 is also 0xabcd.
>
> So the result of this patch is to try to turn the imx7d PCIe root
> bridge into a USB controller.  Which of course breaks it and prevents
> anything behind it, i.e. the endpoint attached to the pci-e bus, from
> working.
>
> Somehow this quirk needs to be more targeted.

Right. Lukas also reported this. It appears that there's a duplicate VID
PID used for the USB controller on HAPS platform. You can review the
discussion subject "Linux Kernel Regression: HAPS quirk breaks PCIe on
i.MX6QP".

I'll send out a patch to resolve this issue. You can check the change to
see if this resolves your issue:



Thanks,
Thinh
Trent Piepho - Feb. 4, 2019, 11:50 p.m.
On Mon, 2019-02-04 at 23:18 +0000, Thinh Nguyen wrote:
> Trent Piepho wrote:

> > On Mon, 2018-12-10 at 14:08 -0800, Thinh Nguyen wrote:

> > > Synopsys USB 3.x host HAPS platform has a class code of

> > > PCI_CLASS_SERIAL_USB_XHCI, and xhci driver can claim it. However, these

> > > devices should use dwc3-haps driver. Change these devices' class code to

> > > PCI_CLASS_SERIAL_USB_DEVICE to prevent the xhci-pci driver from claiming

> > > them.

> > > 

> > This change breaks my IMX7d based device.  This SoC has a PCIe bus

> > based on the Synopsys Designware host controller.  This has a root

> >                                                  

> > 

> Right. Lukas also reported this. It appears that there's a duplicate VID

> PID used for the USB controller on HAPS platform. You can review the

> discussion subject "Linux Kernel Regression: HAPS quirk breaks PCIe on

> i.MX6QP".

> 

> -DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SYNOPSYS, PCI_ANY_ID,

> -                        quirk_synopsys_haps);

> +DECLARE_PCI_FIXUP_CLASS_HEADER(PCI_VENDOR_ID_SYNOPSYS, PCI_ANY_ID,

> +               PCI_CLASS_SERIAL_USB_XHCI, 0, quirk_synopsys_haps);


Thanks for the quick response.  Tested this on imx7d, and as expected,
it fixes the problem.  The host bridge has class PCI_CLASS_BRIDGE_PCI
like it should and the quirk no longer matches.

Patch

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index b0a413f3f7ca..f46e7de9e15d 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -639,8 +639,8 @@  static void quirk_synopsys_haps(struct pci_dev *pdev)
                break;
        }
 }
-DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SYNOPSYS, PCI_ANY_ID,
-                        quirk_synopsys_haps);
+DECLARE_PCI_FIXUP_CLASS_HEADER(PCI_VENDOR_ID_SYNOPSYS, PCI_ANY_ID,
+               PCI_CLASS_SERIAL_USB_XHCI, 0, quirk_synopsys_haps);
 
 /*
  * Let's make the southbridge information explicit instead of having to