Date: Mon, 4 May 2009 12:07:33 GMT From: Hans Petter Selasky <hselasky@FreeBSD.org> To: Perforce Change Reviews <perforce@FreeBSD.org> Subject: PERFORCE change 161551 for review Message-ID: <200905041207.n44C7XXQ040379@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=161551 Change 161551 by hselasky@hselasky_laptop001 on 2009/05/04 12:06:46 USB CORE + USB controller: Revert back some changes done by Andrew Thompson, due to unseen issues. More documentation has been added to prevent similar bugs in the future. Affected files ... .. //depot/projects/usb/src/sys/dev/usb/controller/ehci.c#19 edit .. //depot/projects/usb/src/sys/dev/usb/controller/ohci.c#16 edit .. //depot/projects/usb/src/sys/dev/usb/controller/uhci.c#13 edit .. //depot/projects/usb/src/sys/dev/usb/usb_core.h#14 edit .. //depot/projects/usb/src/sys/dev/usb/usb_device.c#29 edit .. //depot/projects/usb/src/sys/dev/usb/usb_device.h#18 edit .. //depot/projects/usb/src/sys/dev/usb/usb_generic.c#13 edit .. //depot/projects/usb/src/sys/dev/usb/usb_hub.c#16 edit .. //depot/projects/usb/src/sys/dev/usb/usb_revision.h#3 edit .. //depot/projects/usb/src/sys/dev/usb/usb_transfer.c#143 edit Differences ... ==== //depot/projects/usb/src/sys/dev/usb/controller/ehci.c#19 (text+ko) ==== @@ -1964,7 +1964,7 @@ usb2_pc_cpu_flush(qh->page_cache); - if (xfer->xroot->udev->state != USB_STATE_SUSPENDED) { + if (xfer->xroot->udev->flags.self_suspended == 0) { EHCI_APPEND_QH(qh, *qh_last); } } ==== //depot/projects/usb/src/sys/dev/usb/controller/ohci.c#16 (text+ko) ==== @@ -1020,7 +1020,7 @@ * writing the BLF and CLF bits: */ - if (xfer->xroot->udev->state == USB_STATE_SUSPENDED) { + if (xfer->xroot->udev->flags.self_suspended) { /* nothing to do */ } else if (xfer->pipe->methods == &ohci_device_bulk_methods) { ohci_softc_t *sc = OHCI_BUS2SC(xfer->xroot->bus); @@ -1589,7 +1589,7 @@ ed->ed_headp = td->td_self; - if (xfer->xroot->udev->state != USB_STATE_SUSPENDED) { + if (xfer->xroot->udev->flags.self_suspended == 0) { /* the append function will flush the endpoint descriptor */ OHCI_APPEND_QH(ed, *ed_last); ==== //depot/projects/usb/src/sys/dev/usb/controller/uhci.c#13 (text+ko) ==== @@ -1921,7 +1921,7 @@ qh->e_next = td; qh->qh_e_next = td->td_self; - if (xfer->xroot->udev->state != USB_STATE_SUSPENDED) { + if (xfer->xroot->udev->flags.self_suspended == 0) { UHCI_APPEND_QH(qh, sc->sc_bulk_p_last); uhci_add_loop(sc); xfer->flags_int.bandwidth_reclaimed = 1; @@ -1982,7 +1982,7 @@ * NOTE: some devices choke on bandwidth- reclamation for control * transfers */ - if (xfer->xroot->udev->state != USB_STATE_SUSPENDED) { + if (xfer->xroot->udev->flags.self_suspended == 0) { if (xfer->xroot->udev->speed == USB_SPEED_LOW) { UHCI_APPEND_QH(qh, sc->sc_ls_ctl_p_last); } else { @@ -2071,11 +2071,9 @@ qh->e_next = td; qh->qh_e_next = td->td_self; - if (xfer->xroot->udev->state != USB_STATE_SUSPENDED) { - + if (xfer->xroot->udev->flags.self_suspended == 0) { /* enter QHs into the controller data structures */ UHCI_APPEND_QH(qh, sc->sc_intr_p_last[xfer->qh_pos]); - } else { usb2_pc_cpu_flush(qh->page_cache); } ==== //depot/projects/usb/src/sys/dev/usb/usb_core.h#14 (text+ko) ==== @@ -515,7 +515,7 @@ /* prototypes */ const char *usb2_errstr(usb2_error_t error); -const char *usb2_statestr(enum usb_dev_state state); +const char *usb2_statestr(enum usb2_dev_state state); struct usb2_config_descriptor *usb2_get_config_descriptor( struct usb2_device *udev); struct usb2_device_descriptor *usb2_get_device_descriptor( @@ -553,6 +553,6 @@ uint8_t usb2_get_bus_index(struct usb2_device *udev); uint8_t usb2_get_device_index(struct usb2_device *udev); void usb2_set_power_mode(struct usb2_device *udev, uint8_t power_mode); -int usb2_device_attached(struct usb2_device *udev); +uint8_t usb2_device_attached(struct usb2_device *udev); #endif /* _USB2_CORE_H_ */ ==== //depot/projects/usb/src/sys/dev/usb/usb_device.c#29 (text+ko) ==== @@ -96,15 +96,12 @@ [USB_STATE_POWERED] = "POWERED", [USB_STATE_ADDRESSED] = "ADDRESSED", [USB_STATE_CONFIGURED] = "CONFIGURED", - [USB_STATE_SUSPENDED] = "SUSPENDED" }; const char * -usb2_statestr(enum usb_dev_state state) +usb2_statestr(enum usb2_dev_state state) { - KASSERT(state < USB_STATE_MAX, ("invalid udev state")); - - return (statestr[state]); + return ((state < USB_STATE_MAX) ? statestr[state] : "UNKNOWN"); } /*------------------------------------------------------------------------* @@ -999,7 +996,7 @@ udev->port_no, udev->address); if (device_is_attached(dev)) { - if (udev->state == USB_STATE_SUSPENDED) { + if (udev->flags.peer_suspended) { err = DEVICE_RESUME(dev); if (err) { device_printf(dev, "Resume failed!\n"); @@ -1139,7 +1136,7 @@ uaa->temp_dev = NULL; device_set_ivars(iface->subdev, NULL); - if (udev->state == USB_STATE_SUSPENDED) { + if (udev->flags.peer_suspended) { err = DEVICE_SUSPEND(iface->subdev); if (err) device_printf(iface->subdev, "Suspend failed\n"); @@ -1360,12 +1357,12 @@ USB_BUS_LOCK(udev->bus); /* filter the suspend events */ - if ((udev->state == USB_STATE_SUSPENDED && do_suspend) || - (udev->state != USB_STATE_SUSPENDED && !do_suspend)) { + if (udev->flags.peer_suspended == do_suspend) { USB_BUS_UNLOCK(udev->bus); /* nothing to do */ return (0); } + udev->flags.peer_suspended = do_suspend; USB_BUS_UNLOCK(udev->bus); /* do the suspend or resume */ @@ -2466,7 +2463,7 @@ } void -usb2_set_device_state(struct usb2_device *udev, enum usb_dev_state state) +usb2_set_device_state(struct usb2_device *udev, enum usb2_dev_state state) { KASSERT(state < USB_STATE_MAX, ("invalid udev state")); @@ -2476,7 +2473,7 @@ udev->state = state; } -int +uint8_t usb2_device_attached(struct usb2_device *udev) { return (udev->state > USB_STATE_DETACHED); ==== //depot/projects/usb/src/sys/dev/usb/usb_device.h#18 (text+ko) ==== @@ -89,6 +89,14 @@ * strings */ uint8_t remote_wakeup:1; /* set if remote wakeup is enabled */ uint8_t uq_bus_powered:1; /* set if BUS powered quirk is present */ + + /* + * NOTE: Although the flags below will reach the same value + * over time, but the instant values may differ, and + * consequently the flags cannot be merged into one! + */ + uint8_t peer_suspended:1; /* set if peer is suspended */ + uint8_t self_suspended:1; /* set if self is suspended */ }; /* @@ -139,7 +147,7 @@ #endif usb2_ticks_t plugtime; /* copy of "ticks" */ - enum usb_dev_state state; + enum usb2_dev_state state; uint16_t refcount; #define USB_DEV_REF_MAX 0xffff @@ -207,6 +215,6 @@ uint8_t usb2_peer_can_wakeup(struct usb2_device *udev); struct usb2_pipe *usb2_pipe_foreach(struct usb2_device *udev, struct usb2_pipe *pipe); void usb2_set_device_state(struct usb2_device *udev, - enum usb_dev_state state); + enum usb2_dev_state state); #endif /* _USB2_DEVICE_H_ */ ==== //depot/projects/usb/src/sys/dev/usb/usb_generic.c#13 (text+ko) ==== @@ -823,11 +823,7 @@ di->udi_speed = udev->speed; di->udi_mode = udev->flags.usb2_mode; di->udi_power_mode = udev->power_mode; - if (udev->state == USB_STATE_SUSPENDED) { - di->udi_suspended = 1; - } else { - di->udi_suspended = 0; - } + di->udi_suspended = udev->flags.peer_suspended; hub = udev->parent_hub; if (hub) { ==== //depot/projects/usb/src/sys/dev/usb/usb_hub.c#16 (text+ko) ==== @@ -544,7 +544,8 @@ if (udev->depth > USB_HUB_MAX_DEPTH) { return (USB_ERR_TOO_DEEP); } - if (udev->state == USB_STATE_SUSPENDED) { + + if (udev->flags.self_suspended) { /* need to wait until the child signals resume */ DPRINTF("Device is suspended!\n"); return (0); @@ -1518,7 +1519,7 @@ udev->pwr_save.write_refs += val; } - if (udev->state == USB_STATE_SUSPENDED) + if (udev->flags.self_suspended) needs_explore = (udev->pwr_save.write_refs != 0) || ((udev->pwr_save.read_refs != 0) && @@ -1600,7 +1601,7 @@ (rem_wakeup == 0))) { /* check if we are suspended */ - if (udev->state == USB_STATE_SUSPENDED) { + if (udev->flags.self_suspended != 0) { USB_BUS_UNLOCK(bus); usb2_dev_resume_peer(udev); USB_BUS_LOCK(bus); @@ -1608,7 +1609,7 @@ } else if (temp >= limit) { /* check if we are not suspended */ - if (udev->state != USB_STATE_SUSPENDED) { + if (udev->flags.self_suspended == 0) { USB_BUS_UNLOCK(bus); usb2_dev_suspend_peer(udev); USB_BUS_LOCK(bus); @@ -1647,7 +1648,7 @@ if (temp < mintime) mintime = temp; - if (udev->state != USB_STATE_SUSPENDED) { + if (udev->flags.self_suspended == 0) { type_refs[0] += udev->pwr_save.type_refs[0]; type_refs[1] += udev->pwr_save.type_refs[1]; type_refs[2] += udev->pwr_save.type_refs[2]; @@ -1697,7 +1698,7 @@ return; /* check if already resumed */ - if (udev->state != USB_STATE_SUSPENDED) + if (udev->flags.self_suspended == 0) return; /* we need a parent HUB to do resume */ @@ -1737,7 +1738,7 @@ } USB_BUS_LOCK(bus); /* set that this device is now resumed */ - usb2_set_device_state(udev, USB_STATE_CONFIGURED); + udev->flags.self_suspended = 0; #if USB_HAVE_POWERD /* make sure that we don't go into suspend right away */ udev->pwr_save.last_xfer_time = ticks; @@ -1797,7 +1798,7 @@ return; /* check if already suspended */ - if (udev->state == USB_STATE_SUSPENDED) + if (udev->flags.self_suspended) return; /* we need a parent HUB to do suspend */ @@ -1819,7 +1820,7 @@ if (child == NULL) continue; - if (child->state == USB_STATE_SUSPENDED) + if (child->flags.self_suspended) continue; DPRINTFN(1, "Port %u is busy on the HUB!\n", x + 1); @@ -1846,7 +1847,7 @@ * Set that this device is suspended. This variable must be set * before calling USB controller suspend callbacks. */ - usb2_set_device_state(udev, USB_STATE_SUSPENDED); + udev->flags.self_suspended = 1; USB_BUS_UNLOCK(udev->bus); if (udev->bus->methods->device_suspend != NULL) { ==== //depot/projects/usb/src/sys/dev/usb/usb_revision.h#3 (text+ko) ==== @@ -28,9 +28,9 @@ #define _USB2_REVISION_H_ /* - * The "USB_SPEED" macro defines all the supported USB speeds. + * The "USB_SPEED" macros defines all the supported USB speeds. */ -enum { +enum usb2_speed { USB_SPEED_VARIABLE, USB_SPEED_LOW, USB_SPEED_FULL, @@ -40,9 +40,9 @@ }; /* - * The "USB_REV" macro defines all the supported USB revisions. + * The "USB_REV" macros defines all the supported USB revisions. */ -enum { +enum usb2_revision { USB_REV_UNKNOWN, USB_REV_PRE_1_0, USB_REV_1_0, @@ -54,24 +54,23 @@ }; /* - * The "USB_MODE" macro defines all the supported USB modes. + * The "USB_MODE" macros defines all the supported USB modes. */ -enum { +enum usb2_mode { USB_MODE_HOST, USB_MODE_DEVICE, USB_MODE_MAX }; /* - * The "USB_MODE" macro defines all the supported device states. + * The "USB_MODE" macros defines all the supported device states. */ -enum usb_dev_state { +enum usb2_dev_state { USB_STATE_DETACHED, USB_STATE_ATTACHED, USB_STATE_POWERED, USB_STATE_ADDRESSED, USB_STATE_CONFIGURED, - USB_STATE_SUSPENDED, USB_STATE_MAX, }; #endif /* _USB2_REVISION_H_ */ ==== //depot/projects/usb/src/sys/dev/usb/usb_transfer.c#143 (text+ko) ==== @@ -1376,14 +1376,6 @@ xfer, xfer->pipe, xfer->nframes, USB_GET_DATA_ISREAD(xfer) ? "read" : "write"); - /* Check if the device is still alive */ - if (info->udev->state < USB_STATE_POWERED) { - USB_BUS_LOCK(bus); - usb2_transfer_done(xfer, USB_ERR_NOT_CONFIGURED); - USB_BUS_UNLOCK(bus); - return; - } - #if USB_DEBUG if (USB_DEBUG_VAR > 0) { USB_BUS_LOCK(bus); @@ -1444,8 +1436,15 @@ /* clear any previous errors */ xfer->error = 0; + /* Check if the device is still alive */ + if (info->udev->state < USB_STATE_POWERED) { + USB_BUS_LOCK(bus); + usb2_transfer_done(xfer, USB_ERR_NOT_CONFIGURED); + USB_BUS_UNLOCK(bus); + return; + } + /* sanity check */ - if (xfer->nframes == 0) { if (xfer->flags.stall_pipe) { /*
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200905041207.n44C7XXQ040379>