From ddeac4e75f2527a340f9dc655bde49bb2429b39b Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Thu, 15 Jan 2009 17:03:33 -0500 Subject: USB: fix toggle mismatch in disable_endpoint paths This patch (as1200) finishes some fixes that were left incomplete by an earlier patch. Although nobody has addressed this issue in the past, it turns out that we need to distinguish between two different modes of disabling and enabling endpoints. In one mode only the data structures in usbcore are affected, and in the other mode the host controller and device hardware states are affected as well. The earlier patch added an extra argument to the routines in the enable_endpoint pathways to reflect this difference. This patch adds corresponding arguments to the disable_endpoint pathways. Without this change, the endpoint toggle state can get out of sync between the host and the device. The exact mechanism depends on the details of the host controller (whether or not it stores its own copy of the toggle values). Signed-off-by: Alan Stern Reported-by: Dan Streetman Tested-by: Dan Streetman Cc: stable Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/message.c | 40 ++++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 16 deletions(-) (limited to 'drivers/usb/core/message.c') diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c index de51667dd64..31fb204f44c 100644 --- a/drivers/usb/core/message.c +++ b/drivers/usb/core/message.c @@ -1039,14 +1039,15 @@ static void remove_intf_ep_devs(struct usb_interface *intf) * @dev: the device whose endpoint is being disabled * @epaddr: the endpoint's address. Endpoint number for output, * endpoint number + USB_DIR_IN for input + * @reset_hardware: flag to erase any endpoint state stored in the + * controller hardware * - * Deallocates hcd/hardware state for this endpoint ... and nukes all - * pending urbs. - * - * If the HCD hasn't registered a disable() function, this sets the - * endpoint's maxpacket size to 0 to prevent further submissions. + * Disables the endpoint for URB submission and nukes all pending URBs. + * If @reset_hardware is set then also deallocates hcd/hardware state + * for the endpoint. */ -void usb_disable_endpoint(struct usb_device *dev, unsigned int epaddr) +void usb_disable_endpoint(struct usb_device *dev, unsigned int epaddr, + bool reset_hardware) { unsigned int epnum = epaddr & USB_ENDPOINT_NUMBER_MASK; struct usb_host_endpoint *ep; @@ -1056,15 +1057,18 @@ void usb_disable_endpoint(struct usb_device *dev, unsigned int epaddr) if (usb_endpoint_out(epaddr)) { ep = dev->ep_out[epnum]; - dev->ep_out[epnum] = NULL; + if (reset_hardware) + dev->ep_out[epnum] = NULL; } else { ep = dev->ep_in[epnum]; - dev->ep_in[epnum] = NULL; + if (reset_hardware) + dev->ep_in[epnum] = NULL; } if (ep) { ep->enabled = 0; usb_hcd_flush_endpoint(dev, ep); - usb_hcd_disable_endpoint(dev, ep); + if (reset_hardware) + usb_hcd_disable_endpoint(dev, ep); } } @@ -1072,17 +1076,21 @@ void usb_disable_endpoint(struct usb_device *dev, unsigned int epaddr) * usb_disable_interface -- Disable all endpoints for an interface * @dev: the device whose interface is being disabled * @intf: pointer to the interface descriptor + * @reset_hardware: flag to erase any endpoint state stored in the + * controller hardware * * Disables all the endpoints for the interface's current altsetting. */ -void usb_disable_interface(struct usb_device *dev, struct usb_interface *intf) +void usb_disable_interface(struct usb_device *dev, struct usb_interface *intf, + bool reset_hardware) { struct usb_host_interface *alt = intf->cur_altsetting; int i; for (i = 0; i < alt->desc.bNumEndpoints; ++i) { usb_disable_endpoint(dev, - alt->endpoint[i].desc.bEndpointAddress); + alt->endpoint[i].desc.bEndpointAddress, + reset_hardware); } } @@ -1103,8 +1111,8 @@ void usb_disable_device(struct usb_device *dev, int skip_ep0) dev_dbg(&dev->dev, "%s nuking %s URBs\n", __func__, skip_ep0 ? "non-ep0" : "all"); for (i = skip_ep0; i < 16; ++i) { - usb_disable_endpoint(dev, i); - usb_disable_endpoint(dev, i + USB_DIR_IN); + usb_disable_endpoint(dev, i, true); + usb_disable_endpoint(dev, i + USB_DIR_IN, true); } dev->toggle[0] = dev->toggle[1] = 0; @@ -1274,7 +1282,7 @@ int usb_set_interface(struct usb_device *dev, int interface, int alternate) remove_intf_ep_devs(iface); usb_remove_sysfs_intf_files(iface); } - usb_disable_interface(dev, iface); + usb_disable_interface(dev, iface, true); iface->cur_altsetting = alt; @@ -1353,8 +1361,8 @@ int usb_reset_configuration(struct usb_device *dev) */ for (i = 1; i < 16; ++i) { - usb_disable_endpoint(dev, i); - usb_disable_endpoint(dev, i + USB_DIR_IN); + usb_disable_endpoint(dev, i, true); + usb_disable_endpoint(dev, i + USB_DIR_IN, true); } config = dev->actconfig; -- cgit v1.2.3-70-g09d2 From 67f5a4ba9741fcef3f4db3509ad03565d9e33af2 Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Fri, 20 Feb 2009 16:33:08 -0500 Subject: USB: usb_get_string should check the descriptor type This patch (as1218) fixes a problem with a radio-control joystick used in the "walkera 4#3" helicopter. This device responds to the initial Get-String-Descriptor request for string 0 (which is really the list of supported languages) by sending its config descriptor! The usb_get_string() routine needs to check whether it got the right type of descriptor. Oddly enough, this sort of check is already present in usb_get_descriptor(). The patch changes the error code from -EPROTO to -ENODATA, because -EPROTO shows up in so many other contexts to indicate a hardware failure rather than a firmware error. Signed-off-by: Alan Stern Tested-by: Guillermo Jarabo Cc: stable Signed-off-by: Greg Kroah-Hartman =================================================================== --- drivers/usb/core/message.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) (limited to 'drivers/usb/core/message.c') diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c index 31fb204f44c..49e7f56e0d7 100644 --- a/drivers/usb/core/message.c +++ b/drivers/usb/core/message.c @@ -653,7 +653,7 @@ int usb_get_descriptor(struct usb_device *dev, unsigned char type, if (result <= 0 && result != -ETIMEDOUT) continue; if (result > 1 && ((u8 *)buf)[1] != type) { - result = -EPROTO; + result = -ENODATA; continue; } break; @@ -696,8 +696,13 @@ static int usb_get_string(struct usb_device *dev, unsigned short langid, USB_REQ_GET_DESCRIPTOR, USB_DIR_IN, (USB_DT_STRING << 8) + index, langid, buf, size, USB_CTRL_GET_TIMEOUT); - if (!(result == 0 || result == -EPIPE)) - break; + if (result == 0 || result == -EPIPE) + continue; + if (result > 1 && ((u8 *) buf)[1] != USB_DT_STRING) { + result = -ENODATA; + continue; + } + break; } return result; } -- cgit v1.2.3-70-g09d2