Re: [PATCH] xhci: Add BayTrail to list of Intel switchable hosts
On Tue, May 28, 2013 at 10:53:56AM +0300, Heikki Krogerus wrote: Hi Sarah, On Fri, May 24, 2013 at 09:55:25AM -0700, Sarah Sharp wrote: At this point the port switchover quirk is getting unwieldy. I know of at least two more platforms that will need the switchover quirk, and it's silly to keep adding them to the list. Heikki, can you change the code to always try to switchover the ports from EHCI to xHCI if an Intel xHCI host is found, and fail gracefully if there isn't an EHCI host controller on board? I suspect Intel will include the port switchover mechanism until they decide to only include xHCI hosts and no EHCI hosts. In the meantime, we can avoid breaking USB 3.0 new platforms by always attempting to do the switchover. OK, makes sense. I or Mathias will make the patch for you. Great, thanks! Sarah Sharp -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xhci: Add BayTrail to list of Intel switchable hosts
Hi Sarah, On Fri, May 24, 2013 at 09:55:25AM -0700, Sarah Sharp wrote: At this point the port switchover quirk is getting unwieldy. I know of at least two more platforms that will need the switchover quirk, and it's silly to keep adding them to the list. Heikki, can you change the code to always try to switchover the ports from EHCI to xHCI if an Intel xHCI host is found, and fail gracefully if there isn't an EHCI host controller on board? I suspect Intel will include the port switchover mechanism until they decide to only include xHCI hosts and no EHCI hosts. In the meantime, we can avoid breaking USB 3.0 new platforms by always attempting to do the switchover. OK, makes sense. I or Mathias will make the patch for you. Thanks, -- heikki -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xhci: Add BayTrail to list of Intel switchable hosts
On Sat, May 25, 2013 at 03:00:28AM +0400, Sergei Shtylyov wrote: Hello. On 05/25/2013 01:54 AM, Sarah Sharp wrote: From: Chew, Chiau Ee chiau.ee.c...@intel.com Like the xHCI controller on Intel Panther Point and Lynx Point chipsets, the xHCI controller on Intel BayTrail has also ports that can be switched between the EHCI host controller. s/between/to/ Sergei, DO NOT REVIEW PATCHES TO THE XHCI DRIVER. OK, I'll try to follow another rule from you, though I'm getting tired of you telling me what not to do. I've told you not to review patches to the xHCI driver three times now. This is not another rule, this is something I've asked you to avoid already. Strange, I would have remembered if you told me *thrice*, yet what I clearly remember you told me is not to comment on your own patches. Our recollections differ. Fine. I am telling you now, do not provide reviews for ANY xHCI driver patches where the number of noise comments (grammar, spelling fixes, extra parens, etc) outnumber the signal comments (bugs in the patch, missing some idea, etc). Sarah Sharp -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xhci: Add BayTrail to list of Intel switchable hosts
At this point the port switchover quirk is getting unwieldy. I know of
at least two more platforms that will need the switchover quirk, and
it's silly to keep adding them to the list.
Heikki, can you change the code to always try to switchover the ports
from EHCI to xHCI if an Intel xHCI host is found, and fail gracefully if
there isn't an EHCI host controller on board?
I suspect Intel will include the port switchover mechanism until they
decide to only include xHCI hosts and no EHCI hosts. In the meantime,
we can avoid breaking USB 3.0 new platforms by always attempting to do
the switchover.
Sarah Sharp
On Tue, May 21, 2013 at 09:08:16AM +0300, Heikki Krogerus wrote:
From: Chew, Chiau Ee chiau.ee.c...@intel.com
Like the xHCI controller on Intel Panther Point and Lynx Point
chipsets, the xHCI controller on Intel BayTrail has also ports that
can be switched between the EHCI host controller.
This patch should be backported to stable kernels as old as 3.0,
that contain commit 69e848c2090aebba5698a1620604c7dccb448684
Intel xhci: Support EHCI/xHCI port switching.
Signed-off-by: Chew, Chiau Ee chiau.ee.c...@intel.com
Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com
Cc: sta...@vger.kernel.org
---
drivers/usb/host/ehci-pci.c |3 ++-
drivers/usb/host/pci-quirks.c | 12 +++-
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c
index 595d210..a5708d9 100644
--- a/drivers/usb/host/ehci-pci.c
+++ b/drivers/usb/host/ehci-pci.c
@@ -322,7 +322,8 @@ static bool usb_is_intel_switchable_ehci(struct pci_dev
*pdev)
(pdev-device == 0x1E26 ||
pdev-device == 0x8C2D ||
pdev-device == 0x8C26 ||
- pdev-device == 0x9C26);
+ pdev-device == 0x9C26 ||
+ pdev-device == 0x0F34);
}
static void ehci_enable_xhci_companion(void)
diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index 4c338ec..6062822 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -724,6 +724,7 @@ static int handshake(void __iomem *ptr, u32 mask, u32
done,
#define PCI_DEVICE_ID_INTEL_LYNX_POINT_XHCI 0x8C31
#define PCI_DEVICE_ID_INTEL_LYNX_POINT_LP_XHCI 0x9C31
+#define PCI_DEVICE_ID_INTEL_BYT_XHCI 0x0F35
bool usb_is_intel_ppt_switchable_xhci(struct pci_dev *pdev)
{
@@ -741,10 +742,19 @@ bool usb_is_intel_lpt_switchable_xhci(struct pci_dev
*pdev)
pdev-device == PCI_DEVICE_ID_INTEL_LYNX_POINT_LP_XHCI);
}
+/* And so does the Intel BayTrail. */
+bool usb_is_intel_byt_switchable_xhci(struct pci_dev *pdev)
+{
+ return pdev-class == PCI_CLASS_SERIAL_USB_XHCI
+ pdev-vendor == PCI_VENDOR_ID_INTEL
+ pdev-device == PCI_DEVICE_ID_INTEL_BYT_XHCI;
+}
+
bool usb_is_intel_switchable_xhci(struct pci_dev *pdev)
{
return usb_is_intel_ppt_switchable_xhci(pdev) ||
- usb_is_intel_lpt_switchable_xhci(pdev);
+ usb_is_intel_lpt_switchable_xhci(pdev) ||
+ usb_is_intel_byt_switchable_xhci(pdev);
}
EXPORT_SYMBOL_GPL(usb_is_intel_switchable_xhci);
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xhci: Add BayTrail to list of Intel switchable hosts
On Tue, May 21, 2013 at 04:26:43PM +0400, Sergei Shtylyov wrote: Hello. On 21-05-2013 10:08, Heikki Krogerus wrote: From: Chew, Chiau Ee chiau.ee.c...@intel.com Like the xHCI controller on Intel Panther Point and Lynx Point chipsets, the xHCI controller on Intel BayTrail has also ports that can be switched between the EHCI host controller. s/between/to/ Sergei, DO NOT REVIEW PATCHES TO THE XHCI DRIVER. Your grammar comments and complaints about extra parenthesis are not constructive and cause unnecessary patch churn. Sarah Sharp -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xhci: Add BayTrail to list of Intel switchable hosts
Hello. On 05/24/2013 08:56 PM, Sarah Sharp wrote: From: Chew, Chiau Ee chiau.ee.c...@intel.com Like the xHCI controller on Intel Panther Point and Lynx Point chipsets, the xHCI controller on Intel BayTrail has also ports that can be switched between the EHCI host controller. s/between/to/ Sergei, DO NOT REVIEW PATCHES TO THE XHCI DRIVER. OK, I'll try to follow another rule from you, though I'm getting tired of you telling me what not to do. Your grammar comments and complaints about extra parenthesis are not constructive and cause unnecessary patch churn. As if I haven't ever found more serious issues in your own XHCI patches (even after you forbade me to review them). Sarah Sharp WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xhci: Add BayTrail to list of Intel switchable hosts
On Fri, May 24, 2013 at 10:05:01PM +0400, Sergei Shtylyov wrote: Hello. On 05/24/2013 08:56 PM, Sarah Sharp wrote: From: Chew, Chiau Ee chiau.ee.c...@intel.com Like the xHCI controller on Intel Panther Point and Lynx Point chipsets, the xHCI controller on Intel BayTrail has also ports that can be switched between the EHCI host controller. s/between/to/ Sergei, DO NOT REVIEW PATCHES TO THE XHCI DRIVER. OK, I'll try to follow another rule from you, though I'm getting tired of you telling me what not to do. I've told you not to review patches to the xHCI driver three times now. This is not another rule, this is something I've asked you to avoid already. I've been asking you to change how you review your patches because you can't seem to pick up on the social cues that nitpicking patches about only grammar and random code style issues is not constructive. You don't seem to be getting the hint, hence the ALL CAPS and blunt words. Your grammar comments and complaints about extra parenthesis are not constructive and cause unnecessary patch churn. As if I haven't ever found more serious issues in your own XHCI patches (even after you forbade me to review them). Serious issues are fine to bring up, either for my patches or for patches that people send for the xHCI. However, the vast majority of comments I see from you are simply grammar comments or nitpicking of code style that checkpatch.pl doesn't complain about. At some point, your signal to noise ratio is so low that most kernel developers are just going to start tuning you out. Greg already put your email into /dev/null. Do you want other kernel developers to do so as well? If you want *serious bugs* to be acted on, then just send email about *serious bugs*. Or include a mix of comments about grammar/code style in with comments about serious code issues. Please increase your signal to noise ratio, or I will continue to send you scathing emails when you nitpick xHCI patches. Sarah Sharp -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xhci: Add BayTrail to list of Intel switchable hosts
Hello. On 05/25/2013 01:54 AM, Sarah Sharp wrote: From: Chew, Chiau Ee chiau.ee.c...@intel.com Like the xHCI controller on Intel Panther Point and Lynx Point chipsets, the xHCI controller on Intel BayTrail has also ports that can be switched between the EHCI host controller. s/between/to/ Sergei, DO NOT REVIEW PATCHES TO THE XHCI DRIVER. OK, I'll try to follow another rule from you, though I'm getting tired of you telling me what not to do. I've told you not to review patches to the xHCI driver three times now. This is not another rule, this is something I've asked you to avoid already. Strange, I would have remembered if you told me *thrice*, yet what I clearly remember you told me is not to comment on your own patches. I've been asking you to change how you review your patches because you can't seem to pick up on the social cues that nitpicking patches about only grammar and random code style issues is not constructive. You don't seem to be getting the hint, hence the ALL CAPS and blunt words. Maybe because you are the only one (plus maybe Greg) having issues with my reviews. I've never been harassed for my review style on any other mailing lists I'm subscribed to. Usually people are even grateful for whatever review is provided. Your grammar comments and complaints about extra parenthesis are not constructive and cause unnecessary patch churn. As if I haven't ever found more serious issues in your own XHCI patches (even after you forbade me to review them). Serious issues are fine to bring up, either for my patches or for patches that people send for the xHCI. However, the vast majority of comments I see from you are simply grammar comments or nitpicking of code style that checkpatch.pl doesn't complain about. checkpatch.pl unfortunately is not perfect, there's CodingStyle violations it doesn't complain about. And not everybody even runs checkpatch.pl unfortunately (I myself often forget that). At some point, your signal to noise ratio is so low that most kernel developers are just going to start tuning you out. As I said, so far I had serious issues due to my reviews only with one or two people in the past several years. I started appearing on the Linux mailing lists since 2004. Greg already put your email into /dev/null. Do you want other kernel developers to do so as well? I remember Greg has commented to my patches not so long ago. Maybe he did put me into his killfile recently but I don't know what accident might have triggered that. If you want *serious bugs* to be acted on, then just send email about *serious bugs*. Or include a mix of comments about grammar/code style in with comments about serious code issues. Fortunately, serious issues are rare and unfortunately, it's harder to see them (in the xHCI driver particularly, as I'm not familiar with its code). Please increase your signal to noise ratio, or I will continue to send you scathing emails when you nitpick xHCI patches. I've already consented not to review them (at least unless I see something serious enough). Sarah Sharp WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xhci: Add BayTrail to list of Intel switchable hosts
On Tue, May 21, 2013 at 10:37:55AM -0400, Alan Stern wrote:
On Tue, 21 May 2013, Heikki Krogerus wrote:
diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c
index 595d210..a5708d9 100644
--- a/drivers/usb/host/ehci-pci.c
+++ b/drivers/usb/host/ehci-pci.c
@@ -322,7 +322,8 @@ static bool usb_is_intel_switchable_ehci(struct pci_dev
*pdev)
(pdev-device == 0x1E26 ||
pdev-device == 0x8C2D ||
pdev-device == 0x8C26 ||
-pdev-device == 0x9C26);
+pdev-device == 0x9C26 ||
+pdev-device == 0x0F34);
The entries should be kept sorted in numerical order. In fact, you
might even interchange the 0x8C2D and 0x8C26 entries.
OK.
diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index 4c338ec..6062822 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -724,6 +724,7 @@ static int handshake(void __iomem *ptr, u32 mask, u32
done,
#define PCI_DEVICE_ID_INTEL_LYNX_POINT_XHCI0x8C31
#define PCI_DEVICE_ID_INTEL_LYNX_POINT_LP_XHCI 0x9C31
+#define PCI_DEVICE_ID_INTEL_BYT_XHCI 0x0F35
bool usb_is_intel_ppt_switchable_xhci(struct pci_dev *pdev)
{
@@ -741,10 +742,19 @@ bool usb_is_intel_lpt_switchable_xhci(struct pci_dev
*pdev)
pdev-device == PCI_DEVICE_ID_INTEL_LYNX_POINT_LP_XHCI);
}
+/* And so does the Intel BayTrail. */
+bool usb_is_intel_byt_switchable_xhci(struct pci_dev *pdev)
+{
+ return pdev-class == PCI_CLASS_SERIAL_USB_XHCI
+ pdev-vendor == PCI_VENDOR_ID_INTEL
+ pdev-device == PCI_DEVICE_ID_INTEL_BYT_XHCI;
+}
+
bool usb_is_intel_switchable_xhci(struct pci_dev *pdev)
{
return usb_is_intel_ppt_switchable_xhci(pdev) ||
- usb_is_intel_lpt_switchable_xhci(pdev);
+ usb_is_intel_lpt_switchable_xhci(pdev) ||
+ usb_is_intel_byt_switchable_xhci(pdev);
}
EXPORT_SYMBOL_GPL(usb_is_intel_switchable_xhci);
This code cries out for refactoring. Why test pdev-class and
pdev-vendor in the same way in three different places? Those two
tests should be moved directly into usb_is_intel_switchable_xhci().
The other helper functions then become simple comparisons. As far as
I'm concerned, they also could be moved inline into
usb_is_intel_switchable_xhci(). Sarah may disagree, however.
Sarah what do you think?
--
heikki
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xhci: Add BayTrail to list of Intel switchable hosts
On Tue, May 21, 2013 at 04:26:43PM +0400, Sergei Shtylyov wrote: Like the xHCI controller on Intel Panther Point and Lynx Point chipsets, the xHCI controller on Intel BayTrail has also ports that can be switched between the EHCI host controller. s/between/to/ OK. Thanks, -- heikki -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] xhci: Add BayTrail to list of Intel switchable hosts
From: Chew, Chiau Ee chiau.ee.c...@intel.com
Like the xHCI controller on Intel Panther Point and Lynx Point
chipsets, the xHCI controller on Intel BayTrail has also ports that
can be switched between the EHCI host controller.
This patch should be backported to stable kernels as old as 3.0,
that contain commit 69e848c2090aebba5698a1620604c7dccb448684
Intel xhci: Support EHCI/xHCI port switching.
Signed-off-by: Chew, Chiau Ee chiau.ee.c...@intel.com
Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com
Cc: sta...@vger.kernel.org
---
drivers/usb/host/ehci-pci.c |3 ++-
drivers/usb/host/pci-quirks.c | 12 +++-
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c
index 595d210..a5708d9 100644
--- a/drivers/usb/host/ehci-pci.c
+++ b/drivers/usb/host/ehci-pci.c
@@ -322,7 +322,8 @@ static bool usb_is_intel_switchable_ehci(struct pci_dev
*pdev)
(pdev-device == 0x1E26 ||
pdev-device == 0x8C2D ||
pdev-device == 0x8C26 ||
-pdev-device == 0x9C26);
+pdev-device == 0x9C26 ||
+pdev-device == 0x0F34);
}
static void ehci_enable_xhci_companion(void)
diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index 4c338ec..6062822 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -724,6 +724,7 @@ static int handshake(void __iomem *ptr, u32 mask, u32 done,
#define PCI_DEVICE_ID_INTEL_LYNX_POINT_XHCI0x8C31
#define PCI_DEVICE_ID_INTEL_LYNX_POINT_LP_XHCI 0x9C31
+#define PCI_DEVICE_ID_INTEL_BYT_XHCI 0x0F35
bool usb_is_intel_ppt_switchable_xhci(struct pci_dev *pdev)
{
@@ -741,10 +742,19 @@ bool usb_is_intel_lpt_switchable_xhci(struct pci_dev
*pdev)
pdev-device == PCI_DEVICE_ID_INTEL_LYNX_POINT_LP_XHCI);
}
+/* And so does the Intel BayTrail. */
+bool usb_is_intel_byt_switchable_xhci(struct pci_dev *pdev)
+{
+ return pdev-class == PCI_CLASS_SERIAL_USB_XHCI
+ pdev-vendor == PCI_VENDOR_ID_INTEL
+ pdev-device == PCI_DEVICE_ID_INTEL_BYT_XHCI;
+}
+
bool usb_is_intel_switchable_xhci(struct pci_dev *pdev)
{
return usb_is_intel_ppt_switchable_xhci(pdev) ||
- usb_is_intel_lpt_switchable_xhci(pdev);
+ usb_is_intel_lpt_switchable_xhci(pdev) ||
+ usb_is_intel_byt_switchable_xhci(pdev);
}
EXPORT_SYMBOL_GPL(usb_is_intel_switchable_xhci);
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xhci: Add BayTrail to list of Intel switchable hosts
Hello. On 21-05-2013 10:08, Heikki Krogerus wrote: From: Chew, Chiau Ee chiau.ee.c...@intel.com Like the xHCI controller on Intel Panther Point and Lynx Point chipsets, the xHCI controller on Intel BayTrail has also ports that can be switched between the EHCI host controller. s/between/to/ This patch should be backported to stable kernels as old as 3.0, that contain commit 69e848c2090aebba5698a1620604c7dccb448684 Intel xhci: Support EHCI/xHCI port switching. Signed-off-by: Chew, Chiau Ee chiau.ee.c...@intel.com Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com Cc: sta...@vger.kernel.org WBR, Sergei -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
