hi, On Thu, Nov 07, 2013 at 01:40:40PM -0600, Bin Liu wrote: > On Thu, Nov 7, 2013 at 1:02 PM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > > On Thu, 7 Nov 2013, Bin Liu wrote: > > > >> On Thu, Nov 7, 2013 at 12:01 PM, Felipe Balbi <balbi@xxxxxx> wrote: > >> > Hi, > >> > > >> > On Thu, Nov 07, 2013 at 11:54:48AM -0600, Bin Liu wrote: > >> >> I am debugging a weird musb problem in which musb is in high-speed > >> >> device mode loaded with g_mass_storage. For some reason, musb received > >> >> 10 bytes in SETUP packets during enumeration. Please see the following > >> >> log from musb. > >> >> > >> >> <after connected to host> > >> >> [ 447.100858] musb-hdrc musb-hdrc.0: usbintr (1) epintr(0) > >> >> [ 447.250856] musb-hdrc musb-hdrc.0: usbintr (4) epintr(0) > >> >> [ 447.569139] musb-hdrc musb-hdrc.0: usbintr (0) epintr(1) > >> >> [ 447.574515] musb_g_ep0_irq 720: SetupEnd came in a wrong ep0stage setup > >> >> [ 447.581189] musb_g_ep0_irq 815: SETUP packet len 10 != 8 ? > >> >> [ 447.586737] musb-hdrc musb-hdrc.0: usbintr (0) epintr(1) > >> >> [ 447.592103] musb_g_ep0_irq 815: SETUP packet len 10 != 8 ? > >> >> [ 447.597652] musb-hdrc musb-hdrc.0: Unhandled USB IRQ 00000001-00000000 > >> >> ...... > >> >> > >> >> When look at the following code in musb_gadget_ep0.c, when len != 8, > >> >> musb_read_setup() is not called, so it just breaks out without > >> >> flushing the fifo or sending > >> >> MUSB_CSR0_P_SVDRXPKTRDY. Is this a bug? I would think in this error > >> > > >> > in a sense, yes. OTOH, a setup packet with less than 8 bytes is a much > >> > larger bug on the host side, right ? Still, I think that you should > >> > reply with a stall in that case. > >> > >> I am in the early stage of the investigation, there are still many > >> things I need to check, but I am guessing the 10-byte SETUP issue is > >> due to signal integrity. The device chip (musb) and the host chip are > >> on the same board, DP/DM are directly wired together (that is why I > >> cannot hook up an analyzer yet). > >> > >> I just came across to this portion of the code, and was wondering this > >> is a bug in the driver which does not handle this corner case > >> properly. Thanks for the confirmation. > > > > A bug in the driver certainly seems more likely than a SETUP packet > > containing 10 bytes. > > > > In any case, STALL is not an allowed response to a SETUP packet. > > According to section 8.5.3 in the USB-2 spec, the only valid response > > to a SETUP packet is ACK. If the packet is corrupted, the device has > > to discard the packet and return no handshake. > > Ok, in Line 805 instead of 'break;' doing 'goto stall;' will stall the > IN transaction of the SETUP transfer. I think this is the proper way > to handle it. that's what I mean. We will always receive the SETUP packet itself, but following IN/OUT tokens can be responded with stall handshakes. -- balbi
Attachment:
signature.asc
Description: Digital signature
