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

2013-03-15 Thread 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 
> > 
> > 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 
> > Cc: Oliver Neukum 
> > Cc: Thomas Renninger 
> > Cc: Yinghai Lu 
> > Cc: Frederik Himpe 
> > Cc: David Haerdeman 
> > Cc: Alan Stern 
> > 
> > Reviewed-by: Thomas Renninger 
> > Signed-off-by: Hannes Reinecke 
> 
> Argh, I should have added:
> CC: sta...@kernel.org

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

I'll go do it...

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2013-03-15 Thread 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 h...@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 bhelg...@google.com
  Cc: Oliver Neukum oneu...@suse.de
  Cc: Thomas Renninger tr...@suse.de
  Cc: Yinghai Lu ying...@kernel.org
  Cc: Frederik Himpe fhi...@vub.ac.be
  Cc: David Haerdeman da...@hardeman.nu
  Cc: Alan Stern st...@rowland.harvard.edu
  
  Reviewed-by: Thomas Renninger tr...@suse.de
  Signed-off-by: Hannes Reinecke h...@suse.de
 
 Argh, I should have added:
 CC: sta...@kernel.org

You mean sta...@vger.kernel.org :)

I'll go do it...

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2013-03-05 Thread Sarah Sharp 
Looks fine on the xHCI portion.  Thanks for catching the XHCI_BROKEN_MSI
case.

Acked-by: Sarah Sharp 

On Mon, Mar 04, 2013 at 05:14:43PM +0100, Thomas Renninger wrote:
> From: 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.
> 
> v3: Be careful to not break XHCI_BROKEN_MSI workaround (by trenn)
> 
> Cc: Bjorn Helgaas 
> Cc: Oliver Neukum 
> Cc: Thomas Renninger 
> Cc: Yinghai Lu 
> Cc: Frederik Himpe 
> Cc: David Haerdeman 
> Cc: Alan Stern 
> 
> Reviewed-by: Thomas Renninger 
> Signed-off-by: Hannes Reinecke 
> 
> 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,
> - "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,
> + "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, 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, _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 majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2013-03-05 Thread Thomas Renninger 
On Monday, March 04, 2013 05:14:43 PM Thomas Renninger wrote:
> From: 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.
> 
> v3: Be careful to not break XHCI_BROKEN_MSI workaround (by trenn)
> 
> Cc: Bjorn Helgaas 
> Cc: Oliver Neukum 
> Cc: Thomas Renninger 
> Cc: Yinghai Lu 
> Cc: Frederik Himpe 
> Cc: David Haerdeman 
> Cc: Alan Stern 
> 
> Reviewed-by: Thomas Renninger 
> Signed-off-by: Hannes Reinecke 

Argh, I should have added:
CC: sta...@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: sta...@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,
> - "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,
> + "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, 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, _hcd_irq, IRQF_SHARED,
>   hcd->irq_descr, hcd);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2013-03-05 Thread Thomas Renninger 
On Monday, March 04, 2013 05:14:43 PM Thomas Renninger wrote:
 From: Hannes Reinecke h...@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 bhelg...@google.com
 Cc: Oliver Neukum oneu...@suse.de
 Cc: Thomas Renninger tr...@suse.de
 Cc: Yinghai Lu ying...@kernel.org
 Cc: Frederik Himpe fhi...@vub.ac.be
 Cc: David Haerdeman da...@hardeman.nu
 Cc: Alan Stern st...@rowland.harvard.edu
 
 Reviewed-by: Thomas Renninger tr...@suse.de
 Signed-off-by: Hannes Reinecke h...@suse.de

Argh, I should have added:
CC: sta...@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: sta...@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);
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2013-03-05 Thread Sarah Sharp 
Looks fine on the xHCI portion.  Thanks for catching the XHCI_BROKEN_MSI
case.

Acked-by: Sarah Sharp sarah.a.sh...@linux.intel.com

On Mon, Mar 04, 2013 at 05:14:43PM +0100, Thomas Renninger wrote:
 From: Hannes Reinecke h...@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 bhelg...@google.com
 Cc: Oliver Neukum oneu...@suse.de
 Cc: Thomas Renninger tr...@suse.de
 Cc: Yinghai Lu ying...@kernel.org
 Cc: Frederik Himpe fhi...@vub.ac.be
 Cc: David Haerdeman da...@hardeman.nu
 Cc: Alan Stern st...@rowland.harvard.edu
 
 Reviewed-by: Thomas Renninger tr...@suse.de
 Signed-off-by: Hannes Reinecke h...@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 majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2013-03-04 Thread 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2013-03-04 Thread Sergei Shtylyov 

Hello.

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


From: 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.

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

Cc: Bjorn Helgaas
Cc: Oliver Neukum
Cc: Thomas Renninger
Cc: Yinghai Lu
Cc: Frederik Himpe
Cc: David Haerdeman
Cc: Alan Stern

Reviewed-by: Thomas Renninger
Signed-off-by: Hannes Reinecke

   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,
-   "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,
+   "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, 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2013-03-04 Thread Sergei Shtylyov 

Hello.

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


From: Hannes Reineckeh...@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 Helgaasbhelg...@google.com
Cc: Oliver Neukumoneu...@suse.de
Cc: Thomas Renningertr...@suse.de
Cc: Yinghai Luying...@kernel.org
Cc: Frederik Himpefhi...@vub.ac.be
Cc: David Haerdemanda...@hardeman.nu
Cc: Alan Sternst...@rowland.harvard.edu

Reviewed-by: Thomas Renningertr...@suse.de
Signed-off-by: Hannes Reineckeh...@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

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

2013-03-04 Thread 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

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/