2017-08-22 09:15:06

by Kai-Heng Feng

[permalink] [raw]
Subject: [PATCH] Revert "xhci: Limit USB2 port wake support for AMD Promontory hosts"

This reverts commit dec08194ffeccfa1cf085906b53d301930eae18f.

Commit dec08194ffec ("xhci: Limit USB2 port wake support for AMD Promontory
hosts") makes all high speed USB ports on ASUS PRIME B350M-A cease to
function after enabling runtime PM.

All boards with this chipsets will be affected, so revert the commit.

Conflicts:
drivers/usb/host/xhci-pci.c
drivers/usb/host/xhci.h

Signed-off-by: Kai-Heng Feng <[email protected]>
---
drivers/usb/host/xhci-hub.c | 3 ---
drivers/usb/host/xhci-pci.c | 12 ------------
drivers/usb/host/xhci.h | 2 +-
3 files changed, 1 insertion(+), 16 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 00721e8807ab..4bd8654b1233 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -1473,9 +1473,6 @@ int xhci_bus_suspend(struct usb_hcd *hcd)
t2 |= PORT_WKOC_E | PORT_WKCONN_E;
t2 &= ~PORT_WKDISC_E;
}
- if ((xhci->quirks & XHCI_U2_DISABLE_WAKE) &&
- (hcd->speed < HCD_USB3))
- t2 &= ~PORT_WAKE_BITS;
} else
t2 &= ~PORT_WAKE_BITS;

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 8071c8fdd15e..76f392954733 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -54,11 +54,6 @@
#define PCI_DEVICE_ID_INTEL_APL_XHCI 0x5aa8
#define PCI_DEVICE_ID_INTEL_DNV_XHCI 0x19d0

-#define PCI_DEVICE_ID_AMD_PROMONTORYA_4 0x43b9
-#define PCI_DEVICE_ID_AMD_PROMONTORYA_3 0x43ba
-#define PCI_DEVICE_ID_AMD_PROMONTORYA_2 0x43bb
-#define PCI_DEVICE_ID_AMD_PROMONTORYA_1 0x43bc
-
#define PCI_DEVICE_ID_ASMEDIA_1042A_XHCI 0x1142

static const char hcd_name[] = "xhci_hcd";
@@ -142,13 +137,6 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
if (pdev->vendor == PCI_VENDOR_ID_AMD)
xhci->quirks |= XHCI_TRUST_TX_LENGTH;

- if ((pdev->vendor == PCI_VENDOR_ID_AMD) &&
- ((pdev->device == PCI_DEVICE_ID_AMD_PROMONTORYA_4) ||
- (pdev->device == PCI_DEVICE_ID_AMD_PROMONTORYA_3) ||
- (pdev->device == PCI_DEVICE_ID_AMD_PROMONTORYA_2) ||
- (pdev->device == PCI_DEVICE_ID_AMD_PROMONTORYA_1)))
- xhci->quirks |= XHCI_U2_DISABLE_WAKE;
-
if (pdev->vendor == PCI_VENDOR_ID_INTEL) {
xhci->quirks |= XHCI_LPM_SUPPORT;
xhci->quirks |= XHCI_INTEL_HOST;
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index e3e935291ed6..d6b471823a0b 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1819,7 +1819,7 @@ struct xhci_hcd {
/* For controller with a broken Port Disable implementation */
#define XHCI_BROKEN_PORT_PED (1 << 25)
#define XHCI_LIMIT_ENDPOINT_INTERVAL_7 (1 << 26)
-#define XHCI_U2_DISABLE_WAKE (1 << 27)
+/* Reserved. It was XHCI_U2_DISABLE_WAKE */
#define XHCI_ASMEDIA_MODIFY_FLOWCONTROL (1 << 28)

unsigned int num_active_eps;
--
2.14.1


2017-08-28 09:29:54

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] Revert "xhci: Limit USB2 port wake support for AMD Promontory hosts"

On Tue, Aug 22, 2017 at 05:14:47PM +0800, Kai-Heng Feng wrote:
> This reverts commit dec08194ffeccfa1cf085906b53d301930eae18f.
>
> Commit dec08194ffec ("xhci: Limit USB2 port wake support for AMD Promontory
> hosts") makes all high speed USB ports on ASUS PRIME B350M-A cease to
> function after enabling runtime PM.
>
> All boards with this chipsets will be affected, so revert the commit.
>
> Conflicts:
> drivers/usb/host/xhci-pci.c
> drivers/usb/host/xhci.h

Why are these "Conflicts:" lines here, you did fix up the issues, so
there shouldn't be any more conflicts.

And if you revert this, don't we still have the original problem here?

thanks,

greg k-h

2017-08-28 10:10:44

by Mathias Nyman

[permalink] [raw]
Subject: Re: [PATCH] Revert "xhci: Limit USB2 port wake support for AMD Promontory hosts"

On 28.08.2017 12:29, Greg KH wrote:
> On Tue, Aug 22, 2017 at 05:14:47PM +0800, Kai-Heng Feng wrote:
>> This reverts commit dec08194ffeccfa1cf085906b53d301930eae18f.
>>
>> Commit dec08194ffec ("xhci: Limit USB2 port wake support for AMD Promontory
>> hosts") makes all high speed USB ports on ASUS PRIME B350M-A cease to
>> function after enabling runtime PM.
>>
>> All boards with this chipsets will be affected, so revert the commit.
>>
>> Conflicts:
>> drivers/usb/host/xhci-pci.c
>> drivers/usb/host/xhci.h
>
> Why are these "Conflicts:" lines here, you did fix up the issues, so
> there shouldn't be any more conflicts.
>
> And if you revert this, don't we still have the original problem here?
>

Adding more people who were involved in the original patch.

Users are now seeing the unresponsive USB2 ports with Promontory hosts.
Is there any update on a better way to solve the original issue.

To me a "dead" USB2 port seems like a much worse issue for a user
than a BIOS disabled port waking up on plug/unplug (wake on connect/disconnect),
so I'm myself in favor of doing this revert.

But there was a strong push from Promontory developers to get the original fix in,
and I would like to get some comment from them before I do anything about it.

Thanks
-Mathias



2017-08-29 04:57:11

by Kai-Heng Feng

[permalink] [raw]
Subject: Re: [PATCH] Revert "xhci: Limit USB2 port wake support for AMD Promontory hosts"

On Mon, Aug 28, 2017 at 6:14 PM, Mathias Nyman
<[email protected]> wrote:
> On 28.08.2017 12:29, Greg KH wrote:
>>
>> On Tue, Aug 22, 2017 at 05:14:47PM +0800, Kai-Heng Feng wrote:
>>>
>>> This reverts commit dec08194ffeccfa1cf085906b53d301930eae18f.
>>>
>>> Commit dec08194ffec ("xhci: Limit USB2 port wake support for AMD
>>> Promontory
>>> hosts") makes all high speed USB ports on ASUS PRIME B350M-A cease to
>>> function after enabling runtime PM.
>>>
>>> All boards with this chipsets will be affected, so revert the commit.
>>>
>>> Conflicts:
>>> drivers/usb/host/xhci-pci.c
>>> drivers/usb/host/xhci.h
>>
>>
>> Why are these "Conflicts:" lines here, you did fix up the issues, so
>> there shouldn't be any more conflicts.
>>
>> And if you revert this, don't we still have the original problem here?
>>
>
> Adding more people who were involved in the original patch.
>
> Users are now seeing the unresponsive USB2 ports with Promontory hosts.
> Is there any update on a better way to solve the original issue.
>
> To me a "dead" USB2 port seems like a much worse issue for a user
> than a BIOS disabled port waking up on plug/unplug (wake on
> connect/disconnect),
> so I'm myself in favor of doing this revert.

At least I can't find "Disable USB2" on my ASUS PRIME B350M-A, so the
new behavior is quite surprising.

>
> But there was a strong push from Promontory developers to get the original
> fix in,
> and I would like to get some comment from them before I do anything about
> it.

You looped them to the mail thread which I reported the regression two
weeks ago, and there is no response since then...

>
> Thanks
> -Mathias
>

2017-09-16 04:49:15

by Kai-Heng Feng

[permalink] [raw]
Subject: Re: [PATCH] Revert "xhci: Limit USB2 port wake support for AMD Promontory hosts"

On Mon, Aug 28, 2017 at 9:56 PM, Kai-Heng Feng
<[email protected]> wrote:
> On Mon, Aug 28, 2017 at 6:14 PM, Mathias Nyman
> <[email protected]> wrote:
>> On 28.08.2017 12:29, Greg KH wrote:
>>
>> Adding more people who were involved in the original patch.
>>
>> Users are now seeing the unresponsive USB2 ports with Promontory hosts.
>> Is there any update on a better way to solve the original issue.
>>
>> To me a "dead" USB2 port seems like a much worse issue for a user
>> than a BIOS disabled port waking up on plug/unplug (wake on
>> connect/disconnect),
>> so I'm myself in favor of doing this revert.
>
> At least I can't find "Disable USB2" on my ASUS PRIME B350M-A, so the
> new behavior is quite surprising.
>
>>
>> But there was a strong push from Promontory developers to get the original
>> fix in,
>> and I would like to get some comment from them before I do anything about
>> it.
>
> You looped them to the mail thread which I reported the regression two
> weeks ago, and there is no response since then...

Still no response from AMD and ASMedia guys.

I don't like to use out-of-tree patches for myself, but probably it's
the only way here?

>
>>
>> Thanks
>> -Mathias
>>

2017-09-18 08:02:51

by Mathias Nyman

[permalink] [raw]
Subject: Re: [PATCH] Revert "xhci: Limit USB2 port wake support for AMD Promontory hosts"

On 16.09.2017 07:49, Kai-Heng Feng wrote:
> On Mon, Aug 28, 2017 at 9:56 PM, Kai-Heng Feng
> <[email protected]> wrote:
>> On Mon, Aug 28, 2017 at 6:14 PM, Mathias Nyman
>> <[email protected]> wrote:
>>> On 28.08.2017 12:29, Greg KH wrote:
>>>
>>> Adding more people who were involved in the original patch.
>>>
>>> Users are now seeing the unresponsive USB2 ports with Promontory hosts.
>>> Is there any update on a better way to solve the original issue.
>>>
>>> To me a "dead" USB2 port seems like a much worse issue for a user
>>> than a BIOS disabled port waking up on plug/unplug (wake on
>>> connect/disconnect),
>>> so I'm myself in favor of doing this revert.
>>
>> At least I can't find "Disable USB2" on my ASUS PRIME B350M-A, so the
>> new behavior is quite surprising.
>>
>>>
>>> But there was a strong push from Promontory developers to get the original
>>> fix in,
>>> and I would like to get some comment from them before I do anything about
>>> it.
>>
>> You looped them to the mail thread which I reported the regression two
>> weeks ago, and there is no response since then...
>
> Still no response from AMD and ASMedia guys.
>
> I don't like to use out-of-tree patches for myself, but probably it's
> the only way here?

I'll revert it and send it forward now since rc1 is out

-Mathias