On Tue, Jul 10, 2018 at 11:23:50AM -0400, Alan Stern wrote: > 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. MUSB has a DATAEND bit that should be set after all data packets of the DATA stage are received. The theory is that if we delay setting the DATAEND bit then we might be able to delay completion of the DATA stage. > > 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. Oops, sorry; evidently I wasn't. The MUSB spec actually doesn't mention this. In any case, for a control-OUT transfer it seems that MUSB automatically sends the zero-length data packet, or STALL if the SENDSTALL bit is set, so rather than queue a STATUS stage packet we have to set the bit on time. > > (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. I don't think there's a way to do that... maybe if we fill the FIFO we could force the controller to NAK... then on usb_request submission unfill the FIFO... Paul Elder -- 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
