Hi, Thinh Nguyen writes: >>>> Felipe Balbi writes: >>>>> Thinh Nguyen writes: >>>>>> This patch fixes a commit that causes a hang from device waiting for >>>>>> data with the wrong sequence number. The commit ffb80fc672c3 ("usb: >>>>>> dwc3: gadget: skip Set/Clear Halt when invalid") adds a check to return >>>>>> early depending on DWC3_EP_STALL is set or not, prevent sending the ep >>>>>> halt command to HW endpoint to do CLEAR_FEATURE(ENDPOINT_HALT) request. >>>>>> This was to workaround the issue for macOS where the device hangs from >>>>>> sending DWC3 clear stall command. >>>>>> >>>>>> In USB 3.1 spec, 9.4.5, CLEAR_FEATURE(ENDPOINT_HALT) request always >>>>>> results in the data sequence being reinitialized to zero regardless >>>>>> whether the endpoint has been halted or not. Some device class depends >>>>>> on this feature for its protocol. For instance, in mass storage class, >>>>>> there is MSC reset protocol that does CLEAR_FEATURE(ENDPOINT_HALT) on >>>>>> bulk endpoints. This protocol reinitializes the data sequence and >>>>>> ensures that whatever pending data requested from previous CBW will be >>>>>> reset. Otherwise this will cause a hang as the device can wait for the >>>>>> data with the wrong sequence number from the previous CBW. We found this >>>>>> failure in USB CV: MSC Error Recovery Test with f_mass_storage. >>>>>> >>>>>> This patch fixes this issue by checking to see whether the set/halt ep >>>>>> call is a protocol call before early exit to make sure that set/clear >>>>>> halt endpoint command can go through if it is a device class protocol. >>>>>> >>>>>> Fixes: ffb80fc672c3 ("usb: dwc3: gadget: skip Set/Clear Halt when invalid") >>>>>> Signed-off-by: Thinh Nguyen >>>>> >>>>> this will regress the macOS case I wrote that commit for. We need to >>>> >>>> no wait, it won't regress macOS, but we're still left with the problem >>>> of host and peripheral being able to get DataToggle/SeqN out of sync. >>>> >>> >>> This patch is for the regression we have. Can you provide more >>> information for the macOS? I'm not sure if this is the case for macOS, >> >> I need to find a way to reproduce it again first. When I first >> reproduced it was with dwc3 running adb and connecting it to a macOS >> machine. >> >>> but maybe there is still pending transfer when it tries to send the >>> request? (There shouldn't be any before issuing ClearStall command). Do >> >> this could be, I don't remember if I checked this or not :-) >> >> Really, the best way here, IMHO, would be to re-verify what's going on >> with macOS and revert my orignal patch since it's, rather clearly, >> wrong. >> > > Sure. Are you going to make a revert patch or I am? Well, after we really know what's going on with macOS and have a better fix, then who makes the revert is less important as long as problems get sorted out :-) Either way is fine for me. -- balbi