public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] usb/core: skip unnecessary line IRQ request for USB3
@ 2012-03-15  4:48 Alex Shi
  2012-03-15  6:21 ` Xu, Andiry
  2012-03-15 14:29 ` Alan Stern
  0 siblings, 2 replies; 6+ messages in thread
From: Alex Shi @ 2012-03-15  4:48 UTC (permalink / raw)
  To: stern, gregkh; +Cc: sarah.a.sharp, linux-kernel@vger.kernel.org, linux-usb


USB3 requests line IRQ here but will disable it
in later driver->start function and try MSI first.

xhci_hcd 0000:02:00.0: irq 18, io mem 0xfe500000
xhci_hcd 0000:02:00.0: irq 45 for MSI/MSI-X
xhci_hcd 0000:02:00.0: irq 46 for MSI/MSI-X

So it is better to remove the redundant request here. And
save a little time in booting.

Signed-off-by: Alex Shi <alex.shi@intel.com>
---
 drivers/usb/core/hcd.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index e128232..5b09825 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2450,7 +2450,8 @@ int usb_add_hcd(struct usb_hcd *hcd,
 	/* enable irqs just before we start the controller,
 	 * if the BIOS provides legacy PCI irqs.
 	 */
-	if (usb_hcd_is_primary_hcd(hcd) && irqnum) {
+	if (usb_hcd_is_primary_hcd(hcd) && irqnum
+		&& (hcd->driver->flags & HCD_MASK) != HCD_USB3) {
 		retval = usb_hcd_request_irqs(hcd, irqnum, irqflags);
 		if (retval)
 			goto err_request_irq;
-- 
1.7.5.1




^ permalink raw reply related	[flat|nested] 6+ messages in thread

* RE: [PATCH] usb/core: skip unnecessary line IRQ request for USB3
  2012-03-15  4:48 [PATCH] usb/core: skip unnecessary line IRQ request for USB3 Alex Shi
@ 2012-03-15  6:21 ` Xu, Andiry
  2012-03-15  6:37   ` Alex Shi
  2012-03-15 14:29 ` Alan Stern
  1 sibling, 1 reply; 6+ messages in thread
From: Xu, Andiry @ 2012-03-15  6:21 UTC (permalink / raw)
  To: Alex Shi, stern@rowland.harvard.edu, gregkh
  Cc: sarah.a.sharp@linux.intel.com, linux-kernel@vger.kernel.org,
	linux-usb

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2035 bytes --]

> -----Original Message-----
> From: linux-usb-owner@vger.kernel.org [mailto:linux-usb-
> owner@vger.kernel.org] On Behalf Of Alex Shi
> Sent: Thursday, March 15, 2012 12:48 PM
> To: stern@rowland.harvard.edu; gregkh
> Cc: sarah.a.sharp@linux.intel.com; linux-kernel@vger.kernel.org; linux-usb
> Subject: [PATCH] usb/core: skip unnecessary line IRQ request for USB3
> 
> 
> USB3 requests line IRQ here but will disable it
> in later driver->start function and try MSI first.
> 

You may also remove the unregister the legacy interrupt part in
xhci_try_enable_msi().

In fact, I think all the MSI related codes should be moved to xhci-pci.c,
Since MSI is PCI-only.

Thanks,
Andiry

> xhci_hcd 0000:02:00.0: irq 18, io mem 0xfe500000
> xhci_hcd 0000:02:00.0: irq 45 for MSI/MSI-X
> xhci_hcd 0000:02:00.0: irq 46 for MSI/MSI-X
> 
> So it is better to remove the redundant request here. And
> save a little time in booting.
> 
> Signed-off-by: Alex Shi <alex.shi@intel.com>
> ---
>  drivers/usb/core/hcd.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index e128232..5b09825 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -2450,7 +2450,8 @@ int usb_add_hcd(struct usb_hcd *hcd,
>  	/* enable irqs just before we start the controller,
>  	 * if the BIOS provides legacy PCI irqs.
>  	 */
> -	if (usb_hcd_is_primary_hcd(hcd) && irqnum) {
> +	if (usb_hcd_is_primary_hcd(hcd) && irqnum
> +		&& (hcd->driver->flags & HCD_MASK) != HCD_USB3) {
>  		retval = usb_hcd_request_irqs(hcd, irqnum, irqflags);
>  		if (retval)
>  			goto err_request_irq;
> --
> 1.7.5.1
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] usb/core: skip unnecessary line IRQ request for USB3
  2012-03-15  6:21 ` Xu, Andiry
@ 2012-03-15  6:37   ` Alex Shi
  0 siblings, 0 replies; 6+ messages in thread
From: Alex Shi @ 2012-03-15  6:37 UTC (permalink / raw)
  To: Xu, Andiry
  Cc: stern@rowland.harvard.edu, gregkh, sarah.a.sharp@linux.intel.com,
	linux-kernel@vger.kernel.org, linux-usb


> 
> You may also remove the unregister the legacy interrupt part in
> xhci_try_enable_msi().

Sure, thanks!

>From 6ae16651ae33ea1841984f3a5eb4a691ed730a19 Mon Sep 17 00:00:00 2001
From: Alex Shi <alex.shi@intel.com>
Date: Thu, 15 Mar 2012 08:12:47 +0800
Subject: [PATCH] usb/core: skip unnecessary line IRQ request for USB3

USB3(only xhci driver) requests line IRQ here but will disable it
in later driver->start function and try MSI first:

xhci_hcd 0000:02:00.0: irq 18, io mem 0xfe500000
xhci_hcd 0000:02:00.0: irq 45 for MSI/MSI-X
xhci_hcd 0000:02:00.0: irq 46 for MSI/MSI-X

So it is better to remove the redundant request here.

Signed-off-by: Alex Shi <alex.shi@intel.com>
---
 drivers/usb/core/hcd.c  |    3 ++-
 drivers/usb/host/xhci.c |    3 ---
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index e128232..5b09825 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2450,7 +2450,8 @@ int usb_add_hcd(struct usb_hcd *hcd,
 	/* enable irqs just before we start the controller,
 	 * if the BIOS provides legacy PCI irqs.
 	 */
-	if (usb_hcd_is_primary_hcd(hcd) && irqnum) {
+	if (usb_hcd_is_primary_hcd(hcd) && irqnum
+		&& (hcd->driver->flags & HCD_MASK) != HCD_USB3) {
 		retval = usb_hcd_request_irqs(hcd, irqnum, irqflags);
 		if (retval)
 			goto err_request_irq;
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index c939f5f..64979c2 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -338,9 +338,6 @@ static int xhci_try_enable_msi(struct usb_hcd *hcd)
 	if (xhci->quirks & XHCI_BROKEN_MSI)
 		return 0;
 
-	/* unregister the legacy interrupt */
-	if (hcd->irq)
-		free_irq(hcd->irq, hcd);
 	hcd->irq = -1;
 
 	ret = xhci_setup_msix(xhci);
-- 
1.6.3.3


> 
> In fact, I think all the MSI related codes should be moved to xhci-pci.c,
> Since MSI is PCI-only.

A patch moving MSI from xhci to USB core is in reviewing. That walks
far. :) https://lkml.org/lkml/2012/2/20/76 



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] usb/core: skip unnecessary line IRQ request for USB3
  2012-03-15  4:48 [PATCH] usb/core: skip unnecessary line IRQ request for USB3 Alex Shi
  2012-03-15  6:21 ` Xu, Andiry
@ 2012-03-15 14:29 ` Alan Stern
  2012-03-15 14:39   ` Alex Shi
  1 sibling, 1 reply; 6+ messages in thread
From: Alan Stern @ 2012-03-15 14:29 UTC (permalink / raw)
  To: Alex Shi; +Cc: gregkh, sarah.a.sharp, linux-kernel@vger.kernel.org, linux-usb

On Thu, 15 Mar 2012, Alex Shi wrote:

> USB3 requests line IRQ here but will disable it
> in later driver->start function and try MSI first.
> 
> xhci_hcd 0000:02:00.0: irq 18, io mem 0xfe500000
> xhci_hcd 0000:02:00.0: irq 45 for MSI/MSI-X
> xhci_hcd 0000:02:00.0: irq 46 for MSI/MSI-X
> 
> So it is better to remove the redundant request here. And
> save a little time in booting.
> 
> Signed-off-by: Alex Shi <alex.shi@intel.com>
> ---
>  drivers/usb/core/hcd.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index e128232..5b09825 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -2450,7 +2450,8 @@ int usb_add_hcd(struct usb_hcd *hcd,
>  	/* enable irqs just before we start the controller,
>  	 * if the BIOS provides legacy PCI irqs.
>  	 */
> -	if (usb_hcd_is_primary_hcd(hcd) && irqnum) {
> +	if (usb_hcd_is_primary_hcd(hcd) && irqnum
> +		&& (hcd->driver->flags & HCD_MASK) != HCD_USB3) {
>  		retval = usb_hcd_request_irqs(hcd, irqnum, irqflags);
>  		if (retval)
>  			goto err_request_irq;

This is not a good idea.  What happens if some USB-3 controller 
(perhaps a non-xHCI USB-3 controller) doesn't use MSI?

The right way to avoid registering a legacy interrupt handler is to 
call usb_add_hcd() with irqnum equal to 0.

Alan Stern


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] usb/core: skip unnecessary line IRQ request for USB3
  2012-03-15 14:29 ` Alan Stern
@ 2012-03-15 14:39   ` Alex Shi
  2012-03-16  8:05     ` Alex Shi
  0 siblings, 1 reply; 6+ messages in thread
From: Alex Shi @ 2012-03-15 14:39 UTC (permalink / raw)
  To: Alan Stern; +Cc: gregkh, sarah.a.sharp, linux-kernel@vger.kernel.org, linux-usb


> 
> This is not a good idea.  What happens if some USB-3 controller 
> (perhaps a non-xHCI USB-3 controller) doesn't use MSI?
> 


current usb3 driver just xhci. but sure it need to modify if non-xhci
usb pops up.

> The right way to avoid registering a legacy interrupt handler is to 
> call usb_add_hcd() with irqnum equal to 0.


a good trick! will try it tomorrow.

> 
> Alan Stern
> 





^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] usb/core: skip unnecessary line IRQ request for USB3
  2012-03-15 14:39   ` Alex Shi
@ 2012-03-16  8:05     ` Alex Shi
  0 siblings, 0 replies; 6+ messages in thread
From: Alex Shi @ 2012-03-16  8:05 UTC (permalink / raw)
  To: Alan Stern; +Cc: gregkh, sarah.a.sharp, linux-kernel@vger.kernel.org, linux-usb

On Thu, 2012-03-15 at 22:39 +0800, Alex Shi wrote:
> > 
> > This is not a good idea.  What happens if some USB-3 controller 
> > (perhaps a non-xHCI USB-3 controller) doesn't use MSI?
> > 
> 
> 
> current usb3 driver just xhci. but sure it need to modify if non-xhci
> usb pops up.
> 
> > The right way to avoid registering a legacy interrupt handler is to 
> > call usb_add_hcd() with irqnum equal to 0.
> 
> 
> a good trick! will try it tomorrow.

Do you mean the following patch? It tested. :) 
------------
>From 5ae9a6fe88752495af6da6182cad26620f1ab190 Mon Sep 17 00:00:00 2001
From: Alex Shi <alex.shi@intel.com>
Date: Thu, 15 Mar 2012 08:12:47 +0800
Subject: [PATCH] usb/core: skip unnecessary line IRQ request for USB3

USB3(only xhci driver) requests line IRQ here but will disable it
in later driver->start function and try MSI first:

xhci_hcd 0000:02:00.0: irq 18, io mem 0xfe500000
xhci_hcd 0000:02:00.0: irq 45 for MSI/MSI-X
xhci_hcd 0000:02:00.0: irq 46 for MSI/MSI-X

So it is better to remove the redundant request here.

Signed-off-by: Alex Shi <alex.shi@intel.com>
---
 drivers/usb/core/hcd-pci.c |    5 ++++-
 drivers/usb/host/xhci.c    |    3 ---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c
index 81e2c0d..7b73c74 100644
--- a/drivers/usb/core/hcd-pci.c
+++ b/drivers/usb/core/hcd-pci.c
@@ -245,7 +245,10 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 
 	pci_set_master(dev);
 
-	retval = usb_add_hcd(hcd, dev->irq, IRQF_SHARED);
+	if ((driver->flags & HCD_MASK) != HCD_USB3)
+		retval = usb_add_hcd(hcd, dev->irq, IRQF_SHARED);
+	else
+		retval = usb_add_hcd(hcd, 0, IRQF_SHARED);
 	if (retval != 0)
 		goto unmap_registers;
 	set_hs_companion(dev, hcd);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index c939f5f..64979c2 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -338,9 +338,6 @@ static int xhci_try_enable_msi(struct usb_hcd *hcd)
 	if (xhci->quirks & XHCI_BROKEN_MSI)
 		return 0;
 
-	/* unregister the legacy interrupt */
-	if (hcd->irq)
-		free_irq(hcd->irq, hcd);
 	hcd->irq = -1;
 
 	ret = xhci_setup_msix(xhci);
-- 
1.6.3.3



> 
> > 
> > Alan Stern
> > 
> 
> 
> 
> 



^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-03-16  8:06 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-03-15  4:48 [PATCH] usb/core: skip unnecessary line IRQ request for USB3 Alex Shi
2012-03-15  6:21 ` Xu, Andiry
2012-03-15  6:37   ` Alex Shi
2012-03-15 14:29 ` Alan Stern
2012-03-15 14:39   ` Alex Shi
2012-03-16  8:05     ` Alex Shi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox