[PATCH] USB: fix interrupt disabling for HCDs with shared interrupthandlers

[Stefan Becker]

Hi,

here is the proposed patch to fix the problem. I hope I used git 
correctly to generate it...

Regards,

	Stefan

---
Stefan Becker
E-Mail: Stefan.Becker@xxxxxxxxx
From 445575b29a449057294acf61c7d51c5f70490676 Mon Sep 17 00:00:00 2001
From: Stefan Becker <stefan.becker@xxxxxxxxx>
Date: Mon, 30 Jun 2008 21:18:29 +0300
Subject: [PATCH] USB: fix interrupt disabling for HCDs with shared interrupt handlers

As has been discussed several times on LKML, IRQF_SHARED | IRQF_DISABLED
doesn't work together reliably, i.e. a shared interrupt handler CAN'T be
certain to be called with interrupts disabled. Most USB HCD handlers use
IRQF_DISABLED and therefore havoc can break out if they share their
interrupt with a handler that doesn't use it.

On my test machine the yenta_socket interrupt handler (no IRQF_DISABLED)
was registered before ehci_hcd and one uhci_hcd instance. Therefore all
usb_hcd_irq() invocations for ehci_hcd and for one uhci_hcd instance
happened with interrupts enabled. That led to random lockups as USB core
HCD functions that acquire the same spinlock could be called twice
from interrupt handlers.

This patch doesn't modify the interface of usb_add_hcd(), i.e. HCD code
can still use IRQF_DISABLED. It will now silently convert this into an
internal HCD flag and clear IRQF_DISABLED before requesting the interrupt
from the Linux core. usb_hcd_irq() will check this flag to disable/re-enable
the interrupts on a case by case basis.

Signed-off-by: Stefan Becker <stefan.becker@xxxxxxxxx>

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 09a53e7..76eb3fc 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1683,20 +1683,30 @@ EXPORT_SYMBOL_GPL(usb_bus_start_enum);
  */
 irqreturn_t usb_hcd_irq (int irq, void *__hcd)
 {
-	struct usb_hcd		*hcd = __hcd;
-	int			start = hcd->state;
+	struct usb_hcd		*hcd  = __hcd;
+	unsigned int            flags = 0;
+	irqreturn_t             rc;
+
+	if (hcd->disable_irq)
+		local_irq_save(flags);
+
+	if (unlikely(hcd->state == HC_STATE_HALT ||
+		     !test_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags))) {
+		rc = IRQ_NONE;
+	} else if (hcd->driver->irq (hcd) == IRQ_NONE) {
+		rc = IRQ_NONE;
+	} else {
+		set_bit(HCD_FLAG_SAW_IRQ, &hcd->flags);
 
-	if (unlikely(start == HC_STATE_HALT ||
-	    !test_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags)))
-		return IRQ_NONE;
-	if (hcd->driver->irq (hcd) == IRQ_NONE)
-		return IRQ_NONE;
+		if (unlikely(hcd->state == HC_STATE_HALT))
+			usb_hc_died (hcd);
+		rc = IRQ_HANDLED;
+	}
 
-	set_bit(HCD_FLAG_SAW_IRQ, &hcd->flags);
+	if (hcd->disable_irq)
+		local_irq_restore(flags);
 
-	if (unlikely(hcd->state == HC_STATE_HALT))
-		usb_hc_died (hcd);
-	return IRQ_HANDLED;
+	return rc;
 }
 
 /*-------------------------------------------------------------------------*/
@@ -1860,6 +1870,14 @@ int usb_add_hcd(struct usb_hcd *hcd,
 
 	/* enable irqs just before we start the controller */
 	if (hcd->driver->irq) {
+
+		/* IRQF_DISABLED doesn't work as advertised when used together
+		 * with IRQF_SHARED. Therefore we handle interrupt disabling
+		 * ourselves in usb_hcd_irq() when the controller requests it.
+		 */
+		hcd->disable_irq = (irqflags & IRQF_DISABLED) ? 1 : 0;
+		irqflags &= ~IRQF_DISABLED;
+
 		snprintf(hcd->irq_descr, sizeof(hcd->irq_descr), "%s:usb%d",
 				hcd->driver->description, hcd->self.busnum);
 		if ((retval = request_irq(irqnum, &usb_hcd_irq, irqflags,
diff --git a/drivers/usb/core/hcd.h b/drivers/usb/core/hcd.h
index a0bf5df..df404ad 100644
--- a/drivers/usb/core/hcd.h
+++ b/drivers/usb/core/hcd.h
@@ -100,6 +100,7 @@ struct usb_hcd {
 	unsigned		wireless:1;	/* Wireless USB HCD */
 	unsigned		authorized_default:1;
 	unsigned		has_tt:1;	/* Integrated TT in root hub */
+	unsigned		disable_irq:1;	/* Disable IRQs for handler */
 
 	int			irq;		/* irq allocated */
 	void __iomem		*regs;		/* device memory/io */