From ad9aa12975ddc0a0eee76d88196c5a57795eaaaa Mon Sep 17 00:00:00 2001 From: hselasky Date: Wed, 30 Oct 2013 06:18:54 +0000 Subject: [PATCH] MFC r257206: Fix a deadlock when trying to power off a USB device. The deadlock happens because the code in question is trying to modify the parent USB port registers outside the USB explore thread. git-svn-id: svn://svn.freebsd.org/base/stable/9@257373 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f --- sys/dev/usb/usb_dev.c | 2 +- sys/dev/usb/usb_device.h | 3 +++ sys/dev/usb/usb_generic.c | 19 +++++++------------ sys/dev/usb/usb_hub.c | 38 +++++++++++++++++++++++++++++++++----- 4 files changed, 44 insertions(+), 18 deletions(-) diff --git a/sys/dev/usb/usb_dev.c b/sys/dev/usb/usb_dev.c index b5627d8b3..759f9a113 100644 --- a/sys/dev/usb/usb_dev.c +++ b/sys/dev/usb/usb_dev.c @@ -1095,7 +1095,7 @@ usb_ioctl(struct cdev *dev, u_long cmd, caddr_t addr, int fflag, struct thread* /* Wait for re-enumeration, if any */ - while (f->udev->re_enumerate_wait != 0) { + while (f->udev->re_enumerate_wait != USB_RE_ENUM_DONE) { usb_unref_device(cpd, &refs); diff --git a/sys/dev/usb/usb_device.h b/sys/dev/usb/usb_device.h index 8e13e3de1..d4e3602ba 100644 --- a/sys/dev/usb/usb_device.h +++ b/sys/dev/usb/usb_device.h @@ -230,6 +230,9 @@ struct usb_device { uint8_t driver_added_refcount; /* our driver added generation count */ uint8_t power_mode; /* see USB_POWER_XXX */ uint8_t re_enumerate_wait; /* set if re-enum. is in progress */ +#define USB_RE_ENUM_DONE 0 +#define USB_RE_ENUM_START 1 +#define USB_RE_ENUM_PWR_OFF 2 uint8_t ifaces_max; /* number of interfaces present */ uint8_t endpoints_max; /* number of endpoints present */ diff --git a/sys/dev/usb/usb_generic.c b/sys/dev/usb/usb_generic.c index c4b3fbd5e..8c2e84f61 100644 --- a/sys/dev/usb/usb_generic.c +++ b/sys/dev/usb/usb_generic.c @@ -1749,16 +1749,11 @@ ugen_set_power_mode(struct usb_fifo *f, int mode) switch (mode) { case USB_POWER_MODE_OFF: - /* get the device unconfigured */ - err = ugen_set_config(f, USB_UNCONFIG_INDEX); - if (err) { - DPRINTFN(0, "Could not unconfigure " - "device (ignored)\n"); + if (udev->flags.usb_mode == USB_MODE_HOST && + udev->re_enumerate_wait == USB_RE_ENUM_DONE) { + udev->re_enumerate_wait = USB_RE_ENUM_PWR_OFF; } - - /* clear port enable */ - err = usbd_req_clear_port_feature(udev->parent_hub, - NULL, udev->port_no, UHF_PORT_ENABLE); + /* set power mode will wake up the explore thread */ break; case USB_POWER_MODE_ON: @@ -1806,9 +1801,9 @@ ugen_set_power_mode(struct usb_fifo *f, int mode) /* if we are powered off we need to re-enumerate first */ if (old_mode == USB_POWER_MODE_OFF) { - if (udev->flags.usb_mode == USB_MODE_HOST) { - if (udev->re_enumerate_wait == 0) - udev->re_enumerate_wait = 1; + if (udev->flags.usb_mode == USB_MODE_HOST && + udev->re_enumerate_wait == USB_RE_ENUM_DONE) { + udev->re_enumerate_wait = USB_RE_ENUM_START; } /* set power mode will wake up the explore thread */ } diff --git a/sys/dev/usb/usb_hub.c b/sys/dev/usb/usb_hub.c index 3ce1fdd42..059a01cfb 100644 --- a/sys/dev/usb/usb_hub.c +++ b/sys/dev/usb/usb_hub.c @@ -242,7 +242,8 @@ uhub_explore_sub(struct uhub_softc *sc, struct usb_port *up) uint8_t do_unlock; do_unlock = usbd_enum_lock(child); - if (child->re_enumerate_wait) { + switch (child->re_enumerate_wait) { + case USB_RE_ENUM_START: err = usbd_set_config_index(child, USB_UNCONFIG_INDEX); if (err != 0) { @@ -257,8 +258,33 @@ uhub_explore_sub(struct uhub_softc *sc, struct usb_port *up) err = usb_probe_and_attach(child, USB_IFACE_INDEX_ANY); } - child->re_enumerate_wait = 0; + child->re_enumerate_wait = USB_RE_ENUM_DONE; err = 0; + break; + + case USB_RE_ENUM_PWR_OFF: + /* get the device unconfigured */ + err = usbd_set_config_index(child, + USB_UNCONFIG_INDEX); + if (err) { + DPRINTFN(0, "Could not unconfigure " + "device (ignored)\n"); + } + + /* clear port enable */ + err = usbd_req_clear_port_feature(child->parent_hub, + NULL, child->port_no, UHF_PORT_ENABLE); + if (err) { + DPRINTFN(0, "Could not disable port " + "(ignored)\n"); + } + child->re_enumerate_wait = USB_RE_ENUM_DONE; + err = 0; + break; + + default: + child->re_enumerate_wait = USB_RE_ENUM_DONE; + break; } if (do_unlock) usbd_enum_unlock(child); @@ -2073,7 +2099,7 @@ usb_peer_should_wakeup(struct usb_device *udev) return (((udev->power_mode == USB_POWER_MODE_ON) && (udev->flags.usb_mode == USB_MODE_HOST)) || (udev->driver_added_refcount != udev->bus->driver_added_refcount) || - (udev->re_enumerate_wait != 0) || + (udev->re_enumerate_wait != USB_RE_ENUM_DONE) || (udev->pwr_save.type_refs[UE_ISOCHRONOUS] != 0) || (udev->pwr_save.write_refs != 0) || ((udev->pwr_save.read_refs != 0) && @@ -2489,6 +2515,8 @@ usbd_set_power_mode(struct usb_device *udev, uint8_t power_mode) #if USB_HAVE_POWERD usb_bus_power_update(udev->bus); +#else + usb_needs_explore(udev->bus, 0 /* no probe */ ); #endif } @@ -2527,8 +2555,8 @@ usbd_filter_power_mode(struct usb_device *udev, uint8_t power_mode) void usbd_start_re_enumerate(struct usb_device *udev) { - if (udev->re_enumerate_wait == 0) { - udev->re_enumerate_wait = 1; + if (udev->re_enumerate_wait == USB_RE_ENUM_DONE) { + udev->re_enumerate_wait = USB_RE_ENUM_START; usb_needs_explore(udev->bus, 0); } } -- 2.45.0