Re: [patch]ratelimit debounce error messages

Am Freitag, 2. März 2007 17:03 schrieb Alan Stern:
> On Thu, 1 Mar 2007, Oliver Neukum wrote:
> 
> > > Actually I had in mind something simpler, like getting rid of the
> > > GET_DESCRIPTOR_TRIES loop for i (or reducing it to one attempt using the
> > > new scheme and one attempt using the old scheme) and getting rid of the 
> > > SET_ADDRESS_TRIES loop for j, both in hub_port_init().
> > 
> > I would prefer to try very hard once. This is what this patch does.
> 
> Take a look at this log from a little over a year ago:
> 
> http://marc.theaimsgroup.com/?l=linux-usb-devel&m=113521633114762&w=2
> 
> Len Brown had a docking station with an internal hub that wasn't working 
> right.  It is device 3-5; we start trying to enumerate it at time stamp 
> 40.129703 and give up at timestamp 103.761001.  That's just ridiculous.
> Not to mention unfriendly, since khubd is unavailable for anything else 
> during that time _and_ it holds the root hub's device lock, preventing 
> many other things from working.
> 
> Trying hard is fine, but to keep on trying for over 60 seconds is foolish.

How about this one?

	Regards
		Oliver

--- linux-2.6.20-giveup/drivers/usb/core/hub.c.orig	2007-03-06 13:18:46.000000000 +0100
+++ linux-2.6.20-giveup/drivers/usb/core/hub.c	2007-03-06 13:24:38.000000000 +0100
@@ -70,6 +70,7 @@
 
 	unsigned		has_indicators:1;
 	u8			indicator[USB_MAXCHILDREN];
+	unsigned long		last_failed[USB_MAXCHILDREN];
 	struct delayed_work	leds;
 };
 
@@ -2410,7 +2411,7 @@
 	struct usb_device *hdev = hub->hdev;
 	struct device *hub_dev = hub->intfdev;
 	u16 wHubCharacteristics = le16_to_cpu(hub->descriptor->wHubCharacteristics);
-	int status, i;
+	int status, i, retries, timeouts = 0;
  
 	dev_dbg (hub_dev,
 		"port %d, status %04x, change %04x, %s\n",
@@ -2438,7 +2439,7 @@
 			dev_err (hub_dev,
 				"connect-debounce failed, port %d disabled\n",
 				port1);
-			goto done;
+			goto failed;
 		}
 		portstatus = status;
 	}
@@ -2452,7 +2453,7 @@
 			set_port_feature(hdev, port1, USB_PORT_FEAT_POWER);
  
 		if (portstatus & USB_PORT_STAT_ENABLE)
-  			goto done;
+  			goto failed;
 		return;
 	}
 
@@ -2469,7 +2470,11 @@
 	}
 #endif
 
-	for (i = 0; i < SET_CONFIG_TRIES; i++) {
+	/* we give short shrift if we had a failure within the last 2 hours */
+	retries = hub->last_failed[port1 - 1] &&
+		time_after(hub->last_failed[port1 - 1] + HZ * 60 * 60 * 2, jiffies) ?
+		1 : SET_CONFIG_TRIES;
+	for (i = 0; i < retries; i++) {
 		struct usb_device *udev;
 
 		/* reallocate for each attempt, since references
@@ -2571,6 +2576,7 @@
 		if (status)
 			dev_dbg(hub_dev, "%dmA power budget left\n", status);
 
+		hub->last_failed[port1 - 1] = 0; /*all former errors forgiven */
 		return;
 
 loop_disable:
@@ -2579,10 +2585,18 @@
 		ep0_reinit(udev);
 		release_address(udev);
 		usb_put_dev(udev);
+		if (status == -ETIMEDOUT)
+			timeouts++;
+		else
+			timeouts = 0;
 		if (status == -ENOTCONN)
 			break;
+		if (timeouts >= 3) /* number taken from US law */
+			break;
 	}
- 
+
+failed:
+	hub->last_failed[port1 - 1] = jiffies;
 done:
 	hub_port_disable(hub, port1, 1);
 }

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel