From owner-p4-projects@FreeBSD.ORG Sun Sep 23 14:49:32 2007 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 58AB316A421; Sun, 23 Sep 2007 14:49:32 +0000 (UTC) Delivered-To: perforce@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id EBBFF16A418 for ; Sun, 23 Sep 2007 14:49:31 +0000 (UTC) (envelope-from hselasky@FreeBSD.org) Received: from repoman.freebsd.org (repoman.freebsd.org [IPv6:2001:4f8:fff6::29]) by mx1.freebsd.org (Postfix) with ESMTP id CC7FF13C458 for ; Sun, 23 Sep 2007 14:49:31 +0000 (UTC) (envelope-from hselasky@FreeBSD.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.14.1/8.14.1) with ESMTP id l8NEnVXj087571 for ; Sun, 23 Sep 2007 14:49:31 GMT (envelope-from hselasky@FreeBSD.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.14.1/8.14.1/Submit) id l8NEnVSl087562 for perforce@freebsd.org; Sun, 23 Sep 2007 14:49:31 GMT (envelope-from hselasky@FreeBSD.org) Date: Sun, 23 Sep 2007 14:49:31 GMT Message-Id: <200709231449.l8NEnVSl087562@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to hselasky@FreeBSD.org using -f From: Hans Petter Selasky To: Perforce Change Reviews Cc: Subject: PERFORCE change 126734 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 23 Sep 2007 14:49:32 -0000 http://perforce.freebsd.org/chv.cgi?CH=126734 Change 126734 by hselasky@hselasky_laptop001 on 2007/09/23 14:49:06 FYI; The comments follow the P4 diff from top to bottom. - renamed "sc_hub" into "sc_udev" to better match the analogy of the other USB device drivers - convert kernel USB flags (USBD_XXX) into a bitfield (scripted) - all USB BULK/ISOC/INTR -transfers must setup "xfer->frlengths[]" before calling "usbd_start_hardware()". Else the actual length of the previous transfer will be used for transfer length of the next USB transfer. - new internal function "uhub_port_to_sub_device()" which gets the USB device structure connected to a USB port. - factored out code into "uhub_explore_sub()" - USB port numbers and indexes are now 8-bit hence there cannot be more than 255 ports according to the USB specification. - passing a mutex to "usbd_do_request_flags()" and all "usbreq_xxx()" functions is now mandatory. - remove redundant passing of "struct usbd_port *" to "usbd_new_device()" - "struct usbd_device" now has a pointer directly to the parent HUB, "paraent_hub" - be consequent on differating between port index and port number which differ by 1 unit. Affected files ... .. //depot/projects/usb/src/sys/dev/usb/uhub.c#16 edit Differences ... ==== //depot/projects/usb/src/sys/dev/usb/uhub.c#16 (text+ko) ==== @@ -70,14 +70,14 @@ struct uhub_softc { device_t sc_dev; /* base device */ - struct usbd_device *sc_hub; /* USB device */ + struct usbd_device *sc_udev; /* USB device */ struct usbd_xfer *sc_xfer[2]; /* interrupt xfer */ uint8_t sc_flags; #define UHUB_FLAG_RUNNING 0x01 #define UHUB_FLAG_INTR_STALL 0x02 uint8_t sc_name[32]; }; -#define UHUB_PROTO(sc) ((sc)->sc_hub->ddesc.bDeviceProtocol) +#define UHUB_PROTO(sc) ((sc)->sc_udev->ddesc.bDeviceProtocol) #define UHUB_IS_HIGH_SPEED(sc) (UHUB_PROTO(sc) != UDPROTO_FSHUB) #define UHUB_IS_SINGLE_TT(sc) (UHUB_PROTO(sc) == UDPROTO_HSHUBSTT) @@ -101,7 +101,7 @@ .endpoint = UE_ADDR_ANY, .direction = UE_DIR_ANY, .timeout = 0, - .flags = (USBD_PIPE_BOF|USBD_SHORT_XFER_OK), + .flags = { .pipe_bof = 1, .short_xfer_ok = 1, }, .bufsize = 0, /* use wMaxPacketSize */ .callback = &uhub_intr_callback, .interval = UHUB_INTR_INTERVAL, @@ -113,7 +113,7 @@ .direction = UE_DIR_ANY, .timeout = 1000, /* 1 second */ .interval = 50, /* 50ms */ - .flags = 0, + .flags = { }, .bufsize = sizeof(usb_device_request_t), .callback = &uhub_intr_clear_stall_callback, }, @@ -179,12 +179,13 @@ * event handler thread that we need * to be explored again: */ - usb_needs_explore(sc->sc_hub); + usb_needs_explore(sc->sc_udev); tr_setup: if (sc->sc_flags & UHUB_FLAG_INTR_STALL) { usbd_transfer_start(sc->sc_xfer[1]); } else { + xfer->frlengths[0] = xfer->max_data_length; usbd_start_hardware(xfer); } return; @@ -198,17 +199,65 @@ return; } +static struct usbd_device * +uhub_port_to_sub_device(struct usbd_device *udev, struct usbd_port *up) +{ + if ((udev == NULL) || (up == NULL)) { + /* be NULL safe */ + return NULL; + } + + if (up->device_addr == USB_START_ADDR) { + /* nothing to do */ + return NULL; + } + + return udev->bus->devices[up->device_addr]; +} + static usbd_status +uhub_explore_sub(device_t dev, struct usbd_device *udev, struct usbd_port *up) +{ + struct usbd_device *child; + uint8_t refcount = usb_driver_added_refcount; + usbd_status err = 0; + + child = uhub_port_to_sub_device(udev, up); + + if (child == NULL) { + /* nothing to do */ + return 0; + } + + /* check if probe and attach should be done */ + + if (child->driver_added_refcount != refcount) { + child->driver_added_refcount = refcount; + err = usbd_probe_and_attach(dev, child); + } + + /* if a HUB becomes present, do a recursive HUB explore */ + + if (child->hub) { + (child->hub->explore)(child); + } + + return err; +} + +static usbd_status uhub_explore(struct usbd_device *udev) { usb_hub_descriptor_t *hd = &udev->hub->hubdesc; struct uhub_softc *sc = udev->hub->hubsoftc; + struct usbd_device *child; struct usbd_port *up; usbd_status err; - uint16_t port; uint16_t change; uint16_t status; + uint8_t portno; uint8_t speed; + uint8_t x; DPRINTF(sc, 10, "udev=%p addr=%d\n", udev, udev->address); @@ -223,10 +272,12 @@ return (USBD_TOO_DEEP); } - for(port = 1; port <= hd->bNbrPorts; port++) + for (x = 0; x < hd->bNbrPorts; x++) { - up = &udev->hub->ports[port-1]; - err = usbreq_get_port_status(udev, port, &up->status); + up = udev->hub->ports + x; + portno = x + 1; + err = usbreq_get_port_status + (udev, &usb_global_lock, &up->status, portno); if (err) { DPRINTF(sc, 0, "get port status failed, " @@ -236,11 +287,12 @@ status = UGETW(up->status.wPortStatus); change = UGETW(up->status.wPortChange); DPRINTF(sc, 3, "port %d status 0x%04x 0x%04x\n", - port, status, change); + portno, status, change); if(change & UPS_C_PORT_ENABLED) { DPRINTF(sc, 0, "C_PORT_ENABLED 0x%x\n", change); - usbreq_clear_port_feature(udev, port, UHF_C_PORT_ENABLE); + usbreq_clear_port_feature + (udev, &usb_global_lock, portno, UHF_C_PORT_ENABLE); if(change & UPS_C_CONNECT_STATUS) { /* ignore the port error @@ -250,7 +302,7 @@ else if(status & UPS_PORT_ENABLED) { DPRINTF(sc, -1, "illegal enable change, " - "port %d\n", port); + "port %d\n", portno); } else { @@ -258,48 +310,35 @@ if(up->restartcnt) /* no message first time */ { DPRINTF(sc, -1, "port error, restarting " - "port %d\n", port); + "port %d\n", portno); } if(up->restartcnt++ < USBD_RESTART_MAX) { goto disconnect; } else { DPRINTF(sc, -1, "port error, giving up " - "port %d\n", port); + "port %d\n", portno); } } } if(!(change & UPS_C_CONNECT_STATUS)) { DPRINTF(sc, 3, "port=%d !C_CONNECT_" - "STATUS\n", port); + "STATUS\n", portno); + /* no status change, just do recursive explore */ - if(up->device != NULL) - { - if(up->device->hub != NULL) - { - (up->device->hub->explore)(up->device); - } - else - { - /* allow drivers to be hot-plugged */ - - if(up->last_refcount != usb_driver_added_refcount) - { - usbd_probe_and_attach - (sc->sc_dev, port, up); - } - } - } + err = uhub_explore_sub(sc->sc_dev, udev, up); continue; } /* we have a connect status change, handle it */ DPRINTF(sc, 0, "status change hub=%d port=%d\n", - udev->address, port); - usbreq_clear_port_feature(udev, port, UHF_C_PORT_CONNECTION); - /*usbreq_clear_port_feature(udev, port, UHF_C_PORT_ENABLE);*/ + udev->address, portno); + usbreq_clear_port_feature + (udev, &usb_global_lock, portno, UHF_C_PORT_CONNECTION); + /* usbreq_clear_port_feature + (udev, &usb_global_lock, portno, 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 @@ -308,20 +347,21 @@ * the disconnect. */ disconnect: - if(up->device != NULL) - { + child = uhub_port_to_sub_device(udev, up); + if (child) { /* disconnected */ DPRINTF(sc, 0, "device addr=%d disappeared " - "on port %d\n", up->device->address, port); - usbd_free_device(up, 1); - usbreq_clear_port_feature(udev, port, - UHF_C_PORT_CONNECTION); + "on port %d\n", child->address, + portno); + usbd_free_device(child, 1); + usbreq_clear_port_feature + (udev, &usb_global_lock, portno, UHF_C_PORT_CONNECTION); } if(!(status & UPS_CURRENT_CONNECT_STATUS)) { /* nothing connected, just ignore it */ DPRINTF(sc, 3, "port=%d !CURRENT_CONNECT_STATUS\n", - port); + portno); continue; } @@ -330,22 +370,27 @@ if(!(status & UPS_PORT_POWER)) { DPRINTF(sc, -1, "strange, connected port %d " - "has no power\n", port); + "has no power\n", portno); } /* wait for maximum device power up time */ usbd_delay_ms(udev, USB_PORT_POWERUP_DELAY); /* reset port, which implies enabling it */ - if(usbreq_reset_port(udev, port, &up->status)) - { + err = usbreq_reset_port + (udev, &usb_global_lock, &up->status, portno); + + if (err) { DPRINTF(sc, -1, "port %d reset " - "failed\n", port); + "failed, error=%s\n", + portno, usbd_errstr(err)); continue; } /* get port status again, it might have changed during reset */ - err = usbreq_get_port_status(udev, port, &up->status); + err = usbreq_get_port_status + (udev, &usb_global_lock, &up->status, portno); + if(err) { DPRINTF(sc, 0, "get port status failed, " @@ -358,7 +403,7 @@ { /* nothing connected, just ignore it */ DPRINTF(sc, 1, "port %d, device disappeared " - "after reset\n", port); + "after reset\n", portno); continue; } @@ -368,14 +413,18 @@ (status & UPS_LOW_SPEED) ? USB_SPEED_LOW : USB_SPEED_FULL; /* get device info and set its address */ - err = usbd_new_device(sc->sc_dev, udev->bus, - udev->depth + 1, speed, port, up); - if (err) - { + err = usbd_new_device(sc->sc_dev, udev->bus, udev, + udev->depth + 1, speed, x, portno); + if (err == 0) { + err = uhub_explore_sub(sc->sc_dev, udev, up); + } + + if (err) { DPRINTF(sc, -1, "usb_new_device failed, " "error=%s\n", usbd_errstr(err)); /* Avoid addressing problems by disabling. */ - /* usbd_reset_port(udev, port, &up->status); */ + /* usbreq_reset_port + (udev, &usb_global_lock, &up->status, portno); */ /* * The unit refused to accept a new address, or had @@ -383,19 +432,18 @@ * at 0 we have to disable the port instead. */ DPRINTF(sc, -1, "device problem (%s), " - "disabling port %d\n", usbd_errstr(err), port); - usbreq_clear_port_feature(udev, port, UHF_PORT_ENABLE); + "disabling port %d\n", usbd_errstr(err), portno); + usbreq_clear_port_feature + (udev, &usb_global_lock, portno, UHF_PORT_ENABLE); + continue; } - else - { - /* the port set up succeeded, reset error count */ - up->restartcnt = 0; + + /* + * The port setup succeeded, reset error count and + * do recursive explore, if any: + */ + up->restartcnt = 0; - if(up->device->hub) - { - (up->device->hub->explore)(up->device); - } - } } return (USBD_NORMAL_COMPLETION); } @@ -424,20 +472,22 @@ struct uhub_softc *sc = device_get_softc(dev); struct usb_attach_arg *uaa = device_get_ivars(dev); struct usbd_device *udev = uaa->device; + struct usbd_device *parent_hub = udev->parent_hub; struct usbd_hub *hub; usb_device_request_t req; usb_hub_descriptor_t hubdesc; - uint16_t port; - uint16_t nports; - uint16_t removable; uint16_t pwrdly; + uint8_t x; + uint8_t nports; + uint8_t portno; + uint8_t removable; usbd_status err; if (sc == NULL) { return ENOMEM; } - sc->sc_hub = udev; + sc->sc_udev = udev; sc->sc_dev = dev; snprintf(sc->sc_name, sizeof(sc->sc_name), "%s", @@ -460,9 +510,9 @@ "parent->selfpowered=%d\n", udev->depth, udev->self_powered, - udev->powersrc->parent, - udev->powersrc->parent ? - udev->powersrc->parent->self_powered : 0); + parent_hub, + parent_hub ? + parent_hub->self_powered : 0); if(udev->depth > USB_HUB_MAX_DEPTH) { @@ -471,9 +521,8 @@ goto error; } - if(!udev->self_powered && - (udev->powersrc->parent != NULL) && - (!udev->powersrc->parent->self_powered)) + if (!udev->self_powered && parent_hub && + (!parent_hub->self_powered)) { DPRINTF(sc, -1, "bus powered hub connected to " "bus powered hub. HUB ignored!\n"); @@ -490,7 +539,7 @@ USETW(req.wIndex, 0); USETW(req.wLength, USB_HUB_DESCRIPTOR_SIZE); - err = usbd_do_request(udev, &req, &hubdesc); + err = usbd_do_request(udev, &usb_global_lock, &req, &hubdesc); nports = hubdesc.bNbrPorts; @@ -498,7 +547,7 @@ { uint16_t len = (USB_HUB_DESCRIPTOR_SIZE-1) + ((nports+7) / 8); USETW(req.wLength, len); - err = usbd_do_request(udev, &req, &hubdesc); + err = usbd_do_request(udev, &usb_global_lock, &req, &hubdesc); } if(err) @@ -520,16 +569,31 @@ goto error; } - udev->hub = malloc((sizeof(hub[0]) + (sizeof(hub[0].ports[0]) * nports)), - M_USBDEV, M_WAITOK|M_ZERO); + hub = malloc(sizeof(hub[0]) + (sizeof(hub->ports[0]) * nports), + M_USBDEV, M_WAITOK|M_ZERO); - if(udev->hub == NULL) + if (hub == NULL) { goto error; } + udev->hub = hub; + /* init FULL-speed ISOCHRONOUS schedule */ - usbd_fs_isoc_schedule_init_all(udev->hub->fs_isoc_schedule); + usbd_fs_isoc_schedule_init_all(hub->fs_isoc_schedule); + + /* initialize HUB structure */ + hub->hubsoftc = sc; + hub->explore = &uhub_explore; + hub->hubdesc = hubdesc; + hub->hubudev = udev; + + /* if self powered hub, give ports maximum current */ + if (udev->self_powered) { + hub->portpower = USB_MAX_POWER; + } else { + hub->portpower = USB_MIN_POWER; + } /* set up interrupt pipe */ err = usbd_transfer_setup(udev, 0, sc->sc_xfer, @@ -541,12 +605,6 @@ goto error; } - hub = udev->hub; - - hub->hubsoftc = sc; - hub->explore = uhub_explore; - hub->hubdesc = hubdesc; - /* wait with power off for a while */ usbd_delay_ms(udev, USB_POWER_DOWN_TIME); @@ -581,46 +639,37 @@ pwrdly = ((hubdesc.bPwrOn2PwrGood * UHD_PWRON_FACTOR) + USB_EXTRA_POWER_UP_TIME); - for(port = 1; - port <= nports; - port++) - { + for(x = 0; x < nports; x++) { /* set up data structures */ - struct usbd_port *up = &hub->ports[port-1]; - up->device = NULL; - up->parent = udev; - up->portno = port; - - /* self powered hub, give ports maximum current */ - up->power = (udev->self_powered) ? - USB_MAX_POWER : - USB_MIN_POWER ; - + struct usbd_port *up = hub->ports + x; + up->device_addr = USB_START_ADDR; up->restartcnt = 0; + portno = x + 1; /* check if port is removable */ - if(!UHD_NOT_REMOV(&hubdesc, port)) + if(!UHD_NOT_REMOV(&hubdesc, portno)) { removable++; } /* turn the power on */ - err = usbreq_set_port_feature(udev, port, UHF_PORT_POWER); - if(err) - { + err = usbreq_set_port_feature + (udev, &usb_global_lock, portno, UHF_PORT_POWER); + + if (err) { DPRINTF(sc, -1, "port %d power on failed, %s\n", - port, usbd_errstr(err)); + portno, usbd_errstr(err)); } - DPRINTF(sc, 0, "turn on port %d power\n", port); + DPRINTF(sc, 0, "turn on port %d power\n", + portno); /* wait for stable power */ usbd_delay_ms(udev, pwrdly); } device_printf(dev, "%d port%s with %d " - "removable, %s powered\n", - nports, (nports != 1) ? "s" : "", - removable, udev->self_powered ? "self" : "bus"); + "removable, %s powered\n", nports, (nports != 1) ? "s" : "", + removable, udev->self_powered ? "self" : "bus"); /* the usual exploration will finish the setup */ @@ -637,8 +686,7 @@ error: usbd_transfer_unsetup(sc->sc_xfer, 2); - if(udev->hub) - { + if (udev->hub) { free(udev->hub, M_USBDEV); udev->hub = NULL; } @@ -653,10 +701,9 @@ uhub_detach(device_t dev) { struct uhub_softc *sc = device_get_softc(dev); - struct usbd_hub *hub = sc->sc_hub->hub; - struct usbd_port *up; - uint16_t port; - uint16_t nports; + struct usbd_hub *hub = sc->sc_udev->hub; + struct usbd_device *child; + uint8_t x; /* detach all children first */ bus_generic_detach(dev); @@ -666,26 +713,22 @@ return (0); } - nports = hub->hubdesc.bNbrPorts; - for(port = 0; - port < nports; - port++) - { - up = &hub->ports[port]; - if(up->device) - { - /* subdevices are not freed, because - * the caller of uhub_detach() will - * do that - */ - usbd_free_device(up, 0); - } + for (x = 0; x < hub->hubdesc.bNbrPorts; x++) { + + child = uhub_port_to_sub_device(sc->sc_udev, hub->ports + x); + + /* Subdevices are not freed, because + * the caller of uhub_detach() will + * do that. The function we are calling + * is NULL safe. + */ + usbd_free_device(child, 0); } usbd_transfer_unsetup(sc->sc_xfer, 2); free(hub, M_USBDEV); - sc->sc_hub->hub = NULL; + sc->sc_udev->hub = NULL; return (0); } @@ -701,20 +744,18 @@ char *buf, size_t buflen) { struct uhub_softc *sc = device_get_softc(parent); - struct usbd_hub *hub = sc->sc_hub->hub; + struct usbd_hub *hub = sc->sc_udev->hub; struct usbd_device *udev; - uint16_t port; - uint16_t nports; + uint8_t x; + uint8_t nports; uint8_t iface_index; mtx_lock(&usb_global_lock); nports = hub->hubdesc.bNbrPorts; - for(port = 0; - port < nports; - port++) + for (x = 0; x < nports; x++) { - udev = hub->ports[port].device; + udev = uhub_port_to_sub_device(sc->sc_udev, hub->ports + x); if(udev) { device_t * subdev = @@ -754,11 +795,11 @@ if(udev->probed == USBD_PROBED_IFACE_AND_FOUND) { snprintf(buf, buflen, "port=%i interface=%i", - port, iface_index); + x + 1, iface_index); } else { - snprintf(buf, buflen, "port=%i", port); + snprintf(buf, buflen, "port=%i", x + 1); } mtx_unlock(&usb_global_lock); @@ -771,21 +812,19 @@ char *buf, size_t buflen) { struct uhub_softc *sc = device_get_softc(parent); - struct usbd_hub *hub = sc->sc_hub->hub; + struct usbd_hub *hub = sc->sc_udev->hub; struct usbd_interface *iface; struct usbd_device *udev; - uint16_t port; - uint16_t nports; + uint8_t x; + uint8_t nports; uint8_t iface_index; mtx_lock(&usb_global_lock); nports = hub->hubdesc.bNbrPorts; - for(port = 0; - port < nports; - port++) + for (x = 0; x < nports; x++) { - udev = hub->ports[port].device; + udev = uhub_port_to_sub_device(sc->sc_udev, hub->ports + x); if(udev) { device_t * subdev =