Greg: The current mechanism for keeping track of which endpoints in a device are halted is both flawed and unnecessary. It's flawed because devices are free to change the endpoint status whenever they want without telling us, and it's unnecessary because an URB submitted for a halted endpoint will quickly receive an error indication.
This patch removes the code associated with tracking halts: the halted[] member of usb_device, the usb_endpoint_halt, usb_endpoint_halted, and usb_endpoint_running macros, all the places where they are used or checked, and the places in the host controller drivers where they get set. This is part of my ongoing program for cleaning up usbcore. Please apply. Alan Stern Signed-off-by: Alan Stern <[EMAIL PROTECTED]> diff -Nru a/drivers/isdn/hisax/st5481_usb.c b/drivers/isdn/hisax/st5481_usb.c --- a/drivers/isdn/hisax/st5481_usb.c Thu Jun 24 16:44:15 2004 +++ b/drivers/isdn/hisax/st5481_usb.c Thu Jun 24 16:44:15 2004 @@ -143,9 +143,6 @@ if (ctrl_msg->dr.bRequest == USB_REQ_CLEAR_FEATURE) { /* Special case handling for pipe reset */ le16_to_cpus(&ctrl_msg->dr.wIndex); - usb_endpoint_running(adapter->usb_dev, - ctrl_msg->dr.wIndex & ~USB_DIR_IN, - (ctrl_msg->dr.wIndex & USB_DIR_IN) == 0); /* toggle is reset on clear */ usb_settoggle(adapter->usb_dev, diff -Nru a/drivers/usb/core/devices.c b/drivers/usb/core/devices.c --- a/drivers/usb/core/devices.c Thu Jun 24 16:44:16 2004 +++ b/drivers/usb/core/devices.c Thu Jun 24 16:44:16 2004 @@ -283,9 +283,8 @@ /* TBD: * 0. TBDs - * 1. marking active config and ifaces (code lists all, but should mark + * 1. marking active interface altsettings (code lists all, but should mark * which ones are active, if any) - * 2. add <halted> status to each endpoint line */ static char *usb_dump_config_descriptor(char *start, char *end, const struct usb_config_descriptor *desc, int active) diff -Nru a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c --- a/drivers/usb/core/hcd.c Thu Jun 24 16:44:16 2004 +++ b/drivers/usb/core/hcd.c Thu Jun 24 16:44:16 2004 @@ -1311,13 +1311,10 @@ rescan: /* (re)block new requests, as best we can */ - if (endpoint & USB_DIR_IN) { - usb_endpoint_halt (udev, epnum, 0); + if (endpoint & USB_DIR_IN) udev->epmaxpacketin [epnum] = 0; - } else { - usb_endpoint_halt (udev, epnum, 1); + else udev->epmaxpacketout [epnum] = 0; - } /* then kill any current requests */ spin_lock (&hcd_data_lock); diff -Nru a/drivers/usb/core/hcd.h b/drivers/usb/core/hcd.h --- a/drivers/usb/core/hcd.h Thu Jun 24 16:44:16 2004 +++ b/drivers/usb/core/hcd.h Thu Jun 24 16:44:16 2004 @@ -367,8 +367,6 @@ extern int usb_find_interface_driver (struct usb_device *dev, struct usb_interface *interface); -#define usb_endpoint_halt(dev, ep, out) ((dev)->halted[out] |= (1 << (ep))) - #define usb_endpoint_out(ep_dir) (!((ep_dir) & USB_DIR_IN)) /* diff -Nru a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c --- a/drivers/usb/core/hub.c Thu Jun 24 16:44:15 2004 +++ b/drivers/usb/core/hub.c Thu Jun 24 16:44:16 2004 @@ -1451,8 +1451,6 @@ != udev->descriptor.bMaxPacketSize0)) { usb_disable_endpoint(udev, 0 + USB_DIR_IN); usb_disable_endpoint(udev, 0 + USB_DIR_OUT); - usb_endpoint_running(udev, 0, 1); - usb_endpoint_running(udev, 0, 0); udev->epmaxpacketin [0] = udev->descriptor.bMaxPacketSize0; udev->epmaxpacketout[0] = udev->descriptor.bMaxPacketSize0; } @@ -2057,8 +2055,6 @@ * Other endpoints will be handled by re-enumeration. */ usb_disable_endpoint(udev, 0); usb_disable_endpoint(udev, 0 + USB_DIR_IN); - usb_endpoint_running(udev, 0, 0); - usb_endpoint_running(udev, 0, 1); ret = hub_port_init(parent, udev, port); if (ret < 0) @@ -2091,7 +2087,7 @@ struct usb_interface *intf = udev->actconfig->interface[i]; struct usb_interface_descriptor *desc; - /* set_interface resets host side toggle and halt status even + /* set_interface resets host side toggle even * for altsetting zero. the interface may have no driver. */ desc = &intf->cur_altsetting->desc; diff -Nru a/drivers/usb/core/message.c b/drivers/usb/core/message.c --- a/drivers/usb/core/message.c Thu Jun 24 16:44:15 2004 +++ b/drivers/usb/core/message.c Thu Jun 24 16:44:15 2004 @@ -738,9 +738,8 @@ * the copy in usb-storage, for as long as we need two copies. */ - /* toggle was reset by the clear, then ep was reactivated */ + /* toggle was reset by the clear */ usb_settoggle(dev, usb_pipeendpoint(pipe), usb_pipeout(pipe), 0); - usb_endpoint_running(dev, usb_pipeendpoint(pipe), usb_pipeout(pipe)); return 0; } @@ -754,9 +753,8 @@ * Deallocates hcd/hardware state for this endpoint ... and nukes all * pending urbs. * - * If the HCD hasn't registered a disable() function, this marks the - * endpoint as halted and sets its maxpacket size to 0 to prevent - * further submissions. + * If the HCD hasn't registered a disable() function, this sets the + * endpoint's maxpacket size to 0 to prevent further submissions. */ void usb_disable_endpoint(struct usb_device *dev, unsigned int epaddr) { @@ -765,13 +763,10 @@ else { unsigned int epnum = epaddr & USB_ENDPOINT_NUMBER_MASK; - if (usb_endpoint_out(epaddr)) { - usb_endpoint_halt(dev, epnum, 1); + if (usb_endpoint_out(epaddr)) dev->epmaxpacketout[epnum] = 0; - } else { - usb_endpoint_halt(dev, epnum, 0); + else dev->epmaxpacketin[epnum] = 0; - } } } @@ -814,7 +809,6 @@ usb_disable_endpoint(dev, i + USB_DIR_IN); } dev->toggle[0] = dev->toggle[1] = 0; - dev->halted[0] = dev->halted[1] = 0; /* getting rid of interfaces will disconnect * any drivers bound to them (a key side effect) @@ -850,9 +844,8 @@ * @dev: the device whose interface is being enabled * @epd: pointer to the endpoint descriptor * - * Marks the endpoint as running, resets its toggle, and stores - * its maxpacket value. For control endpoints, both the input - * and output sides are handled. + * Resets the endpoint toggle and stores its maxpacket value. + * For control endpoints, both the input and output sides are handled. */ void usb_enable_endpoint(struct usb_device *dev, struct usb_endpoint_descriptor *epd) @@ -864,12 +857,10 @@ USB_ENDPOINT_XFER_CONTROL); if (usb_endpoint_out(epaddr) || is_control) { - usb_endpoint_running(dev, epnum, 1); usb_settoggle(dev, epnum, 1, 0); dev->epmaxpacketout[epnum] = maxsize; } if (!usb_endpoint_out(epaddr) || is_control) { - usb_endpoint_running(dev, epnum, 0); usb_settoggle(dev, epnum, 0, 0); dev->epmaxpacketin[epnum] = maxsize; } @@ -1052,7 +1043,6 @@ } dev->toggle[0] = dev->toggle[1] = 0; - dev->halted[0] = dev->halted[1] = 0; /* re-init hc/hcd interface/endpoint state */ for (i = 0; i < config->desc.bNumInterfaces; i++) { diff -Nru a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c --- a/drivers/usb/core/urb.c Thu Jun 24 16:44:16 2004 +++ b/drivers/usb/core/urb.c Thu Jun 24 16:44:16 2004 @@ -256,13 +256,6 @@ if (!usb_pipecontrol (pipe) && dev->state < USB_STATE_CONFIGURED) return -ENODEV; - /* (actually HCDs may need to duplicate this, endpoint might yet - * stall due to queued bulk/intr transactions that complete after - * we check) - */ - if (usb_endpoint_halted (dev, usb_pipeendpoint (pipe), is_out)) - return -EPIPE; - /* FIXME there should be a sharable lock protecting us against * config/altsetting changes and disconnects, kicking in here. * (here == before maxpacket, and eventually endpoint type, diff -Nru a/drivers/usb/host/ehci-q.c b/drivers/usb/host/ehci-q.c --- a/drivers/usb/host/ehci-q.c Thu Jun 24 16:44:16 2004 +++ b/drivers/usb/host/ehci-q.c Thu Jun 24 16:44:16 2004 @@ -153,17 +153,9 @@ usb_pipein (urb->pipe) ? "in" : "out", token, urb->status); - /* stall indicates some recovery action is needed */ - if (urb->status == -EPIPE) { - int pipe = urb->pipe; - - if (!usb_pipecontrol (pipe)) - usb_endpoint_halt (urb->dev, - usb_pipeendpoint (pipe), - usb_pipeout (pipe)); - /* if async CSPLIT failed, try cleaning out the TT buffer */ - } else if (urb->dev->tt && !usb_pipeint (urb->pipe) + if (urb->status != -EPIPE + && urb->dev->tt && !usb_pipeint (urb->pipe) && ((token & QTD_STS_MMF) != 0 || QTD_CERR(token) == 0) && (!ehci_is_ARC(ehci) diff -Nru a/drivers/usb/host/hc_simple.c b/drivers/usb/host/hc_simple.c --- a/drivers/usb/host/hc_simple.c Thu Jun 24 16:44:15 2004 +++ b/drivers/usb/host/hc_simple.c Thu Jun 24 16:44:15 2004 @@ -146,11 +146,6 @@ if (!urb->dev || !urb->dev->bus || urb->hcpriv) return -EINVAL; - if (usb_endpoint_halted - (urb->dev, usb_pipeendpoint (pipe), usb_pipeout (pipe))) { - printk ("hci_submit_urb: endpoint_halted\n"); - return -EPIPE; - } hci = (hci_t *) urb->dev->bus->hcpriv; /* a request to the virtual root hub */ diff -Nru a/drivers/usb/host/ohci-q.c b/drivers/usb/host/ohci-q.c --- a/drivers/usb/host/ohci-q.c Thu Jun 24 16:44:16 2004 +++ b/drivers/usb/host/ohci-q.c Thu Jun 24 16:44:16 2004 @@ -751,12 +751,6 @@ cc = TD_CC_GET (tdINFO); - /* control endpoints only have soft stalls */ - if (type != PIPE_CONTROL && cc == TD_CC_STALL) - usb_endpoint_halt (urb->dev, - usb_pipeendpoint (urb->pipe), - usb_pipeout (urb->pipe)); - /* update packet status if needed (short is normally ok) */ if (cc == TD_DATAUNDERRUN && !(urb->transfer_flags & URB_SHORT_NOT_OK)) diff -Nru a/drivers/usb/host/uhci-hcd.c b/drivers/usb/host/uhci-hcd.c --- a/drivers/usb/host/uhci-hcd.c Thu Jun 24 16:44:16 2004 +++ b/drivers/usb/host/uhci-hcd.c Thu Jun 24 16:44:16 2004 @@ -1120,10 +1120,6 @@ td_error: ret = uhci_map_status(status, uhci_packetout(td_token(td))); - if (ret == -EPIPE) - /* endpoint has stalled - mark it halted */ - usb_endpoint_halt(urb->dev, uhci_endpoint(td_token(td)), - uhci_packetout(td_token(td))); err: /* diff -Nru a/drivers/usb/image/microtek.c b/drivers/usb/image/microtek.c --- a/drivers/usb/image/microtek.c Thu Jun 24 16:44:16 2004 +++ b/drivers/usb/image/microtek.c Thu Jun 24 16:44:16 2004 @@ -214,8 +214,8 @@ #ifdef MTS_DO_DEBUG static inline void mts_debug_dump(struct mts_desc* desc) { - MTS_DEBUG("desc at 0x%x: halted = %02x%02x, toggle = %02x%02x\n", - (int)desc,(int)desc->usb_dev->halted[1],(int)desc->usb_dev->halted[0], + MTS_DEBUG("desc at 0x%x: toggle = %02x%02x\n", + (int)desc, (int)desc->usb_dev->toggle[1],(int)desc->usb_dev->toggle[0] ); MTS_DEBUG("ep_out=%x ep_response=%x ep_image=%x\n", diff -Nru a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c --- a/drivers/usb/storage/transport.c Thu Jun 24 16:44:16 2004 +++ b/drivers/usb/storage/transport.c Thu Jun 24 16:44:16 2004 @@ -260,9 +260,7 @@ USB_ENDPOINT_HALT, endp, NULL, 0, 3*HZ); - /* reset the toggles and endpoint flags */ - usb_endpoint_running(us->pusb_dev, usb_pipeendpoint(pipe), - usb_pipeout(pipe)); + /* reset the endpoint toggle */ usb_settoggle(us->pusb_dev, usb_pipeendpoint(pipe), usb_pipeout(pipe), 0); diff -Nru a/include/linux/usb.h b/include/linux/usb.h --- a/include/linux/usb.h Thu Jun 24 16:44:16 2004 +++ b/include/linux/usb.h Thu Jun 24 16:44:16 2004 @@ -312,8 +312,6 @@ struct semaphore serialize; unsigned int toggle[2]; /* one bit for each endpoint ([0] = IN, [1] = OUT) */ - unsigned int halted[2]; /* endpoint halts; one bit per endpoint # & direction; */ - /* [0] = IN, [1] = OUT */ int epmaxpacketin[16]; /* INput endpoint specific maximums */ int epmaxpacketout[16]; /* OUTput endpoint specific maximums */ @@ -1102,10 +1100,6 @@ #define usb_gettoggle(dev, ep, out) (((dev)->toggle[out] >> (ep)) & 1) #define usb_dotoggle(dev, ep, out) ((dev)->toggle[out] ^= (1 << (ep))) #define usb_settoggle(dev, ep, out, bit) ((dev)->toggle[out] = ((dev)->toggle[out] & ~(1 << (ep))) | ((bit) << (ep))) - -/* Endpoint halt control/status ... likewise USE WITH CAUTION */ -#define usb_endpoint_running(dev, ep, out) ((dev)->halted[out] &= ~(1 << (ep))) -#define usb_endpoint_halted(dev, ep, out) ((dev)->halted[out] & (1 << (ep))) static inline unsigned int __create_pipe(struct usb_device *dev, unsigned int endpoint) ------------------------------------------------------- This SF.Net email sponsored by Black Hat Briefings & Training. Attend Black Hat Briefings & Training, Las Vegas July 24-29 - digital self defense, top technical experts, no vendor pitches, unmatched networking opportunities. Visit www.blackhat.com _______________________________________________ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel