[PATCH][v2] xhci: correctly enable interrupts

[PATCH][v2] xhci: correctly enable interrupts

[Hannes Reinecke]

xhci has its own interrupt enabling routine, which will try to
use MSI-X/MSI if present. So the usb core shouldn't try to enable
legacy interrupts; on some machines the xhci legacy IRQ setting
is invalid.

Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Oliver Neukum <oneukum@suse.de>
Cc: Thomas Renninger <trenn@suse.de>
Cc: Yinghai Lu <yinghai@kernel.org>
Cc: Frederik Himpe <fhimpe@vub.ac.be>
Cc: David Haerdeman <david@hardeman.nu>
Cc: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Hannes Reinecke <hare@suse.de>

diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c
index 622b4a4..2647e75 100644
--- a/drivers/usb/core/hcd-pci.c
+++ b/drivers/usb/core/hcd-pci.c
@@ -173,6 +173,7 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	struct hc_driver	*driver;
 	struct usb_hcd		*hcd;
 	int			retval;
+	int			hcd_irq = 0;
 
 	if (usb_disabled())
 		return -ENODEV;
@@ -187,15 +188,21 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 		return -ENODEV;
 	dev->current_state = PCI_D0;
 
-	/* The xHCI driver supports MSI and MSI-X,
-	 * so don't fail if the BIOS doesn't provide a legacy IRQ.
+	/*
+	 * The xHCI driver supports MSI and MSI-X,
+	 * so don't fail if the BIOS doesn't provide a legacy IRQ
+	 * and do not try to enable legacy IRQs.
 	 */
-	if (!dev->irq && (driver->flags & HCD_MASK) != HCD_USB3) {
-		dev_err(&dev->dev,
-			"Found HC with no IRQ.  Check BIOS/PCI %s setup!\n",
-			pci_name(dev));
-		retval = -ENODEV;
-		goto disable_pci;
+	if ((driver->flags & HCD_MASK) != HCD_USB3) {
+		if (!dev->irq) {
+			dev_err(&dev->dev,
+				"Found HC with no IRQ.  "
+				"Check BIOS/PCI %s setup!\n",
+				pci_name(dev));
+			retval = -ENODEV;
+			goto disable_pci;
+		}
+		hcd_irq = dev->irq;
 	}
 
 	hcd = usb_create_hcd(driver, &dev->dev, pci_name(dev));
@@ -245,7 +252,7 @@ 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);
+	retval = usb_add_hcd(hcd, hcd_irq, IRQF_SHARED);
 	if (retval != 0)
 		goto unmap_registers;
 	set_hs_companion(dev, hcd);

Re: [PATCH][v2] xhci: correctly enable interrupts

[David Härdeman]

On Mon, Mar 04, 2013 at 09:22:04AM +0100, Hannes Reinecke wrote:
>xhci has its own interrupt enabling routine, which will try to
>use MSI-X/MSI if present. So the usb core shouldn't try to enable
>legacy interrupts; on some machines the xhci legacy IRQ setting
>is invalid.
>
>Cc: Bjorn Helgaas <bhelgaas@google.com>
>Cc: Oliver Neukum <oneukum@suse.de>
>Cc: Thomas Renninger <trenn@suse.de>
>Cc: Yinghai Lu <yinghai@kernel.org>
>Cc: Frederik Himpe <fhimpe@vub.ac.be>
>Cc: David Haerdeman <david@hardeman.nu>
>Cc: Alan Stern <stern@rowland.harvard.edu>
>Signed-off-by: Hannes Reinecke <hare@suse.de>

No idea if it's the "right" solution but it works for me.

Tested-by: David Härdeman <david@hardeman.nu>

>
>diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c
>index 622b4a4..2647e75 100644
>--- a/drivers/usb/core/hcd-pci.c
>+++ b/drivers/usb/core/hcd-pci.c
>@@ -173,6 +173,7 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
> 	struct hc_driver	*driver;
> 	struct usb_hcd		*hcd;
> 	int			retval;
>+	int			hcd_irq = 0;
> 
> 	if (usb_disabled())
> 		return -ENODEV;
>@@ -187,15 +188,21 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
> 		return -ENODEV;
> 	dev->current_state = PCI_D0;
> 
>-	/* The xHCI driver supports MSI and MSI-X,
>-	 * so don't fail if the BIOS doesn't provide a legacy IRQ.
>+	/*
>+	 * The xHCI driver supports MSI and MSI-X,
>+	 * so don't fail if the BIOS doesn't provide a legacy IRQ
>+	 * and do not try to enable legacy IRQs.
> 	 */
>-	if (!dev->irq && (driver->flags & HCD_MASK) != HCD_USB3) {
>-		dev_err(&dev->dev,
>-			"Found HC with no IRQ.  Check BIOS/PCI %s setup!\n",
>-			pci_name(dev));
>-		retval = -ENODEV;
>-		goto disable_pci;
>+	if ((driver->flags & HCD_MASK) != HCD_USB3) {
>+		if (!dev->irq) {
>+			dev_err(&dev->dev,
>+				"Found HC with no IRQ.  "
>+				"Check BIOS/PCI %s setup!\n",
>+				pci_name(dev));
>+			retval = -ENODEV;
>+			goto disable_pci;
>+		}
>+		hcd_irq = dev->irq;
> 	}
> 
> 	hcd = usb_create_hcd(driver, &dev->dev, pci_name(dev));
>@@ -245,7 +252,7 @@ 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);
>+	retval = usb_add_hcd(hcd, hcd_irq, IRQF_SHARED);
> 	if (retval != 0)
> 		goto unmap_registers;
> 	set_hs_companion(dev, hcd);
>

-- 
David Härdeman

Re: [PATCH][v2] xhci: correctly enable interrupts

[Sergei Shtylyov]

Hello.

On 04-03-2013 12:22, Hannes Reinecke wrote:

> xhci has its own interrupt enabling routine, which will try to
> use MSI-X/MSI if present. So the usb core shouldn't try to enable
> legacy interrupts; on some machines the xhci legacy IRQ setting
> is invalid.

> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Oliver Neukum <oneukum@suse.de>
> Cc: Thomas Renninger <trenn@suse.de>
> Cc: Yinghai Lu <yinghai@kernel.org>
> Cc: Frederik Himpe <fhimpe@vub.ac.be>
> Cc: David Haerdeman <david@hardeman.nu>
> Cc: Alan Stern <stern@rowland.harvard.edu>
> Signed-off-by: Hannes Reinecke <hare@suse.de>

> diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c
> index 622b4a4..2647e75 100644
> --- a/drivers/usb/core/hcd-pci.c
> +++ b/drivers/usb/core/hcd-pci.c
[...]
> @@ -187,15 +188,21 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
>   		return -ENODEV;
>   	dev->current_state = PCI_D0;
>
> -	/* The xHCI driver supports MSI and MSI-X,
> -	 * so don't fail if the BIOS doesn't provide a legacy IRQ.
> +	/*
> +	 * The xHCI driver supports MSI and MSI-X,
> +	 * so don't fail if the BIOS doesn't provide a legacy IRQ
> +	 * and do not try to enable legacy IRQs.
>   	 */
> -	if (!dev->irq && (driver->flags & HCD_MASK) != HCD_USB3) {
> -		dev_err(&dev->dev,
> -			"Found HC with no IRQ.  Check BIOS/PCI %s setup!\n",
> -			pci_name(dev));
> -		retval = -ENODEV;
> -		goto disable_pci;
> +	if ((driver->flags & HCD_MASK) != HCD_USB3) {
> +		if (!dev->irq) {
> +			dev_err(&dev->dev,
> +				"Found HC with no IRQ.  "
> +				"Check BIOS/PCI %s setup!\n",

    The message strings are allowed to take more than 80 chars (to facilitate 
grepping for them), so no need to break this one; checkpatch.pl should not 
complain...

> +				pci_name(dev));
> +			retval = -ENODEV;
> +			goto disable_pci;
> +		}
> +		hcd_irq = dev->irq;
>   	}
>
>   	hcd = usb_create_hcd(driver, &dev->dev, pci_name(dev));

WBR, Sergei

Re: [PATCH][v2] xhci: correctly enable interrupts

[Alan Stern]

On Mon, 4 Mar 2013, Hannes Reinecke wrote:

> xhci has its own interrupt enabling routine, which will try to
> use MSI-X/MSI if present. So the usb core shouldn't try to enable
> legacy interrupts; on some machines the xhci legacy IRQ setting
> is invalid.

This version of the patch is much better than the first one, IMO.  
Thank you.

Alan Stern

Re: [PATCH][v2] xhci: correctly enable interrupts

[Thomas Renninger]

On Monday, March 04, 2013 10:26:40 AM Alan Stern wrote:
> On Mon, 4 Mar 2013, Hannes Reinecke wrote:
> > xhci has its own interrupt enabling routine, which will try to
> > use MSI-X/MSI if present. So the usb core shouldn't try to enable
> > legacy interrupts; on some machines the xhci legacy IRQ setting
> > is invalid.
> 
> This version of the patch is much better than the first one, IMO.

I also have this issue. Unfortunately pci read only gives an irq of 255
in secure boot mode and I don't want to struggle with kernel/module
signing to test this.

I found one issue with this patch:
For xhci legacy PCI is not tried to be set up now anymore (before MSI(x)
is tried) which is a correct fix.

But in xhci_try_enable_msi() drivers/usb/host/xhci.c if MSI is known broken
(xhci->quirks & XHCI_BROKEN_MSI), it relies on legacy IRQ being enabled
already.
Instead it should use the "enable legacy IRQ" code later in the same function 
which is the fallback if MSI setup does not succeed.

I send an updated version taking care about above and including the "do not 
split string" concern Sergei mentioned.

        Thomas

[PATCH][v3] xhci: correctly enable interrupts

[Thomas Renninger]

From: Hannes Reinecke <hare@suse.de>

xhci has its own interrupt enabling routine, which will try to
use MSI-X/MSI if present. So the usb core shouldn't try to enable
legacy interrupts; on some machines the xhci legacy IRQ setting
is invalid.

v3: Be careful to not break XHCI_BROKEN_MSI workaround (by trenn)

Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Oliver Neukum <oneukum@suse.de>
Cc: Thomas Renninger <trenn@suse.de>
Cc: Yinghai Lu <yinghai@kernel.org>
Cc: Frederik Himpe <fhimpe@vub.ac.be>
Cc: David Haerdeman <david@hardeman.nu>
Cc: Alan Stern <stern@rowland.harvard.edu>

Reviewed-by: Thomas Renninger <trenn@suse.de>
Signed-off-by: Hannes Reinecke <hare@suse.de>

diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c
index 622b4a4..2b487d4 100644
--- a/drivers/usb/core/hcd-pci.c
+++ b/drivers/usb/core/hcd-pci.c
@@ -173,6 +173,7 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	struct hc_driver	*driver;
 	struct usb_hcd		*hcd;
 	int			retval;
+	int			hcd_irq = 0;
 
 	if (usb_disabled())
 		return -ENODEV;
@@ -187,15 +188,19 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 		return -ENODEV;
 	dev->current_state = PCI_D0;
 
-	/* The xHCI driver supports MSI and MSI-X,
-	 * so don't fail if the BIOS doesn't provide a legacy IRQ.
+	/*
+	 * The xHCI driver has its own irq management
+	 * make sure irq setup is not touched for xhci in generic hcd code
 	 */
-	if (!dev->irq && (driver->flags & HCD_MASK) != HCD_USB3) {
-		dev_err(&dev->dev,
-			"Found HC with no IRQ.  Check BIOS/PCI %s setup!\n",
-			pci_name(dev));
-		retval = -ENODEV;
-		goto disable_pci;
+	if ((driver->flags & HCD_MASK) != HCD_USB3) {
+		if (!dev->irq) {
+			dev_err(&dev->dev,
+			"Found HC with no IRQ. Check BIOS/PCI %s setup!\n",
+				pci_name(dev));
+			retval = -ENODEV;
+			goto disable_pci;
+		}
+		hcd_irq = dev->irq;
 	}
 
 	hcd = usb_create_hcd(driver, &dev->dev, pci_name(dev));
@@ -245,7 +250,7 @@ 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);
+	retval = usb_add_hcd(hcd, hcd_irq, 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 f1f01a8..849470b 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -350,7 +350,7 @@ static int xhci_try_enable_msi(struct usb_hcd *hcd)
 	 * generate interrupts.  Don't even try to enable MSI.
 	 */
 	if (xhci->quirks & XHCI_BROKEN_MSI)
-		return 0;
+		goto legacy_irq;
 
 	/* unregister the legacy interrupt */
 	if (hcd->irq)
@@ -371,6 +371,7 @@ static int xhci_try_enable_msi(struct usb_hcd *hcd)
 		return -EINVAL;
 	}
 
+ legacy_irq:
 	/* fall back to legacy interrupt*/
 	ret = request_irq(pdev->irq, &usb_hcd_irq, IRQF_SHARED,
 			hcd->irq_descr, hcd);

Re: [PATCH][v3] xhci: correctly enable interrupts

[Sergei Shtylyov]

Hello.

On 03/04/2013 07:14 PM, Thomas Renninger wrote:

> From: Hannes Reinecke<hare@suse.de>
>
> xhci has its own interrupt enabling routine, which will try to
> use MSI-X/MSI if present. So the usb core shouldn't try to enable
> legacy interrupts; on some machines the xhci legacy IRQ setting
> is invalid.
>
> v3: Be careful to not break XHCI_BROKEN_MSI workaround (by trenn)
>
> Cc: Bjorn Helgaas<bhelgaas@google.com>
> Cc: Oliver Neukum<oneukum@suse.de>
> Cc: Thomas Renninger<trenn@suse.de>
> Cc: Yinghai Lu<yinghai@kernel.org>
> Cc: Frederik Himpe<fhimpe@vub.ac.be>
> Cc: David Haerdeman<david@hardeman.nu>
> Cc: Alan Stern<stern@rowland.harvard.edu>
>
> Reviewed-by: Thomas Renninger<trenn@suse.de>
> Signed-off-by: Hannes Reinecke<hare@suse.de>
    Still a couple of style comments...

[...]

> @@ -187,15 +188,19 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
>   		return -ENODEV;
>   	dev->current_state = PCI_D0;
>
> -	/* The xHCI driver supports MSI and MSI-X,
> -	 * so don't fail if the BIOS doesn't provide a legacy IRQ.
> +	/*
> +	 * The xHCI driver has its own irq management
> +	 * make sure irq setup is not touched for xhci in generic hcd code
>   	 */
> -	if (!dev->irq&&  (driver->flags&  HCD_MASK) != HCD_USB3) {
> -		dev_err(&dev->dev,
> -			"Found HC with no IRQ.  Check BIOS/PCI %s setup!\n",
> -			pci_name(dev));
> -		retval = -ENODEV;
> -		goto disable_pci;
> +	if ((driver->flags&  HCD_MASK) != HCD_USB3) {
> +		if (!dev->irq) {
> +			dev_err(&dev->dev,
> +			"Found HC with no IRQ. Check BIOS/PCI %s setup!\n",
>    

    Could you indent the string like the line below?

> +				pci_name(dev));
> +			retval = -ENODEV;
> +			goto disable_pci;
> +		}
> +		hcd_irq = dev->irq;
>   	}
>
>   	hcd = usb_create_hcd(driver,&dev->dev, pci_name(dev));
>    
[...]
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index f1f01a8..849470b 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
>    
[...]
> @@ -371,6 +371,7 @@ static int xhci_try_enable_msi(struct usb_hcd *hcd)
>   		return -EINVAL;
>   	}
>
> + legacy_irq:
>    

    Labels shouldn't be indented by a space (unless the existing coding 
style has them indented already, of course).
Although that might be a rule dropped by checkpatch.pl already -- it 
doesn't complain.

WBR, Sergei

Re: [PATCH][v3] xhci: correctly enable interrupts

[Alan Stern]

On Mon, 4 Mar 2013, Sergei Shtylyov wrote:

> > @@ -371,6 +371,7 @@ static int xhci_try_enable_msi(struct usb_hcd *hcd)
> >   		return -EINVAL;
> >   	}
> >
> > + legacy_irq:
> >    
> 
>     Labels shouldn't be indented by a space (unless the existing coding 
> style has them indented already, of course).
> Although that might be a rule dropped by checkpatch.pl already -- it 
> doesn't complain.

Indeed, there's a definite advantage to putting a space before a label:  
The diff program doesn't get confused into thinking the label is the
start of a new function.

Alan Stern

Re: [PATCH][v3] xhci: correctly enable interrupts

[Thomas Renninger]

On Monday, March 04, 2013 05:14:43 PM Thomas Renninger wrote:
> From: Hannes Reinecke <hare@suse.de>
> 
> xhci has its own interrupt enabling routine, which will try to
> use MSI-X/MSI if present. So the usb core shouldn't try to enable
> legacy interrupts; on some machines the xhci legacy IRQ setting
> is invalid.
> 
> v3: Be careful to not break XHCI_BROKEN_MSI workaround (by trenn)
> 
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Oliver Neukum <oneukum@suse.de>
> Cc: Thomas Renninger <trenn@suse.de>
> Cc: Yinghai Lu <yinghai@kernel.org>
> Cc: Frederik Himpe <fhimpe@vub.ac.be>
> Cc: David Haerdeman <david@hardeman.nu>
> Cc: Alan Stern <stern@rowland.harvard.edu>
> 
> Reviewed-by: Thomas Renninger <trenn@suse.de>
> Signed-off-by: Hannes Reinecke <hare@suse.de>

Argh, I should have added:
CC: stable@kernel.org


This seem to be a common issue on new machines.
And having xhci not functional at all there is nasty.

The patch is not complex and should fulfill all stable@ criteria.

Is this the official USB mainline subtree?:
https://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git

Adding Felipe, would be great if this one can get queued for 3.9
inclusion. Would be nice if:
CC: stable@kernel.org
can be added manually, I can also explicitly submit it to stable@
as soon as it hits Linus' tree.

Thanks,

            Thomas

> 
> diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c
> index 622b4a4..2b487d4 100644
> --- a/drivers/usb/core/hcd-pci.c
> +++ b/drivers/usb/core/hcd-pci.c
> @@ -173,6 +173,7 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct
> pci_device_id *id) struct hc_driver	*driver;
>  	struct usb_hcd		*hcd;
>  	int			retval;
> +	int			hcd_irq = 0;
> 
>  	if (usb_disabled())
>  		return -ENODEV;
> @@ -187,15 +188,19 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const
> struct pci_device_id *id) return -ENODEV;
>  	dev->current_state = PCI_D0;
> 
> -	/* The xHCI driver supports MSI and MSI-X,
> -	 * so don't fail if the BIOS doesn't provide a legacy IRQ.
> +	/*
> +	 * The xHCI driver has its own irq management
> +	 * make sure irq setup is not touched for xhci in generic hcd code
>  	 */
> -	if (!dev->irq && (driver->flags & HCD_MASK) != HCD_USB3) {
> -		dev_err(&dev->dev,
> -			"Found HC with no IRQ.  Check BIOS/PCI %s setup!\n",
> -			pci_name(dev));
> -		retval = -ENODEV;
> -		goto disable_pci;
> +	if ((driver->flags & HCD_MASK) != HCD_USB3) {
> +		if (!dev->irq) {
> +			dev_err(&dev->dev,
> +			"Found HC with no IRQ. Check BIOS/PCI %s setup!\n",
> +				pci_name(dev));
> +			retval = -ENODEV;
> +			goto disable_pci;
> +		}
> +		hcd_irq = dev->irq;
>  	}
> 
>  	hcd = usb_create_hcd(driver, &dev->dev, pci_name(dev));
> @@ -245,7 +250,7 @@ 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);
> +	retval = usb_add_hcd(hcd, hcd_irq, 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 f1f01a8..849470b 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -350,7 +350,7 @@ static int xhci_try_enable_msi(struct usb_hcd *hcd)
>  	 * generate interrupts.  Don't even try to enable MSI.
>  	 */
>  	if (xhci->quirks & XHCI_BROKEN_MSI)
> -		return 0;
> +		goto legacy_irq;
> 
>  	/* unregister the legacy interrupt */
>  	if (hcd->irq)
> @@ -371,6 +371,7 @@ static int xhci_try_enable_msi(struct usb_hcd *hcd)
>  		return -EINVAL;
>  	}
> 
> + legacy_irq:
>  	/* fall back to legacy interrupt*/
>  	ret = request_irq(pdev->irq, &usb_hcd_irq, IRQF_SHARED,
>  			hcd->irq_descr, hcd);

Re: [PATCH][v3] xhci: correctly enable interrupts

[Greg KH]

On Tue, Mar 05, 2013 at 11:45:28AM +0100, Thomas Renninger wrote:
> On Monday, March 04, 2013 05:14:43 PM Thomas Renninger wrote:
> > From: Hannes Reinecke <hare@suse.de>
> > 
> > xhci has its own interrupt enabling routine, which will try to
> > use MSI-X/MSI if present. So the usb core shouldn't try to enable
> > legacy interrupts; on some machines the xhci legacy IRQ setting
> > is invalid.
> > 
> > v3: Be careful to not break XHCI_BROKEN_MSI workaround (by trenn)
> > 
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: Oliver Neukum <oneukum@suse.de>
> > Cc: Thomas Renninger <trenn@suse.de>
> > Cc: Yinghai Lu <yinghai@kernel.org>
> > Cc: Frederik Himpe <fhimpe@vub.ac.be>
> > Cc: David Haerdeman <david@hardeman.nu>
> > Cc: Alan Stern <stern@rowland.harvard.edu>
> > 
> > Reviewed-by: Thomas Renninger <trenn@suse.de>
> > Signed-off-by: Hannes Reinecke <hare@suse.de>
> 
> Argh, I should have added:
> CC: stable@kernel.org

You mean "stable@vger.kernel.org" :)

I'll go do it...

thanks,

greg k-h

Re: [PATCH][v3] xhci: correctly enable interrupts

[Sarah Sharp]

Looks fine on the xHCI portion.  Thanks for catching the XHCI_BROKEN_MSI
case.

Acked-by: Sarah Sharp <sarah.a.sharp@linux.intel.com>

On Mon, Mar 04, 2013 at 05:14:43PM +0100, Thomas Renninger wrote:
> From: Hannes Reinecke <hare@suse.de>
> 
> xhci has its own interrupt enabling routine, which will try to
> use MSI-X/MSI if present. So the usb core shouldn't try to enable
> legacy interrupts; on some machines the xhci legacy IRQ setting
> is invalid.
> 
> v3: Be careful to not break XHCI_BROKEN_MSI workaround (by trenn)
> 
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Oliver Neukum <oneukum@suse.de>
> Cc: Thomas Renninger <trenn@suse.de>
> Cc: Yinghai Lu <yinghai@kernel.org>
> Cc: Frederik Himpe <fhimpe@vub.ac.be>
> Cc: David Haerdeman <david@hardeman.nu>
> Cc: Alan Stern <stern@rowland.harvard.edu>
> 
> Reviewed-by: Thomas Renninger <trenn@suse.de>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> 
> diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c
> index 622b4a4..2b487d4 100644
> --- a/drivers/usb/core/hcd-pci.c
> +++ b/drivers/usb/core/hcd-pci.c
> @@ -173,6 +173,7 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
>  	struct hc_driver	*driver;
>  	struct usb_hcd		*hcd;
>  	int			retval;
> +	int			hcd_irq = 0;
>  
>  	if (usb_disabled())
>  		return -ENODEV;
> @@ -187,15 +188,19 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
>  		return -ENODEV;
>  	dev->current_state = PCI_D0;
>  
> -	/* The xHCI driver supports MSI and MSI-X,
> -	 * so don't fail if the BIOS doesn't provide a legacy IRQ.
> +	/*
> +	 * The xHCI driver has its own irq management
> +	 * make sure irq setup is not touched for xhci in generic hcd code
>  	 */
> -	if (!dev->irq && (driver->flags & HCD_MASK) != HCD_USB3) {
> -		dev_err(&dev->dev,
> -			"Found HC with no IRQ.  Check BIOS/PCI %s setup!\n",
> -			pci_name(dev));
> -		retval = -ENODEV;
> -		goto disable_pci;
> +	if ((driver->flags & HCD_MASK) != HCD_USB3) {
> +		if (!dev->irq) {
> +			dev_err(&dev->dev,
> +			"Found HC with no IRQ. Check BIOS/PCI %s setup!\n",
> +				pci_name(dev));
> +			retval = -ENODEV;
> +			goto disable_pci;
> +		}
> +		hcd_irq = dev->irq;
>  	}
>  
>  	hcd = usb_create_hcd(driver, &dev->dev, pci_name(dev));
> @@ -245,7 +250,7 @@ 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);
> +	retval = usb_add_hcd(hcd, hcd_irq, 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 f1f01a8..849470b 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -350,7 +350,7 @@ static int xhci_try_enable_msi(struct usb_hcd *hcd)
>  	 * generate interrupts.  Don't even try to enable MSI.
>  	 */
>  	if (xhci->quirks & XHCI_BROKEN_MSI)
> -		return 0;
> +		goto legacy_irq;
>  
>  	/* unregister the legacy interrupt */
>  	if (hcd->irq)
> @@ -371,6 +371,7 @@ static int xhci_try_enable_msi(struct usb_hcd *hcd)
>  		return -EINVAL;
>  	}
>  
> + legacy_irq:
>  	/* fall back to legacy interrupt*/
>  	ret = request_irq(pdev->irq, &usb_hcd_irq, IRQF_SHARED,
>  			hcd->irq_descr, hcd);
> 
> --
> 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