On Fri, Oct 31, 2014 at 11:19:22PM +0100, 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?
wunderbar!
> 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 31 Oct 2014 22:00:38 -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)) {
> + 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) {
> @@ -556,6 +565,8 @@ 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;
> @@ -563,6 +574,11 @@ uhub_intr(struct usbd_xfer *xfer, void *
> 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;
>
--
Antoine
