On Fri, 7 May 2010 10:41:10 -0400 (EDT)
Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
> This patch (as1378) fixes the bug by moving the initialization
> statements up into usbhid_probe().
BTW, just in case, here's my "fault emulator" that fakes the devices
sprinkling -71s on us:
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 8b0c235..7c750fd 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2675,6 +2675,13 @@ hub_port_init (struct usb_hub *hub, struct usb_device *udev, int port1,
udev->tt = &hub->tt;
udev->ttport = port1;
}
+
+ if (udev->speed == USB_SPEED_LOW &&
+ (jiffies & 1) != 0) { /* Save main keyboard */
+ udev->fault_forced = 1;
+ udev->fault_count = 2 + (jiffies & 31);
+ dev_info(&udev->dev, "fault forced in %d\n", udev->fault_count);
+ }
/* Why interleave GET_DESCRIPTOR and SET_ADDRESS this way?
* Because device hardware and firmware is sometimes buggy in
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index f5f5601..0c0d142 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -376,6 +376,60 @@ static void ehci_watchdog(unsigned long param)
spin_unlock_irqrestore (&ehci->lock, flags);
}
+static int ehci_fault_retire_one(struct ehci_hcd *ehci, int status)
+{
+ struct urb *urb;
+ unsigned long flags;
+ int rc;
+
+ spin_lock_irqsave(&ehci->lock, flags);
+ if (list_empty(&ehci->fault_list)) {
+ spin_unlock_irqrestore(&ehci->lock, flags);
+ return 0;
+ }
+
+ urb = list_entry(ehci->fault_list.next, struct urb, urb_list);
+ list_del(&urb->urb_list);
+
+ rc = list_empty(&ehci->fault_list) ? 0 : 1;
+ spin_unlock_irqrestore(&ehci->lock, flags);
+
+ usb_hcd_giveback_urb(ehci_to_hcd(ehci), urb, status);
+ return rc;
+}
+
+static void ehci_fault_kick(unsigned long param)
+{
+ struct ehci_hcd *ehci = (struct ehci_hcd *) param;
+
+ if (ehci_fault_retire_one(ehci, -EPROTO))
+ mod_timer(&ehci->fault_timer, jiffies + msecs_to_jiffies(2));
+}
+
+static int ehci_fault_check(struct ehci_hcd *ehci, struct urb *urb)
+{
+ unsigned long flags;
+ struct usb_device *udev = urb->dev;
+
+ if (!udev->fault_forced)
+ return -1;
+
+ spin_lock_irqsave(&ehci->lock, flags);
+ if (udev->fault_count) {
+ --udev->fault_count;
+ if (udev->fault_count == 0) {
+ dev_info(&udev->dev, "start faulting for address %d\n", udev->devnum);
+ }
+ spin_unlock_irqrestore(&ehci->lock, flags);
+ return -1;
+ }
+ if (list_empty(&ehci->fault_list))
+ mod_timer(&ehci->fault_timer, jiffies + msecs_to_jiffies(2));
+ list_add_tail(&urb->urb_list, &ehci->fault_list);
+ spin_unlock_irqrestore(&ehci->lock, flags);
+ return 0;
+}
+
/* On some systems, leaving remote wakeup enabled prevents system shutdown.
* The firmware seems to think that powering off is a wakeup event!
* This routine turns off remote wakeup and everything else, on all ports.
@@ -480,6 +534,9 @@ static void ehci_stop (struct usb_hcd *hcd)
ehci_dbg (ehci, "stop\n");
+ del_timer_sync(&ehci->fault_timer);
+ while (ehci_fault_retire_one(ehci, -ENODEV)) { }
+
/* no more interrupts ... */
del_timer_sync (&ehci->watchdog);
del_timer_sync(&ehci->iaa_watchdog);
@@ -537,6 +594,11 @@ static int ehci_init(struct usb_hcd *hcd)
ehci->iaa_watchdog.function = ehci_iaa_watchdog;
ehci->iaa_watchdog.data = (unsigned long) ehci;
+ init_timer(&ehci->fault_timer);
+ ehci->fault_timer.function = ehci_fault_kick;
+ ehci->fault_timer.data = (unsigned long) ehci;
+ INIT_LIST_HEAD(&ehci->fault_list);
+
/*
* hw default: 1K periodic list heads, one per frame.
* periodic_size can shrink by USBCMD update if hcc_params allows.
@@ -838,6 +900,9 @@ static int ehci_urb_enqueue (
struct ehci_hcd *ehci = hcd_to_ehci (hcd);
struct list_head qtd_list;
+ if (ehci_fault_check(ehci, urb) == 0)
+ return 0;
+
INIT_LIST_HEAD (&qtd_list);
switch (usb_pipetype (urb->pipe)) {
diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index 2d85e21..f015284 100644
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -121,6 +121,9 @@ struct ehci_hcd { /* one per controller */
ktime_t last_periodic_enable;
u32 command;
+ struct list_head fault_list;
+ struct timer_list fault_timer;
+
/* SILICON QUIRKS */
unsigned no_selective_suspend:1;
unsigned has_fsl_port_bug:1; /* FreeScale */
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 0c22c64..4c1998c 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -480,7 +480,9 @@ struct usb_device {
unsigned authorized:1;
unsigned authenticated:1;
unsigned wusb:1;
+ unsigned fault_forced:1; /* fault_count is meaningful */
int string_langid;
+ int fault_count;
/* static strings from the device */
char *product;
With this in kernel, I just needed to pull the plug on keyboard maybe
5 times to crash the kernel. The patch equivalent to what you just
posted and the one before that relocates usbhid->intf fixes the problem.
Notice I only put the hook into ehci because my box only has EHCI
with TT and no UHCI.
-- Pete
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
