On Wed, 30 Mar 2005, David Brownell wrote:
> > Sure, but it's not the same.
>
> Actually it's _exactly_ the same, except without giving the upper level
> code a chance (via SET_INTERFACE) to properly shut down and re-activate
> each endpoint's request queue.
I'm still going to nitpick and say that the host telling a device to
_clear_ an endpoint halt is not the same as a gadget driver telling its
controller to _set_ a halt. But never mind that now... :-)
> Now, if the endpoint were actually halted, the CLEAR_HALT would have
> done the right thing... but it wasn't (had never _been_ halted), so
> what happened instead was that the host went in and trashed all the
> state which had been correctly re-initialized. This includes both
> FIFO state and (the really troublesome stuff) DMA transfer state.
>
> The specific failure I was seeing was a hang because the endpoint
> I/O queue wasn't getting restarted. It never needed to restart
> before ... previously, CLEAR_HALT only got called when the queue
> was already empty. (E.g. if the endpoint was really halted.)
This is disturbing. There already is a section of code in usb-storage
that calls usb_clear_halt at a spot where only _one_ type of device (an
old model ZIP drive) is known to halt its bulk endpoints. I had thought
that for every other device this wouldn't cause a problem; apparently I
was wrong.
In case you want to test it, here is a patch for g_file_storage to provoke
that behavior from usb-storage (normally g_file_storage is well behaved
and avoids that pathway):
===== drivers/usb/gadget/file_storage.c 1.42 vs edited =====
--- 1.42/drivers/usb/gadget/file_storage.c 2005-03-21 05:39:23 -05:00
+++ edited/drivers/usb/gadget/file_storage.c 2005-03-30 16:55:16 -05:00
@@ -1312,7 +1312,7 @@
}
VDBG(fsg, "get max LUN\n");
*(u8 *) req->buf = fsg->nluns - 1;
- value = min(w_length, (u16) 1);
+ value = -EDOM;
break;
}
}
> > > Or do usb_clear_halt() at the driver level,
> > > rather than making usbcore break devices that behave more sanely.
> >
> > Ironically enough, that was Pete's original patch!
>
> Hmm ... ;)
Note that Pete's original patch unconditionally called usb_clear_halt for
pipes that almost certainly weren't halted. In other words, it did
exactly what you don't want! So it's probably not a very good solution...
> > The reason I put the patch into usbcore was to protect against other
> > devices misbehaving in the same way as Pete's drives. Putting the
> > clear-halts back into usb-storage will fix Pete's problem, but what
> > happens to the next guy to run into the same thing?
>
> Drivers for those devices will just have to know about this quirk,
> like they know about other device quirks.
Or maybe they won't have to know. Ordinary retry mechanisms should be
able to get past the trouble. It was only the SCSI error handler issue
that prevented things from working for Pete.
I'll send a reversion patch to Greg later on, and raise this matter on
the SCSI development list.
Alan Stern
-------------------------------------------------------
This SF.net email is sponsored by Demarc:
A global provider of Threat Management Solutions.
Download our HomeAdmin security software for free today!
http://www.demarc.com/info/Sentarus/hamr30
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
