On Tue, 10 Jul 2018, Paul Elder wrote: > > > 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. > > > > Correct. Even if userspace wasn't involved, you still wouldn't be able > > to check the data and halt the endpoint in time, because the UDC will > > automatically queue a positive (i.e., zlp) response for the STATUS > > stage when the DATA stage is complete. > > Laurent and I have theorized that in the case of MUSB it might be > possible to do this by delaying the completion of the DATA phase > (and by extension delay the host moving to the STATUS phase). I still How would you delay completion of the DATA stage? As far as I can see, the only way to do that is to send a NAK handshake back to the host, and you mention below that MUSB will not do this. > need to test to see if this theory will work, plus we still need some > way to allow userspace to tell the driver that it should reply with > STALL or not :/ > > > This is a weakness in the gadget API, and there has been at least one > > discussion about changing it. In particular, Felipe recommended that > > STATUS stage packets should be queued explicitly by gadget drivers, not > > sent automatically by UDCs. But as far as I know, this suggestion has > > not been implemented. It would require changing pretty much every UDC > > driver and every gadget driver. > > I'm wondering if gadget drivers explicitly queueing STATUS stage packets > would help, because what I see in the MUSB spec is that the controller > automatically ACKs (or STALLs if that bit in the register is set) > the STATUS phase, so the driver isn't involved in the STATUS reply > except for setting the STALL bit. Are you reading the spec correctly? ACK or STALL is an appropriate response to the STATUS stage of a control-IN transfer. But for a control-OUT transfer, the device has to send a zero-length data packet to complete the STATUS stage. > (If it is indeed possible to delay the completion of > the DATA phase and then have an ioctl actually complete the DATA > phase and set the STALL bit if necessary then it might work...) > > Also, the interrupt signaling the STATUS phase doesn't always happen and > sometimes gets coalesced with the next request: > (in musb_g_ep0_irq, in drivers/usb/musb/musb_gadget_ep0.c) > /* > * This state is typically (but not always) indiscernible > * from the status states since the corresponding interrupts > * tend to happen within too little period of time (with only > * a zero-length packet in between) and so get coalesced... > */ > I guess this isn't too much of an issue but just wanted to point out > that we can't expect to do anything in response to a STATUS phase > interrupt if its arrival isn't reliable. > > > The current situation is that currently the only way for a gadget > > driver to send a STALL to the host in response to a control-OUT > > transfer is to do so before queuing the DATA-stage request. In other > > words, the decision has to be made before the data has been received. > > > > The situation for control-IN transfers is better. The gadget driver > > has no reason to stall the STATUS stage once it has sent the data to > > the host, so it doesn't matter that there is currently no mechanism for > > doing so. > > > > > After clarifying the API (and possibly extending it...), let's move to the > > > MUSB driver and see how to fix it. > > > > Well, that's a separate issue. If an endpoint is enabled but there are > > no outstanding requests queued for it, then any packet received for > > that endpoint should get a STALL response if and only if the endpoint > > is halted, and a NAK response otherwise. > > I think this is a hardware issue. Nowhere in the MUSB spec (nor code) > can I find anything about how to send NAK (or ACK). It seems that it's > either STALL or auto-ACK. Plus, the controller has no reason to NAK, > since its FIFO has space to accept the data; it neither knows nor cares > that the software doesn't have its usb_request ready to accept the data. Okay, that's understandable. But perhaps you could disable the FIFO until the gadget driver has submitted an appropriate usb_request. Alan Stern -- 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