On Thu, May 08, 2014 at 06:56:02PM +0000, Paul Zimmerman wrote: > > From: linux-usb-owner@xxxxxxxxxxxxxxx [mailto:linux-usb-owner@xxxxxxxxxxxxxxx] On Behalf Of Paul Zimmerman > > Sent: Thursday, May 08, 2014 11:42 AM > > > > > From: linux-usb-owner@xxxxxxxxxxxxxxx [mailto:linux-usb-owner@xxxxxxxxxxxxxxx] On Behalf Of Alan Stern > > > Sent: Thursday, May 08, 2014 10:40 AM > > > > > > On Thu, 8 May 2014, Felipe Balbi wrote: > > > > > > > > The dwc3 driver should always prepare a buffer for the next ep0 SETUP > > > > > packet as soon as it retrieves the information for the current SETUP > > > > > packet from the buffer. > > > > > > > > > > Otherwise, as you described it, if the gadget driver never sends its > > > > > delayed status response then the UDC will never respond to any more > > > > > control transfers. > > > > > > > > we _do_ prepare transfers for setup packet everytime a Status phase is > > > > completed (we also restart ep0 in case of stalls): > > > > > > I said that dwc3 should prepare a buffer for a SETUP packet every time > > > a _SETUP_ stage is completed -- not every time a _STATUS_ stage is > > > completed. > > > > > > In principle the host can send a SETUP packet at any time, even in the > > > middle of an ongoing transfer. (Consider the case where a driver on > > > the host unlinks a control transfer before it has completed and then > > > submits a new one.) Right, that's all taken care of and even properly commented around in the code. The way this IP works though is kinda funny. If we do get a new setup packet before we go through all other phases, we still need to initiate data/status phases. They will complete straight away without doing any transfers, nothing will be shifted into the wire. We know about that because of a SETUP_PENDING bit in the XferComplete event - which can only be used for printing a debugging message. Because of that, restarting ep0 on status completion alone is enough as we are mandated to go through the ep0 phases anyway. I assure you we had countless hours of stress testing ep0 handling with all certification test cases. In fact, link layer test td7.9 (iirc) runs with 8 (again, IIRC) back-to-back setup packets. If you're interested in details, check git log on drivers/usb/dwc3/ep0.c > > Just FYI, the DWC3 core is designed to always respond to SETUP packets. > > It has a 3-deep input buffer for SETUPs, provided the RxFIFO is set up > > properly according to the databook. If the buffer fills up, then > > further SETUP packets will get lost, but they will still be ACKed. > > I forgot to say, this means that it is not necessary to prepare a > buffer for the next SETUP immediately after the previous SETUP stage > ends, since the next SETUP will be cached in the controller. > > There is a flowchart in the databook that describes the programming > flow for Control transfers. I think the DWC3 driver follows this > pretty closely, although it has been a while since I last reviewed it. yeah, we _do_ follow it pretty closely. We were very careful to take care of all corner cases, including the one which caused the programming model (and documentation) to be changed. Quite frankly, I'd prefer to have XferNotReady(SETUP) so that we would only start any TRBs (for ep0 at least) when we know what we're supposed to start. EP0 would've been a lot easier to implement/read/maintain should we have that working. But then again, XferNotReady(DATA) is broken in back-to-back SETUP packets, so what the heck ? :-) cheers -- balbi
Attachment:
signature.asc
Description: Digital signature
