ChangeSet 1.1722.83.20, 2004/06/04 15:14:11-07:00, stern@rowland.harvard.edu

[PATCH] USB: Debounce all connect change events

This patch makes the hub driver debounce all connection changes.  Right
now the driver only does so if the status happens to be CONNECTED at one
particular instant.  However, the whole point of debouncing is that the
connection is subject to transient interruptions until it has stabilized;
hence deciding whether to debounce based on a single initial test defeats
the entire purpose.

There are some additional smaller changes that go along with the major
one:

	Comments added to hub_port_connect_change() detailing the
	conditions under which it will be called.

	Don't clear the port's connect-changed feature if it wasn't
	set.

	Skip debouncing if there wasn't a physical connection change
	but only a logical port-enable change (or a firmware-download-
	induced device morph -- not yet implemented).

	Clear all the hub status change indicators in hub_events()
	before handling a connect change.  This will reduce syslog
	clutter from status change bits that remain set while khubd
	is busy taking care of a new device.

The patch includes no changes to the debounce routine itself.  Please
apply.


Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>


 drivers/usb/core/hub.c |   63 +++++++++++++++++++++++++++++++------------------
 1 files changed, 41 insertions(+), 22 deletions(-)


diff -Nru a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
--- a/drivers/usb/core/hub.c	Fri Jun 18 11:03:13 2004
+++ b/drivers/usb/core/hub.c	Fri Jun 18 11:03:13 2004
@@ -1486,7 +1486,14 @@
 	}
 	return remaining;
 }
- 
+
+/* Handle physical or logical connection change events.
+ * This routine is called when:
+ * 	a port connection-change occurs;
+ *	a port enable-change occurs (often caused by EMI);
+ *	usb_reset_device() encounters changed descriptors (as from
+ *		a firmware download)
+ */
 static void hub_port_connect_change(struct usb_hub *hub, int port,
 					u16 portstatus, u16 portchange)
 {
@@ -1497,9 +1504,6 @@
 	dev_dbg (hub_dev,
 		"port %d, status %04x, change %04x, %s\n",
 		port + 1, portstatus, portchange, portspeed (portstatus));
- 
-	/* Clear the connection change status */
-	clear_port_feature(hdev, port + 1, USB_PORT_FEAT_C_CONNECTION);
 
 	if (hub->has_indicators) {
 		set_port_led(hdev, port + 1, HUB_LED_AUTO);
@@ -1510,6 +1514,18 @@
 	if (hdev->children[port])
 		usb_disconnect(&hdev->children[port]);
 
+	if (portchange & USB_PORT_STAT_C_CONNECTION) {
+		status = hub_port_debounce(hdev, port);
+		if (status == -ENOTCONN)
+			portstatus = 0;
+		else if (status < 0) {
+			dev_err (hub_dev,
+				"connect-debounce failed, port %d disabled\n",
+				port+1);
+			goto done;
+		}
+	}
+
 	/* Return now if nothing is connected */
 	if (!(portstatus & USB_PORT_STAT_CONNECTION)) {
 
@@ -1523,13 +1539,6 @@
   			goto done;
 		return;
 	}
-  
-	if (hub_port_debounce(hdev, port)) {
-		dev_err (hub_dev,
-			"connect-debounce failed, port %d disabled\n",
-			port+1);
-		goto done;
-	}
 
 	for (i = 0; i < SET_CONFIG_TRIES; i++) {
 		struct usb_device *udev;
@@ -1631,6 +1640,7 @@
 	u16 portstatus;
 	u16 portchange;
 	int i, ret;
+	int connect_change;
 
 	/*
 	 *  We restart the list every time to avoid a deadlock with
@@ -1676,16 +1686,22 @@
 
 		for (i = 0; i < hub->descriptor->bNbrPorts; i++) {
 			ret = hub_port_status(hdev, i, &portstatus, &portchange);
-			if (ret < 0) {
+			if (ret < 0)
 				continue;
-			}
+			connect_change = 0;
 
 			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_dev,
-					"port %d enable change, status %08x\n",
-					i + 1, portstatus);
+				clear_port_feature(hdev,
+					i + 1, USB_PORT_FEAT_C_CONNECTION);
+				connect_change = 1;
+			}
+
+			if (portchange & USB_PORT_STAT_C_ENABLE) {
+				if (!connect_change)
+					dev_dbg (hub_dev,
+						"port %d enable change, "
+						"status %08x\n",
+						i + 1, portstatus);
 				clear_port_feature(hdev,
 					i + 1, USB_PORT_FEAT_C_ENABLE);
 
@@ -1696,15 +1712,14 @@
 				 * Works at least with mouse driver. 
 				 */
 				if (!(portstatus & USB_PORT_STAT_ENABLE)
-				    && (portstatus & USB_PORT_STAT_CONNECTION)
-				    && (hdev->children[i])) {
+				    && !connect_change
+				    && hdev->children[i]) {
 					dev_err (hub_dev,
 					    "port %i "
 					    "disabled by hub (EMI?), "
 					    "re-enabling...",
 						i + 1);
-					hub_port_connect_change(hub,
-						i, portstatus, portchange);
+					connect_change = 1;
 				}
 			}
 
@@ -1732,6 +1747,10 @@
 				clear_port_feature(hdev,
 					i + 1, USB_PORT_FEAT_C_RESET);
 			}
+
+			if (connect_change)
+				hub_port_connect_change(hub, i,
+						portstatus, portchange);
 		} /* end for i */
 
 		/* deal with hub status changes */