Re: Linux Kernel Regression: HAPS quirk breaks PCIe on i.MX6QP

Re: Linux Kernel Regression: HAPS quirk breaks PCIe on i.MX6QP

[Bjorn Helgaas]

[+cc linux-pci, linux-kernel]

On Thu, Jan 31, 2019 at 11:21 AM Lukas F. Hartmann <lukas@mntmn.com> wrote:
>
> Hi Thinh,
>
> I'm writing you because you're the author in this commit:
> https://github.com/torvalds/linux/commit/03e6742584af8866ba014874669d0440bed3a623
>
> This quirk workaround breaks the PCIe on i.MX6QP, at least on my test
> devices. The reason is because the Synposys PCIe IP in i.MX6QP has the
> same device ID as the HAPS USB3 controller you are targeting in the
> commit: 0xabcd.
>
> Definition:
>
> https://github.com/torvalds/linux/blob/master/include/linux/pci_ids.h#L2364
>
> Here is a bit of lspci output on my i.MX6QP machine (without the problem):
>
> mntmn@reform:~/code/linux$ lspci -v
> 00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01) (prog-if 00
>                                           ^^^^
> [Normal decode])
>         Flags: bus master, fast devsel, latency 0, IRQ 307
>         Memory at 01000000 (32-bit, non-prefetchable) [size=1M]
>         Bus: primary=00, secondary=01, subordinate=ff, sec-latency=0
> ...
>
> The failure mode is that the PCIe controller shows up as a USB
> controller and my ath9k wireless PCIe card cannot be assigned the
> proper resources anymore (-ENOMEM even for very small BARs).
>
> Reverting this commit fixes the problem for me. I suggest to make the
> check more specific to the platform/chipset you are targeting.

So Synopsys apparently re-used Device ID 0xabcd?  That's a pretty bad problem.

It looks like we merged 03e6742584af ("PCI: Override Synopsys USB 3.x
HAPS device class") for v5.0, and v5.0-final hasn't been released yet,
so if we don't hear from Thinh with a resolution, we can still revert
it.

I set myself a reminder to revert this on Feb 11, but hopefully we'll
have a better resolution before then.

Bjorn

Re: Linux Kernel Regression: HAPS quirk breaks PCIe on i.MX6QP

[Thinh Nguyen]

Hi Lukas,

Bjorn Helgaas wrote:
> [+cc linux-pci, linux-kernel]
>
> On Thu, Jan 31, 2019 at 11:21 AM Lukas F. Hartmann <lukas@mntmn.com> wrote:
>> Hi Thinh,
>>
>> I'm writing you because you're the author in this commit:
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_torvalds_linux_commit_03e6742584af8866ba014874669d0440bed3a623&d=DwIBaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=VKipRJmm95P2RbIxyKKYrcUCOGNlQtjlV-5zhrVhIik&m=2sOrowYXlsC3rl0LfHQhIZzImag0jFZXGvR6NIQDsh8&s=oJoBWRE8_LGAYbX2alh6QnjYZTTmzcgLw4MtOMToNyo&e=
>>
>> This quirk workaround breaks the PCIe on i.MX6QP, at least on my test
>> devices. The reason is because the Synposys PCIe IP in i.MX6QP has the
>> same device ID as the HAPS USB3 controller you are targeting in the
>> commit: 0xabcd.
>>
>> Definition:
>>
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_torvalds_linux_blob_master_include_linux_pci-5Fids.h-23L2364&d=DwIBaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=VKipRJmm95P2RbIxyKKYrcUCOGNlQtjlV-5zhrVhIik&m=2sOrowYXlsC3rl0LfHQhIZzImag0jFZXGvR6NIQDsh8&s=-FVCwe81XNjJsYHYk1w-kdAzSOLunTGeNc73azO2QYw&e=
>>
>> Here is a bit of lspci output on my i.MX6QP machine (without the problem):
>>
>> mntmn@reform:~/code/linux$ lspci -v
>> 00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01) (prog-if 00
>>                                           ^^^^
>> [Normal decode])
>>         Flags: bus master, fast devsel, latency 0, IRQ 307
>>         Memory at 01000000 (32-bit, non-prefetchable) [size=1M]
>>         Bus: primary=00, secondary=01, subordinate=ff, sec-latency=0
>> ...
>>
>> The failure mode is that the PCIe controller shows up as a USB
>> controller and my ath9k wireless PCIe card cannot be assigned the
>> proper resources anymore (-ENOMEM even for very small BARs).
>>
>> Reverting this commit fixes the problem for me. I suggest to make the
>> check more specific to the platform/chipset you are targeting.
> So Synopsys apparently re-used Device ID 0xabcd?  That's a pretty bad problem.
>
> It looks like we merged 03e6742584af ("PCI: Override Synopsys USB 3.x
> HAPS device class") for v5.0, and v5.0-final hasn't been released yet,
> so if we don't hear from Thinh with a resolution, we can still revert
> it.
>
> I set myself a reminder to revert this on Feb 11, but hopefully we'll
> have a better resolution before then.
>
> Bjorn
>

This is really odd that the PID for the PCIe controller in i.MX6QP is
the same as PID Synopsys use for USB controller. We use a different set
of PIDs for PCIe controllers and track VID and PID in our releases.

Is the Root Complex (00:00.0) part of the SoC? Or is this a Synopsys
prototype connected to your board? If it is the latter, then the FPGA
configuration needs to be updated to other PID.

We're investigating the workaround in case this is on a taped-out SoC.

Thanks,
Thinh

Re: Linux Kernel Regression: HAPS quirk breaks PCIe on i.MX6QP

[Thinh Nguyen]

Hi Lukas,

Thinh Nguyen wrote:
> Bjorn Helgaas wrote:
>> [+cc linux-pci, linux-kernel]
>>
>> On Thu, Jan 31, 2019 at 11:21 AM Lukas F. Hartmann <lukas@mntmn.com> wrote:
>>> Hi Thinh,
>>>
>>> I'm writing you because you're the author in this commit:
>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_torvalds_linux_commit_03e6742584af8866ba014874669d0440bed3a623&d=DwIBaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=VKipRJmm95P2RbIxyKKYrcUCOGNlQtjlV-5zhrVhIik&m=2sOrowYXlsC3rl0LfHQhIZzImag0jFZXGvR6NIQDsh8&s=oJoBWRE8_LGAYbX2alh6QnjYZTTmzcgLw4MtOMToNyo&e=
>>>
>>> This quirk workaround breaks the PCIe on i.MX6QP, at least on my test
>>> devices. The reason is because the Synposys PCIe IP in i.MX6QP has the
>>> same device ID as the HAPS USB3 controller you are targeting in the
>>> commit: 0xabcd.
>>>
>>> Definition:
>>>
>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_torvalds_linux_blob_master_include_linux_pci-5Fids.h-23L2364&d=DwIBaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=VKipRJmm95P2RbIxyKKYrcUCOGNlQtjlV-5zhrVhIik&m=2sOrowYXlsC3rl0LfHQhIZzImag0jFZXGvR6NIQDsh8&s=-FVCwe81XNjJsYHYk1w-kdAzSOLunTGeNc73azO2QYw&e=
>>>
>>> Here is a bit of lspci output on my i.MX6QP machine (without the problem):
>>>
>>> mntmn@reform:~/code/linux$ lspci -v
>>> 00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01) (prog-if 00
>>>                                           ^^^^
>>> [Normal decode])
>>>         Flags: bus master, fast devsel, latency 0, IRQ 307
>>>         Memory at 01000000 (32-bit, non-prefetchable) [size=1M]
>>>         Bus: primary=00, secondary=01, subordinate=ff, sec-latency=0
>>> ...
>>>
>>> The failure mode is that the PCIe controller shows up as a USB
>>> controller and my ath9k wireless PCIe card cannot be assigned the
>>> proper resources anymore (-ENOMEM even for very small BARs).
>>>
>>> Reverting this commit fixes the problem for me. I suggest to make the
>>> check more specific to the platform/chipset you are targeting.
>> So Synopsys apparently re-used Device ID 0xabcd?  That's a pretty bad problem.
>>
>> It looks like we merged 03e6742584af ("PCI: Override Synopsys USB 3.x
>> HAPS device class") for v5.0, and v5.0-final hasn't been released yet,
>> so if we don't hear from Thinh with a resolution, we can still revert
>> it.
>>
>> I set myself a reminder to revert this on Feb 11, but hopefully we'll
>> have a better resolution before then.
>>
>> Bjorn
>>
> This is really odd that the PID for the PCIe controller in i.MX6QP is
> the same as PID Synopsys use for USB controller. We use a different set
> of PIDs for PCIe controllers and track VID and PID in our releases.
>
> Is the Root Complex (00:00.0) part of the SoC? Or is this a Synopsys
> prototype connected to your board? If it is the latter, then the FPGA
> configuration needs to be updated to other PID.
>
> We're investigating the workaround in case this is on a taped-out SoC.

Can you try to see if this will help?

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index b0a413f3f7ca..64623fbdd1e5 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -629,6 +629,9 @@ static void quirk_synopsys_haps(struct pci_dev *pdev)
 {
        u32 class = pdev->class;
 
+       if (class != PCI_CLASS_SERIAL_USB_XHCI)
+               return;
+
        switch (pdev->device) {
        case PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3:
        case PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3_AXI:

Thanks,
Thinh

Re: Linux Kernel Regression: HAPS quirk breaks PCIe on i.MX6QP

[Lukas Hartmann]

Hi Thinh,

this is in a taped out i.MX6QP 1.2GHz SoC, I also confirmed it with the 1.0GHz version, it has the same 0xabcd device id integrated.

I will try your patch ASAP.

Cheers
Lukas

> On 1. Feb 2019, at 00:46, Thinh Nguyen <thinh.nguyen@synopsys.com> wrote:
> 
> Hi Lukas,
> 
> Thinh Nguyen wrote:
>> Bjorn Helgaas wrote:
>>> [+cc linux-pci, linux-kernel]
>>> 
>>>> On Thu, Jan 31, 2019 at 11:21 AM Lukas F. Hartmann <lukas@mntmn.com> wrote:
>>>> Hi Thinh,
>>>> 
>>>> I'm writing you because you're the author in this commit:
>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_torvalds_linux_commit_03e6742584af8866ba014874669d0440bed3a623&d=DwIBaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=VKipRJmm95P2RbIxyKKYrcUCOGNlQtjlV-5zhrVhIik&m=2sOrowYXlsC3rl0LfHQhIZzImag0jFZXGvR6NIQDsh8&s=oJoBWRE8_LGAYbX2alh6QnjYZTTmzcgLw4MtOMToNyo&e=
>>>> 
>>>> This quirk workaround breaks the PCIe on i.MX6QP, at least on my test
>>>> devices. The reason is because the Synposys PCIe IP in i.MX6QP has the
>>>> same device ID as the HAPS USB3 controller you are targeting in the
>>>> commit: 0xabcd.
>>>> 
>>>> Definition:
>>>> 
>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_torvalds_linux_blob_master_include_linux_pci-5Fids.h-23L2364&d=DwIBaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=VKipRJmm95P2RbIxyKKYrcUCOGNlQtjlV-5zhrVhIik&m=2sOrowYXlsC3rl0LfHQhIZzImag0jFZXGvR6NIQDsh8&s=-FVCwe81XNjJsYHYk1w-kdAzSOLunTGeNc73azO2QYw&e=
>>>> 
>>>> Here is a bit of lspci output on my i.MX6QP machine (without the problem):
>>>> 
>>>> mntmn@reform:~/code/linux$ lspci -v
>>>> 00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01) (prog-if 00
>>>>                                          ^^^^
>>>> [Normal decode])
>>>>        Flags: bus master, fast devsel, latency 0, IRQ 307
>>>>        Memory at 01000000 (32-bit, non-prefetchable) [size=1M]
>>>>        Bus: primary=00, secondary=01, subordinate=ff, sec-latency=0
>>>> ...
>>>> 
>>>> The failure mode is that the PCIe controller shows up as a USB
>>>> controller and my ath9k wireless PCIe card cannot be assigned the
>>>> proper resources anymore (-ENOMEM even for very small BARs).
>>>> 
>>>> Reverting this commit fixes the problem for me. I suggest to make the
>>>> check more specific to the platform/chipset you are targeting.
>>> So Synopsys apparently re-used Device ID 0xabcd?  That's a pretty bad problem.
>>> 
>>> It looks like we merged 03e6742584af ("PCI: Override Synopsys USB 3.x
>>> HAPS device class") for v5.0, and v5.0-final hasn't been released yet,
>>> so if we don't hear from Thinh with a resolution, we can still revert
>>> it.
>>> 
>>> I set myself a reminder to revert this on Feb 11, but hopefully we'll
>>> have a better resolution before then.
>>> 
>>> Bjorn
>>> 
>> This is really odd that the PID for the PCIe controller in i.MX6QP is
>> the same as PID Synopsys use for USB controller. We use a different set
>> of PIDs for PCIe controllers and track VID and PID in our releases.
>> 
>> Is the Root Complex (00:00.0) part of the SoC? Or is this a Synopsys
>> prototype connected to your board? If it is the latter, then the FPGA
>> configuration needs to be updated to other PID.
>> 
>> We're investigating the workaround in case this is on a taped-out SoC.
> 
> Can you try to see if this will help?
> 
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index b0a413f3f7ca..64623fbdd1e5 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -629,6 +629,9 @@ static void quirk_synopsys_haps(struct pci_dev *pdev)
> {
>        u32 class = pdev->class;
> 
> +       if (class != PCI_CLASS_SERIAL_USB_XHCI)
> +               return;
> +
>        switch (pdev->device) {
>        case PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3:
>        case PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3_AXI:
> 
> Thanks,
> Thinh
> 
> 

Re: Linux Kernel Regression: HAPS quirk breaks PCIe on i.MX6QP

[Lukas Wunner]

On Thu, Jan 31, 2019 at 11:46:23PM +0000, Thinh Nguyen wrote:
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -629,6 +629,9 @@ static void quirk_synopsys_haps(struct pci_dev *pdev)
>  {
>         u32 class = pdev->class;
>  
> +       if (class != PCI_CLASS_SERIAL_USB_XHCI)
> +               return;
> +
>         switch (pdev->device) {
>         case PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3:
>         case PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3_AXI:

Please use DECLARE_PCI_FIXUP_CLASS_HEADER() instead.

Thanks,

Lukas

Re: Linux Kernel Regression: HAPS quirk breaks PCIe on i.MX6QP

[Thinh Nguyen]

Hi Lukas,

Lukas Hartmann wrote:
> Hi Thinh,
>
> this is in a taped out i.MX6QP 1.2GHz SoC, I also confirmed it with the 1.0GHz version, it has the same 0xabcd device id integrated.

I see.

> I will try your patch ASAP.

Thank you.
Thinh


>
>> On 1. Feb 2019, at 00:46, Thinh Nguyen <thinh.nguyen@synopsys.com> wrote:
>>
>> Hi Lukas,
>>
>> Thinh Nguyen wrote:
>>> Bjorn Helgaas wrote:
>>>> [+cc linux-pci, linux-kernel]
>>>>
>>>>> On Thu, Jan 31, 2019 at 11:21 AM Lukas F. Hartmann <lukas@mntmn.com> wrote:
>>>>> Hi Thinh,
>>>>>
>>>>> I'm writing you because you're the author in this commit:
>>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_torvalds_linux_commit_03e6742584af8866ba014874669d0440bed3a623&d=DwIBaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=VKipRJmm95P2RbIxyKKYrcUCOGNlQtjlV-5zhrVhIik&m=2sOrowYXlsC3rl0LfHQhIZzImag0jFZXGvR6NIQDsh8&s=oJoBWRE8_LGAYbX2alh6QnjYZTTmzcgLw4MtOMToNyo&e=
>>>>>
>>>>> This quirk workaround breaks the PCIe on i.MX6QP, at least on my test
>>>>> devices. The reason is because the Synposys PCIe IP in i.MX6QP has the
>>>>> same device ID as the HAPS USB3 controller you are targeting in the
>>>>> commit: 0xabcd.
>>>>>
>>>>> Definition:
>>>>>
>>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_torvalds_linux_blob_master_include_linux_pci-5Fids.h-23L2364&d=DwIBaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=VKipRJmm95P2RbIxyKKYrcUCOGNlQtjlV-5zhrVhIik&m=2sOrowYXlsC3rl0LfHQhIZzImag0jFZXGvR6NIQDsh8&s=-FVCwe81XNjJsYHYk1w-kdAzSOLunTGeNc73azO2QYw&e=
>>>>>
>>>>> Here is a bit of lspci output on my i.MX6QP machine (without the problem):
>>>>>
>>>>> mntmn@reform:~/code/linux$ lspci -v
>>>>> 00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01) (prog-if 00
>>>>>                                          ^^^^
>>>>> [Normal decode])
>>>>>        Flags: bus master, fast devsel, latency 0, IRQ 307
>>>>>        Memory at 01000000 (32-bit, non-prefetchable) [size=1M]
>>>>>        Bus: primary=00, secondary=01, subordinate=ff, sec-latency=0
>>>>> ...
>>>>>
>>>>> The failure mode is that the PCIe controller shows up as a USB
>>>>> controller and my ath9k wireless PCIe card cannot be assigned the
>>>>> proper resources anymore (-ENOMEM even for very small BARs).
>>>>>
>>>>> Reverting this commit fixes the problem for me. I suggest to make the
>>>>> check more specific to the platform/chipset you are targeting.
>>>> So Synopsys apparently re-used Device ID 0xabcd?  That's a pretty bad problem.
>>>>
>>>> It looks like we merged 03e6742584af ("PCI: Override Synopsys USB 3.x
>>>> HAPS device class") for v5.0, and v5.0-final hasn't been released yet,
>>>> so if we don't hear from Thinh with a resolution, we can still revert
>>>> it.
>>>>
>>>> I set myself a reminder to revert this on Feb 11, but hopefully we'll
>>>> have a better resolution before then.
>>>>
>>>> Bjorn
>>>>
>>> This is really odd that the PID for the PCIe controller in i.MX6QP is
>>> the same as PID Synopsys use for USB controller. We use a different set
>>> of PIDs for PCIe controllers and track VID and PID in our releases.
>>>
>>> Is the Root Complex (00:00.0) part of the SoC? Or is this a Synopsys
>>> prototype connected to your board? If it is the latter, then the FPGA
>>> configuration needs to be updated to other PID.
>>>

Re: Linux Kernel Regression: HAPS quirk breaks PCIe on i.MX6QP

[Thinh Nguyen]

Hi Lukas,

Lukas Wunner wrote:
> On Thu, Jan 31, 2019 at 11:46:23PM +0000, Thinh Nguyen wrote:
>> --- a/drivers/pci/quirks.c
>> +++ b/drivers/pci/quirks.c
>> @@ -629,6 +629,9 @@ static void quirk_synopsys_haps(struct pci_dev *pdev)
>>  {
>>         u32 class = pdev->class;
>>  
>> +       if (class != PCI_CLASS_SERIAL_USB_XHCI)
>> +               return;
>> +
>>         switch (pdev->device) {
>>         case PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3:
>>         case PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3_AXI:
> Please use DECLARE_PCI_FIXUP_CLASS_HEADER() instead.

Sure. That's a better option. Can you test this with your setup?

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: Linux Kernel Regression: HAPS quirk breaks PCIe on i.MX6QP

[Bjorn Helgaas]

On Fri, Feb 01, 2019 at 08:27:00PM +0000, Thinh Nguyen wrote:
> Lukas Wunner wrote:
> > On Thu, Jan 31, 2019 at 11:46:23PM +0000, Thinh Nguyen wrote:
> >> --- a/drivers/pci/quirks.c
> >> +++ b/drivers/pci/quirks.c
> >> @@ -629,6 +629,9 @@ static void quirk_synopsys_haps(struct pci_dev *pdev)
> >>  {
> >>         u32 class = pdev->class;
> >>  
> >> +       if (class != PCI_CLASS_SERIAL_USB_XHCI)
> >> +               return;
> >> +
> >>         switch (pdev->device) {
> >>         case PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3:
> >>         case PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3_AXI:
> > Please use DECLARE_PCI_FIXUP_CLASS_HEADER() instead.
> 
> Sure. That's a better option. Can you test this with your setup?
> 
> 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
> 
> 

Can we get a formal patch, including details about the issue (I assume
Synopsys released two different parts with Device ID 0xabcd) and a
signed-off-by?

I'd like to get this into for-linus as soon as possible for v5.0.

Bjorn

Re: Linux Kernel Regression: HAPS quirk breaks PCIe on i.MX6QP

[Thinh Nguyen]

Hi Bjorn,

Bjorn Helgaas wrote:
> On Fri, Feb 01, 2019 at 08:27:00PM +0000, Thinh Nguyen wrote:
>> Lukas Wunner wrote:
>>> On Thu, Jan 31, 2019 at 11:46:23PM +0000, Thinh Nguyen wrote:
>>>> --- a/drivers/pci/quirks.c
>>>> +++ b/drivers/pci/quirks.c
>>>> @@ -629,6 +629,9 @@ static void quirk_synopsys_haps(struct pci_dev *pdev)
>>>>  {
>>>>         u32 class = pdev->class;
>>>>  
>>>> +       if (class != PCI_CLASS_SERIAL_USB_XHCI)
>>>> +               return;
>>>> +
>>>>         switch (pdev->device) {
>>>>         case PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3:
>>>>         case PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3_AXI:
>>> Please use DECLARE_PCI_FIXUP_CLASS_HEADER() instead.
>> Sure. That's a better option. Can you test this with your setup?
>>
>> 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
>>
>>
> Can we get a formal patch, including details about the issue (I assume
> Synopsys released two different parts with Device ID 0xabcd) and a
> signed-off-by?
>
> I'd like to get this into for-linus as soon as possible for v5.0.
>

I already submitted a patch for this. Please review patch subject
"[PATCH] PCI: Check for USB xHCI class for HAPS platform".

Thanks,
Thinh

Re: Linux Kernel Regression: HAPS quirk breaks PCIe on i.MX6QP

[Bjorn Helgaas]

On Tue, Feb 05, 2019 at 08:38:58PM +0000, Thinh Nguyen wrote:
> Hi Bjorn,
> 
> Bjorn Helgaas wrote:
> > On Fri, Feb 01, 2019 at 08:27:00PM +0000, Thinh Nguyen wrote:
> >> Lukas Wunner wrote:
> >>> On Thu, Jan 31, 2019 at 11:46:23PM +0000, Thinh Nguyen wrote:
> >>>> --- a/drivers/pci/quirks.c
> >>>> +++ b/drivers/pci/quirks.c
> >>>> @@ -629,6 +629,9 @@ static void quirk_synopsys_haps(struct pci_dev *pdev)
> >>>>  {
> >>>>         u32 class = pdev->class;
> >>>>  
> >>>> +       if (class != PCI_CLASS_SERIAL_USB_XHCI)
> >>>> +               return;
> >>>> +
> >>>>         switch (pdev->device) {
> >>>>         case PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3:
> >>>>         case PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3_AXI:
> >>> Please use DECLARE_PCI_FIXUP_CLASS_HEADER() instead.
> >> Sure. That's a better option. Can you test this with your setup?
> >>
> >> 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
> >>
> >>
> > Can we get a formal patch, including details about the issue (I assume
> > Synopsys released two different parts with Device ID 0xabcd) and a
> > signed-off-by?
> >
> > I'd like to get this into for-linus as soon as possible for v5.0.
> 
> I already submitted a patch for this. Please review patch subject
> "[PATCH] PCI: Check for USB xHCI class for HAPS platform".

OK, maybe I'm missing something.  I don't see it on the linux-pci list
[1] or the PCI patchwork [2] yet.  Maybe it's still making its way
through the mailing list servers.  If you have a URL to the archive or
patchwork, let me know.

Bjorn

[1] https://lore.kernel.org/linux-pci/
[2] https://patchwork.ozlabs.org/project/linux-pci/list

Re: Linux Kernel Regression: HAPS quirk breaks PCIe on i.MX6QP

[Thinh Nguyen]

Bjorn Helgaas wrote:
> On Tue, Feb 05, 2019 at 08:38:58PM +0000, Thinh Nguyen wrote:
>> Hi Bjorn,
>>
>> Bjorn Helgaas wrote:
>>> On Fri, Feb 01, 2019 at 08:27:00PM +0000, Thinh Nguyen wrote:
>>>> Lukas Wunner wrote:
>>>>> On Thu, Jan 31, 2019 at 11:46:23PM +0000, Thinh Nguyen wrote:
>>>>>> --- a/drivers/pci/quirks.c
>>>>>> +++ b/drivers/pci/quirks.c
>>>>>> @@ -629,6 +629,9 @@ static void quirk_synopsys_haps(struct pci_dev *pdev)
>>>>>>  {
>>>>>>         u32 class = pdev->class;
>>>>>>  
>>>>>> +       if (class != PCI_CLASS_SERIAL_USB_XHCI)
>>>>>> +               return;
>>>>>> +
>>>>>>         switch (pdev->device) {
>>>>>>         case PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3:
>>>>>>         case PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3_AXI:
>>>>> Please use DECLARE_PCI_FIXUP_CLASS_HEADER() instead.
>>>> Sure. That's a better option. Can you test this with your setup?
>>>>
>>>> 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
>>>>
>>>>
>>> Can we get a formal patch, including details about the issue (I assume
>>> Synopsys released two different parts with Device ID 0xabcd) and a
>>> signed-off-by?
>>>
>>> I'd like to get this into for-linus as soon as possible for v5.0.
>> I already submitted a patch for this. Please review patch subject
>> "[PATCH] PCI: Check for USB xHCI class for HAPS platform".
> OK, maybe I'm missing something.  I don't see it on the linux-pci list
> [1] or the PCI patchwork [2] yet.  Maybe it's still making its way
> through the mailing list servers.  If you have a URL to the archive or
> patchwork, let me know.
>
> Bjorn
>
> [1] https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_linux-2Dpci_&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=u9FYoxKtyhjrGFcyixFYqTjw1ZX0VsG2d8FCmzkTY-w&m=gf8tJEWOt92UvWl-0yPhkFlhOXuG0fn-pG0zPdqsHv8&s=vQgsCLqN63Bd-d7ZMY5HPVmBa4Htnz64oO96cG-6pNA&e=
> [2] https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_project_linux-2Dpci_list&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=u9FYoxKtyhjrGFcyixFYqTjw1ZX0VsG2d8FCmzkTY-w&m=gf8tJEWOt92UvWl-0yPhkFlhOXuG0fn-pG0zPdqsHv8&s=Tbm0SOa0CCxMxzGcwT6YJrYYzVEU6x7sR8JB9l82VGg&e=
>

That's odd. I sent it yesterday. I'll resend it then.

Thanks,
Thinh

Re: Linux Kernel Regression: HAPS quirk breaks PCIe on i.MX6QP

[Thinh Nguyen]

Thinh Nguyen wrote:
> Bjorn Helgaas wrote:
>> On Tue, Feb 05, 2019 at 08:38:58PM +0000, Thinh Nguyen wrote:
>>> Hi Bjorn,
>>>
>>> Bjorn Helgaas wrote:
>>>> On Fri, Feb 01, 2019 at 08:27:00PM +0000, Thinh Nguyen wrote:
>>>>> Lukas Wunner wrote:
>>>>>> On Thu, Jan 31, 2019 at 11:46:23PM +0000, Thinh Nguyen wrote:
>>>>>>> --- a/drivers/pci/quirks.c
>>>>>>> +++ b/drivers/pci/quirks.c
>>>>>>> @@ -629,6 +629,9 @@ static void quirk_synopsys_haps(struct pci_dev *pdev)
>>>>>>>  {
>>>>>>>         u32 class = pdev->class;
>>>>>>>  
>>>>>>> +       if (class != PCI_CLASS_SERIAL_USB_XHCI)
>>>>>>> +               return;
>>>>>>> +
>>>>>>>         switch (pdev->device) {
>>>>>>>         case PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3:
>>>>>>>         case PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3_AXI:
>>>>>> Please use DECLARE_PCI_FIXUP_CLASS_HEADER() instead.
>>>>> Sure. That's a better option. Can you test this with your setup?
>>>>>
>>>>> 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
>>>>>
>>>>>
>>>> Can we get a formal patch, including details about the issue (I assume
>>>> Synopsys released two different parts with Device ID 0xabcd) and a
>>>> signed-off-by?
>>>>
>>>> I'd like to get this into for-linus as soon as possible for v5.0.
>>> I already submitted a patch for this. Please review patch subject
>>> "[PATCH] PCI: Check for USB xHCI class for HAPS platform".
>> OK, maybe I'm missing something.  I don't see it on the linux-pci list
>> [1] or the PCI patchwork [2] yet.  Maybe it's still making its way
>> through the mailing list servers.  If you have a URL to the archive or
>> patchwork, let me know.
>>
>> Bjorn
>>
>> [1] https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_linux-2Dpci_&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=u9FYoxKtyhjrGFcyixFYqTjw1ZX0VsG2d8FCmzkTY-w&m=gf8tJEWOt92UvWl-0yPhkFlhOXuG0fn-pG0zPdqsHv8&s=vQgsCLqN63Bd-d7ZMY5HPVmBa4Htnz64oO96cG-6pNA&e=
>> [2] https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_project_linux-2Dpci_list&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=u9FYoxKtyhjrGFcyixFYqTjw1ZX0VsG2d8FCmzkTY-w&m=gf8tJEWOt92UvWl-0yPhkFlhOXuG0fn-pG0zPdqsHv8&s=Tbm0SOa0CCxMxzGcwT6YJrYYzVEU6x7sR8JB9l82VGg&e=

After I removed some names in the CC list, it looks like it appears on
PCI patchwork now.
https://patchwork.ozlabs.org/patch/1037211/

Thanks,
Thinh