linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: Regression: Inoperative bluetooth, Intel chipset, mainline kernel 6.6.2+
       [not found]     ` <6a710423-e76c-437e-ba59-b9cefbda3194@moonlit-rail.com>
@ 2023-12-02  7:23       ` Paul Menzel
  2023-12-02  7:50         ` Greg KH
  0 siblings, 1 reply; 22+ messages in thread
From: Paul Menzel @ 2023-12-02  7:23 UTC (permalink / raw)
  To: Basavaraj Natikar, Kris Karas (Bug Reporting)
  Cc: Greg KH, stable, Thorsten Leemhuis, regressions, linux-bluetooth,
	Mario Limonciello, Mathias Nyman, linux-usb

[Cc: +Mario, Mathias, linux-usb]

Am 02.12.23 um 07:43 schrieb Kris Karas (Bug Reporting):
> Greg KH wrote:
>> On Fri, Dec 01, 2023 at 07:33:03AM +0100, Thorsten Leemhuis wrote:
>>> CCing a few lists and people. Greg is among them, who might know if this
>>> is a known issue that 6.6.4-rc1 et. al. might already fix.
>>
>> Not known to me, bisection is needed so we can track down the problem
>> please.
> 
> And the winner is...
> 
>> commit 14a51fa544225deb9ac2f1f9f3c10dedb29f5d2f
>> Author: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
>> Date:   Thu Oct 19 13:29:19 2023 +0300
>>
>>     xhci: Loosen RPM as default policy to cover for AMD xHC 1.1
>> >>     [ Upstream commit 4baf1218150985ee3ab0a27220456a1f027ea0ac ]
>>
>>     The AMD USB host controller (1022:43f7) isn't going into PCI D3 by default
>>     without anything connected. This is because the policy that was introduced
>>     by commit a611bf473d1f ("xhci-pci: Set runtime PM as default policy on all
>>     xHC 1.2 or later devices") only covered 1.2 or later.
>> [ snip ]
>> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
>> index b9ae5c2a2527..bde43cef8846 100644
>> --- a/drivers/usb/host/xhci-pci.c
>> +++ b/drivers/usb/host/xhci-pci.c
>> @@ -535,6 +535,8 @@ static void xhci_pci_quirks(struct device *dev, 
>> struct xhci_hcd *xhci)
>>         /* xHC spec requires PCI devices to support D3hot and D3cold */
>>         if (xhci->hci_version >= 0x120)
>>                 xhci->quirks |= XHCI_DEFAULT_PM_RUNTIME_ALLOW;
>> +       else if (pdev->vendor == PCI_VENDOR_ID_AMD && xhci->hci_version >= 0x110)
>> +               xhci->quirks |= XHCI_DEFAULT_PM_RUNTIME_ALLOW;
>>
>>         if (xhci->quirks & XHCI_RESET_ON_RESUME)
>>                 xhci_dbg_trace(xhci, trace_xhci_dbg_quirks,
> 
> 
> Huh, OK, I was expecting this to be a patch made to the bluetooth code, 
> as it caused bluetoothd to bomb with "opcode 0x0c03 failed".  But I just 
> verified I did the bisect correctly by backing this two-liner out of 
> vanilla 6.6.3, and bluetooth returned to normal operation.  Huzzah!
> 
> Just a brief recap:
> 
> This bug appears to be rather hardware-specific, as only a few folks 
> have reported it.  In my case, the hardware is an ASrock "X470 Taichi" 
> motherboard, and its on-board bluetooth hardware, reporting itself as:
> lspci: 0f:00.3 USB controller: Advanced Micro Devices, Inc. [AMD] Zeppelin USB 3.0 xHCI Compliant Host Controller
> lsusb: ID 8087:0aa7 Intel Corp. Wireless-AC 3168 Bluetooth
> 
> When Basavaraj's patch is applied (in mainline 6.6.2+), bluetooth stops 
> functioning on my motherboard.
> 
> Originally from bugzilla #218142 [1]
[1]: https://bugzilla.kernel.org/show_bug.cgi?id=218142

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Regression: Inoperative bluetooth, Intel chipset, mainline kernel 6.6.2+
  2023-12-02  7:23       ` Regression: Inoperative bluetooth, Intel chipset, mainline kernel 6.6.2+ Paul Menzel
@ 2023-12-02  7:50         ` Greg KH
  2023-12-02  7:58           ` Kris Karas (Bug Reporting)
  0 siblings, 1 reply; 22+ messages in thread
From: Greg KH @ 2023-12-02  7:50 UTC (permalink / raw)
  To: Paul Menzel
  Cc: Basavaraj Natikar, Kris Karas (Bug Reporting), stable,
	Thorsten Leemhuis, regressions, linux-bluetooth,
	Mario Limonciello, Mathias Nyman, linux-usb

On Sat, Dec 02, 2023 at 08:23:55AM +0100, Paul Menzel wrote:
> [Cc: +Mario, Mathias, linux-usb]
> 
> Am 02.12.23 um 07:43 schrieb Kris Karas (Bug Reporting):
> > Greg KH wrote:
> > > On Fri, Dec 01, 2023 at 07:33:03AM +0100, Thorsten Leemhuis wrote:
> > > > CCing a few lists and people. Greg is among them, who might know if this
> > > > is a known issue that 6.6.4-rc1 et. al. might already fix.
> > > 
> > > Not known to me, bisection is needed so we can track down the problem
> > > please.
> > 
> > And the winner is...
> > 
> > > commit 14a51fa544225deb9ac2f1f9f3c10dedb29f5d2f
> > > Author: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
> > > Date:   Thu Oct 19 13:29:19 2023 +0300
> > > 
> > >     xhci: Loosen RPM as default policy to cover for AMD xHC 1.1
> > > >>     [ Upstream commit 4baf1218150985ee3ab0a27220456a1f027ea0ac ]
> > > 
> > >     The AMD USB host controller (1022:43f7) isn't going into PCI D3 by default
> > >     without anything connected. This is because the policy that was introduced
> > >     by commit a611bf473d1f ("xhci-pci: Set runtime PM as default policy on all
> > >     xHC 1.2 or later devices") only covered 1.2 or later.
> > > [ snip ]
> > > diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> > > index b9ae5c2a2527..bde43cef8846 100644
> > > --- a/drivers/usb/host/xhci-pci.c
> > > +++ b/drivers/usb/host/xhci-pci.c
> > > @@ -535,6 +535,8 @@ static void xhci_pci_quirks(struct device *dev,
> > > struct xhci_hcd *xhci)
> > >         /* xHC spec requires PCI devices to support D3hot and D3cold */
> > >         if (xhci->hci_version >= 0x120)
> > >                 xhci->quirks |= XHCI_DEFAULT_PM_RUNTIME_ALLOW;
> > > +       else if (pdev->vendor == PCI_VENDOR_ID_AMD && xhci->hci_version >= 0x110)
> > > +               xhci->quirks |= XHCI_DEFAULT_PM_RUNTIME_ALLOW;
> > > 
> > >         if (xhci->quirks & XHCI_RESET_ON_RESUME)
> > >                 xhci_dbg_trace(xhci, trace_xhci_dbg_quirks,
> > 
> > 
> > Huh, OK, I was expecting this to be a patch made to the bluetooth code,
> > as it caused bluetoothd to bomb with "opcode 0x0c03 failed".  But I just
> > verified I did the bisect correctly by backing this two-liner out of
> > vanilla 6.6.3, and bluetooth returned to normal operation.  Huzzah!
> > 
> > Just a brief recap:
> > 
> > This bug appears to be rather hardware-specific, as only a few folks
> > have reported it.  In my case, the hardware is an ASrock "X470 Taichi"
> > motherboard, and its on-board bluetooth hardware, reporting itself as:
> > lspci: 0f:00.3 USB controller: Advanced Micro Devices, Inc. [AMD] Zeppelin USB 3.0 xHCI Compliant Host Controller
> > lsusb: ID 8087:0aa7 Intel Corp. Wireless-AC 3168 Bluetooth
> > 
> > When Basavaraj's patch is applied (in mainline 6.6.2+), bluetooth stops
> > functioning on my motherboard.
> > 
> > Originally from bugzilla #218142 [1]
> [1]: https://bugzilla.kernel.org/show_bug.cgi?id=218142

Should already be fixed in the 6.6.3 release, can you please verify that
this is broken there?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Regression: Inoperative bluetooth, Intel chipset, mainline kernel 6.6.2+
  2023-12-02  7:50         ` Greg KH
@ 2023-12-02  7:58           ` Kris Karas (Bug Reporting)
  2023-12-02  8:15             ` Greg KH
  0 siblings, 1 reply; 22+ messages in thread
From: Kris Karas (Bug Reporting) @ 2023-12-02  7:58 UTC (permalink / raw)
  To: Greg KH, Paul Menzel
  Cc: Basavaraj Natikar, stable, Thorsten Leemhuis, regressions,
	linux-bluetooth, Mario Limonciello, Mathias Nyman, linux-usb

Greg KH wrote:
>> Am 02.12.23 um 07:43 schrieb Kris Karas (Bug Reporting):
>>> When Basavaraj's patch is applied (in mainline 6.6.2+), bluetooth stops
>>> functioning on my motherboard.
>>>
>>> Originally from bugzilla #218142 [1]
>> [1]: https://bugzilla.kernel.org/show_bug.cgi?id=218142
> 
> Should already be fixed in the 6.6.3 release, can you please verify that
> this is broken there?

Double-checked and confirmed.  6.6.3 shows the bug (hci0: Opcode 0x0c03 
failed: -110) and my currently-running system (6.6.3 with 
14a51fa544225deb9ac2f1f9f3c10dedb29f5d2f backed out) is running fine 
(with its MX Master 3S bluetooth mouse).

Cheers,
Kris

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Regression: Inoperative bluetooth, Intel chipset, mainline kernel 6.6.2+
  2023-12-02  7:58           ` Kris Karas (Bug Reporting)
@ 2023-12-02  8:15             ` Greg KH
  2023-12-03  8:32               ` Kris Karas (Bug Reporting)
  0 siblings, 1 reply; 22+ messages in thread
From: Greg KH @ 2023-12-02  8:15 UTC (permalink / raw)
  To: Kris Karas (Bug Reporting)
  Cc: Paul Menzel, Basavaraj Natikar, stable, Thorsten Leemhuis,
	regressions, linux-bluetooth, Mario Limonciello, Mathias Nyman,
	linux-usb

On Sat, Dec 02, 2023 at 02:58:32AM -0500, Kris Karas (Bug Reporting) wrote:
> Greg KH wrote:
> > > Am 02.12.23 um 07:43 schrieb Kris Karas (Bug Reporting):
> > > > When Basavaraj's patch is applied (in mainline 6.6.2+), bluetooth stops
> > > > functioning on my motherboard.
> > > > 
> > > > Originally from bugzilla #218142 [1]
> > > [1]: https://bugzilla.kernel.org/show_bug.cgi?id=218142
> > 
> > Should already be fixed in the 6.6.3 release, can you please verify that
> > this is broken there?
> 
> Double-checked and confirmed.  6.6.3 shows the bug (hci0: Opcode 0x0c03
> failed: -110) and my currently-running system (6.6.3 with
> 14a51fa544225deb9ac2f1f9f3c10dedb29f5d2f backed out) is running fine (with
> its MX Master 3S bluetooth mouse).

Thanks for testing, any chance you can try 6.6.4-rc1?  Or wait a few
hours for me to release 6.6.4 if you don't want to mess with a -rc
release.

Also, is this showing up in 6.7-rc3?  If so, that would be a big help in
tracking this down.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Regression: Inoperative bluetooth, Intel chipset, mainline kernel 6.6.2+
  2023-12-02  8:15             ` Greg KH
@ 2023-12-03  8:32               ` Kris Karas (Bug Reporting)
  2023-12-03  8:38                 ` Greg KH
  0 siblings, 1 reply; 22+ messages in thread
From: Kris Karas (Bug Reporting) @ 2023-12-03  8:32 UTC (permalink / raw)
  To: Greg KH
  Cc: Paul Menzel, Basavaraj Natikar, stable, Thorsten Leemhuis,
	regressions, linux-bluetooth, Mario Limonciello, Mathias Nyman,
	linux-usb

Greg KH wrote:
> Thanks for testing, any chance you can try 6.6.4-rc1?  Or wait a few
> hours for me to release 6.6.4 if you don't want to mess with a -rc
> release.

As I mentioned to Greg off-list (to save wasting other peoples' 
bandwidth), I couldn't find 6.6.4-rc1.  Looking in wrong git tree?  But 
6.6.4 is now out, which I have tested and am running at the moment, 
albeit with the problem commit from 6.6.2 backed out.

There is no change with respect to this bug.  The problematic patch 
introduced in 6.6.2 was neither reverted nor amended.  The "opcode 
0x0c03 failed" lines to the kernel log continue to be present.

> Also, is this showing up in 6.7-rc3?  If so, that would be a big help in
> tracking this down.

The bug shows up in 6.7-rc3 as well, exactly as it does here in 6.6.2+ 
and in 6.1.63+.  The problematic patch bisected earlier appears 
identically (and seems to have been introduced simultaneously) in these 
recent releases.

Kris

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Regression: Inoperative bluetooth, Intel chipset, mainline kernel 6.6.2+
  2023-12-03  8:32               ` Kris Karas (Bug Reporting)
@ 2023-12-03  8:38                 ` Greg KH
  2023-12-03 16:16                   ` Basavaraj Natikar
  2023-12-04  9:12                   ` Mathias Nyman
  0 siblings, 2 replies; 22+ messages in thread
From: Greg KH @ 2023-12-03  8:38 UTC (permalink / raw)
  To: Kris Karas (Bug Reporting)
  Cc: Paul Menzel, Basavaraj Natikar, stable, Thorsten Leemhuis,
	regressions, linux-bluetooth, Mario Limonciello, Mathias Nyman,
	linux-usb

On Sun, Dec 03, 2023 at 03:32:52AM -0500, Kris Karas (Bug Reporting) wrote:
> Greg KH wrote:
> > Thanks for testing, any chance you can try 6.6.4-rc1?  Or wait a few
> > hours for me to release 6.6.4 if you don't want to mess with a -rc
> > release.
> 
> As I mentioned to Greg off-list (to save wasting other peoples' bandwidth),
> I couldn't find 6.6.4-rc1.  Looking in wrong git tree?  But 6.6.4 is now
> out, which I have tested and am running at the moment, albeit with the
> problem commit from 6.6.2 backed out.
> 
> There is no change with respect to this bug.  The problematic patch
> introduced in 6.6.2 was neither reverted nor amended.  The "opcode 0x0c03
> failed" lines to the kernel log continue to be present.
> 
> > Also, is this showing up in 6.7-rc3?  If so, that would be a big help in
> > tracking this down.
> 
> The bug shows up in 6.7-rc3 as well, exactly as it does here in 6.6.2+ and
> in 6.1.63+.  The problematic patch bisected earlier appears identically (and
> seems to have been introduced simultaneously) in these recent releases.

Ok, in a way, this is good as that means I haven't missed a fix, but bad
in that this does affect everyone more.

So let's start over, you found the offending commit, and nothing has
fixed it, so what do we do?  xhci/amd developers, any ideas?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Regression: Inoperative bluetooth, Intel chipset, mainline kernel 6.6.2+
  2023-12-03  8:38                 ` Greg KH
@ 2023-12-03 16:16                   ` Basavaraj Natikar
  2023-12-03 16:24                     ` Basavaraj Natikar
  2023-12-04  9:12                   ` Mathias Nyman
  1 sibling, 1 reply; 22+ messages in thread
From: Basavaraj Natikar @ 2023-12-03 16:16 UTC (permalink / raw)
  To: Greg KH, Kris Karas (Bug Reporting)
  Cc: Paul Menzel, Basavaraj Natikar, stable, Thorsten Leemhuis,
	regressions, linux-bluetooth, Mario Limonciello, Mathias Nyman,
	linux-usb


On 12/3/2023 2:08 PM, Greg KH wrote:
> On Sun, Dec 03, 2023 at 03:32:52AM -0500, Kris Karas (Bug Reporting) wrote:
>> Greg KH wrote:
>>> Thanks for testing, any chance you can try 6.6.4-rc1?  Or wait a few
>>> hours for me to release 6.6.4 if you don't want to mess with a -rc
>>> release.
>> As I mentioned to Greg off-list (to save wasting other peoples' bandwidth),
>> I couldn't find 6.6.4-rc1.  Looking in wrong git tree?  But 6.6.4 is now
>> out, which I have tested and am running at the moment, albeit with the
>> problem commit from 6.6.2 backed out.
>>
>> There is no change with respect to this bug.  The problematic patch
>> introduced in 6.6.2 was neither reverted nor amended.  The "opcode 0x0c03
>> failed" lines to the kernel log continue to be present.
>>
>>> Also, is this showing up in 6.7-rc3?  If so, that would be a big help in
>>> tracking this down.
>> The bug shows up in 6.7-rc3 as well, exactly as it does here in 6.6.2+ and
>> in 6.1.63+.  The problematic patch bisected earlier appears identically (and
>> seems to have been introduced simultaneously) in these recent releases.
> Ok, in a way, this is good as that means I haven't missed a fix, but bad
> in that this does affect everyone more.
>
> So let's start over, you found the offending commit, and nothing has
> fixed it, so what do we do?  xhci/amd developers, any ideas?

Can we enable RPM on specific controllers for AMD xHC 1.1
instead to cover all AMD xHC 1.1? 

Please find below the proposed changes and let me know if it is OK?
 
Author: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
Date:   Sun Dec 3 18:28:27 2023 +0530

    xhci: Remove RPM as default policy to cover AMD xHC 1.1

    xHC 1.1 runtime PM as default policy causes issues on few AMD controllers.
    Hence remove RPM as default policy to cover AMD xHC 1.1 and add only
    AMD USB host controller (1022:43f7) which has RPM support. 

    Fixes: 4baf12181509 ("xhci: Loosen RPM as default policy to cover for AMD xHC 1.1")
    Link: https://lore.kernel.org/all/2023120329-length-strum-9ee1@gregkh
    Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 95ed9404f6f8..7ffd6b8227cc 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -535,7 +535,7 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
        /* xHC spec requires PCI devices to support D3hot and D3cold */
        if (xhci->hci_version >= 0x120)
                xhci->quirks |= XHCI_DEFAULT_PM_RUNTIME_ALLOW;
-       else if (pdev->vendor == PCI_VENDOR_ID_AMD && xhci->hci_version >= 0x110)
+       else if (pdev->vendor == PCI_VENDOR_ID_AMD && pdev->vendor == 0x43f7)
                xhci->quirks |= XHCI_DEFAULT_PM_RUNTIME_ALLOW;

        if (xhci->quirks & XHCI_RESET_ON_RESUME)

Thanks,
--
Basavaraj

>
> thanks,
>
> greg k-h


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: Regression: Inoperative bluetooth, Intel chipset, mainline kernel 6.6.2+
  2023-12-03 16:16                   ` Basavaraj Natikar
@ 2023-12-03 16:24                     ` Basavaraj Natikar
  2023-12-03 19:52                       ` Oleksandr Natalenko
  0 siblings, 1 reply; 22+ messages in thread
From: Basavaraj Natikar @ 2023-12-03 16:24 UTC (permalink / raw)
  To: Greg KH, Kris Karas (Bug Reporting)
  Cc: Paul Menzel, Basavaraj Natikar, stable, Thorsten Leemhuis,
	regressions, linux-bluetooth, Mario Limonciello, Mathias Nyman,
	linux-usb


On 12/3/2023 9:46 PM, Basavaraj Natikar wrote:
> On 12/3/2023 2:08 PM, Greg KH wrote:
>> On Sun, Dec 03, 2023 at 03:32:52AM -0500, Kris Karas (Bug Reporting) wrote:
>>> Greg KH wrote:
>>>> Thanks for testing, any chance you can try 6.6.4-rc1?  Or wait a few
>>>> hours for me to release 6.6.4 if you don't want to mess with a -rc
>>>> release.
>>> As I mentioned to Greg off-list (to save wasting other peoples' bandwidth),
>>> I couldn't find 6.6.4-rc1.  Looking in wrong git tree?  But 6.6.4 is now
>>> out, which I have tested and am running at the moment, albeit with the
>>> problem commit from 6.6.2 backed out.
>>>
>>> There is no change with respect to this bug.  The problematic patch
>>> introduced in 6.6.2 was neither reverted nor amended.  The "opcode 0x0c03
>>> failed" lines to the kernel log continue to be present.
>>>
>>>> Also, is this showing up in 6.7-rc3?  If so, that would be a big help in
>>>> tracking this down.
>>> The bug shows up in 6.7-rc3 as well, exactly as it does here in 6.6.2+ and
>>> in 6.1.63+.  The problematic patch bisected earlier appears identically (and
>>> seems to have been introduced simultaneously) in these recent releases.
>> Ok, in a way, this is good as that means I haven't missed a fix, but bad
>> in that this does affect everyone more.
>>
>> So let's start over, you found the offending commit, and nothing has
>> fixed it, so what do we do?  xhci/amd developers, any ideas?
> Can we enable RPM on specific controllers for AMD xHC 1.1
> instead to cover all AMD xHC 1.1? 
>
> Please find below the proposed changes and let me know if it is OK?
>  
> Author: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
> Date:   Sun Dec 3 18:28:27 2023 +0530
>
>     xhci: Remove RPM as default policy to cover AMD xHC 1.1
>
>     xHC 1.1 runtime PM as default policy causes issues on few AMD controllers.
>     Hence remove RPM as default policy to cover AMD xHC 1.1 and add only
>     AMD USB host controller (1022:43f7) which has RPM support. 
>
>     Fixes: 4baf12181509 ("xhci: Loosen RPM as default policy to cover for AMD xHC 1.1")
>     Link: https://lore.kernel.org/all/2023120329-length-strum-9ee1@gregkh
>     Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
>
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index 95ed9404f6f8..7ffd6b8227cc 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -535,7 +535,7 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
>         /* xHC spec requires PCI devices to support D3hot and D3cold */
>         if (xhci->hci_version >= 0x120)
>                 xhci->quirks |= XHCI_DEFAULT_PM_RUNTIME_ALLOW;
> -       else if (pdev->vendor == PCI_VENDOR_ID_AMD && xhci->hci_version >= 0x110)
> +       else if (pdev->vendor == PCI_VENDOR_ID_AMD && pdev->vendor == 0x43f7)

sorry its 
pdev->device == 0x43f7

Incorrect ---> else if (pdev->vendor == PCI_VENDOR_ID_AMD && pdev->vendor == 0x43f7)
correct line --> else if (pdev->vendor == PCI_VENDOR_ID_AMD && pdev->device == 0x43f7)

>                 xhci->quirks |= XHCI_DEFAULT_PM_RUNTIME_ALLOW;
>
>         if (xhci->quirks & XHCI_RESET_ON_RESUME)
>
> Thanks,
> --
> Basavaraj
>
>> thanks,
>>
>> greg k-h


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Regression: Inoperative bluetooth, Intel chipset, mainline kernel 6.6.2+
  2023-12-03 16:24                     ` Basavaraj Natikar
@ 2023-12-03 19:52                       ` Oleksandr Natalenko
  0 siblings, 0 replies; 22+ messages in thread
From: Oleksandr Natalenko @ 2023-12-03 19:52 UTC (permalink / raw)
  To: Greg KH, Kris Karas (Bug Reporting), Basavaraj Natikar
  Cc: Paul Menzel, Basavaraj Natikar, stable, Thorsten Leemhuis,
	regressions, linux-bluetooth, Mario Limonciello, Mathias Nyman,
	linux-usb

[-- Attachment #1: Type: text/plain, Size: 5236 bytes --]

Hello.

On neděle 3. prosince 2023 17:24:28 CET Basavaraj Natikar wrote:
> 
> On 12/3/2023 9:46 PM, Basavaraj Natikar wrote:
> > On 12/3/2023 2:08 PM, Greg KH wrote:
> >> On Sun, Dec 03, 2023 at 03:32:52AM -0500, Kris Karas (Bug Reporting) wrote:
> >>> Greg KH wrote:
> >>>> Thanks for testing, any chance you can try 6.6.4-rc1?  Or wait a few
> >>>> hours for me to release 6.6.4 if you don't want to mess with a -rc
> >>>> release.
> >>> As I mentioned to Greg off-list (to save wasting other peoples' bandwidth),
> >>> I couldn't find 6.6.4-rc1.  Looking in wrong git tree?  But 6.6.4 is now
> >>> out, which I have tested and am running at the moment, albeit with the
> >>> problem commit from 6.6.2 backed out.
> >>>
> >>> There is no change with respect to this bug.  The problematic patch
> >>> introduced in 6.6.2 was neither reverted nor amended.  The "opcode 0x0c03
> >>> failed" lines to the kernel log continue to be present.
> >>>
> >>>> Also, is this showing up in 6.7-rc3?  If so, that would be a big help in
> >>>> tracking this down.
> >>> The bug shows up in 6.7-rc3 as well, exactly as it does here in 6.6.2+ and
> >>> in 6.1.63+.  The problematic patch bisected earlier appears identically (and
> >>> seems to have been introduced simultaneously) in these recent releases.
> >> Ok, in a way, this is good as that means I haven't missed a fix, but bad
> >> in that this does affect everyone more.
> >>
> >> So let's start over, you found the offending commit, and nothing has
> >> fixed it, so what do we do?  xhci/amd developers, any ideas?
> > Can we enable RPM on specific controllers for AMD xHC 1.1
> > instead to cover all AMD xHC 1.1? 
> >
> > Please find below the proposed changes and let me know if it is OK?
> >  
> > Author: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
> > Date:   Sun Dec 3 18:28:27 2023 +0530
> >
> >     xhci: Remove RPM as default policy to cover AMD xHC 1.1
> >
> >     xHC 1.1 runtime PM as default policy causes issues on few AMD controllers.
> >     Hence remove RPM as default policy to cover AMD xHC 1.1 and add only
> >     AMD USB host controller (1022:43f7) which has RPM support. 
> >
> >     Fixes: 4baf12181509 ("xhci: Loosen RPM as default policy to cover for AMD xHC 1.1")
> >     Link: https://lore.kernel.org/all/2023120329-length-strum-9ee1@gregkh
> >     Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
> >
> > diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> > index 95ed9404f6f8..7ffd6b8227cc 100644
> > --- a/drivers/usb/host/xhci-pci.c
> > +++ b/drivers/usb/host/xhci-pci.c
> > @@ -535,7 +535,7 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
> >         /* xHC spec requires PCI devices to support D3hot and D3cold */
> >         if (xhci->hci_version >= 0x120)
> >                 xhci->quirks |= XHCI_DEFAULT_PM_RUNTIME_ALLOW;
> > -       else if (pdev->vendor == PCI_VENDOR_ID_AMD && xhci->hci_version >= 0x110)
> > +       else if (pdev->vendor == PCI_VENDOR_ID_AMD && pdev->vendor == 0x43f7)
> 
> sorry its 
> pdev->device == 0x43f7
> 
> Incorrect ---> else if (pdev->vendor == PCI_VENDOR_ID_AMD && pdev->vendor == 0x43f7)
> correct line --> else if (pdev->vendor == PCI_VENDOR_ID_AMD && pdev->device == 0x43f7)
> 
> >                 xhci->quirks |= XHCI_DEFAULT_PM_RUNTIME_ALLOW;
> >
> >         if (xhci->quirks & XHCI_RESET_ON_RESUME)

Given the following hardware:

[~]> lspci -nn | grep -i usb
06:00.4 USB controller [0c03]: Realtek Semiconductor Co., Ltd. RTL811x EHCI host controller [10ec:816d] (rev 1a)
07:00.1 USB controller [0c03]: Advanced Micro Devices, Inc. [AMD] Matisse USB 3.0 Host Controller [1022:149c]
07:00.3 USB controller [0c03]: Advanced Micro Devices, Inc. [AMD] Matisse USB 3.0 Host Controller [1022:149c]
0f:00.3 USB controller [0c03]: Advanced Micro Devices, Inc. [AMD] Matisse USB 3.0 Host Controller [1022:149c]

and v6.6.4 kernel, without this patch:

[~]> LC_TIME=C jctl -kb -1 --grep 'hci version'
Dec 03 13:22:03 archlinux kernel: xhci_hcd 0000:07:00.1: hcc params 0x0278ffe5 hci version 0x110 quirks 0x0000000200000410
Dec 03 13:22:03 archlinux kernel: xhci_hcd 0000:07:00.3: hcc params 0x0278ffe5 hci version 0x110 quirks 0x0000000200000410
Dec 03 13:22:03 archlinux kernel: xhci_hcd 0000:0f:00.3: hcc params 0x0278ffe5 hci version 0x110 quirks 0x0000000200000410

With the patch applied:

[~]> LC_TIME=C jctl -kb --grep 'hci version'
Dec 03 20:46:59 archlinux kernel: xhci_hcd 0000:07:00.1: hcc params 0x0278ffe5 hci version 0x110 quirks 0x0000000000000410
Dec 03 20:46:59 archlinux kernel: xhci_hcd 0000:07:00.3: hcc params 0x0278ffe5 hci version 0x110 quirks 0x0000000000000410
Dec 03 20:46:59 archlinux kernel: xhci_hcd 0000:0f:00.3: hcc params 0x0278ffe5 hci version 0x110 quirks 0x0000000000000410

(note the difference in `quirks` as expected)

Hence, feel free to add:

Tested-by: Oleksandr Natalenko <oleksandr@natalenko.name>
Link: https://lore.kernel.org/lkml/5993222.lOV4Wx5bFT@natalenko.name/

Thank you.

> >
> > Thanks,
> > --
> > Basavaraj
> >
> >> thanks,
> >>
> >> greg k-h
> 
> 
> 


-- 
Oleksandr Natalenko (post-factum)

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Regression: Inoperative bluetooth, Intel chipset, mainline kernel 6.6.2+
  2023-12-03  8:38                 ` Greg KH
  2023-12-03 16:16                   ` Basavaraj Natikar
@ 2023-12-04  9:12                   ` Mathias Nyman
  2023-12-04 10:08                     ` [PATCH 1/2] Revert "xhci: Enable RPM on controllers that support low-power states" Mathias Nyman
  1 sibling, 1 reply; 22+ messages in thread
From: Mathias Nyman @ 2023-12-04  9:12 UTC (permalink / raw)
  To: Greg KH, Kris Karas (Bug Reporting)
  Cc: Paul Menzel, Basavaraj Natikar, stable, Thorsten Leemhuis,
	regressions, linux-bluetooth, Mario Limonciello, Mathias Nyman,
	linux-usb

On 3.12.2023 10.38, Greg KH wrote:
> On Sun, Dec 03, 2023 at 03:32:52AM -0500, Kris Karas (Bug Reporting) wrote:
>> Greg KH wrote:
>>> Thanks for testing, any chance you can try 6.6.4-rc1?  Or wait a few
>>> hours for me to release 6.6.4 if you don't want to mess with a -rc
>>> release.
>>
>> As I mentioned to Greg off-list (to save wasting other peoples' bandwidth),
>> I couldn't find 6.6.4-rc1.  Looking in wrong git tree?  But 6.6.4 is now
>> out, which I have tested and am running at the moment, albeit with the
>> problem commit from 6.6.2 backed out.
>>
>> There is no change with respect to this bug.  The problematic patch
>> introduced in 6.6.2 was neither reverted nor amended.  The "opcode 0x0c03
>> failed" lines to the kernel log continue to be present.
>>
>>> Also, is this showing up in 6.7-rc3?  If so, that would be a big help in
>>> tracking this down.
>>
>> The bug shows up in 6.7-rc3 as well, exactly as it does here in 6.6.2+ and
>> in 6.1.63+.  The problematic patch bisected earlier appears identically (and
>> seems to have been introduced simultaneously) in these recent releases.
> 
> Ok, in a way, this is good as that means I haven't missed a fix, but bad
> in that this does affect everyone more.
> 
> So let's start over, you found the offending commit, and nothing has
> fixed it, so what do we do?  xhci/amd developers, any ideas?
> thanks,
> 
> greg k-h
> 

I suggest reverting these two patches from everywhere (all stable):
a5d6264b638e xhci: Enable RPM on controllers that support low-power states
4baf12181509 xhci: Loosen RPM as default policy to cover for AMD xHC 1.1

Then write a new well tested patch that adds default runtime pm to those AMD
hosts that support it. And only add that to usb-next

-Mathias





^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 1/2] Revert "xhci: Enable RPM on controllers that support low-power states"
  2023-12-04  9:12                   ` Mathias Nyman
@ 2023-12-04 10:08                     ` Mathias Nyman
  2023-12-04 10:08                       ` [PATCH 2/2] Revert "xhci: Loosen RPM as default policy to cover for AMD xHC 1.1" Mathias Nyman
                                         ` (2 more replies)
  0 siblings, 3 replies; 22+ messages in thread
From: Mathias Nyman @ 2023-12-04 10:08 UTC (permalink / raw)
  To: gregkh
  Cc: linux-usb, linux-bluetooth, mario.limonciello, regressions,
	regressions, Basavaraj.Natikar, pmenzel, bugs-a21, Mathias Nyman,
	stable

This reverts commit a5d6264b638efeca35eff72177fd28d149e0764b.

This patch was an attempt to solve issues seen when enabling runtime PM
as default for all AMD 1.1 xHC hosts. see commit 4baf12181509
("xhci: Loosen RPM as default policy to cover for AMD xHC 1.1")

This was not enough, regressions are still seen, so start from a clean
slate and revert both of them.

This patch went to stable and should be reverted from there as well

Fixes: a5d6264b638e ("xhci: Enable RPM on controllers that support low-power states")
Cc: stable@vger.kernel.org
Cc: Mario Limonciello <mario.limonciello@amd.com>
Cc: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-pci.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 95ed9404f6f8..bde43cef8846 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -695,9 +695,7 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	/* USB-2 and USB-3 roothubs initialized, allow runtime pm suspend */
 	pm_runtime_put_noidle(&dev->dev);
 
-	if (pci_choose_state(dev, PMSG_SUSPEND) == PCI_D0)
-		pm_runtime_forbid(&dev->dev);
-	else if (xhci->quirks & XHCI_DEFAULT_PM_RUNTIME_ALLOW)
+	if (xhci->quirks & XHCI_DEFAULT_PM_RUNTIME_ALLOW)
 		pm_runtime_allow(&dev->dev);
 
 	dma_set_max_seg_size(&dev->dev, UINT_MAX);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 2/2] Revert "xhci: Loosen RPM as default policy to cover for AMD xHC 1.1"
  2023-12-04 10:08                     ` [PATCH 1/2] Revert "xhci: Enable RPM on controllers that support low-power states" Mathias Nyman
@ 2023-12-04 10:08                       ` Mathias Nyman
  2023-12-04 10:49                       ` [PATCH 1/2] Revert "xhci: Enable RPM on controllers that support low-power states" Basavaraj Natikar
  2023-12-15 16:53                       ` [PATCH 1/2] Revert "xhci: Enable RPM on controllers that support low-power states" patchwork-bot+bluetooth
  2 siblings, 0 replies; 22+ messages in thread
From: Mathias Nyman @ 2023-12-04 10:08 UTC (permalink / raw)
  To: gregkh
  Cc: linux-usb, linux-bluetooth, mario.limonciello, regressions,
	regressions, Basavaraj.Natikar, pmenzel, bugs-a21, Mathias Nyman,
	stable

This reverts commit 4baf1218150985ee3ab0a27220456a1f027ea0ac.

Enabling runtime pm as default for all AMD xHC 1.1 controllers caused
regression. An initial attempt to fix those was done in commit a5d6264b638e
("xhci: Enable RPM on controllers that support low-power states") but new
issues are still seen.

Revert them both and start from a clean slate.

This patch went to stable an needs to be reverted from there as well.

Fixes: 4baf12181509 ("xhci: Loosen RPM as default policy to cover for AMD xHC 1.1")
Link: https://lore.kernel.org/linux-usb/55c50bf5-bffb-454e-906e-4408c591cb63@molgen.mpg.de
Cc: Mario Limonciello <mario.limonciello@amd.com>
Cc: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
Cc: stable@vger.kernel.org
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-pci.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index bde43cef8846..b9ae5c2a2527 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -535,8 +535,6 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
 	/* xHC spec requires PCI devices to support D3hot and D3cold */
 	if (xhci->hci_version >= 0x120)
 		xhci->quirks |= XHCI_DEFAULT_PM_RUNTIME_ALLOW;
-	else if (pdev->vendor == PCI_VENDOR_ID_AMD && xhci->hci_version >= 0x110)
-		xhci->quirks |= XHCI_DEFAULT_PM_RUNTIME_ALLOW;
 
 	if (xhci->quirks & XHCI_RESET_ON_RESUME)
 		xhci_dbg_trace(xhci, trace_xhci_dbg_quirks,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] Revert "xhci: Enable RPM on controllers that support low-power states"
  2023-12-04 10:08                     ` [PATCH 1/2] Revert "xhci: Enable RPM on controllers that support low-power states" Mathias Nyman
  2023-12-04 10:08                       ` [PATCH 2/2] Revert "xhci: Loosen RPM as default policy to cover for AMD xHC 1.1" Mathias Nyman
@ 2023-12-04 10:49                       ` Basavaraj Natikar
  2023-12-04 14:22                         ` Mathias Nyman
  2023-12-15 16:53                       ` [PATCH 1/2] Revert "xhci: Enable RPM on controllers that support low-power states" patchwork-bot+bluetooth
  2 siblings, 1 reply; 22+ messages in thread
From: Basavaraj Natikar @ 2023-12-04 10:49 UTC (permalink / raw)
  To: Mathias Nyman, gregkh
  Cc: linux-usb, linux-bluetooth, mario.limonciello, regressions,
	regressions, Basavaraj.Natikar, pmenzel, bugs-a21, stable


On 12/4/2023 3:38 PM, Mathias Nyman wrote:
> This reverts commit a5d6264b638efeca35eff72177fd28d149e0764b.
>
> This patch was an attempt to solve issues seen when enabling runtime PM
> as default for all AMD 1.1 xHC hosts. see commit 4baf12181509
> ("xhci: Loosen RPM as default policy to cover for AMD xHC 1.1")

AFAK, only 4baf12181509 commit has regression on AMD xHc 1.1 below is not regression
patch and its unrelated to AMD xHC 1.1.

Only [PATCH 2/2] Revert "xhci: Loosen RPM as default policy to cover for AMD xHC 1.1"
alone in this series solves regression issues.

>
> This was not enough, regressions are still seen, so start from a clean
> slate and revert both of them.
>
> This patch went to stable and should be reverted from there as well
>
> Fixes: a5d6264b638e ("xhci: Enable RPM on controllers that support low-power states")
> Cc: stable@vger.kernel.org
> Cc: Mario Limonciello <mario.limonciello@amd.com>
> Cc: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> ---
>  drivers/usb/host/xhci-pci.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index 95ed9404f6f8..bde43cef8846 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -695,9 +695,7 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
>  	/* USB-2 and USB-3 roothubs initialized, allow runtime pm suspend */
>  	pm_runtime_put_noidle(&dev->dev);
>  
> -	if (pci_choose_state(dev, PMSG_SUSPEND) == PCI_D0)
> -		pm_runtime_forbid(&dev->dev);
> -	else if (xhci->quirks & XHCI_DEFAULT_PM_RUNTIME_ALLOW)
> +	if (xhci->quirks & XHCI_DEFAULT_PM_RUNTIME_ALLOW)
>  		pm_runtime_allow(&dev->dev);
>  
>  	dma_set_max_seg_size(&dev->dev, UINT_MAX);


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] Revert "xhci: Enable RPM on controllers that support low-power states"
  2023-12-04 10:49                       ` [PATCH 1/2] Revert "xhci: Enable RPM on controllers that support low-power states" Basavaraj Natikar
@ 2023-12-04 14:22                         ` Mathias Nyman
  2023-12-04 14:49                           ` Basavaraj Natikar
  0 siblings, 1 reply; 22+ messages in thread
From: Mathias Nyman @ 2023-12-04 14:22 UTC (permalink / raw)
  To: Basavaraj Natikar, gregkh
  Cc: linux-usb, linux-bluetooth, mario.limonciello, regressions,
	regressions, Basavaraj.Natikar, pmenzel, bugs-a21, stable

On 4.12.2023 12.49, Basavaraj Natikar wrote:
> 
> On 12/4/2023 3:38 PM, Mathias Nyman wrote:
>> This reverts commit a5d6264b638efeca35eff72177fd28d149e0764b.
>>
>> This patch was an attempt to solve issues seen when enabling runtime PM
>> as default for all AMD 1.1 xHC hosts. see commit 4baf12181509
>> ("xhci: Loosen RPM as default policy to cover for AMD xHC 1.1")
> 
> AFAK, only 4baf12181509 commit has regression on AMD xHc 1.1 below is not regression
> patch and its unrelated to AMD xHC 1.1.
> 
> Only [PATCH 2/2] Revert "xhci: Loosen RPM as default policy to cover for AMD xHC 1.1"
> alone in this series solves regression issues.
> 

Patch a5d6264b638e ("xhci: Enable RPM on controllers that support low-power states")
was originally not supposed to go to stable. It was added later as it solved some
cases triggered by 4baf12181509 ("xhci: Loosen RPM as default policy to cover for AMD xHC 1.1")
see:
https://lore.kernel.org/linux-usb/5993222.lOV4Wx5bFT@natalenko.name/

Turns out it wasn't enough.

If we now revert 4baf12181509 "xhci: Loosen RPM as default policy to cover for AMD xHC 1.1"
I still think it makes sense to also revert a5d6264b638e.
Especially from the stable kernels.

This way we roll back this whole issue to a known working state.

Thanks
Mathias

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] Revert "xhci: Enable RPM on controllers that support low-power states"
  2023-12-04 14:22                         ` Mathias Nyman
@ 2023-12-04 14:49                           ` Basavaraj Natikar
  2023-12-04 15:06                             ` Mathias Nyman
  0 siblings, 1 reply; 22+ messages in thread
From: Basavaraj Natikar @ 2023-12-04 14:49 UTC (permalink / raw)
  To: Mathias Nyman, gregkh
  Cc: linux-usb, linux-bluetooth, mario.limonciello, regressions,
	regressions, Basavaraj.Natikar, pmenzel, bugs-a21, stable


On 12/4/2023 7:52 PM, Mathias Nyman wrote:
> On 4.12.2023 12.49, Basavaraj Natikar wrote:
>>
>> On 12/4/2023 3:38 PM, Mathias Nyman wrote:
>>> This reverts commit a5d6264b638efeca35eff72177fd28d149e0764b.
>>>
>>> This patch was an attempt to solve issues seen when enabling runtime PM
>>> as default for all AMD 1.1 xHC hosts. see commit 4baf12181509
>>> ("xhci: Loosen RPM as default policy to cover for AMD xHC 1.1")
>>
>> AFAK, only 4baf12181509 commit has regression on AMD xHc 1.1 below is
>> not regression
>> patch and its unrelated to AMD xHC 1.1.
>>
>> Only [PATCH 2/2] Revert "xhci: Loosen RPM as default policy to cover
>> for AMD xHC 1.1"
>> alone in this series solves regression issues.
>>
>
> Patch a5d6264b638e ("xhci: Enable RPM on controllers that support
> low-power states")
> was originally not supposed to go to stable. It was added later as it
> solved some
> cases triggered by 4baf12181509 ("xhci: Loosen RPM as default policy
> to cover for AMD xHC 1.1")
> see:
> https://lore.kernel.org/linux-usb/5993222.lOV4Wx5bFT@natalenko.name/
>
> Turns out it wasn't enough.
>
> If we now revert 4baf12181509 "xhci: Loosen RPM as default policy to
> cover for AMD xHC 1.1"
> I still think it makes sense to also revert a5d6264b638e.
> Especially from the stable kernels.

Yes , a5d6264b638e still solves other issues if underlying hardware doesn't support RPM
if we revert a5d6264b638e on stable releases then new issues (not related to regression)
other than AMD xHC 1.1 controllers including xHC 1.2 will still exist on stable releases.
If revert then we can backport to stable release later if required.

Sure, will send a follow up patch to fix 4baf12181509 alone on mainline if revert on all releases.

>
> This way we roll back this whole issue to a known working state.

Sure, for at-least a5d6264b638e if not revert on mainline then will not resend the same patch.

Thanks,
--
Basavaraj

>
> Thanks
> Mathias


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] Revert "xhci: Enable RPM on controllers that support low-power states"
  2023-12-04 14:49                           ` Basavaraj Natikar
@ 2023-12-04 15:06                             ` Mathias Nyman
  2023-12-04 15:29                               ` Basavaraj Natikar
  0 siblings, 1 reply; 22+ messages in thread
From: Mathias Nyman @ 2023-12-04 15:06 UTC (permalink / raw)
  To: Basavaraj Natikar, gregkh
  Cc: linux-usb, linux-bluetooth, mario.limonciello, regressions,
	regressions, Basavaraj.Natikar, pmenzel, bugs-a21, stable

On 4.12.2023 16.49, Basavaraj Natikar wrote:
> 
> On 12/4/2023 7:52 PM, Mathias Nyman wrote:
>> On 4.12.2023 12.49, Basavaraj Natikar wrote:
>>>
>>> On 12/4/2023 3:38 PM, Mathias Nyman wrote:
>>>> This reverts commit a5d6264b638efeca35eff72177fd28d149e0764b.
>>>>
>>>> This patch was an attempt to solve issues seen when enabling runtime PM
>>>> as default for all AMD 1.1 xHC hosts. see commit 4baf12181509
>>>> ("xhci: Loosen RPM as default policy to cover for AMD xHC 1.1")
>>>
>>> AFAK, only 4baf12181509 commit has regression on AMD xHc 1.1 below is
>>> not regression
>>> patch and its unrelated to AMD xHC 1.1.
>>>
>>> Only [PATCH 2/2] Revert "xhci: Loosen RPM as default policy to cover
>>> for AMD xHC 1.1"
>>> alone in this series solves regression issues.
>>>
>>
>> Patch a5d6264b638e ("xhci: Enable RPM on controllers that support
>> low-power states")
>> was originally not supposed to go to stable. It was added later as it
>> solved some
>> cases triggered by 4baf12181509 ("xhci: Loosen RPM as default policy
>> to cover for AMD xHC 1.1")
>> see:
>> https://lore.kernel.org/linux-usb/5993222.lOV4Wx5bFT@natalenko.name/
>>
>> Turns out it wasn't enough.
>>
>> If we now revert 4baf12181509 "xhci: Loosen RPM as default policy to
>> cover for AMD xHC 1.1"
>> I still think it makes sense to also revert a5d6264b638e.
>> Especially from the stable kernels.
> 
> Yes , a5d6264b638e still solves other issues if underlying hardware doesn't support RPM
> if we revert a5d6264b638e on stable releases then new issues (not related to regression)
> other than AMD xHC 1.1 controllers including xHC 1.2 will still exist on stable releases.

Ok, got it, so a5d6264b638e also solves other issues than those exposed by 4baf12181509.
And that one (a5d6264b638) should originally have been marked for stable.

So only revert 4baf12181509, PATCH 2/2 in this series

Thanks
Mathias

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] Revert "xhci: Enable RPM on controllers that support low-power states"
  2023-12-04 15:06                             ` Mathias Nyman
@ 2023-12-04 15:29                               ` Basavaraj Natikar
  2023-12-04 23:55                                 ` Greg KH
  0 siblings, 1 reply; 22+ messages in thread
From: Basavaraj Natikar @ 2023-12-04 15:29 UTC (permalink / raw)
  To: Mathias Nyman, gregkh
  Cc: linux-usb, linux-bluetooth, mario.limonciello, regressions,
	regressions, Basavaraj.Natikar, pmenzel, bugs-a21, stable


On 12/4/2023 8:36 PM, Mathias Nyman wrote:
> On 4.12.2023 16.49, Basavaraj Natikar wrote:
>>
>> On 12/4/2023 7:52 PM, Mathias Nyman wrote:
>>> On 4.12.2023 12.49, Basavaraj Natikar wrote:
>>>>
>>>> On 12/4/2023 3:38 PM, Mathias Nyman wrote:
>>>>> This reverts commit a5d6264b638efeca35eff72177fd28d149e0764b.
>>>>>
>>>>> This patch was an attempt to solve issues seen when enabling
>>>>> runtime PM
>>>>> as default for all AMD 1.1 xHC hosts. see commit 4baf12181509
>>>>> ("xhci: Loosen RPM as default policy to cover for AMD xHC 1.1")
>>>>
>>>> AFAK, only 4baf12181509 commit has regression on AMD xHc 1.1 below is
>>>> not regression
>>>> patch and its unrelated to AMD xHC 1.1.
>>>>
>>>> Only [PATCH 2/2] Revert "xhci: Loosen RPM as default policy to cover
>>>> for AMD xHC 1.1"
>>>> alone in this series solves regression issues.
>>>>
>>>
>>> Patch a5d6264b638e ("xhci: Enable RPM on controllers that support
>>> low-power states")
>>> was originally not supposed to go to stable. It was added later as it
>>> solved some
>>> cases triggered by 4baf12181509 ("xhci: Loosen RPM as default policy
>>> to cover for AMD xHC 1.1")
>>> see:
>>> https://lore.kernel.org/linux-usb/5993222.lOV4Wx5bFT@natalenko.name/
>>>
>>> Turns out it wasn't enough.
>>>
>>> If we now revert 4baf12181509 "xhci: Loosen RPM as default policy to
>>> cover for AMD xHC 1.1"
>>> I still think it makes sense to also revert a5d6264b638e.
>>> Especially from the stable kernels.
>>
>> Yes , a5d6264b638e still solves other issues if underlying hardware
>> doesn't support RPM
>> if we revert a5d6264b638e on stable releases then new issues (not
>> related to regression)
>> other than AMD xHC 1.1 controllers including xHC 1.2 will still exist
>> on stable releases.
>
> Ok, got it, so a5d6264b638e also solves other issues than those
> exposed by 4baf12181509.
> And that one (a5d6264b638) should originally have been marked for stable.
>
> So only revert 4baf12181509, PATCH 2/2 in this series

Thank you, that is correct.

Thanks,
--
Basavaraj

>
> Thanks
> Mathias


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] Revert "xhci: Enable RPM on controllers that support low-power states"
  2023-12-04 15:29                               ` Basavaraj Natikar
@ 2023-12-04 23:55                                 ` Greg KH
  2023-12-05  9:05                                   ` [PATCH v2] Revert "xhci: Loosen RPM as default policy to cover for AMD xHC 1.1" Mathias Nyman
  0 siblings, 1 reply; 22+ messages in thread
From: Greg KH @ 2023-12-04 23:55 UTC (permalink / raw)
  To: Basavaraj Natikar
  Cc: Mathias Nyman, linux-usb, linux-bluetooth, mario.limonciello,
	regressions, regressions, Basavaraj.Natikar, pmenzel, bugs-a21,
	stable

On Mon, Dec 04, 2023 at 08:59:35PM +0530, Basavaraj Natikar wrote:
> 
> On 12/4/2023 8:36 PM, Mathias Nyman wrote:
> > On 4.12.2023 16.49, Basavaraj Natikar wrote:
> >>
> >> On 12/4/2023 7:52 PM, Mathias Nyman wrote:
> >>> On 4.12.2023 12.49, Basavaraj Natikar wrote:
> >>>>
> >>>> On 12/4/2023 3:38 PM, Mathias Nyman wrote:
> >>>>> This reverts commit a5d6264b638efeca35eff72177fd28d149e0764b.
> >>>>>
> >>>>> This patch was an attempt to solve issues seen when enabling
> >>>>> runtime PM
> >>>>> as default for all AMD 1.1 xHC hosts. see commit 4baf12181509
> >>>>> ("xhci: Loosen RPM as default policy to cover for AMD xHC 1.1")
> >>>>
> >>>> AFAK, only 4baf12181509 commit has regression on AMD xHc 1.1 below is
> >>>> not regression
> >>>> patch and its unrelated to AMD xHC 1.1.
> >>>>
> >>>> Only [PATCH 2/2] Revert "xhci: Loosen RPM as default policy to cover
> >>>> for AMD xHC 1.1"
> >>>> alone in this series solves regression issues.
> >>>>
> >>>
> >>> Patch a5d6264b638e ("xhci: Enable RPM on controllers that support
> >>> low-power states")
> >>> was originally not supposed to go to stable. It was added later as it
> >>> solved some
> >>> cases triggered by 4baf12181509 ("xhci: Loosen RPM as default policy
> >>> to cover for AMD xHC 1.1")
> >>> see:
> >>> https://lore.kernel.org/linux-usb/5993222.lOV4Wx5bFT@natalenko.name/
> >>>
> >>> Turns out it wasn't enough.
> >>>
> >>> If we now revert 4baf12181509 "xhci: Loosen RPM as default policy to
> >>> cover for AMD xHC 1.1"
> >>> I still think it makes sense to also revert a5d6264b638e.
> >>> Especially from the stable kernels.
> >>
> >> Yes , a5d6264b638e still solves other issues if underlying hardware
> >> doesn't support RPM
> >> if we revert a5d6264b638e on stable releases then new issues (not
> >> related to regression)
> >> other than AMD xHC 1.1 controllers including xHC 1.2 will still exist
> >> on stable releases.
> >
> > Ok, got it, so a5d6264b638e also solves other issues than those
> > exposed by 4baf12181509.
> > And that one (a5d6264b638) should originally have been marked for stable.
> >
> > So only revert 4baf12181509, PATCH 2/2 in this series
> 
> Thank you, that is correct.

So just take patch 2/2 here, or will someone be sending me a new patch?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v2] Revert "xhci: Loosen RPM as default policy to cover for AMD xHC 1.1"
  2023-12-04 23:55                                 ` Greg KH
@ 2023-12-05  9:05                                   ` Mathias Nyman
  2023-12-05 18:36                                     ` Mario Limonciello
  2023-12-15 16:53                                     ` patchwork-bot+bluetooth
  0 siblings, 2 replies; 22+ messages in thread
From: Mathias Nyman @ 2023-12-05  9:05 UTC (permalink / raw)
  To: gregkh
  Cc: linux-usb, linux-bluetooth, mario.limonciello, regressions,
	regressions, Basavaraj.Natikar, pmenzel, bugs-a21, Mathias Nyman,
	stable

This reverts commit 4baf1218150985ee3ab0a27220456a1f027ea0ac.

Enabling runtime pm as default for all AMD xHC 1.1 controllers caused
regression. An initial attempt to fix those was done in commit a5d6264b638e
("xhci: Enable RPM on controllers that support low-power states") but new
issues are still seen.

Revert this to get those AMD xHC 1.1 systems working

This patch went to stable an needs to be reverted from there as well.

Fixes: 4baf12181509 ("xhci: Loosen RPM as default policy to cover for AMD xHC 1.1")
Link: https://lore.kernel.org/linux-usb/55c50bf5-bffb-454e-906e-4408c591cb63@molgen.mpg.de
Cc: Mario Limonciello <mario.limonciello@amd.com>
Cc: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
Cc: stable@vger.kernel.org
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
v1 -> v2
Revert only one patch, keep commit a5d6264b638
Minor commit message changes

 drivers/usb/host/xhci-pci.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 95ed9404f6f8..d6fc08e5db8f 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -535,8 +535,6 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
 	/* xHC spec requires PCI devices to support D3hot and D3cold */
 	if (xhci->hci_version >= 0x120)
 		xhci->quirks |= XHCI_DEFAULT_PM_RUNTIME_ALLOW;
-	else if (pdev->vendor == PCI_VENDOR_ID_AMD && xhci->hci_version >= 0x110)
-		xhci->quirks |= XHCI_DEFAULT_PM_RUNTIME_ALLOW;
 
 	if (xhci->quirks & XHCI_RESET_ON_RESUME)
 		xhci_dbg_trace(xhci, trace_xhci_dbg_quirks,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] Revert "xhci: Loosen RPM as default policy to cover for AMD xHC 1.1"
  2023-12-05  9:05                                   ` [PATCH v2] Revert "xhci: Loosen RPM as default policy to cover for AMD xHC 1.1" Mathias Nyman
@ 2023-12-05 18:36                                     ` Mario Limonciello
  2023-12-15 16:53                                     ` patchwork-bot+bluetooth
  1 sibling, 0 replies; 22+ messages in thread
From: Mario Limonciello @ 2023-12-05 18:36 UTC (permalink / raw)
  To: Mathias Nyman, gregkh
  Cc: linux-usb, linux-bluetooth, regressions, regressions,
	Basavaraj.Natikar, pmenzel, bugs-a21, stable

On 12/5/2023 03:05, Mathias Nyman wrote:
> This reverts commit 4baf1218150985ee3ab0a27220456a1f027ea0ac.
> 
> Enabling runtime pm as default for all AMD xHC 1.1 controllers caused
> regression. An initial attempt to fix those was done in commit a5d6264b638e
> ("xhci: Enable RPM on controllers that support low-power states") but new
> issues are still seen.
> 
> Revert this to get those AMD xHC 1.1 systems working
> 
> This patch went to stable an needs to be reverted from there as well.
> 
> Fixes: 4baf12181509 ("xhci: Loosen RPM as default policy to cover for AMD xHC 1.1")
> Link: https://lore.kernel.org/linux-usb/55c50bf5-bffb-454e-906e-4408c591cb63@molgen.mpg.de
> Cc: Mario Limonciello <mario.limonciello@amd.com>
> Cc: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>

Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>

This presumes that Basavaraj is going to send up another patch for the 
ID it for sure improves, works and is needed.

> ---
> v1 -> v2
> Revert only one patch, keep commit a5d6264b638
> Minor commit message changes
> 
>   drivers/usb/host/xhci-pci.c | 2 --
>   1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index 95ed9404f6f8..d6fc08e5db8f 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -535,8 +535,6 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
>   	/* xHC spec requires PCI devices to support D3hot and D3cold */
>   	if (xhci->hci_version >= 0x120)
>   		xhci->quirks |= XHCI_DEFAULT_PM_RUNTIME_ALLOW;
> -	else if (pdev->vendor == PCI_VENDOR_ID_AMD && xhci->hci_version >= 0x110)
> -		xhci->quirks |= XHCI_DEFAULT_PM_RUNTIME_ALLOW;
>   
>   	if (xhci->quirks & XHCI_RESET_ON_RESUME)
>   		xhci_dbg_trace(xhci, trace_xhci_dbg_quirks,


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2] Revert "xhci: Loosen RPM as default policy to cover for AMD xHC 1.1"
  2023-12-05  9:05                                   ` [PATCH v2] Revert "xhci: Loosen RPM as default policy to cover for AMD xHC 1.1" Mathias Nyman
  2023-12-05 18:36                                     ` Mario Limonciello
@ 2023-12-15 16:53                                     ` patchwork-bot+bluetooth
  1 sibling, 0 replies; 22+ messages in thread
From: patchwork-bot+bluetooth @ 2023-12-15 16:53 UTC (permalink / raw)
  To: Mathias Nyman
  Cc: gregkh, linux-usb, linux-bluetooth, mario.limonciello,
	regressions, regressions, Basavaraj.Natikar, pmenzel, bugs-a21,
	stable

Hello:

This patch was applied to bluetooth/bluetooth-next.git (master)
by Greg Kroah-Hartman <gregkh@linuxfoundation.org>:

On Tue,  5 Dec 2023 11:05:48 +0200 you wrote:
> This reverts commit 4baf1218150985ee3ab0a27220456a1f027ea0ac.
> 
> Enabling runtime pm as default for all AMD xHC 1.1 controllers caused
> regression. An initial attempt to fix those was done in commit a5d6264b638e
> ("xhci: Enable RPM on controllers that support low-power states") but new
> issues are still seen.
> 
> [...]

Here is the summary with links:
  - [v2] Revert "xhci: Loosen RPM as default policy to cover for AMD xHC 1.1"
    https://git.kernel.org/bluetooth/bluetooth-next/c/24be0b3c4059

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/2] Revert "xhci: Enable RPM on controllers that support low-power states"
  2023-12-04 10:08                     ` [PATCH 1/2] Revert "xhci: Enable RPM on controllers that support low-power states" Mathias Nyman
  2023-12-04 10:08                       ` [PATCH 2/2] Revert "xhci: Loosen RPM as default policy to cover for AMD xHC 1.1" Mathias Nyman
  2023-12-04 10:49                       ` [PATCH 1/2] Revert "xhci: Enable RPM on controllers that support low-power states" Basavaraj Natikar
@ 2023-12-15 16:53                       ` patchwork-bot+bluetooth
  2 siblings, 0 replies; 22+ messages in thread
From: patchwork-bot+bluetooth @ 2023-12-15 16:53 UTC (permalink / raw)
  To: Mathias Nyman
  Cc: gregkh, linux-usb, linux-bluetooth, mario.limonciello,
	regressions, regressions, Basavaraj.Natikar, pmenzel, bugs-a21,
	stable

Hello:

This series was applied to bluetooth/bluetooth-next.git (master)
by Greg Kroah-Hartman <gregkh@linuxfoundation.org>:

On Mon,  4 Dec 2023 12:08:58 +0200 you wrote:
> This reverts commit a5d6264b638efeca35eff72177fd28d149e0764b.
> 
> This patch was an attempt to solve issues seen when enabling runtime PM
> as default for all AMD 1.1 xHC hosts. see commit 4baf12181509
> ("xhci: Loosen RPM as default policy to cover for AMD xHC 1.1")
> 
> This was not enough, regressions are still seen, so start from a clean
> slate and revert both of them.
> 
> [...]

Here is the summary with links:
  - [1/2] Revert "xhci: Enable RPM on controllers that support low-power states"
    (no matching commit)
  - [2/2] Revert "xhci: Loosen RPM as default policy to cover for AMD xHC 1.1"
    https://git.kernel.org/bluetooth/bluetooth-next/c/24be0b3c4059

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2023-12-15 16:53 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <ee109942-ef8e-45b9-8cb9-a98a787fe094@moonlit-rail.com>
     [not found] ` <8d6070c8-3f82-4a12-8c60-7f1862fef9d9@leemhuis.info>
     [not found]   ` <2023120119-bonus-judgingly-bf57@gregkh>
     [not found]     ` <6a710423-e76c-437e-ba59-b9cefbda3194@moonlit-rail.com>
2023-12-02  7:23       ` Regression: Inoperative bluetooth, Intel chipset, mainline kernel 6.6.2+ Paul Menzel
2023-12-02  7:50         ` Greg KH
2023-12-02  7:58           ` Kris Karas (Bug Reporting)
2023-12-02  8:15             ` Greg KH
2023-12-03  8:32               ` Kris Karas (Bug Reporting)
2023-12-03  8:38                 ` Greg KH
2023-12-03 16:16                   ` Basavaraj Natikar
2023-12-03 16:24                     ` Basavaraj Natikar
2023-12-03 19:52                       ` Oleksandr Natalenko
2023-12-04  9:12                   ` Mathias Nyman
2023-12-04 10:08                     ` [PATCH 1/2] Revert "xhci: Enable RPM on controllers that support low-power states" Mathias Nyman
2023-12-04 10:08                       ` [PATCH 2/2] Revert "xhci: Loosen RPM as default policy to cover for AMD xHC 1.1" Mathias Nyman
2023-12-04 10:49                       ` [PATCH 1/2] Revert "xhci: Enable RPM on controllers that support low-power states" Basavaraj Natikar
2023-12-04 14:22                         ` Mathias Nyman
2023-12-04 14:49                           ` Basavaraj Natikar
2023-12-04 15:06                             ` Mathias Nyman
2023-12-04 15:29                               ` Basavaraj Natikar
2023-12-04 23:55                                 ` Greg KH
2023-12-05  9:05                                   ` [PATCH v2] Revert "xhci: Loosen RPM as default policy to cover for AMD xHC 1.1" Mathias Nyman
2023-12-05 18:36                                     ` Mario Limonciello
2023-12-15 16:53                                     ` patchwork-bot+bluetooth
2023-12-15 16:53                       ` [PATCH 1/2] Revert "xhci: Enable RPM on controllers that support low-power states" patchwork-bot+bluetooth

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).