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

login
register
mail settings
Submitter Thinh Nguyen
Date Feb. 1, 2019, 8:27 p.m.
Message ID <30102591E157244384E984126FC3CB4F639BF351@us01wembx1.internal.synopsys.com>
Download mbox | patch
Permalink /patch/716251/
State New
Headers show

Comments

Thinh Nguyen - Feb. 1, 2019, 8:27 p.m.
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?



Thanks,
Thinh
Bjorn Helgaas - Feb. 5, 2019, 8:09 p.m.
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
Thinh Nguyen - Feb. 5, 2019, 8:38 p.m.
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
Bjorn Helgaas - Feb. 5, 2019, 8:59 p.m.
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
Thinh Nguyen - Feb. 5, 2019, 9:02 p.m.
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
Thinh Nguyen - Feb. 5, 2019, 9:33 p.m.
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

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