[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <200806301331.21077.david-b@pacbell.net>
Date: Mon, 30 Jun 2008 13:31:20 -0700
From: David Brownell <david-b@...bell.net>
To: Alan Stern <stern@...land.harvard.edu>
Cc: Stefan Becker <Stefan.Becker@...ia.com>,
linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org,
Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH] USB: fix interrupt disabling for HCDs with shared interrupt handlers
On Monday 30 June 2008, Alan Stern wrote:
> Don't bother with this extra stuff. All USB host controller drivers
> want to have interrupts disabled when their IRQ handlers run.
How about this one instead? I think it's probably almost midnight
where Stefan is, so I'd not expect Stefan to have an updated
patch very soon ... :)
- dave
====== CUT HERE
Add a workaround for the way the IRQ framework can disregard
IRQF_DISABLED when IRQF_SHARED is set and lockdep isn't in use.
This leaves IRQF_DISABLED active for unshared IRQs, but clears
that flag otherwise (since the genirq code should eventually
start warning folk about that bad combination of flags).
(Thanks to Stefan Becker <Stefan.Becker@...ia.com> for tracking
down the source of hard lockups on his hardware; on some systems
this problem could lead to oopsing.)
Signed-off-by: David Brownell <dbrownell@...rs.sourceforge.net>
---
UNTESTED but "obviously right" yes? Candidate for 2.6.26-rc8+ ...
drivers/usb/core/hcd.c | 22 ++++++++++++++++++----
1 file changed, 18 insertions(+), 4 deletions(-)
--- a/drivers/usb/core/hcd.c 2008-06-30 13:09:00.000000000 -0700
+++ b/drivers/usb/core/hcd.c 2008-06-30 13:28:04.000000000 -0700
@@ -1687,19 +1687,31 @@ 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;
+ int start;
+ unsigned long flags;
+ irqreturn_t value = IRQ_NONE;
+
+ /* IRQF_DISABLED doesn't work correctly with shared IRQs
+ * when the first handler doesn't use it. So let's just
+ * assume it's never used.
+ */
+ local_irq_save(flags);
+ start = hcd->state;
if (unlikely(start == HC_STATE_HALT ||
!test_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags)))
- return IRQ_NONE;
+ goto done;
if (hcd->driver->irq (hcd) == IRQ_NONE)
- return IRQ_NONE;
+ goto done;
+ value = IRQ_HANDLED;
set_bit(HCD_FLAG_SAW_IRQ, &hcd->flags);
if (unlikely(hcd->state == HC_STATE_HALT))
usb_hc_died (hcd);
- return IRQ_HANDLED;
+done:
+ local_irq_restore(flags);
+ return value;
}
/*-------------------------------------------------------------------------*/
@@ -1865,6 +1877,8 @@ int usb_add_hcd(struct usb_hcd *hcd,
if (hcd->driver->irq) {
snprintf(hcd->irq_descr, sizeof(hcd->irq_descr), "%s:usb%d",
hcd->driver->description, hcd->self.busnum);
+ /* work around (IRQF_SHARED|IRQF_DISABLED) misbehavior.. */
+ irqflags &= ~IRQF_DISABLED;
if ((retval = request_irq(irqnum, &usb_hcd_irq, irqflags,
hcd->irq_descr, hcd)) != 0) {
dev_err(hcd->self.controller,
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists