Date: Fri, 7 Aug 2009 12:20:33 GMT From: Hans Petter Selasky <hselasky@FreeBSD.org> To: Perforce Change Reviews <perforce@FreeBSD.org> Subject: PERFORCE change 167087 for review Message-ID: <200908071220.n77CKXhP016250@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=167087 Change 167087 by hselasky@hselasky_laptop001 on 2009/08/07 12:20:27 USB CORE: - newbus lock cleanup Affected files ... .. //depot/projects/usb/src/sys/dev/usb/controller/usb_controller.c#28 edit .. //depot/projects/usb/src/sys/dev/usb/usb_dev.c#35 edit .. //depot/projects/usb/src/sys/dev/usb/usb_device.c#47 edit .. //depot/projects/usb/src/sys/dev/usb/usb_device.h#29 edit .. //depot/projects/usb/src/sys/dev/usb/usb_handle_request.c#20 edit .. //depot/projects/usb/src/sys/dev/usb/usb_hub.c#30 edit Differences ... ==== //depot/projects/usb/src/sys/dev/usb/controller/usb_controller.c#28 (text+ko) ==== @@ -67,7 +67,6 @@ static device_detach_t usb_detach; static void usb_attach_sub(device_t, struct usb_bus *); -static void usb_post_init(void *); /* static variables */ @@ -84,8 +83,6 @@ SYSCTL_INT(_hw_usb, OID_AUTO, no_boot_wait, CTLFLAG_RDTUN, &usb_no_boot_wait, 0, "No device enumerate waiting at boot."); -static uint8_t usb_post_init_called = 0; - static devclass_t usb_devclass; static device_method_t usb_methods[] = { @@ -142,10 +139,8 @@ bus->bus_roothold = root_mount_hold(device_get_nameunit(dev)); } - if (usb_post_init_called) { - usb_attach_sub(dev, bus); - usb_needs_explore(bus, 1); - } + usb_attach_sub(dev, bus); + return (0); /* return success */ } @@ -269,10 +264,10 @@ device_set_softc(dev, NULL); USB_BUS_UNLOCK(bus); + /* detach children first */ newbus_xlock(); - - /* detach children first */ bus_generic_detach(dev); + newbus_xunlock(); /* * Free USB Root device, but not any sub-devices, hence they @@ -281,7 +276,6 @@ usb_free_device(udev, USB_UNCFG_FLAG_FREE_EP0); - newbus_xunlock(); USB_BUS_LOCK(bus); /* clear bdev variable last */ bus->bdev = NULL; @@ -356,7 +350,6 @@ } USB_BUS_UNLOCK(bus); - newbus_xlock(); /* default power_mask value */ bus->hw_power_state = @@ -389,7 +382,6 @@ err = USB_ERR_NOMEM; } - newbus_xunlock(); USB_BUS_LOCK(bus); if (err) { @@ -407,17 +399,18 @@ /*------------------------------------------------------------------------* * usb_attach_sub * - * This function creates a thread which runs the USB attach code. It - * is factored out, hence it can be called at two different places in - * time. During bootup this function is called from - * "usb_post_init". During hot-plug it is called directly from the - * "usb_attach()" method. + * This function creates a thread which runs the USB attach code. *------------------------------------------------------------------------*/ static void usb_attach_sub(device_t dev, struct usb_bus *bus) { const char *pname = device_get_nameunit(dev); + newbus_xlock(); + if (usb_devclass_ptr == NULL) + usb_devclass_ptr = devclass_find("usbus"); + newbus_xunlock(); + /* Initialise USB process messages */ bus->explore_msg[0].hdr.pm_callback = &usb_bus_explore; bus->explore_msg[0].bus = bus; @@ -460,52 +453,12 @@ /* ignore */ } USB_BUS_UNLOCK(bus); - } -} -/*------------------------------------------------------------------------* - * usb_post_init - * - * This function is called to attach all USB busses that were found - * during bootup. - *------------------------------------------------------------------------*/ -static void -usb_post_init(void *arg) -{ - struct usb_bus *bus; - devclass_t dc; - device_t dev; - int max; - int n; - - newbus_xlock(); - - usb_devclass_ptr = devclass_find("usbus"); - - dc = usb_devclass_ptr; - if (dc) { - max = devclass_get_maxunit(dc) + 1; - for (n = 0; n != max; n++) { - dev = devclass_get_device(dc, n); - if (dev && device_is_attached(dev)) { - bus = device_get_ivars(dev); - if (bus) - usb_attach_sub(dev, bus); - } - } - } else { - DPRINTFN(0, "no devclass\n"); + /* Do initial explore */ + usb_needs_explore(bus, 1); } - usb_post_init_called = 1; - - /* explore all USB busses in parallell */ - - usb_needs_explore_all(); - - newbus_xunlock(); } -SYSINIT(usb_post_init, SI_SUB_KICK_SCHEDULER, SI_ORDER_ANY, usb_post_init, NULL); SYSUNINIT(usb_bus_unload, SI_SUB_KLD, SI_ORDER_ANY, usb_bus_unload, NULL); /*------------------------------------------------------------------------* ==== //depot/projects/usb/src/sys/dev/usb/usb_dev.c#35 (text+ko) ==== @@ -217,7 +217,7 @@ * We need to grab the sx-lock before grabbing the * FIFO refs to avoid deadlock at detach! */ - sx_xlock(cpd->udev->default_sx + 1); + usbd_enum_lock(cpd->udev); mtx_lock(&usb_ref_lock); @@ -275,14 +275,12 @@ } mtx_unlock(&usb_ref_lock); - if (crd->is_uref) { - mtx_lock(&Giant); /* XXX */ - } return (0); error: if (crd->is_uref) { - sx_unlock(cpd->udev->default_sx + 1); + usbd_enum_unlock(cpd->udev); + if (--(cpd->udev->refcount) == 0) { cv_signal(cpd->udev->default_cv + 1); } @@ -334,10 +332,9 @@ DPRINTFN(2, "cpd=%p is_uref=%d\n", cpd, crd->is_uref); - if (crd->is_uref) { - mtx_unlock(&Giant); /* XXX */ - sx_unlock(cpd->udev->default_sx + 1); - } + if (crd->is_uref) + usbd_enum_unlock(cpd->udev); + mtx_lock(&usb_ref_lock); if (crd->is_read) { if (--(crd->rxfifo->refcount) == 0) { @@ -1040,16 +1037,11 @@ * Performance optimisation: We try to check for IOCTL's that * don't need the USB reference first. Then we grab the USB * reference if we need it! - * Note that some ioctl_post handlers would need to run with the - * newbus lock held. It cannot be acquired later because it can - * introduce a LOR, so acquire it here. */ - newbus_xlock(); err = usb_ref_device(cpd, &refs, 0 /* no uref */ ); - if (err) { - newbus_xunlock(); + if (err) return (ENXIO); - } + fflags = cpd->fflags; f = NULL; /* set default value */ @@ -1081,7 +1073,6 @@ } done: usb_unref_device(cpd, &refs); - newbus_xunlock(); return (err); } ==== //depot/projects/usb/src/sys/dev/usb/usb_device.c#47 (text+ko) ==== @@ -402,11 +402,11 @@ uint8_t do_unlock; /* automatic locking */ - if (sx_xlocked(udev->default_sx + 1)) { + if (usbd_enum_is_locked(udev)) { do_unlock = 0; } else { do_unlock = 1; - sx_xlock(udev->default_sx + 1); + usbd_enum_lock(udev); } /* detach all interface drivers */ @@ -442,9 +442,8 @@ udev->curr_config_no = USB_UNCONFIG_NO; udev->curr_config_index = USB_UNCONFIG_INDEX; - if (do_unlock) { - sx_unlock(udev->default_sx + 1); - } + if (do_unlock) + usbd_enum_unlock(udev); } /*------------------------------------------------------------------------* @@ -472,11 +471,11 @@ DPRINTFN(6, "udev=%p index=%d\n", udev, index); /* automatic locking */ - if (sx_xlocked(udev->default_sx + 1)) { + if (usbd_enum_is_locked(udev)) { do_unlock = 0; } else { do_unlock = 1; - sx_xlock(udev->default_sx + 1); + usbd_enum_lock(udev); } usb_unconfigure(udev, USB_UNCFG_FLAG_FREE_SUBDEV); @@ -585,9 +584,8 @@ if (err) { usb_unconfigure(udev, USB_UNCFG_FLAG_FREE_SUBDEV); } - if (do_unlock) { - sx_unlock(udev->default_sx + 1); - } + if (do_unlock) + usbd_enum_unlock(udev); return (err); } @@ -823,11 +821,11 @@ uint8_t do_unlock; /* automatic locking */ - if (sx_xlocked(udev->default_sx + 1)) { + if (usbd_enum_is_locked(udev)) { do_unlock = 0; } else { do_unlock = 1; - sx_xlock(udev->default_sx + 1); + usbd_enum_lock(udev); } if (iface == NULL) { err = USB_ERR_INVAL; @@ -863,9 +861,9 @@ iface->idesc->bAlternateSetting); done: - if (do_unlock) { - sx_unlock(udev->default_sx + 1); - } + if (do_unlock) + usbd_enum_unlock(udev); + return (err); } @@ -1230,11 +1228,11 @@ return (USB_ERR_INVAL); } /* automatic locking */ - if (sx_xlocked(udev->default_sx + 1)) { + if (usbd_enum_is_locked(udev)) { do_unlock = 0; } else { do_unlock = 1; - sx_xlock(udev->default_sx + 1); + usbd_enum_lock(udev); } if (udev->curr_config_index == USB_UNCONFIG_INDEX) { @@ -1315,9 +1313,9 @@ } } done: - if (do_unlock) { - sx_unlock(udev->default_sx + 1); - } + if (do_unlock) + usbd_enum_unlock(udev); + return (0); } @@ -2448,3 +2446,32 @@ { return (udev->state > USB_STATE_DETACHED); } + +/* The following function locks enumerating the given USB device. */ + +void +usbd_enum_lock(struct usb_device *udev) +{ + newbus_xlock(); + sx_xlock(udev->default_sx + 1); +} + +/* The following function unlocks enumerating the given USB device. */ + +void +usbd_enum_unlock(struct usb_device *udev) +{ + sx_xunlock(udev->default_sx + 1); + newbus_xunlock(); +} + +/* + * The following function checks the enumerating lock for the given + * USB device. + */ + +uint8_t +usbd_enum_is_locked(struct usb_device *udev) +{ + return (sx_xlocked(udev->default_sx + 1)); +} ==== //depot/projects/usb/src/sys/dev/usb/usb_device.h#29 (text+ko) ==== @@ -211,5 +211,8 @@ struct usb_endpoint *usb_endpoint_foreach(struct usb_device *udev, struct usb_endpoint *ep); void usb_set_device_state(struct usb_device *udev, enum usb_dev_state state); +void usbd_enum_lock(struct usb_device *); +void usbd_enum_unlock(struct usb_device *); +uint8_t usbd_enum_is_locked(struct usb_device *); #endif /* _USB_DEVICE_H_ */ ==== //depot/projects/usb/src/sys/dev/usb/usb_handle_request.c#20 (text+ko) ==== @@ -152,8 +152,8 @@ * attach: */ USB_XFER_UNLOCK(xfer); - newbus_xlock(); - sx_xlock(udev->default_sx + 1); + + usbd_enum_lock(udev); if (conf_no == USB_UNCONFIG_NO) { conf_no = USB_UNCONFIG_INDEX; @@ -176,8 +176,7 @@ goto done; } done: - sx_unlock(udev->default_sx + 1); - newbus_xunlock(); + usbd_enum_unlock(udev); USB_XFER_LOCK(xfer); return (err); } @@ -190,19 +189,19 @@ usb_error_t err = 0; /* automatic locking */ - if (sx_xlocked(udev->default_sx + 1)) { + if (usbd_enum_is_locked(udev)) { do_unlock = 0; } else { do_unlock = 1; - sx_xlock(udev->default_sx + 1); + usbd_enum_lock(udev); } if (alt_index >= usbd_get_no_alts(udev->cdesc, iface->idesc)) err = USB_ERR_INVAL; - if (do_unlock) { - sx_unlock(udev->default_sx + 1); - } + if (do_unlock) + usbd_enum_unlock(udev); + return (err); } @@ -236,8 +235,8 @@ * attach: */ USB_XFER_UNLOCK(xfer); - newbus_xlock(); - sx_xlock(udev->default_sx + 1); + + usbd_enum_lock(udev); error = ENXIO; @@ -353,20 +352,17 @@ goto tr_stalled; } tr_valid: - sx_unlock(udev->default_sx + 1); - newbus_xunlock(); + usbd_enum_unlock(udev); USB_XFER_LOCK(xfer); return (0); tr_short: - sx_unlock(udev->default_sx + 1); - newbus_xunlock(); + usbd_enum_unlock(udev); USB_XFER_LOCK(xfer); return (USB_ERR_SHORT_XFER); tr_stalled: - sx_unlock(udev->default_sx + 1); - newbus_xunlock(); + usbd_enum_unlock(udev); USB_XFER_LOCK(xfer); return (USB_ERR_STALLED); } ==== //depot/projects/usb/src/sys/dev/usb/usb_hub.c#30 (text+ko) ==== @@ -96,6 +96,7 @@ struct uhub_softc { struct uhub_current_state sc_st;/* current state */ device_t sc_dev; /* base device */ + struct mtx sc_mtx; /* our mutex */ struct usb_device *sc_udev; /* USB device */ struct usb_xfer *sc_xfer[UHUB_N_TRANSFER]; /* interrupt xfer */ uint8_t sc_flags; @@ -234,10 +235,8 @@ if (child->driver_added_refcount != refcount) { child->driver_added_refcount = refcount; - newbus_xlock(); err = usb_probe_and_attach(child, USB_IFACE_INDEX_ANY); - newbus_xunlock(); if (err) { goto done; } @@ -320,11 +319,9 @@ /* detach any existing devices */ if (child) { - newbus_xlock(); usb_free_device(child, USB_UNCFG_FLAG_FREE_SUBDEV | USB_UNCFG_FLAG_FREE_EP0); - newbus_xunlock(); child = NULL; } /* get fresh status */ @@ -432,10 +429,8 @@ mode = USB_MODE_HOST; /* need to create a new child */ - newbus_xlock(); child = usb_alloc_device(sc->sc_dev, udev->bus, udev, udev->depth + 1, portno - 1, portno, speed, mode); - newbus_xunlock(); if (child == NULL) { DPRINTFN(0, "could not allocate new device!\n"); goto error; @@ -444,11 +439,9 @@ error: if (child) { - newbus_xlock(); usb_free_device(child, USB_UNCFG_FLAG_FREE_SUBDEV | USB_UNCFG_FLAG_FREE_EP0); - newbus_xunlock(); child = NULL; } if (err == 0) { @@ -698,6 +691,8 @@ sc->sc_udev = udev; sc->sc_dev = dev; + mtx_init(&sc->sc_mtx, "USB HUB mutex", NULL, MTX_DEF); + snprintf(sc->sc_name, sizeof(sc->sc_name), "%s", device_get_nameunit(dev)); @@ -781,7 +776,7 @@ } else { /* normal HUB */ err = usbd_transfer_setup(udev, &iface_index, sc->sc_xfer, - uhub_config, UHUB_N_TRANSFER, sc, &Giant); + uhub_config, UHUB_N_TRANSFER, sc, &sc->sc_mtx); } if (err) { DPRINTFN(0, "cannot setup interrupt transfer, " @@ -857,9 +852,9 @@ /* Start the interrupt endpoint, if any */ if (sc->sc_xfer[0] != NULL) { - USB_XFER_LOCK(sc->sc_xfer[0]); + mtx_lock(&sc->sc_mtx); usbd_transfer_start(sc->sc_xfer[0]); - USB_XFER_UNLOCK(sc->sc_xfer[0]); + mtx_unlock(&sc->sc_mtx); } /* Enable automatic power save on all USB HUBs */ @@ -875,6 +870,9 @@ free(udev->hub, M_USBDEV); udev->hub = NULL; } + + mtx_destroy(&sc->sc_mtx); + return (ENXIO); } @@ -915,6 +913,9 @@ free(hub, M_USBDEV); sc->sc_udev->hub = NULL; + + mtx_destroy(&sc->sc_mtx); + return (0); } @@ -1778,13 +1779,13 @@ /* always update hardware power! */ (bus->methods->set_hw_power) (bus); } - newbus_xlock(); - sx_xlock(udev->default_sx + 1); + + usbd_enum_lock(udev); /* notify all sub-devices about resume */ err = usb_suspend_resume(udev, 0); - sx_unlock(udev->default_sx + 1); - newbus_xunlock(); + + usbd_enum_unlock(udev); /* check if peer has wakeup capability */ if (usb_peer_can_wakeup(udev)) { @@ -1850,13 +1851,12 @@ } } - newbus_xlock(); - sx_xlock(udev->default_sx + 1); + usbd_enum_lock(udev); /* notify all sub-devices about suspend */ err = usb_suspend_resume(udev, 1); - sx_unlock(udev->default_sx + 1); - newbus_xunlock(); + + usbd_enum_unlock(udev); if (usb_peer_can_wakeup(udev)) { /* allow device to do remote wakeup */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200908071220.n77CKXhP016250>