2011/1/19 Michal Nazarewicz <mina86@xxxxxxxxxx>: >> On Thu, Jan 13, 2011 at 6:18 PM, Maulik Mankad <maulik@xxxxxx> wrote: >>> >>> When USB CV MSC tests are run on f_mass_storage gadget >>> Bulk Only Mass Storage Reset fails since req->length >>> is set to USB_BUFSIZ=1024 in composite_setup(). >>> >>> Set req->length to zero since the reset request does not >>> contain any data transfers. >>> >>> Signed-off-by: Maulik Mankad <maulik@xxxxxx> >>> Cc: Michal Nazarewicz <mina86@xxxxxxxxxx> >>> Cc: Felipe Balbi <balbi@xxxxxx> >>> Cc: Greg Kroah-Hartman <gregkh@xxxxxxx> >>> --- >>> Update Michal's email address. > > Yeah... I'm wondering if I should send a patch fixing my email in > source files. > >>> Rebased to Linus's master. >>> >>> drivers/usb/gadget/f_mass_storage.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> Index: mainline/drivers/usb/gadget/f_mass_storage.c >>> =================================================================== >>> --- mainline.orig/drivers/usb/gadget/f_mass_storage.c >>> +++ mainline/drivers/usb/gadget/f_mass_storage.c >>> @@ -626,6 +626,7 @@ static int fsg_setup(struct usb_function >>> * and reinitialize our state. >>> */ >>> DBG(fsg, "bulk reset request\n"); >>> + fsg->common->ep0req->length = 0; >>> raise_exception(fsg->common, FSG_STATE_RESET); >>> return DELAYED_STATUS; >>> > > On Tue, 18 Jan 2011 16:14:07 +0100, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> > wrote: >> >> This really should be handled by Michal. > > Yeah, sorry, I've just moved to Zurich so been postponing looking at > the code. > >> But AFAICS the fix is not correct. The whole DELAYED_STATUS definition >> and interpretation needs to be put into the composite core. > > Actually I think that handling of DELAYED_STATUS is pretty much the same in > FSG and MSF. In FSG no request is queued and the DELAYED_STATUS is returned > back to the UDC driver and the same things happens in MSF since composite > does not queue any requests in the "non-core control request" path but just > returns what ->setup() yields. > > Still, I'm not entirely sure how the above change affects anything since no > one should touch the ep0req after fsg_setup() returns. > What I have observed while debugging is with MSF when BOT MSC reset is issued, composite_setup() ends up calling fsg_setup(). Note that req->length is set to USB_BUFSIZ = 1024 in composite_setup(). While handling this class specific request, ep0_queue() gets called under FSG_STATE_RESET case of handle_exception() function. Thus ep0_queue() gets called with req->length set to 1024. Hence the controller driver reports a timeout error on endpoint zero. In case of FSG, the length parameter is initialized to zero upfront in fsg_setup() of file_storage.c. Hence the issue is not seen with FSG. The issue can be easily reproduced by running USB CV MSC tests with MSF. Below is the relevant logs for this issue. musb_read_fifo 286: RX ep0 fifo fc0ab020 count 8 buf c09a5d24 musb_read_setup 605: SETUP req21.ff v0000 i0000 l0 musb_g_ep0_irq 853: handled 0, csr 0001, ep0stage wait BOT RESET FROM DRIVER musb_g_tx 491: <== ep1in, txcsr 2000 musb_g_giveback 188: ep1out request efeadfc0, 0/512 fault -104 FSG_STATE_RESET Now calling ep0 queue musb_g_ep0_queue 959: queue to ep0 (OUT/RX), length=1024 g_mass_storage gadget: error in submission: ep0 --> -22 ep0_queue function end Regards, Maulik -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
