In case you're interested, I've started getting reports that this patch does fix the problem. I haven't tested it myself yet, but it seems that other people have.
David, I think you should send this to Greg K-H for inclusion.
Matt
On Fri, Oct 18, 2002 at 03:27:08PM -0700, David Brownell wrote:
>
> >>>I'd really like an HCD guy to look at this. I've tested this every way I
> >>>can think of -- attempts to clear the halt on the bulk-in endpoint all
> >>>return -EPIPE. I know that the device is okay, because the same sequence
> >>>on 2.4.x works just fine.
>
> I think I got it. Basically, in 2.5 usb_pipein() uses the "true != 0"
> convention that C uses, where 2.4 used a "true == 1" solution costing
> an extra mask and shift. But only the clear_halt code tried to use
> "true" as an integer, not as a boolean ... all other users of that macro
> in the kernel used the boolean "!= 0" convention!
>
> Try the patch I've attached. It got me a happy little message about how
> clear halt "WORKED!".
>
> I suspect it'd be better to just remove that "check endpoint status" code
> in usbcore, there's no point in Linux expecting it to work if neither MSFT
> nor AAPL will expect reading that flag to work. One more area where the USB
> spec and the real world disagree ... :)
>
> - Dave
>
> --- ./drivers-dist/usb/core/message.c Fri Oct 18 09:48:17 2002
> +++ ./drivers/usb/core/message.c Fri Oct 18 14:24:44 2002
> @@ -687,6 +687,7 @@
> * 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 device
> + * 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
> @@ -702,21 +703,29 @@
> {
> 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 */
> result = usb_control_msg(dev, usb_sndctrlpipe(dev, 0),
> - USB_REQ_CLEAR_FEATURE, USB_RECIP_ENDPOINT, 0, endp, NULL, 0,
> + USB_REQ_CLEAR_FEATURE, USB_RECIP_ENDPOINT,
> + 0 /* feature id for endpoint halt */, endp, NULL, 0,
> HZ * USB_CTRL_SET_TIMEOUT);
>
> - /* don't clear if failed */
> + /* don't un-halt or force DATA0 except on success */
> if (result < 0)
> return result;
>
> +#if 0
> + {
> + __u16 status;
> + unsigned char *buffer;
> +
> + /* NOTE: seems like Microsoft and Apple don't bother verifying
> + * the halt "took", so some devices may seize up if you check...
> + * such as the Hagiwara FlashGate DUAL. We shouldn't ever need
> + * to verify this, anyway.
> + */
> buffer = kmalloc(sizeof(status), GFP_KERNEL);
> if (!buffer) {
> @@ -727,6 +736,5 @@
> 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);
> + buffer, sizeof(status), HZ * USB_CTRL_GET_TIMEOUT);
>
> memcpy(&status, buffer, sizeof(status));
> @@ -738,10 +746,10 @@
> if (le16_to_cpu(status) & 1)
> return -EPIPE; /* still halted */
> + }
> +#endif
>
> - usb_endpoint_running(dev, usb_pipeendpoint(pipe), usb_pipeout(pipe));
> -
> - /* toggle is reset on clear */
> -
> + /* 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;
> --- ./drivers-dist/usb/storage/transport.c Wed Oct 16 14:02:11 2002
> +++ ./drivers/usb/storage/transport.c Fri Oct 18 14:29:25 2002
> @@ -519,5 +519,8 @@
> {
> 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,
--
Matthew Dharm Home: [EMAIL PROTECTED]
Maintainer, Linux USB Mass Storage Driver
M: No, Windows doesn't have any nag screens.
C: Then what are those blue and white screens I get every day?
-- Mike and Cobb
User Friendly, 1/4/1999
msg08746/pgp00000.pgp
Description: PGP signature
