> From: Felipe Balbi [mailto:balbi@xxxxxx] > > On Thu, Sep 25, 2014 at 07:08:12PM +0000, Paul Zimmerman wrote: > > > From: Alan Stern [mailto:stern@xxxxxxxxxxxxxxxxxxx] > > > Sent: Thursday, September 25, 2014 11:08 AM > > > > > > On Thu, 25 Sep 2014, Paul Zimmerman wrote: > > > > > > > That's why I don't understand how this can happen for IN. AFAIK, a STALL > > > > is only sent in response to something the host sent in the CBW. At that > > > > point, there shouldn't be any IN transfers active. > > > > > > The gadget may send a partial response to the CBW. The end of the > > > response is marked with a STALL. The mass-storage gadget driver > > > submits the partial response and then calls usb_ep_set_halt() without > > > waiting for the IN data to be delivered. It relies on the UDC driver > > > returning -EAGAIN if any data is still pending. > > > > I guess you're referring to the code under the DATA_DIR_TO_HOST case > > in finish_reply(). > > > > Felipe, in our vendor driver, the ep_set_halt() and ep_set_wedge() > > functions check the request queue for the endpoint, and if it is not > > empty, they return -EAGAIN. > > is that really enough ? The request is deleted (rather, moved to another > list) as soon as StartTransfer is issued. I can certainly check both > lists (and already do) but I fear that we might still race with the HW > because there's no documentation to guarantee that I won't :-) > > > I see your patch for the dwc3 driver does add that, in addition to the > > FIFO empty check. Does it still work OK if you remove the FIFO empty > > check portion? > > It probably does, but I can't be sure there won't be a corner case where > the list is empty (Xfercomplete was issued and request given back) but > host hasn't moved all the data. Synopsys databook doesn't guarantee that > will never be the case. > > Can you confirm, without a shadow of a doubt, that XferComplete will > only be issued after host has moved every single bit of data ? If that > can be updated to the databook, then sure, I can remove the FIFO space > check; otherwise we might leave a corner case that will take us a few > days to debug again because it will be very difficult to trigger :-) >From the 2.70a databook, section 8.2.2: "While processing TRBs, two conditions may cause the core to write out an event and raise an interrupt line: - TRB Complete: - For OUT endpoints, a packet is received which reduces the remaining byte count in the TRB buffer to zero. - For IN endpoints, an acknowledgement is received for a transmitted packet which reduces the remaining byte count in the TRB buffer to zero. - Short Packet Received: - For OUT endpoints only. While writing to a TRB buffer, the endpoint receives a packet that is smaller than the endpoint's MaxPacketSize. " So for an IN, the completion event doesn't come until the TRB has been ACKed, which means all the data has been sent. But, I'm not saying you *have* to remove the FIFO space check. I think we now know the problem was caused by the missing -EAGAIN. So having the FIFO check there shouldn't hurt anything, it's not masking some other problem. And you may need it in the future for one of those other cases the databook talks about. -- Paul -- 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
