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>