On 31/10/14(Fri) 23:19, Martin Pieuchot wrote: > Device below changes uhub(4) to no longer check *all* the ports status > of *all* the hubs on bus when one of those hubs requests an explore. > > This has the nice effect of reducing the delay/bus traffic when a port > status change is detected which reduces (avoid) some timing related > problems when connection devices. > > To do so, we only query the status of the ports having reported a status > changed in the interrupt. > > I'd appreciate tests on various HC/archs. > > Comments, ok?
Updated diff that correctly deals with devices asking for a reconnect and xhci(4)'s root hub reporting only one port change per interrupt. Index: uhub.c =================================================================== RCS file: /cvs/src/sys/dev/usb/uhub.c,v retrieving revision 1.74 diff -u -p -r1.74 uhub.c --- uhub.c 1 Oct 2014 08:29:01 -0000 1.74 +++ uhub.c 1 Nov 2014 00:21:04 -0000 @@ -57,8 +57,11 @@ struct uhub_softc { struct device sc_dev; /* base device */ struct usbd_device *sc_hub; /* USB device */ struct usbd_pipe *sc_ipipe; /* interrupt pipe */ - u_int8_t *sc_statusbuf; /* per port status buffer */ - size_t sc_statuslen; /* status bufferlen */ + + uint32_t sc_status; /* status from last interrupt */ + uint8_t *sc_statusbuf; /* per port status buffer */ + size_t sc_statuslen; /* status bufferlen */ + u_char sc_running; }; #define UHUB_PROTO(sc) ((sc)->sc_hub->ddesc.bDeviceProtocol) @@ -349,18 +352,24 @@ uhub_explore(struct usbd_device *dev) for (port = 1; port <= dev->hub->nports; port++) { up = &dev->hub->ports[port-1]; - err = usbd_get_port_status(dev, port, &up->status); - if (err) { - DPRINTF("%s: get port %d status failed, error=%s\n", - sc->sc_dev.dv_xname, port, usbd_errstr(err)); - continue; - } - status = UGETW(up->status.wPortStatus); - change = UGETW(up->status.wPortChange); + reconnect = up->reattach; up->reattach = 0; - DPRINTF("%s: port %d status=0x%04x change=0x%04x\n", - sc->sc_dev.dv_xname, port, status, change); + change = 0; + status = 0; + + if ((sc->sc_status & (1 << port)) || reconnect) { + sc->sc_status &= ~(1 << port); + + if (usbd_get_port_status(dev, port, &up->status)) + continue; + + status = UGETW(up->status.wPortStatus); + change = UGETW(up->status.wPortChange); + DPRINTF("%s: port %d status=0x%04x change=0x%04x\n", + sc->sc_dev.dv_xname, port, status, change); + } + if (change & UPS_C_PORT_ENABLED) { usbd_clear_port_feature(dev, port, UHF_C_PORT_ENABLE); if (change & UPS_C_CONNECT_STATUS) { @@ -393,7 +402,7 @@ uhub_explore(struct usbd_device *dev) /* We have a connect status change, handle it. */ usbd_clear_port_feature(dev, port, UHF_C_PORT_CONNECTION); - /*usbd_clear_port_feature(dev, port, UHF_C_PORT_ENABLE);*/ + /* * If there is already a device on the port the change status * must mean that is has disconnected. Looking at the @@ -556,13 +565,21 @@ void uhub_intr(struct usbd_xfer *xfer, void *addr, usbd_status status) { struct uhub_softc *sc = addr; + uint32_t stats; + int i; if (usbd_is_dying(sc->sc_hub)) return; DPRINTF("%s: intr status=%d\n", sc->sc_dev.dv_xname, status); + if (status == USBD_STALLED) usbd_clear_endpoint_stall_async(sc->sc_ipipe); - else if (status == USBD_NORMAL_COMPLETION) + else if (status == USBD_NORMAL_COMPLETION) { + for (i = 0; i < xfer->actlen; i++) + stats |= (uint32_t)(xfer->buffer[i]) << (i * 8); + sc->sc_status |= stats; + usb_needs_explore(sc->sc_hub, 0); + } } Index: usbdivar.h =================================================================== RCS file: /cvs/src/sys/dev/usb/usbdivar.h,v retrieving revision 1.64 diff -u -p -r1.64 usbdivar.h --- usbdivar.h 31 Oct 2014 12:43:33 -0000 1.64 +++ usbdivar.h 31 Oct 2014 21:12:55 -0000 @@ -182,7 +182,7 @@ struct usbd_pipe { struct usbd_xfer { struct usbd_pipe *pipe; void *priv; - void *buffer; + char *buffer; u_int32_t length; u_int32_t actlen; u_int16_t flags;