diff mbox

[Xenial] xhci: AMD Promontory USB disable port support

Message ID 1496393789-18420-2-git-send-email-acelan.kao@canonical.com
State New
Headers show

Commit Message

AceLan Kao June 2, 2017, 8:56 a.m. UTC 
From: Jiahau Chang <jiahau@gmail.com>

v3: Fix some checkpatch.pl warnings

BugLink: http://bugs.launchpad.net/bugs/1695216

For AMD Promontory xHCI host, although you can disable USB 2.0 ports in BIOS
settings, those ports will be enabled anyway after you remove a device on
that port and re-plug it in again. It's a known limitation of the chip.
As a workaround we can clear the PORT_WAKE_BITS.

Signed-off-by: Jiahau Chang <Lars_Chang@asmedia.com.tw>
Signed-off-by: AceLan Kao <acelan.kao@canonical.com>
---
 drivers/usb/host/xhci-hub.c | 19 ++++++++++++++-----
 drivers/usb/host/xhci-pci.c | 13 +++++++++++++
 drivers/usb/host/xhci.h     |  2 ++
 3 files changed, 29 insertions(+), 5 deletions(-)

Comments

Stefan Bader June 2, 2017, 10:14 a.m. UTC | #1 
On 02.06.2017 10:56, AceLan Kao wrote:
> From: Jiahau Chang <jiahau@gmail.com>
> 
> v3: Fix some checkpatch.pl warnings
> 
> BugLink: http://bugs.launchpad.net/bugs/1695216
> 
> For AMD Promontory xHCI host, although you can disable USB 2.0 ports in BIOS
> settings, those ports will be enabled anyway after you remove a device on
> that port and re-plug it in again. It's a known limitation of the chip.
> As a workaround we can clear the PORT_WAKE_BITS.
> 
> Signed-off-by: Jiahau Chang <Lars_Chang@asmedia.com.tw>
> Signed-off-by: AceLan Kao <acelan.kao@canonical.com>

What is the source of this? upstream? linux-next? random git tree?

-Stefan

> ---
>  drivers/usb/host/xhci-hub.c | 19 ++++++++++++++-----
>  drivers/usb/host/xhci-pci.c | 13 +++++++++++++
>  drivers/usb/host/xhci.h     |  2 ++
>  3 files changed, 29 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
> index 7e2c0de..bc9349b 100644
> --- a/drivers/usb/host/xhci-hub.c
> +++ b/drivers/usb/host/xhci-hub.c
> @@ -1105,12 +1105,19 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
>  			xhci_dbg(xhci, "set port reset, actual port %d status  = 0x%x\n", wIndex, temp);
>  			break;
>  		case USB_PORT_FEAT_REMOTE_WAKE_MASK:
> -			xhci_set_remote_wake_mask(xhci, port_array,
> +			if ((xhci->quirks & XHCI_U2_DISABLE_WAKE) && (hcd->speed < HCD_USB3)) {
> +				temp = readl(port_array[wIndex]);
> +				xhci_dbg(xhci, "skip set port remote wake mask, "
> +						"actual port %d status  = 0x%x\n",
> +						wIndex, temp);
> +			} else {
> +				xhci_set_remote_wake_mask(xhci, port_array,
>  					wIndex, wake_mask);
> -			temp = readl(port_array[wIndex]);
> -			xhci_dbg(xhci, "set port remote wake mask, "
> -					"actual port %d status  = 0x%x\n",
> -					wIndex, temp);
> +				temp = readl(port_array[wIndex]);
> +				xhci_dbg(xhci, "set port remote wake mask, "
> +						"actual port %d status  = 0x%x\n",
> +						wIndex, temp);
> +			}
>  			break;
>  		case USB_PORT_FEAT_BH_PORT_RESET:
>  			temp |= PORT_WR;
> @@ -1342,6 +1349,8 @@ 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 dd262f4..e4369e8 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -53,6 +53,11 @@
>  #define PCI_DEVICE_ID_INTEL_BROXTON_B_XHCI		0x1aa8
>  #define PCI_DEVICE_ID_INTEL_APL_XHCI			0x5aa8
>  
> +#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
> +
>  static const char hcd_name[] = "xhci_hcd";
>  
>  static struct hc_driver __read_mostly xhci_pci_hc_driver;
> @@ -135,6 +140,14 @@ 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 2e34c9f..d723fef 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -1644,6 +1644,8 @@ struct xhci_hcd {
>  #define XHCI_BROKEN_STREAMS	(1 << 19)
>  #define XHCI_PME_STUCK_QUIRK	(1 << 20)
>  #define XHCI_MISSING_CAS	(1 << 24)
> +#define XHCI_U2_DISABLE_WAKE    (1 << 27)
> +
>  	unsigned int		num_active_eps;
>  	unsigned int		limit_active_eps;
>  	/* There are two roothubs to keep track of bus suspend info for */
>
AceLan Kao June 22, 2017, 2:50 a.m. UTC | #2 
Hi Stefan,

This patch is not shown up in any tree yet, but it just accepted by
the maintainer the v4 version.
I'll create another SRU for v4 patch, please ignore this one.
Thanks.

Best regards,
AceLan Kao.

2017-06-02 18:14 GMT+08:00 Stefan Bader <stefan.bader@canonical.com>:
> On 02.06.2017 10:56, AceLan Kao wrote:
>> From: Jiahau Chang <jiahau@gmail.com>
>>
>> v3: Fix some checkpatch.pl warnings
>>
>> BugLink: http://bugs.launchpad.net/bugs/1695216
>>
>> For AMD Promontory xHCI host, although you can disable USB 2.0 ports in BIOS
>> settings, those ports will be enabled anyway after you remove a device on
>> that port and re-plug it in again. It's a known limitation of the chip.
>> As a workaround we can clear the PORT_WAKE_BITS.
>>
>> Signed-off-by: Jiahau Chang <Lars_Chang@asmedia.com.tw>
>> Signed-off-by: AceLan Kao <acelan.kao@canonical.com>
>
> What is the source of this? upstream? linux-next? random git tree?
>
> -Stefan
>
>> ---
>>  drivers/usb/host/xhci-hub.c | 19 ++++++++++++++-----
>>  drivers/usb/host/xhci-pci.c | 13 +++++++++++++
>>  drivers/usb/host/xhci.h     |  2 ++
>>  3 files changed, 29 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
>> index 7e2c0de..bc9349b 100644
>> --- a/drivers/usb/host/xhci-hub.c
>> +++ b/drivers/usb/host/xhci-hub.c
>> @@ -1105,12 +1105,19 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
>>                       xhci_dbg(xhci, "set port reset, actual port %d status  = 0x%x\n", wIndex, temp);
>>                       break;
>>               case USB_PORT_FEAT_REMOTE_WAKE_MASK:
>> -                     xhci_set_remote_wake_mask(xhci, port_array,
>> +                     if ((xhci->quirks & XHCI_U2_DISABLE_WAKE) && (hcd->speed < HCD_USB3)) {
>> +                             temp = readl(port_array[wIndex]);
>> +                             xhci_dbg(xhci, "skip set port remote wake mask, "
>> +                                             "actual port %d status  = 0x%x\n",
>> +                                             wIndex, temp);
>> +                     } else {
>> +                             xhci_set_remote_wake_mask(xhci, port_array,
>>                                       wIndex, wake_mask);
>> -                     temp = readl(port_array[wIndex]);
>> -                     xhci_dbg(xhci, "set port remote wake mask, "
>> -                                     "actual port %d status  = 0x%x\n",
>> -                                     wIndex, temp);
>> +                             temp = readl(port_array[wIndex]);
>> +                             xhci_dbg(xhci, "set port remote wake mask, "
>> +                                             "actual port %d status  = 0x%x\n",
>> +                                             wIndex, temp);
>> +                     }
>>                       break;
>>               case USB_PORT_FEAT_BH_PORT_RESET:
>>                       temp |= PORT_WR;
>> @@ -1342,6 +1349,8 @@ 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 dd262f4..e4369e8 100644
>> --- a/drivers/usb/host/xhci-pci.c
>> +++ b/drivers/usb/host/xhci-pci.c
>> @@ -53,6 +53,11 @@
>>  #define PCI_DEVICE_ID_INTEL_BROXTON_B_XHCI           0x1aa8
>>  #define PCI_DEVICE_ID_INTEL_APL_XHCI                 0x5aa8
>>
>> +#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
>> +
>>  static const char hcd_name[] = "xhci_hcd";
>>
>>  static struct hc_driver __read_mostly xhci_pci_hc_driver;
>> @@ -135,6 +140,14 @@ 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 2e34c9f..d723fef 100644
>> --- a/drivers/usb/host/xhci.h
>> +++ b/drivers/usb/host/xhci.h
>> @@ -1644,6 +1644,8 @@ struct xhci_hcd {
>>  #define XHCI_BROKEN_STREAMS  (1 << 19)
>>  #define XHCI_PME_STUCK_QUIRK (1 << 20)
>>  #define XHCI_MISSING_CAS     (1 << 24)
>> +#define XHCI_U2_DISABLE_WAKE    (1 << 27)
>> +
>>       unsigned int            num_active_eps;
>>       unsigned int            limit_active_eps;
>>       /* There are two roothubs to keep track of bus suspend info for */
>>
>
>
>
> --
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
>
Po-Hsu Lin June 22, 2017, 3:02 a.m. UTC | #3 
NAK this one as per AceLan's comment.

On Thu, Jun 22, 2017 at 10:50 AM, AceLan Kao <acelan.kao@canonical.com> wrote:
> Hi Stefan,
>
> This patch is not shown up in any tree yet, but it just accepted by
> the maintainer the v4 version.
> I'll create another SRU for v4 patch, please ignore this one.
> Thanks.
>
> Best regards,
> AceLan Kao.
>
diff mbox

Patch

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 7e2c0de..bc9349b 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -1105,12 +1105,19 @@  int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 			xhci_dbg(xhci, "set port reset, actual port %d status  = 0x%x\n", wIndex, temp);
 			break;
 		case USB_PORT_FEAT_REMOTE_WAKE_MASK:
-			xhci_set_remote_wake_mask(xhci, port_array,
+			if ((xhci->quirks & XHCI_U2_DISABLE_WAKE) && (hcd->speed < HCD_USB3)) {
+				temp = readl(port_array[wIndex]);
+				xhci_dbg(xhci, "skip set port remote wake mask, "
+						"actual port %d status  = 0x%x\n",
+						wIndex, temp);
+			} else {
+				xhci_set_remote_wake_mask(xhci, port_array,
 					wIndex, wake_mask);
-			temp = readl(port_array[wIndex]);
-			xhci_dbg(xhci, "set port remote wake mask, "
-					"actual port %d status  = 0x%x\n",
-					wIndex, temp);
+				temp = readl(port_array[wIndex]);
+				xhci_dbg(xhci, "set port remote wake mask, "
+						"actual port %d status  = 0x%x\n",
+						wIndex, temp);
+			}
 			break;
 		case USB_PORT_FEAT_BH_PORT_RESET:
 			temp |= PORT_WR;
@@ -1342,6 +1349,8 @@  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 dd262f4..e4369e8 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -53,6 +53,11 @@ 
 #define PCI_DEVICE_ID_INTEL_BROXTON_B_XHCI		0x1aa8
 #define PCI_DEVICE_ID_INTEL_APL_XHCI			0x5aa8
 
+#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
+
 static const char hcd_name[] = "xhci_hcd";
 
 static struct hc_driver __read_mostly xhci_pci_hc_driver;
@@ -135,6 +140,14 @@  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 2e34c9f..d723fef 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1644,6 +1644,8 @@  struct xhci_hcd {
 #define XHCI_BROKEN_STREAMS	(1 << 19)
 #define XHCI_PME_STUCK_QUIRK	(1 << 20)
 #define XHCI_MISSING_CAS	(1 << 24)
+#define XHCI_U2_DISABLE_WAKE    (1 << 27)
+
 	unsigned int		num_active_eps;
 	unsigned int		limit_active_eps;
 	/* There are two roothubs to keep track of bus suspend info for */