Hi Felipe,
On 5/11/2017 6:12 PM, Thinh Nguyen wrote:
> On 4/11/2017 11:03 PM, Felipe Balbi wrote:
>> Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> writes:
>>>>>> Felipe Balbi <balbi@xxxxxxxxxx> writes:
>>>>>>> Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> 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 <thinhn@xxxxxxxxxxxx>
>>>>>>>
>>>>>>> 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.
>>
>
> Do you have any update on this issue?
>
The patch ffb80fc672c3 ("usb: dwc3: gadget: skip Set/Clear Halt when
invalid") still causes a regression for us. As there hasn't any update
for the macOS issue, can I submit a revert patch for this?
Thanks,
Thinh