Patchwork Intel xhci: Only switch the switchable ports

login
register
mail settings
Submitter Keng-Yu Lin
Date Aug. 9, 2012, 10:06 a.m.
Message ID <1344506817-11547-2-git-send-email-kengyu@canonical.com>
Download mbox | patch
Permalink /patch/176044/
State New
Headers show

Comments

Keng-Yu Lin - Aug. 9, 2012, 10:06 a.m.
With a previous patch to enable the EHCI/XHCI port switching, it switches
all the available ports.

The assumption is not correct because the BIOS may expect some ports
not switchable by the OS.

There are two more registers that contains the information of the switchable
and non-switchable ports.

This patch adds the checking code for the two register so that only the
switchable ports are altered.

Signed-off-by: Keng-Yu Lin <kengyu@canonical.com>
---
 drivers/usb/host/pci-quirks.c |   27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)
Andy Whitcroft - Aug. 9, 2012, 11:26 a.m.
On Thu, Aug 09, 2012 at 06:06:57PM +0800, Keng-Yu Lin wrote:
> With a previous patch to enable the EHCI/XHCI port switching, it switches
> all the available ports.
> 
> The assumption is not correct because the BIOS may expect some ports
> not switchable by the OS.
> 
> There are two more registers that contains the information of the switchable
> and non-switchable ports.
> 
> This patch adds the checking code for the two register so that only the
> switchable ports are altered.
> 
> Signed-off-by: Keng-Yu Lin <kengyu@canonical.com>
> ---
>  drivers/usb/host/pci-quirks.c |   27 +++++++++++++++++++++++----
>  1 file changed, 23 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
> index 833b3c6..89f62f2 100644
> --- a/drivers/usb/host/pci-quirks.c
> +++ b/drivers/usb/host/pci-quirks.c
> @@ -75,7 +75,9 @@
>  #define	NB_PIF0_PWRDOWN_1	0x01100013
>  
>  #define USB_INTEL_XUSB2PR      0xD0
> +#define USB_INTEL_USB2PRM      0xD4
>  #define USB_INTEL_USB3_PSSEN   0xD8
> +#define USB_INTEL_USB3PRM      0xDC
>  
>  static struct amd_chipset_info {
>  	struct pci_dev	*nb_dev;
> @@ -772,10 +774,18 @@ void usb_enable_xhci_ports(struct pci_dev *xhci_pdev)
>  		return;
>  	}
>  
> -	ports_available = 0xffffffff;
> +	/* Read USB3PRM, the USB 3.0 Port Routing Mask Register
> +	 * Indicate the ports that can be changed from OS.
> +	 */
> +	pci_read_config_dword(xhci_pdev, USB_INTEL_USB3PRM,
> +			&ports_available);
> +
> +	dev_dbg(&xhci_pdev->dev, "Configurable ports to enable SuperSpeed: 0x%x\n",
> +			ports_available);
> +
>  	/* Write USB3_PSSEN, the USB 3.0 Port SuperSpeed Enable
> -	 * Register, to turn on SuperSpeed terminations for all
> -	 * available ports.
> +	 * Register, to turn on SuperSpeed terminations for the
> +	 * switchable ports.
>  	 */
>  	pci_write_config_dword(xhci_pdev, USB_INTEL_USB3_PSSEN,
>  			cpu_to_le32(ports_available));
> @@ -785,7 +795,16 @@ void usb_enable_xhci_ports(struct pci_dev *xhci_pdev)
>  	dev_dbg(&xhci_pdev->dev, "USB 3.0 ports that are now enabled "
>  			"under xHCI: 0x%x\n", ports_available);
>  
> -	ports_available = 0xffffffff;
> +	/* Read XUSB2PRM, xHCI USB 2.0 Port Routing Mask Register
> +	 * Indicate the port to be controlled by the EHCI host.
> +	 */
> +
> +	pci_read_config_dword(xhci_pdev, USB_INTEL_USB2PRM,
> +			&ports_available);
> +
> +	dev_dbg(&xhci_pdev->dev, "Configurable ports to hand over the ECHI host:
> +			0x%x\n", ports_available);
> +
>  	/* Write XUSB2PR, the xHC USB 2.0 Port Routing Register, to
>  	 * switch the USB 2.0 power and data lines over to the xHCI
>  	 * host.

The patch _looks_ reasonable.  Has this been tested on anything without
the affected hardware?  Is it going upstream?

-apw
Leann Ogasawara - Aug. 9, 2012, 6:13 p.m.
On 08/09/2012 04:26 AM, Andy Whitcroft wrote:
> On Thu, Aug 09, 2012 at 06:06:57PM +0800, Keng-Yu Lin wrote:
>> With a previous patch to enable the EHCI/XHCI port switching, it switches
>> all the available ports.
>>
>> The assumption is not correct because the BIOS may expect some ports
>> not switchable by the OS.
>>
>> There are two more registers that contains the information of the switchable
>> and non-switchable ports.
>>
>> This patch adds the checking code for the two register so that only the
>> switchable ports are altered.
>>
>> Signed-off-by: Keng-Yu Lin <kengyu@canonical.com>
>> ---
>>  drivers/usb/host/pci-quirks.c |   27 +++++++++++++++++++++++----
>>  1 file changed, 23 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
>> index 833b3c6..89f62f2 100644
>> --- a/drivers/usb/host/pci-quirks.c
>> +++ b/drivers/usb/host/pci-quirks.c
>> @@ -75,7 +75,9 @@
>>  #define	NB_PIF0_PWRDOWN_1	0x01100013
>>  
>>  #define USB_INTEL_XUSB2PR      0xD0
>> +#define USB_INTEL_USB2PRM      0xD4
>>  #define USB_INTEL_USB3_PSSEN   0xD8
>> +#define USB_INTEL_USB3PRM      0xDC
>>  
>>  static struct amd_chipset_info {
>>  	struct pci_dev	*nb_dev;
>> @@ -772,10 +774,18 @@ void usb_enable_xhci_ports(struct pci_dev *xhci_pdev)
>>  		return;
>>  	}
>>  
>> -	ports_available = 0xffffffff;
>> +	/* Read USB3PRM, the USB 3.0 Port Routing Mask Register
>> +	 * Indicate the ports that can be changed from OS.
>> +	 */
>> +	pci_read_config_dword(xhci_pdev, USB_INTEL_USB3PRM,
>> +			&ports_available);
>> +
>> +	dev_dbg(&xhci_pdev->dev, "Configurable ports to enable SuperSpeed: 0x%x\n",
>> +			ports_available);
>> +
>>  	/* Write USB3_PSSEN, the USB 3.0 Port SuperSpeed Enable
>> -	 * Register, to turn on SuperSpeed terminations for all
>> -	 * available ports.
>> +	 * Register, to turn on SuperSpeed terminations for the
>> +	 * switchable ports.
>>  	 */
>>  	pci_write_config_dword(xhci_pdev, USB_INTEL_USB3_PSSEN,
>>  			cpu_to_le32(ports_available));
>> @@ -785,7 +795,16 @@ void usb_enable_xhci_ports(struct pci_dev *xhci_pdev)
>>  	dev_dbg(&xhci_pdev->dev, "USB 3.0 ports that are now enabled "
>>  			"under xHCI: 0x%x\n", ports_available);
>>  
>> -	ports_available = 0xffffffff;
>> +	/* Read XUSB2PRM, xHCI USB 2.0 Port Routing Mask Register
>> +	 * Indicate the port to be controlled by the EHCI host.
>> +	 */
>> +
>> +	pci_read_config_dword(xhci_pdev, USB_INTEL_USB2PRM,
>> +			&ports_available);
>> +
>> +	dev_dbg(&xhci_pdev->dev, "Configurable ports to hand over the ECHI host:
>> +			0x%x\n", ports_available);
>> +
>>  	/* Write XUSB2PR, the xHC USB 2.0 Port Routing Register, to
>>  	 * switch the USB 2.0 power and data lines over to the xHCI
>>  	 * host.
> The patch _looks_ reasonable.  Has this been tested on anything without
> the affected hardware?  Is it going upstream?

It does look like it's been sent upstream, although sparking some
additional discussion:

https://lkml.org/lkml/2012/8/9/356

For Quantal, since we have some time before we release, I'd like to wait
and see how this shakes out upstream first.

Thanks,
Leann
Keng-Yu Lin - Aug. 13, 2012, 6:25 a.m.
On Thu, Aug 9, 2012 at 7:26 PM, Andy Whitcroft <apw@canonical.com> wrote:
>
> The patch _looks_ reasonable.  Has this been tested on anything without
> the affected hardware?  Is it going upstream?
>
> -apw

I did test the patch on various Panther Point-based machines. They all
work well with the patch. On the affected machines, the webcam issue
is fixed. On the unaffected machines USB works.

The previous upstream port switching patch made it only do the port
switching on Panter Point platform by checking the PCI IDs. So my
patch in consequence does this way too.


There are really affected and unaffected hardware, though they share
the same PCI device ID 0x1E26. HM70 platform is affected by this
issue, HM77 is not.
But at the moment I have no idea on the chipset mystery. (I've been
always assuming the same PCI IDs means the same thing, in fact it's
not in the case).

However in general, the XUSB2PRM and USB3PRM registers do contain the
information of which ports should not be altered from its original
value set-up by BIOS.
My patch just implements the check and make the kernel follow the
designed behaviour.

  cheers
-kengyu
Keng-Yu Lin - Sept. 12, 2012, 12:38 p.m.
On Fri, Aug 10, 2012 at 2:13 AM, Leann Ogasawara
<leann.ogasawara@canonical.com> wrote:
> On 08/09/2012 04:26 AM, Andy Whitcroft wrote:
>> On Thu, Aug 09, 2012 at 06:06:57PM +0800, Keng-Yu Lin wrote:
>>> With a previous patch to enable the EHCI/XHCI port switching, it switches
>>> all the available ports.
>>>
>>> The assumption is not correct because the BIOS may expect some ports
>>> not switchable by the OS.
>>>
>>> There are two more registers that contains the information of the switchable
>>> and non-switchable ports.
>>>
>>> This patch adds the checking code for the two register so that only the
>>> switchable ports are altered.
>>>
>>> Signed-off-by: Keng-Yu Lin <kengyu@canonical.com>
>>> ---
>>>  drivers/usb/host/pci-quirks.c |   27 +++++++++++++++++++++++----
>>>  1 file changed, 23 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
>>> index 833b3c6..89f62f2 100644
>>> --- a/drivers/usb/host/pci-quirks.c
>>> +++ b/drivers/usb/host/pci-quirks.c
>>> @@ -75,7 +75,9 @@
>>>  #define     NB_PIF0_PWRDOWN_1       0x01100013
>>>
>>>  #define USB_INTEL_XUSB2PR      0xD0
>>> +#define USB_INTEL_USB2PRM      0xD4
>>>  #define USB_INTEL_USB3_PSSEN   0xD8
>>> +#define USB_INTEL_USB3PRM      0xDC
>>>
>>>  static struct amd_chipset_info {
>>>      struct pci_dev  *nb_dev;
>>> @@ -772,10 +774,18 @@ void usb_enable_xhci_ports(struct pci_dev *xhci_pdev)
>>>              return;
>>>      }
>>>
>>> -    ports_available = 0xffffffff;
>>> +    /* Read USB3PRM, the USB 3.0 Port Routing Mask Register
>>> +     * Indicate the ports that can be changed from OS.
>>> +     */
>>> +    pci_read_config_dword(xhci_pdev, USB_INTEL_USB3PRM,
>>> +                    &ports_available);
>>> +
>>> +    dev_dbg(&xhci_pdev->dev, "Configurable ports to enable SuperSpeed: 0x%x\n",
>>> +                    ports_available);
>>> +
>>>      /* Write USB3_PSSEN, the USB 3.0 Port SuperSpeed Enable
>>> -     * Register, to turn on SuperSpeed terminations for all
>>> -     * available ports.
>>> +     * Register, to turn on SuperSpeed terminations for the
>>> +     * switchable ports.
>>>       */
>>>      pci_write_config_dword(xhci_pdev, USB_INTEL_USB3_PSSEN,
>>>                      cpu_to_le32(ports_available));
>>> @@ -785,7 +795,16 @@ void usb_enable_xhci_ports(struct pci_dev *xhci_pdev)
>>>      dev_dbg(&xhci_pdev->dev, "USB 3.0 ports that are now enabled "
>>>                      "under xHCI: 0x%x\n", ports_available);
>>>
>>> -    ports_available = 0xffffffff;
>>> +    /* Read XUSB2PRM, xHCI USB 2.0 Port Routing Mask Register
>>> +     * Indicate the port to be controlled by the EHCI host.
>>> +     */
>>> +
>>> +    pci_read_config_dword(xhci_pdev, USB_INTEL_USB2PRM,
>>> +                    &ports_available);
>>> +
>>> +    dev_dbg(&xhci_pdev->dev, "Configurable ports to hand over the ECHI host:
>>> +                    0x%x\n", ports_available);
>>> +
>>>      /* Write XUSB2PR, the xHC USB 2.0 Port Routing Register, to
>>>       * switch the USB 2.0 power and data lines over to the xHCI
>>>       * host.
>> The patch _looks_ reasonable.  Has this been tested on anything without
>> the affected hardware?  Is it going upstream?
>
> It does look like it's been sent upstream, although sparking some
> additional discussion:
>
> https://lkml.org/lkml/2012/8/9/356
>
> For Quantal, since we have some time before we release, I'd like to wait
> and see how this shakes out upstream first.
>
> Thanks,
> Leann

Hi all:
  The patch has arrived in greg's usb git tree
(https://git.kernel.org/?p=linux/kernel/git/gregkh/usb.git;a=commit;h=a96874a2a92feaef607ddd3137277a788cb927a6)
and I just see another machine affected as bug 1034814 described.

  Just wonder if there is any chance that we speed up the SRU will be
very appreciated.

  cheers,
-kengyu
Leann Ogasawara - Sept. 12, 2012, 4:17 p.m.
On 09/12/2012 05:38 AM, Keng-Yu Lin wrote:
> On Fri, Aug 10, 2012 at 2:13 AM, Leann Ogasawara
> <leann.ogasawara@canonical.com> wrote:
>> On 08/09/2012 04:26 AM, Andy Whitcroft wrote:
>>> On Thu, Aug 09, 2012 at 06:06:57PM +0800, Keng-Yu Lin wrote:
>>>> With a previous patch to enable the EHCI/XHCI port switching, it switches
>>>> all the available ports.
>>>>
>>>> The assumption is not correct because the BIOS may expect some ports
>>>> not switchable by the OS.
>>>>
>>>> There are two more registers that contains the information of the switchable
>>>> and non-switchable ports.
>>>>
>>>> This patch adds the checking code for the two register so that only the
>>>> switchable ports are altered.
>>>>
>>>> Signed-off-by: Keng-Yu Lin <kengyu@canonical.com>
>>>> ---
>>>>  drivers/usb/host/pci-quirks.c |   27 +++++++++++++++++++++++----
>>>>  1 file changed, 23 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
>>>> index 833b3c6..89f62f2 100644
>>>> --- a/drivers/usb/host/pci-quirks.c
>>>> +++ b/drivers/usb/host/pci-quirks.c
>>>> @@ -75,7 +75,9 @@
>>>>  #define     NB_PIF0_PWRDOWN_1       0x01100013
>>>>
>>>>  #define USB_INTEL_XUSB2PR      0xD0
>>>> +#define USB_INTEL_USB2PRM      0xD4
>>>>  #define USB_INTEL_USB3_PSSEN   0xD8
>>>> +#define USB_INTEL_USB3PRM      0xDC
>>>>
>>>>  static struct amd_chipset_info {
>>>>      struct pci_dev  *nb_dev;
>>>> @@ -772,10 +774,18 @@ void usb_enable_xhci_ports(struct pci_dev *xhci_pdev)
>>>>              return;
>>>>      }
>>>>
>>>> -    ports_available = 0xffffffff;
>>>> +    /* Read USB3PRM, the USB 3.0 Port Routing Mask Register
>>>> +     * Indicate the ports that can be changed from OS.
>>>> +     */
>>>> +    pci_read_config_dword(xhci_pdev, USB_INTEL_USB3PRM,
>>>> +                    &ports_available);
>>>> +
>>>> +    dev_dbg(&xhci_pdev->dev, "Configurable ports to enable SuperSpeed: 0x%x\n",
>>>> +                    ports_available);
>>>> +
>>>>      /* Write USB3_PSSEN, the USB 3.0 Port SuperSpeed Enable
>>>> -     * Register, to turn on SuperSpeed terminations for all
>>>> -     * available ports.
>>>> +     * Register, to turn on SuperSpeed terminations for the
>>>> +     * switchable ports.
>>>>       */
>>>>      pci_write_config_dword(xhci_pdev, USB_INTEL_USB3_PSSEN,
>>>>                      cpu_to_le32(ports_available));
>>>> @@ -785,7 +795,16 @@ void usb_enable_xhci_ports(struct pci_dev *xhci_pdev)
>>>>      dev_dbg(&xhci_pdev->dev, "USB 3.0 ports that are now enabled "
>>>>                      "under xHCI: 0x%x\n", ports_available);
>>>>
>>>> -    ports_available = 0xffffffff;
>>>> +    /* Read XUSB2PRM, xHCI USB 2.0 Port Routing Mask Register
>>>> +     * Indicate the port to be controlled by the EHCI host.
>>>> +     */
>>>> +
>>>> +    pci_read_config_dword(xhci_pdev, USB_INTEL_USB2PRM,
>>>> +                    &ports_available);
>>>> +
>>>> +    dev_dbg(&xhci_pdev->dev, "Configurable ports to hand over the ECHI host:
>>>> +                    0x%x\n", ports_available);
>>>> +
>>>>      /* Write XUSB2PR, the xHC USB 2.0 Port Routing Register, to
>>>>       * switch the USB 2.0 power and data lines over to the xHCI
>>>>       * host.
>>> The patch _looks_ reasonable.  Has this been tested on anything without
>>> the affected hardware?  Is it going upstream?
>> It does look like it's been sent upstream, although sparking some
>> additional discussion:
>>
>> https://lkml.org/lkml/2012/8/9/356
>>
>> For Quantal, since we have some time before we release, I'd like to wait
>> and see how this shakes out upstream first.
>>
>> Thanks,
>> Leann
> Hi all:
>   The patch has arrived in greg's usb git tree
> (https://git.kernel.org/?p=linux/kernel/git/gregkh/usb.git;a=commit;h=a96874a2a92feaef607ddd3137277a788cb927a6)
> and I just see another machine affected as bug 1034814 described.
>
>   Just wonder if there is any chance that we speed up the SRU will be
> very appreciated.

I've applied this to Quantal master-next to get some testing.  I'll also
Ack for Precise.

Acked-by: Leann Ogasawara <leann.ogasawara@canonical.com>

Thanks,
Leann
Tim Gardner - Sept. 12, 2012, 5:01 p.m.

Patch

diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index 833b3c6..89f62f2 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -75,7 +75,9 @@ 
 #define	NB_PIF0_PWRDOWN_1	0x01100013
 
 #define USB_INTEL_XUSB2PR      0xD0
+#define USB_INTEL_USB2PRM      0xD4
 #define USB_INTEL_USB3_PSSEN   0xD8
+#define USB_INTEL_USB3PRM      0xDC
 
 static struct amd_chipset_info {
 	struct pci_dev	*nb_dev;
@@ -772,10 +774,18 @@  void usb_enable_xhci_ports(struct pci_dev *xhci_pdev)
 		return;
 	}
 
-	ports_available = 0xffffffff;
+	/* Read USB3PRM, the USB 3.0 Port Routing Mask Register
+	 * Indicate the ports that can be changed from OS.
+	 */
+	pci_read_config_dword(xhci_pdev, USB_INTEL_USB3PRM,
+			&ports_available);
+
+	dev_dbg(&xhci_pdev->dev, "Configurable ports to enable SuperSpeed: 0x%x\n",
+			ports_available);
+
 	/* Write USB3_PSSEN, the USB 3.0 Port SuperSpeed Enable
-	 * Register, to turn on SuperSpeed terminations for all
-	 * available ports.
+	 * Register, to turn on SuperSpeed terminations for the
+	 * switchable ports.
 	 */
 	pci_write_config_dword(xhci_pdev, USB_INTEL_USB3_PSSEN,
 			cpu_to_le32(ports_available));
@@ -785,7 +795,16 @@  void usb_enable_xhci_ports(struct pci_dev *xhci_pdev)
 	dev_dbg(&xhci_pdev->dev, "USB 3.0 ports that are now enabled "
 			"under xHCI: 0x%x\n", ports_available);
 
-	ports_available = 0xffffffff;
+	/* Read XUSB2PRM, xHCI USB 2.0 Port Routing Mask Register
+	 * Indicate the port to be controlled by the EHCI host.
+	 */
+
+	pci_read_config_dword(xhci_pdev, USB_INTEL_USB2PRM,
+			&ports_available);
+
+	dev_dbg(&xhci_pdev->dev, "Configurable ports to hand over the ECHI host:
+			0x%x\n", ports_available);
+
 	/* Write XUSB2PR, the xHC USB 2.0 Port Routing Register, to
 	 * switch the USB 2.0 power and data lines over to the xHCI
 	 * host.