[PATCH v2 1/2] PCI: Move Synopsys HAPS platform device IDs

[PATCH v2 1/2] PCI: Move Synopsys HAPS platform device IDs

[Thinh Nguyen]

Move Synopsys HAPS platform device IDs to pci_ids.h so that both
drivers/pci/quirks.c and dwc3-haps driver can reference these IDs.

Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
Acked-by: Felipe Balbi <felipe.balbi@linux.intel.com>
---
Change in v2:
- Revise title to fit PCI patches' convention
- Revise commit message to include the change purpose

 drivers/usb/dwc3/dwc3-haps.c | 4 ----
 include/linux/pci_ids.h      | 3 +++
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-haps.c b/drivers/usb/dwc3/dwc3-haps.c
index c9cc33881bef..02d57d98ef9b 100644
--- a/drivers/usb/dwc3/dwc3-haps.c
+++ b/drivers/usb/dwc3/dwc3-haps.c
@@ -15,10 +15,6 @@
 #include <linux/platform_device.h>
 #include <linux/property.h>
 
-#define PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3		0xabcd
-#define PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3_AXI	0xabce
-#define PCI_DEVICE_ID_SYNOPSYS_HAPSUSB31	0xabcf
-
 /**
  * struct dwc3_haps - Driver private structure
  * @dwc3: child dwc3 platform_device
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 69f0abe1ba1a..25db0c1586ea 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -2358,6 +2358,9 @@
 #define PCI_DEVICE_ID_CENATEK_IDE	0x0001
 
 #define PCI_VENDOR_ID_SYNOPSYS		0x16c3
+#define PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3		0xabcd
+#define PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3_AXI	0xabce
+#define PCI_DEVICE_ID_SYNOPSYS_HAPSUSB31	0xabcf
 
 #define PCI_VENDOR_ID_VITESSE		0x1725
 #define PCI_DEVICE_ID_VITESSE_VSC7174	0x7174
-- 
2.11.0

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

[Thinh Nguyen]

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.

Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
---
Change in v2:
- Revise title to fit PCI patches' convention
- Override pci class instead of driver

 drivers/pci/quirks.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 4700d24e5d55..c84f81c42a1f 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -618,6 +618,32 @@ static void quirk_amd_nl_class(struct pci_dev *pdev)
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_NL_USB,
 		quirk_amd_nl_class);
 
+/*
+ * 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.
+ */
+static void quirk_synopsys_haps(struct pci_dev *pdev)
+{
+	u32 class = pdev->class;
+
+	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);
+
 /*
  * Let's make the southbridge information explicit instead of having to
  * worry about people probing the ACPI areas, for example.. (Yes, it
-- 
2.11.0

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

[Bjorn Helgaas]

On Mon, Dec 10, 2018 at 02:08:01PM -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.
> 
> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>

I applied both of these to pci/misc for v4.21, thanks!

> ---
> Change in v2:
> - Revise title to fit PCI patches' convention
> - Override pci class instead of driver
> 
>  drivers/pci/quirks.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 4700d24e5d55..c84f81c42a1f 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -618,6 +618,32 @@ static void quirk_amd_nl_class(struct pci_dev *pdev)
>  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_NL_USB,
>  		quirk_amd_nl_class);
>  
> +/*
> + * 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.
> + */
> +static void quirk_synopsys_haps(struct pci_dev *pdev)
> +{
> +	u32 class = pdev->class;
> +
> +	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;

I dropped this "default: return;" because it's superfluous.

> +	}
> +}
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SYNOPSYS, PCI_ANY_ID,
> +			 quirk_synopsys_haps);
> +
>  /*
>   * Let's make the southbridge information explicit instead of having to
>   * worry about people probing the ACPI areas, for example.. (Yes, it
> -- 
> 2.11.0
> 

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

[Trent Piepho]

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.

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

[Thinh Nguyen]

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:

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


Thanks,
Thinh

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

[Trent Piepho]

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.