ChangeSet 1.1722.83.21, 2004/06/04 15:14:42-07:00, stern@rowland.harvard.edu [PATCH] USB: Superficial improvements to hub_port_debounce() Since my previous suggestions for changes to hub_port_debounce() encountered so much resistance, this patch makes some fairly superficial improvements to the code while leaving the logic of the algorithm almost completely intact. The only behavioral change is that it actually requests the port status at the start, rather than assuming the status is not CONNECTED. Changes include: Vastly improved comments that are now unambiguous and accurately descriptive of the code. Local variables changed to more sensible names. The stability period is now reported in milliseconds rather than a meaningless poll count. The sleep interval is moved from the start of the loop to the end, so that the first time through we read the port status immediately. If the connection has not stabilized after the total timeout expires, -ETIMEDOUT is returned rather than whatever the current connect status happens to be. If the connection does stabilize then the port status is returned so that hub_port_connect_change() will have an up-to-date value for the status rather than relying on the pre-debounce value. The changes I wanted to make but other people were worried about are included as comments. A later (small) patch will uncomment them for testing. Signed-off-by: Alan Stern Signed-off-by: Greg Kroah-Hartman drivers/usb/core/hub.c | 64 ++++++++++++++++++++++++++----------------------- 1 files changed, 34 insertions(+), 30 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:02:58 2004 +++ b/drivers/usb/core/hub.c Fri Jun 18 11:02:58 2004 @@ -1190,57 +1190,62 @@ /* 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 400ms total debounce timeout; if the + * connection isn't stable by then it returns -ETIMEDOUT. It checks + * every 25ms for transient disconnects. When the port status has been + * unchanged for 100ms it returns the port status. */ -#define HUB_DEBOUNCE_TIMEOUT 400 -#define HUB_DEBOUNCE_STEP 25 -#define HUB_DEBOUNCE_STABLE 4 +#define HUB_DEBOUNCE_TIMEOUT 400 /* 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 = 0; u16 portchange, portstatus; - unsigned connection; - - connection = 0; - stable_count = 0; - for (delay_time = 0; delay_time < HUB_DEBOUNCE_TIMEOUT; delay_time += HUB_DEBOUNCE_STEP) { - msleep(HUB_DEBOUNCE_STEP); + unsigned connection = 0xffff; + for (total_time = 0; ; total_time += 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) { + stable_time += HUB_DEBOUNCE_STEP; + if (stable_time >= HUB_DEBOUNCE_STABLE && connection /* */) + 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); + if (portchange & USB_PORT_STAT_C_CONNECTION) { + clear_port_feature(hdev, port+1, + USB_PORT_FEAT_C_CONNECTION); } + + if (total_time >= HUB_DEBOUNCE_TIMEOUT) + break; + msleep(HUB_DEBOUNCE_STEP); } 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 -ETIMEDOUT; + return portstatus; } static int hub_set_address(struct usb_device *udev) @@ -1516,14 +1521,13 @@ if (portchange & USB_PORT_STAT_C_CONNECTION) { status = hub_port_debounce(hdev, port); - if (status == -ENOTCONN) - portstatus = 0; - else if (status < 0) { + if (status < 0) { dev_err (hub_dev, "connect-debounce failed, port %d disabled\n", port+1); goto done; } + portstatus = status; } /* Return now if nothing is connected */