Hi Alan, On Monday, 2 July 2018 18:13:27 EEST Alan Stern wrote: > On Mon, 2 Jul 2018, Paul Elder wrote: > > Hello Felipe, > > > > We have discovered an issue in MUSB gadget when receiving control OUT > > transfers. I have specifically observed it very frequently on the second > > SET_CUR UVC request when trying to stream video with yavta from a UVC > > gadget. > > > > What happens is that in the DATA phase, the controller copies the > > incoming data to FIFO which generates an interrupt (as per the docs), > > then the interrupt handler (actually ep0_rxstate) sends a stall because > > there is no usb_request to put the data into. > > > > Currently this usb_request is supposed to come from userspace by ioctl > > UVCIOC_SEND_RESPONSE, which means that (more often than not) if the > > interrupt corresponding to the DATA phase comes before userspace has a > > chance to call the ioctl (or before the ioctl handler has a chance to > > finish), then it will send a stall (usbmon ends with EPIPE). > > Shouldn't the UDC send a NAK install of a STALL under these > circumstances? In fact, shouldn't the controller not even bother to > copy the incoming data to its FIFO? > > I realize the MUSB hardware has a number of limitations, but this seems > a little peculiar. Those are questions I've asked myself when trying to help Paul with this issue. I realized that I couldn't tell whether MUSB was doing the right thing, as I didn't know what the UDC API required. When validating a control OUT request we need to validate both the data part of the SETUP phase (bRequest, bRequestType, wValue, wIndex) and the data part of the DATA phase. In the case of the UVC gadget, both validation occurs in userspace, so the SETUP validation can't be performed synchronously with the function's .setup() handler. In case of control writes, the STALL handshake can be returned in either the DATA phase or the STATUS phase (as far as I understand the USB specification doesn't indicate in which use cases it is appropriate to return a STALL handshake in the DATA or STATUS phase). As the data validation can obviously only be performed after the data is received, I assume that signaling a data error through a STALL handshake can only be performed in the STATUS stage. Signaling a setup error, on the other hand, could be done in either phase. The question is thus what options does the USB gadget API offer function drivers to STALL the control transfer in the DATA or STATUS phase. As far as I can tell, this can be done by returning an error from the .setup() handler (in which case I suppose the DATA stage will be STALLed), or calling usb_ep_set_halt() before the request completion handler for the DATA stage returns. Is that right ? Are there other mechanisms ? If there are no other mechanism, I won't be able to validate the DATA stage in userspace as I can only send the data to userspace in the DATA stage request completion handler, and have to then return before receiving a response from userspace. After clarifying the API (and possibly extending it...), let's move to the MUSB driver and see how to fix it. -- Regards, Laurent Pinchart -- 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
