linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH v1] usb: hcd: Try MSI interrupts on PCI devices
@ 2022-02-28 20:12 micklorain
  2022-02-28 20:27 ` gregkh
  2022-02-28 20:59 ` andriy.shevchenko
  0 siblings, 2 replies; 9+ messages in thread
From: micklorain @ 2022-02-28 20:12 UTC (permalink / raw)
  To: andriy.shevchenko@linux.intel.com
  Cc: gregkh@linuxfoundation.org, linux-usb@vger.kernel.org,
	mathias.nyman@intel.com

Hi,

This patch breaks USB for me. I noticed when I upgraded from debian's 4.19.0-18 (working) to 5.10.0-10 (broken). I git bisect'ed until I found that this patch is the culprit. Upstream 5.17.0-rc2 is still broken, but 5.17.0-rc2 with this patch reverted works.

lsusb when things work :
https://paste.debian.net/hidden/2a964425/

lsusb when things are broken :
https://paste.debian.net/hidden/0376920c/

dmesg when things are broken :
https://paste.debian.net/hidden/780ca112/

dmesg when things work :
https://paste.debian.net/hidden/4d1bfc0f/

Let me know if you need anything else from me.

Thanks !
Mick Lorain

-------- Original Message --------

On 02/07/2020 16:30, Andy Shevchenko wrote:

> It appears that some platforms share same IRQ line between several devices,
> some of which are EHCI and OHCI controllers. This is neither practical nor
> performance-wise, especially in the case when they are supporting MSI.
>
> In order to improve the situation try to allocate MSI and fallback to legacy
> IRQ if no MSI available.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/usb/core/hcd-pci.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c
> index 1547aa6e5314..4dc443aaef5c 100644
> --- a/drivers/usb/core/hcd-pci.c
> +++ b/drivers/usb/core/hcd-pci.c
> @@ -194,20 +194,21 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct pci_device_id *id,
>  	 * make sure irq setup is not touched for xhci in generic hcd code
>  	 */
>  	if ((driver->flags & HCD_MASK) < HCD_USB3) {
> -		if (!dev->irq) {
> +		retval = pci_alloc_irq_vectors(dev, 1, 1, PCI_IRQ_LEGACY | PCI_IRQ_MSI);
> +		if (retval < 0) {
>  			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_irq = pci_irq_vector(dev, 0);
>  	}
>
>  	hcd = usb_create_hcd(driver, &dev->dev, pci_name(dev));
>  	if (!hcd) {
>  		retval = -ENOMEM;
> -		goto disable_pci;
> +		goto free_irq_vectors;
>  	}
>
>  	hcd->amd_resume_bug = (usb_hcd_amd_remote_wakeup_quirk(dev) &&
> @@ -286,6 +287,9 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct pci_device_id *id,
>
>  put_hcd:
>  	usb_put_hcd(hcd);
> +free_irq_vectors:
> +	if ((driver->flags & HCD_MASK) < HCD_USB3)
> +		pci_free_irq_vectors(dev);
>  disable_pci:
>  	pci_disable_device(dev);
>  	dev_err(&dev->dev, "init %s fail, %d\n", pci_name(dev), retval);
> @@ -343,6 +347,8 @@ void usb_hcd_pci_remove(struct pci_dev *dev)
>  		up_read(&companions_rwsem);
>  	}
>  	usb_put_hcd(hcd);
> +	if ((hcd->driver->flags & HCD_MASK) < HCD_USB3)
> +		pci_free_irq_vectors(dev);
>  	pci_disable_device(dev);
>  }
>  EXPORT_SYMBOL_GPL(usb_hcd_pci_remove);
> @@ -454,7 +460,7 @@ static int suspend_common(struct device *dev, bool do_wakeup)
>  	 * synchronized here.
>  	 */
>  	if (!hcd->msix_enabled)
> -		synchronize_irq(pci_dev->irq);
> +		synchronize_irq(pci_irq_vector(pci_dev, 0));
>
>  	/* Downstream ports from this root hub should already be quiesced, so
>  	 * there will be no DMA activity.  Now we can shut down the upstream
> --
> 2.27.0



^ permalink raw reply	[flat|nested] 9+ messages in thread
* [PATCH v1] usb: hcd: Try MSI interrupts on PCI devices
@ 2020-07-02 14:30 Andy Shevchenko
  0 siblings, 0 replies; 9+ messages in thread
From: Andy Shevchenko @ 2020-07-02 14:30 UTC (permalink / raw)
  To: Greg Kroah-Hartman, linux-usb, Mathias Nyman; +Cc: Andy Shevchenko

It appears that some platforms share same IRQ line between several devices,
some of which are EHCI and OHCI controllers. This is neither practical nor
performance-wise, especially in the case when they are supporting MSI.

In order to improve the situation try to allocate MSI and fallback to legacy
IRQ if no MSI available.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/usb/core/hcd-pci.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c
index 1547aa6e5314..4dc443aaef5c 100644
--- a/drivers/usb/core/hcd-pci.c
+++ b/drivers/usb/core/hcd-pci.c
@@ -194,20 +194,21 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct pci_device_id *id,
 	 * make sure irq setup is not touched for xhci in generic hcd code
 	 */
 	if ((driver->flags & HCD_MASK) < HCD_USB3) {
-		if (!dev->irq) {
+		retval = pci_alloc_irq_vectors(dev, 1, 1, PCI_IRQ_LEGACY | PCI_IRQ_MSI);
+		if (retval < 0) {
 			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_irq = pci_irq_vector(dev, 0);
 	}
 
 	hcd = usb_create_hcd(driver, &dev->dev, pci_name(dev));
 	if (!hcd) {
 		retval = -ENOMEM;
-		goto disable_pci;
+		goto free_irq_vectors;
 	}
 
 	hcd->amd_resume_bug = (usb_hcd_amd_remote_wakeup_quirk(dev) &&
@@ -286,6 +287,9 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct pci_device_id *id,
 
 put_hcd:
 	usb_put_hcd(hcd);
+free_irq_vectors:
+	if ((driver->flags & HCD_MASK) < HCD_USB3)
+		pci_free_irq_vectors(dev);
 disable_pci:
 	pci_disable_device(dev);
 	dev_err(&dev->dev, "init %s fail, %d\n", pci_name(dev), retval);
@@ -343,6 +347,8 @@ void usb_hcd_pci_remove(struct pci_dev *dev)
 		up_read(&companions_rwsem);
 	}
 	usb_put_hcd(hcd);
+	if ((hcd->driver->flags & HCD_MASK) < HCD_USB3)
+		pci_free_irq_vectors(dev);
 	pci_disable_device(dev);
 }
 EXPORT_SYMBOL_GPL(usb_hcd_pci_remove);
@@ -454,7 +460,7 @@ static int suspend_common(struct device *dev, bool do_wakeup)
 	 * synchronized here.
 	 */
 	if (!hcd->msix_enabled)
-		synchronize_irq(pci_dev->irq);
+		synchronize_irq(pci_irq_vector(pci_dev, 0));
 
 	/* Downstream ports from this root hub should already be quiesced, so
 	 * there will be no DMA activity.  Now we can shut down the upstream
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-03-14  9:58 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-02-28 20:12 [PATCH v1] usb: hcd: Try MSI interrupts on PCI devices micklorain
2022-02-28 20:27 ` gregkh
2022-03-13 21:53   ` micklorain
2022-02-28 20:59 ` andriy.shevchenko
2022-03-06 16:09   ` micklorain
2022-03-09 17:47     ` andriy.shevchenko
2022-03-13 21:44       ` micklorain
2022-03-14  9:57         ` andriy.shevchenko
  -- strict thread matches above, loose matches on Subject: below --
2020-07-02 14:30 Andy Shevchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).