On Wed, 26 May 2004, Pete Zaitcev wrote: > I am trying to get you to explain what you do not like about comments. > They match the code perfectly (checked in 2.6.6).
I think they are unclear and confusing. I disagree that they match the code perfectly because I can't tell what they really mean. In my revision below the comments _do_ match the code. They might both be incorrect -- but that's a separate issue! > > I think it wouldn't hurt to change two aspects of this routine. The first > > is simple: Nils Faerber has requested that the HUB_DEBOUNCE_TIMEOUT value > > be increased from 400 ms to 1500 ms. That wouldn't affect normal stable > > connections but it would give slightly flaky devices a better chance of > > connecting properly. > > I am dubious. Do you have an actual report from him? http://marc.theaimsgroup.com/?l=linux-usb-devel&m=107943746325823&w=2 > > The second is to treat connect-change status properly -- make it reset the > > stable_count value back to 0. The way it is now, if the connection status > > is unstable and drops & returns within a 25 ms period, the routine won't > > realize that anything has happened. > > *IF* I understand what you are talking about, you want to track > USB_PORT_STAT_C_CONNECTION. I saw hubs with USB_PORT_STAT_C_CONNECTION now > working right, this is why the "connection" variable is present at all. > Otherwise, we would not need it. IIRC, 2.2 did not have it. You're right; if USB_PORT_STAT_C_CONNECTION always worked right then "connection" wouldn't be needed. > In any case, this discussion is too vague. If you have an idea for a code > change, please post a patch. Fair enough... Here's my current approach. It has one additional change from the current code: If the port has been stably disconnected for 100 ms it returns immediately without waiting for the full timeout period. Maybe that's too optimistic, I don't know. I haven't tried testing this yet. Alan Stern ===== drivers/usb/core/hub.c 1.158 vs edited ===== --- 1.158/drivers/usb/core/hub.c Tue May 25 16:24:27 2004 +++ edited/drivers/usb/core/hub.c Wed May 26 16:00:51 2004 @@ -958,57 +958,64 @@ /* USB 2.0 spec, 7.1.7.3 / fig 7-29: * * Between connect detection and reset signaling there must be a delay - * of 100ms at least for debounce and power-settling. The corresponding + * of 100ms at least for debounce and power-settling. The corresponding * timer shall restart whenever the downstream port detects a disconnect. * - * Apparently there are some bluetooth and irda-dongles and a number - * of low-speed devices which require longer delays of about 200-400ms. + * Apparently there are some bluetooth and irda-dongles and a number of + * low-speed devices for which this debounce period may last over a second. * Not covered by the spec - but easy to deal with. * - * This implementation uses 400ms minimum debounce timeout and checks - * every 25ms for transient disconnects to restart the delay. + * This implementation uses a 1500ms total debounce timeout; if the + * connection isn't stable by then it returns -EBUSY. It checks every + * 25ms for transient disconnects. When the port status has been + * unchanged for 100ms it returns 0 for a successful connection or + * -ENOTCONN for no connection. */ -#define HUB_DEBOUNCE_TIMEOUT 400 -#define HUB_DEBOUNCE_STEP 25 -#define HUB_DEBOUNCE_STABLE 4 +#define HUB_DEBOUNCE_TIMEOUT 1500 +#define HUB_DEBOUNCE_STEP 25 +#define HUB_DEBOUNCE_STABLE 100 static int hub_port_debounce(struct usb_device *hdev, int port) { int ret; - int delay_time, stable_count; + int total_time, stable_time; u16 portchange, portstatus; unsigned connection; + stable_time = 0; connection = 0; - stable_count = 0; - for (delay_time = 0; delay_time < HUB_DEBOUNCE_TIMEOUT; delay_time += HUB_DEBOUNCE_STEP) { + for (total_time = 0; total_time < HUB_DEBOUNCE_TIMEOUT; + total_time += HUB_DEBOUNCE_STEP) { msleep(HUB_DEBOUNCE_STEP); ret = hub_port_status(hdev, port, &portstatus, &portchange); if (ret < 0) return ret; - if ((portstatus & USB_PORT_STAT_CONNECTION) == connection) { - if (connection) { - if (++stable_count == HUB_DEBOUNCE_STABLE) - break; - } + if (!(portchange & USB_PORT_STAT_C_CONNECTION) && + (portstatus & USB_PORT_STAT_CONNECTION) == connection) { + if ((stable_time += HUB_DEBOUNCE_STEP) >= + HUB_DEBOUNCE_STABLE) + break; } else { - stable_count = 0; + stable_time = 0; + connection = portstatus & USB_PORT_STAT_CONNECTION; } - connection = portstatus & USB_PORT_STAT_CONNECTION; if ((portchange & USB_PORT_STAT_C_CONNECTION)) { - clear_port_feature(hdev, port+1, USB_PORT_FEAT_C_CONNECTION); + clear_port_feature(hdev, port+1, + USB_PORT_FEAT_C_CONNECTION); } } dev_dbg (hubdev (hdev), - "debounce: port %d: delay %dms stable %d status 0x%x\n", - port + 1, delay_time, stable_count, portstatus); + "debounce: port %d: total %dms stable %dms status 0x%x\n", + port + 1, total_time, stable_time, portstatus); - return (portstatus & USB_PORT_STAT_CONNECTION) ? 0 : -ENOTCONN; + if (stable_time < HUB_DEBOUNCE_STABLE) + return -EBUSY; + return connection ? 0 : -ENOTCONN; } static int hub_set_address(struct usb_device *udev) ------------------------------------------------------- This SF.Net email is sponsored by: Oracle 10g Get certified on the hottest thing ever to hit the market... Oracle 10g. Take an Oracle 10g class now, and we'll give you the exam FREE. http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click _______________________________________________ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
