From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Mxfk0-00007R-0p for qemu-devel@nongnu.org; Tue, 13 Oct 2009 07:40:32 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Mxfjv-00005b-F4 for qemu-devel@nongnu.org; Tue, 13 Oct 2009 07:40:31 -0400 Received: from [199.232.76.173] (port=40967 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Mxfjv-00005N-7J for qemu-devel@nongnu.org; Tue, 13 Oct 2009 07:40:27 -0400 Received: from smtp-out1.tiscali.nl ([195.241.79.176]:55618) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1Mxfju-00080I-Gm for qemu-devel@nongnu.org; Tue, 13 Oct 2009 07:40:26 -0400 From: Paul Bolle Content-Type: text/plain; charset="UTF-8" Date: Tue, 13 Oct 2009 13:40:08 +0200 Message-Id: <1255434008.1817.40.camel@localhost.localdomain> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] [PATCH] usb-linux: return USB_RET_STALL on -EPIPE List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Anthony Liguori , Mark Burkley , Max Krasnyansky 0) This is an attempt to get an issue in usb-linux.c, for which a patch was posted about a year ago, finally fixed. 1) Mark Burkley submitted a "EHCI emulation module" for review in in October 2008 (see: http://lists.gnu.org/archive/html/qemu-devel/2008-10/msg01326.html). No EHCI emulation module was ever committed to qemu. 2) Part of that (large) patch was a fix for a separate issue in usb-linux.c. Max Krasnyansky has ACK'ed that fix (see: http://lists.gnu.org/archive/html/qemu-devel/2008-11/msg00032.html). 3) I already asked whether this fix was ready to be committed in last April (see: http://lists.gnu.org/archive/html/qemu-devel/2009-04/msg01763.html) 4) Maybe submitting this fix as a separate patch (with a really long commit message but without a Signed-off-by) and cc-ing everbody involved will help if actually getting this issue fixed. Paul Bolle --- usb-linux: return USB_RET_STALL on -EPIPE Max Krasnyansky wrote: > Anthony Liguori wrote: >> Mark Burkley wrote: >>> Hi Anthony, >>> [...] >>> I am seeing an issue with a >>> memory key I am using for testing. ioctl returns EPIPE (which I >>> would have thought was a STALL) to an asynchronous IN completion in >>> usb-linux.c but then this is returned as USB_RET_NAK to EHCI which >>> confuses my WinXP target because the transfer is then never >>> completed. >>> >>> Can I just check that it was intentional to return NAK for EPIPE >>> returns in asynchronous completions? If so, then I will try to >>> detect the stall in my implementation and treat differently to a >>> NAK. It's just that if I modify usb-linux.c to return >>> USB_RET_STALL on -EPIPE then it works fine. > > I just looked at the usb-linuc.c:async_complete() code and it looks like > it was intentional but I cannot remember why I wrote that way. And what > you're saying makes sense. ie It should be a STALL. In fact I think that > might fix the regression with USB storage devices that some people have > reported. > I'll play some more with this later today. I want to make sure that the > change we're talking about does not break existing devices that I > thoroughly tested as part of the usb async re-write. If everything works > as expected then we'll change it. Ok. I just tested that change (ie returning STALL instead of NAK on EPIPE) with a bunch of devices: USB serial adapter, CF card reader, USB webcam (MS VX-3000) and MS USB mouse. All that stuff was hooked up to XP-SP3 and all of them are perfectly usable at the same time. In other words here is my ACK :) Acked-by: Max Krasnyansky Tested-by: Paul Bolle --- usb-linux.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/usb-linux.c b/usb-linux.c index 9e5d9c4..d712134 100644 --- a/usb-linux.c +++ b/usb-linux.c @@ -275,7 +275,9 @@ static void async_complete(void *opaque) case -EPIPE: set_halt(s, p->devep); - /* fall through */ + p->len = USB_RET_STALL; + break; + default: p->len = USB_RET_NAK; break; -- 1.6.5.rc2