PERFORCE change 126734 for review

Hans Petter Selasky hselasky at FreeBSD.org
Sun Sep 23 07:49:32 PDT 2007


http://perforce.freebsd.org/chv.cgi?CH=126734

Change 126734 by hselasky at 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 = 


More information about the p4-projects mailing list