ChangeSet 1.808.2.4, 2002/10/21 23:22:52-07:00, dbrownell@users.sourceforge.net [PATCH] usb: problem clearing halts This is a slightly cleaned up version of that earlier patch: - Makes both copies of the clear_halt() logic know that usb_pipein() returns boolean (zero/not) not integer (0/1). This resolves a problem folk have had with usb-storage. (I looked at kernel uses of usb_pipein and it really was only the clear_halt logic that cares.) - Removes some code from the "standard" version; no point in Linux expecting devices to do something neither Microsoft nor Apple will test for. diff -Nru a/drivers/usb/core/message.c b/drivers/usb/core/message.c --- a/drivers/usb/core/message.c Mon Oct 28 13:56:25 2002 +++ b/drivers/usb/core/message.c Mon Oct 28 13:56:25 2002 @@ -686,8 +686,9 @@ * as reported by URB completion status. Endpoints that are halted are * sometimes referred to as being "stalled". Such endpoints are unable * to transmit or receive data until the halt status is cleared. Any URBs - * queued queued for such an endpoint should normally be unlinked before - * clearing the halt condition. + * queued for such an endpoint should normally be unlinked by the driver + * before clearing the halt condition, as described in sections 5.7.5 + * and 5.8.5 of the USB 2.0 spec. * * Note that control and isochronous endpoints don't halt, although control * endpoints report "protocol stall" (for unsupported requests) using the @@ -701,48 +702,34 @@ int usb_clear_halt(struct usb_device *dev, int pipe) { int result; - __u16 status; - unsigned char *buffer; - int endp=usb_pipeendpoint(pipe)|(usb_pipein(pipe)<<7); - -/* - if (!usb_endpoint_halted(dev, endp & 0x0f, usb_endpoint_out(endp))) - return 0; -*/ - + int endp = usb_pipeendpoint(pipe); + + if (usb_pipein (pipe)) + endp |= USB_DIR_IN; + + /* we don't care if it wasn't halted first. in fact some devices + * (like some ibmcam model 1 units) seem to expect hosts to make + * this request for iso endpoints, which can't halt! + */ result = usb_control_msg(dev, usb_sndctrlpipe(dev, 0), USB_REQ_CLEAR_FEATURE, USB_RECIP_ENDPOINT, 0, endp, NULL, 0, HZ * USB_CTRL_SET_TIMEOUT); - /* don't clear if failed */ - if (result < 0) - return result; - - buffer = kmalloc(sizeof(status), GFP_KERNEL); - if (!buffer) { - err("unable to allocate memory for configuration descriptors"); - return -ENOMEM; - } - - result = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0), - USB_REQ_GET_STATUS, USB_DIR_IN | USB_RECIP_ENDPOINT, 0, endp, - // FIXME USB_CTRL_GET_TIMEOUT, yes? why not usb_get_status() ? - buffer, sizeof(status), HZ * USB_CTRL_SET_TIMEOUT); - - memcpy(&status, buffer, sizeof(status)); - kfree(buffer); - + /* don't un-halt or force to DATA0 except on success */ if (result < 0) return result; - if (le16_to_cpu(status) & 1) - return -EPIPE; /* still halted */ - - usb_endpoint_running(dev, usb_pipeendpoint(pipe), usb_pipeout(pipe)); - - /* toggle is reset on clear */ + /* NOTE: seems like Microsoft and Apple don't bother verifying + * the clear "took", so some devices could lock up if you check... + * such as the Hagiwara FlashGate DUAL. So we won't bother. + * + * NOTE: make sure the logic here doesn't diverge much from + * the copy in usb-storage, for as long as we need two copies. + */ + /* toggle was reset by the clear, then ep was reactivated */ usb_settoggle(dev, usb_pipeendpoint(pipe), usb_pipeout(pipe), 0); + usb_endpoint_running(dev, usb_pipeendpoint(pipe), usb_pipeout(pipe)); return 0; } diff -Nru a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c --- a/drivers/usb/storage/transport.c Mon Oct 28 13:56:25 2002 +++ b/drivers/usb/storage/transport.c Mon Oct 28 13:56:25 2002 @@ -518,7 +518,10 @@ int usb_stor_clear_halt(struct us_data *us, unsigned int pipe) { int result; - int endp = usb_pipeendpoint(pipe) | (usb_pipein(pipe) << 7); + int endp = usb_pipeendpoint(pipe); + + if (usb_pipein (pipe)) + endp |= USB_DIR_IN; result = usb_stor_control_msg(us, us->send_ctrl_pipe, USB_REQ_CLEAR_FEATURE, USB_RECIP_ENDPOINT, 0,