Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 28 Sep 2009 07:01:54 +0000 (UTC)
From:      Andrew Thompson <thompsa@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r197553 - in head/sys/dev/usb: . controller
Message-ID:  <200909280701.n8S71sBD046019@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: thompsa
Date: Mon Sep 28 07:01:54 2009
New Revision: 197553
URL: http://svn.freebsd.org/changeset/base/197553

Log:
  MFp4 @ 168387
  
  - clean up USB detach logic. There seems to be some problems detaching multiple
    USB HUBs connected in series from the root.
  
  - after this patch the rule is:
    1) Always use device_detach() on the USB HUB first.
    2) Never just device_delete_child() on the USB HUB, because that function
    will traverse to all the device leaves and free them first, and then the USB
    stack will free the devices twice which doesn't work very well.
  
  - make sure the did DMA delay gets set after the timeout has elapsed to make
    logic more clear. There is no functional difference.
  
  Submitted by:	Hans Petter Selasky

Modified:
  head/sys/dev/usb/controller/usb_controller.c
  head/sys/dev/usb/usb_device.c
  head/sys/dev/usb/usb_device.h
  head/sys/dev/usb/usb_hub.c
  head/sys/dev/usb/usb_transfer.c

Modified: head/sys/dev/usb/controller/usb_controller.c
==============================================================================
--- head/sys/dev/usb/controller/usb_controller.c	Mon Sep 28 03:32:35 2009	(r197552)
+++ head/sys/dev/usb/controller/usb_controller.c	Mon Sep 28 07:01:54 2009	(r197553)
@@ -270,11 +270,9 @@ usb_bus_detach(struct usb_proc_msg *pm)
 	mtx_unlock(&Giant);
 
 	/*
-	 * Free USB Root device, but not any sub-devices, hence they
-	 * are freed by the caller of this function:
+	 * Free USB device and all subdevices, if any.
 	 */
-	usb_free_device(udev,
-	    USB_UNCFG_FLAG_FREE_EP0);
+	usb_free_device(udev, 0);
 
 	USB_BUS_LOCK(bus);
 	/* clear bdev variable last */

Modified: head/sys/dev/usb/usb_device.c
==============================================================================
--- head/sys/dev/usb/usb_device.c	Mon Sep 28 03:32:35 2009	(r197552)
+++ head/sys/dev/usb/usb_device.c	Mon Sep 28 07:01:54 2009	(r197553)
@@ -478,7 +478,7 @@ usbd_set_config_index(struct usb_device 
 		usbd_enum_lock(udev);
 	}
 
-	usb_unconfigure(udev, USB_UNCFG_FLAG_FREE_SUBDEV);
+	usb_unconfigure(udev, 0);
 
 	if (index == USB_UNCONFIG_INDEX) {
 		/*
@@ -582,7 +582,7 @@ usbd_set_config_index(struct usb_device 
 done:
 	DPRINTF("error=%s\n", usbd_errstr(err));
 	if (err) {
-		usb_unconfigure(udev, USB_UNCFG_FLAG_FREE_SUBDEV);
+		usb_unconfigure(udev, 0);
 	}
 	if (do_unlock)
 		usbd_enum_unlock(udev);
@@ -989,18 +989,13 @@ usb_detach_device_sub(struct usb_device 
 	device_t dev;
 	int err;
 
-	if (!(flag & USB_UNCFG_FLAG_FREE_SUBDEV)) {
-
-		*ppdev = NULL;
-
-	} else if (*ppdev) {
-
+	dev = *ppdev;
+	if (dev) {
 		/*
 		 * NOTE: It is important to clear "*ppdev" before deleting
 		 * the child due to some device methods being called late
 		 * during the delete process !
 		 */
-		dev = *ppdev;
 		*ppdev = NULL;
 
 		device_printf(dev, "at %s, port %d, addr %d "
@@ -1778,7 +1773,7 @@ repeat_set_config:
 		} else if (usb_test_huawei_autoinst_p(udev, &uaa) == 0) {
 			DPRINTFN(0, "Found Huawei auto-install disk!\n");
 			/* leave device unconfigured */
-			usb_unconfigure(udev, USB_UNCFG_FLAG_FREE_SUBDEV);
+			usb_unconfigure(udev, 0);
 		}
 	} else {
 		err = 0;		/* set success */
@@ -1803,10 +1798,10 @@ repeat_set_config:
 #endif
 done:
 	if (err) {
-		/* free device  */
-		usb_free_device(udev,
-		    USB_UNCFG_FLAG_FREE_SUBDEV |
-		    USB_UNCFG_FLAG_FREE_EP0);
+		/*
+		 * Free USB device and all subdevices, if any.
+		 */
+		usb_free_device(udev, 0);
 		udev = NULL;
 	}
 	return (udev);
@@ -1926,9 +1921,10 @@ usb_cdev_cleanup(void* arg)
 /*------------------------------------------------------------------------*
  *	usb_free_device
  *
- * This function is NULL safe and will free an USB device.
+ * This function is NULL safe and will free an USB device and its
+ * children devices, if any.
  *
- * Flag values, see "USB_UNCFG_FLAG_XXX".
+ * Flag values: Reserved, set to zero.
  *------------------------------------------------------------------------*/
 void
 usb_free_device(struct usb_device *udev, uint8_t flag)
@@ -1982,7 +1978,7 @@ usb_free_device(struct usb_device *udev,
 	}
 
 	/* the following will get the device unconfigured in software */
-	usb_unconfigure(udev, flag);
+	usb_unconfigure(udev, USB_UNCFG_FLAG_FREE_EP0);
 
 	/* unsetup any leftover default USB transfers */
 	usbd_transfer_unsetup(udev->default_xfer, USB_DEFAULT_XFER_MAX);

Modified: head/sys/dev/usb/usb_device.h
==============================================================================
--- head/sys/dev/usb/usb_device.h	Mon Sep 28 03:32:35 2009	(r197552)
+++ head/sys/dev/usb/usb_device.h	Mon Sep 28 07:01:54 2009	(r197553)
@@ -41,7 +41,6 @@ struct usb_device;		/* linux compat */
 /* "usb_unconfigure()" flags */
 
 #define	USB_UNCFG_FLAG_NONE 0x00
-#define	USB_UNCFG_FLAG_FREE_SUBDEV 0x01		/* subdevices are freed */
 #define	USB_UNCFG_FLAG_FREE_EP0	0x02		/* endpoint zero is freed */
 
 struct usb_clear_stall_msg {

Modified: head/sys/dev/usb/usb_hub.c
==============================================================================
--- head/sys/dev/usb/usb_hub.c	Mon Sep 28 03:32:35 2009	(r197552)
+++ head/sys/dev/usb/usb_hub.c	Mon Sep 28 07:01:54 2009	(r197553)
@@ -316,12 +316,13 @@ repeat:
 	if (err) {
 		goto error;
 	}
-	/* detach any existing devices */
+	/* check if there is a child */
 
-	if (child) {
-		usb_free_device(child,
-		    USB_UNCFG_FLAG_FREE_SUBDEV |
-		    USB_UNCFG_FLAG_FREE_EP0);
+	if (child != NULL) {
+		/*
+		 * Free USB device and all subdevices, if any.
+		 */
+		usb_free_device(child, 0);
 		child = NULL;
 	}
 	/* get fresh status */
@@ -438,10 +439,11 @@ repeat:
 	return (0);			/* success */
 
 error:
-	if (child) {
-		usb_free_device(child,
-		    USB_UNCFG_FLAG_FREE_SUBDEV |
-		    USB_UNCFG_FLAG_FREE_EP0);
+	if (child != NULL) {
+		/*
+		 * Free USB device and all subdevices, if any.
+		 */
+		usb_free_device(child, 0);
 		child = NULL;
 	}
 	if (err == 0) {
@@ -888,12 +890,14 @@ uhub_detach(device_t dev)
 	struct usb_device *child;
 	uint8_t x;
 
-	/* detach all children first */
-	bus_generic_detach(dev);
-
 	if (hub == NULL) {		/* must be partially working */
 		return (0);
 	}
+
+	/* Make sure interrupt transfer is gone. */
+	usbd_transfer_unsetup(sc->sc_xfer, UHUB_N_TRANSFER);
+
+	/* Detach all ports */
 	for (x = 0; x != hub->nports; x++) {
 
 		child = usb_bus_port_get_device(sc->sc_udev->bus, hub->ports + x);
@@ -901,16 +905,13 @@ uhub_detach(device_t dev)
 		if (child == NULL) {
 			continue;
 		}
+
 		/*
-		 * Subdevices are not freed, because the caller of
-		 * uhub_detach() will do that.
+		 * Free USB device and all subdevices, if any.
 		 */
-		usb_free_device(child,
-		    USB_UNCFG_FLAG_FREE_EP0);
+		usb_free_device(child, 0);
 	}
 
-	usbd_transfer_unsetup(sc->sc_xfer, UHUB_N_TRANSFER);
-
 	free(hub, M_USBDEV);
 	sc->sc_udev->hub = NULL;
 
@@ -984,10 +985,19 @@ static int
 uhub_child_location_string(device_t parent, device_t child,
     char *buf, size_t buflen)
 {
-	struct uhub_softc *sc = device_get_softc(parent);
-	struct usb_hub *hub = sc->sc_udev->hub;
+	struct uhub_softc *sc;
+	struct usb_hub *hub;
 	struct hub_result res;
 
+	if (!device_is_attached(parent)) {
+		if (buflen)
+			buf[0] = 0;
+		return (0);
+	}
+
+	sc = device_get_softc(parent);
+	hub = sc->sc_udev->hub;
+
 	mtx_lock(&Giant);
 	uhub_find_iface_index(hub, child, &res);
 	if (!res.udev) {
@@ -1009,11 +1019,20 @@ static int
 uhub_child_pnpinfo_string(device_t parent, device_t child,
     char *buf, size_t buflen)
 {
-	struct uhub_softc *sc = device_get_softc(parent);
-	struct usb_hub *hub = sc->sc_udev->hub;
+	struct uhub_softc *sc;
+	struct usb_hub *hub;
 	struct usb_interface *iface;
 	struct hub_result res;
 
+	if (!device_is_attached(parent)) {
+		if (buflen)
+			buf[0] = 0;
+		return (0);
+	}
+
+	sc = device_get_softc(parent);
+	hub = sc->sc_udev->hub;
+
 	mtx_lock(&Giant);
 	uhub_find_iface_index(hub, child, &res);
 	if (!res.udev) {

Modified: head/sys/dev/usb/usb_transfer.c
==============================================================================
--- head/sys/dev/usb/usb_transfer.c	Mon Sep 28 03:32:35 2009	(r197552)
+++ head/sys/dev/usb/usb_transfer.c	Mon Sep 28 07:01:54 2009	(r197553)
@@ -2121,6 +2121,9 @@ usb_dma_delay_done_cb(void *arg)
 
 	DPRINTFN(3, "Completed %p\n", xfer);
 
+	/* only delay once */
+	xfer->flags_int.did_dma_delay = 1;
+
 	/* queue callback for execution, again */
 	usbd_transfer_done(xfer, 0);
 }
@@ -2488,9 +2491,6 @@ usbd_callback_wrapper_sub(struct usb_xfe
 
 		usb_timeout_t temp;
 
-		/* only delay once */
-		xfer->flags_int.did_dma_delay = 1;
-
 		/* we can not cancel this delay */
 		xfer->flags_int.can_cancel_immed = 0;
 



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200909280701.n8S71sBD046019>