David:

In my testing, the system entered a peculiar state in which the HC was 
repeatedly enabled and disabled, over and over.  I had seen this before, 
but was never able to generate it reproducibly until now.

It turned out to be caused by the khubd thread hanging.  One of the bugs I
fixed in usbtest caused the test routine to hang if the first URB failed
upon submission.  When that happened it retained the usbtest device lock,
blocking the disconnect routine and thus the khubd thread.

The problem kicked in when I unplugged the USB device.  The hub_events()
routine in hub.c is written to handle port-connect-change events fully
before moving on to handle port-enable-change events.  Since the
connect-change handler was hung, the enable-change handler never got to
run, so the port-enable-change bit in the root hub controller was never
reset.  For whatever reason, this bit caused the resume-detect signal to
turn on, which tells the driver to wakeup the HC.  Since no devices were
actually connected, the driver then proceded to suspend the HC, and the 
sequence repeated.

This patch simply moves the connect-change and enable-change handlers to
the end, so they get run after all the port-change features are reset.  
Clearly this is only a stopgap measure.  A much better approach would be
something like what you have suggested in the past: to have the hub thread
send all the reset-feature messages and then invoke a separate work queue
to manage each individual port.  That would be more like the traditional
division of labor into top halves (which do just enough to turn off a
device's interrupt request) and bottom halves (which do the actual work).

Alan Stern


===== hub.c 1.112 vs edited =====
--- 1.112/drivers/usb/core/hub.c        Fri Jun 20 13:12:57 2003
+++ edited/drivers/hub/core/hub.c       Mon Jul 21 10:03:01 2003
@@ -876,13 +876,6 @@
        unsigned int delay = HUB_SHORT_RESET_TIME;
        int i;
 
-       dev_dbg (&hubstate->intf->dev,
-               "port %d, status %x, change %x, %s\n",
-               port + 1, portstatus, portchange, portspeed (portstatus));
-
-       /* Clear the connection change status */
-       clear_port_feature(hub, port + 1, USB_PORT_FEAT_C_CONNECTION);
-
        /* Disconnect any existing devices under this port */
        if (hub->children[port])
                usb_disconnect(&hub->children[port]);
@@ -1051,33 +1044,21 @@
                        }
 
                        if (portchange & USB_PORT_STAT_C_CONNECTION) {
-                               hub_port_connect_change(hub, i, portstatus, 
portchange);
-                       } else if (portchange & USB_PORT_STAT_C_ENABLE) {
+                               dev_dbg (&hub->intf->dev,
+                                       "port %d, status %x, change %x, %s\n",
+                                       i + 1, portstatus, portchange,
+                                       portspeed (portstatus));
+                               clear_port_feature(dev,
+                                       i + 1, USB_PORT_FEAT_C_CONNECTION);
+                       }
+
+                       if (portchange & USB_PORT_STAT_C_ENABLE) {
                                dev_dbg (hubdev (dev),
                                        "port %d enable change, status %x\n",
                                        i + 1, portstatus);
                                clear_port_feature(dev,
                                        i + 1, USB_PORT_FEAT_C_ENABLE);
 
-                               /*
-                                * EM interference sometimes causes badly
-                                * shielded USB devices to be shutdown by
-                                * the hub, this hack enables them again.
-                                * Works at least with mouse driver. 
-                                */
-                               if (!(portstatus & USB_PORT_STAT_ENABLE)
-                                   && (portstatus & USB_PORT_STAT_CONNECTION)
-                                   && (dev->children[i])) {
-                                       dev_err (&hub->intf->dev,
-                                           "port %i "
-                                           "disabled by hub (EMI?), "
-                                           "re-enabling...",
-                                               i + 1);
-                                       hub_port_connect_change(hub,
-                                               i, portstatus, portchange);
-                               }
-                       }
-
                        if (portchange & USB_PORT_STAT_C_SUSPEND) {
                                dev_dbg (&hub->intf->dev,
                                        "suspend change on port %d\n",
@@ -1101,6 +1082,32 @@
                                        i + 1);
                                clear_port_feature(dev,
                                        i + 1, USB_PORT_FEAT_C_RESET);
+                       }
+
+                       if (portchange & USB_PORT_STAT_C_CONNECTION) {
+                               hub_port_connect_change(hub, i, portstatus,
+                                       portchange);
+                       }
+
+                       if (portchange & USB_PORT_STAT_C_ENABLE) {
+
+                               /*
+                                * EM interference sometimes causes badly
+                                * shielded USB devices to be shutdown by
+                                * the hub, this hack enables them again.
+                                * Works at least with mouse driver. 
+                                */
+                               if (!(portstatus & USB_PORT_STAT_ENABLE)
+                                   && (portstatus & USB_PORT_STAT_CONNECTION)
+                                   && (dev->children[i])) {
+                                       dev_err (&hub->intf->dev,
+                                           "port %i "
+                                           "disabled by hub (EMI?), "
+                                           "re-enabling...",
+                                               i + 1);
+                                       hub_port_connect_change(hub,
+                                               i, portstatus, portchange);
+                               }
                        }
                } /* end for i */
 



-------------------------------------------------------
This SF.net email is sponsored by: VM Ware
With VMware you can run multiple operating systems on a single machine.
WITHOUT REBOOTING! Mix Linux / Windows / Novell virtual machines at the
same time. Free trial click here: http://www.vmware.com/wl/offer/345/0
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to