On Wed, 13 Feb 2002, Greg KH wrote:
> > Yes, I do totally agree with David: there is absolutely _no_ debounce
> > interval (connect-detect-to-port-reset) implemented in 2.4.18-pre9 hub.c
>
> Good catch. I'll send out a patch in a bit to fix this.
Me too - see below
> > struct usb_port_status - which becomes the transfer_buffer for the
> > usb_control_msg) call. Mapping stack locations for PCI-DMA however is
>
> Bleah, yes this should be fixed. Patches _gladly_ accepted for this
> problem.
Hi,
the attached patch below addresses both problems:
* usb_hub_port_debounce() provides the required debounce delay including
the "restart delay when spurious disconnect detected" behaviour
described in 7.1.7.1. However, I've used a minimum delay of 200ms
(instead TATTDB > 100ms) as this turned out to be needed to solve
the "not accepting new address" problem relyably for my usb-irda dongle.
I've kept the additional 400ms delay for low speed device as I wasn't
able to test whether this issue is still there.
Did some testing with both the usb-irda and the EZ-USB FX dev. board,
where the latter one turned out to be a pretty good trigger for connect/
disconnect races on hub-port, when one starts repeatedly pressing the
reset/disconnect button - Looks good for me now!
* usb_hub_port_status() was introduced to wrap the calls for
usb_get_port_status and uses a kmalloced temporary buffer instead
of exposing stack locations to PCI-DMA. Had it included during the
debounce tests and it work as expected for me.
Patch created and tested with 2.4.18-pre9.
Martin
----------------------
--- linux-2.4.18-pre9/drivers/usb/hub.c Wed Nov 28 15:16:48 2001
+++ v2.4.18-pre9-md/drivers/usb/hub.c Thu Feb 14 01:49:34 2002
@@ -496,6 +496,29 @@
err("cannot disconnect hub %d", dev->devnum);
}
+static int usb_hub_port_status(struct usb_device *hub, int port,
+ unsigned short *status, unsigned short *change)
+{
+ struct usb_port_status *portsts;
+ int ret = -1;
+
+ portsts = kmalloc(sizeof(portsts), GFP_KERNEL);
+ if (portsts) {
+ ret = usb_get_port_status(hub, port + 1, portsts);
+ if (ret < 0)
+ err("get_port_status(%d) failed (err = %d)", port + 1, ret);
+ else {
+ *status = le16_to_cpu(portsts->wPortStatus);
+ *change = le16_to_cpu(portsts->wPortChange);
+ dbg("port %d, portstatus %x, change %x, %s", port + 1,
+ *status, *change, portspeed(*status));
+ ret = 0;
+ }
+ kfree(portsts);
+ }
+ return ret;
+}
+
#define HUB_RESET_TRIES 5
#define HUB_PROBE_TRIES 2
#define HUB_SHORT_RESET_TIME 10
@@ -507,7 +530,6 @@
struct usb_device *dev, unsigned int delay)
{
int delay_time, ret;
- struct usb_port_status portsts;
unsigned short portchange, portstatus;
for (delay_time = 0; delay_time < HUB_RESET_TIMEOUT; delay_time += delay) {
@@ -515,17 +537,11 @@
wait_ms(delay);
/* read and decode port status */
- ret = usb_get_port_status(hub, port + 1, &portsts);
+ ret = usb_hub_port_status(hub, port, &portstatus, &portchange);
if (ret < 0) {
- err("get_port_status(%d) failed (err = %d)", port + 1, ret);
return -1;
}
- portstatus = le16_to_cpu(portsts.wPortStatus);
- portchange = le16_to_cpu(portsts.wPortChange);
- dbg("port %d, portstatus %x, change %x, %s", port + 1,
- portstatus, portchange, portspeed (portstatus));
-
/* bomb out completely if something weird happened */
if ((portchange & USB_PORT_STAT_C_CONNECTION))
return -1;
@@ -592,17 +608,49 @@
port + 1, hub->devnum, ret);
}
+#define HUB_DEBOUNCE_TIMEOUT 200 /* 7.1.7.1: min 100ms - restart if disconnect
+*/
+#define HUB_DEBOUNCE_STEP 10 /* interval when to check for spurious
+disconnect */
+
+/* return: -1 on error, 0 on success, 1 on disconnect. */
+static int usb_hub_port_debounce(struct usb_device *hub, int port)
+{
+ int delay_time, ret, bogus_discon;
+ unsigned short portchange, portstatus;
+
+ bogus_discon = 0;
+ for (delay_time = 0; delay_time < HUB_DEBOUNCE_TIMEOUT; /* empty */ ) {
+
+ /* wait for debounce and to give the device stable buspower */
+
+ wait_ms(HUB_DEBOUNCE_STEP);
+
+ ret = usb_hub_port_status(hub, port, &portstatus, &portchange);
+ if (ret < 0)
+ return -1;
+
+ if (!(portstatus & USB_PORT_STAT_CONNECTION)) {
+ delay_time = 0;
+ if (++bogus_discon > 3) { /* 3 discon in a row are taken
+serious */
+ info("port %d: disconnect detected during
+connect-debounce", port+1);
+ return 1;
+ }
+ }
+ else {
+ delay_time += HUB_DEBOUNCE_STEP;
+ bogus_discon = 0;
+ }
+ }
+ return 0;
+}
+
static void usb_hub_port_connect_change(struct usb_device *hub, int port,
- struct usb_port_status *portsts)
+ unsigned char portstatus, unsigned char portchange)
{
struct usb_device *dev;
- unsigned short portstatus, portchange;
unsigned int delay = HUB_SHORT_RESET_TIME;
int i;
char *portstr, *tempstr;
- portstatus = le16_to_cpu(portsts->wPortStatus);
- portchange = le16_to_cpu(portsts->wPortChange);
dbg("port %d, portstatus %x, change %x, %s",
port + 1, portstatus, portchange, portspeed (portstatus));
@@ -621,6 +669,12 @@
return;
}
+ if (usb_hub_port_debounce(hub, port)) {
+ err("connect-debounce failed, port %d disabled", port+1);
+ usb_hub_port_disable(hub, port);
+ return;
+ }
+
/* Some low speed devices have problems with the quick delay, so */
/* be a bit pessimistic with those devices. RHbug #23670 */
if (portstatus & USB_PORT_STAT_LOW_SPEED) {
@@ -751,22 +805,18 @@
}
for (i = 0; i < hub->descriptor->bNbrPorts; i++) {
- struct usb_port_status portsts;
unsigned short portstatus, portchange;
- ret = usb_get_port_status(dev, i + 1, &portsts);
+ ret = usb_hub_port_status(dev, i, &portstatus, &portchange);
if (ret < 0) {
err("get_port_status failed (err = %d)", ret);
continue;
}
- portstatus = le16_to_cpu(portsts.wPortStatus);
- portchange = le16_to_cpu(portsts.wPortChange);
-
if (portchange & USB_PORT_STAT_C_CONNECTION) {
dbg("port %d connection change", i + 1);
- usb_hub_port_connect_change(dev, i, &portsts);
+ usb_hub_port_connect_change(dev, i, portstatus,
+portchange);
} else if (portchange & USB_PORT_STAT_C_ENABLE) {
dbg("port %d enable change, status %x", i + 1,
portstatus);
usb_clear_port_feature(dev, i + 1,
USB_PORT_FEAT_C_ENABLE);
@@ -780,7 +830,7 @@
(portstatus & USB_PORT_STAT_CONNECTION) &&
(dev->children[i])) {
err("already running port %i disabled by hub
(EMI?), re-enabling...",
i + 1);
- usb_hub_port_connect_change(dev, i, &portsts);
+ usb_hub_port_connect_change(dev, i,
+portstatus, portchange);
}
}
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
