On Thu, Aug 9, 2012 at 10:24 PM, Sarah Sharp
<sarah.a.sharp@xxxxxxxxxxxxxxx> wrote:
> On Thu, Aug 09, 2012 at 05:31:51PM +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.
>
> Why would the BIOS expect some ports to not be switchable? I know that
> we internally at Intel had discussed some theoretical reasons why it
> might not be good to switch some ports, but when I presented the
> original patch with this same code in it to Linux USB mailing list, both
> Alan and Greg said, "Why not unconditionally switch ports?" I had no
> good examples at the time.
>
> Is this causing issues with some particular BIOS?
>
Yes, this is causing the internal webcam missing on the USB bus as I
observed on some HM70-based laptops.
The internal webcam is attached to one port that is controlled by the
xhci host.
But the other ports with the outer plugs work well after booting. I
cannot test the USB port of the internal webcam easily (without
tearing down the laptop :-/).
I also tried some similar HM77-based models. HM77 has no this issue.
This could be some chipset mystery I am not aware now.
Some bisecting led to your original patch.
>> 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@xxxxxxxxxxxxx>
>> ---
>> 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.
>
> Your code is correct, but your comment is wrong. XUSB2PRM is the USB
> 2.0 ports that should be controlled by the xHCI host.
>
Thanks for the explanation.
>> + */
>> +
>> + 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);
>
> Again, this should be "Configurable USB 2.0 ports to hand over to xHCI:"
> Also, don't split strings, it makes it hard to grep for them later.
>
Thanks. I will re-sent a patch with the correct comment.
>> +
>> /* 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.
>
> Sarah Sharp
-kengyu
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
