From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us01smtprelay-2.synopsys.com ([198.182.60.111]:36936 "EHLO smtprelay.synopsys.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1033443AbdEWUf0 (ORCPT ); Tue, 23 May 2017 16:35:26 -0400 From: Thinh Nguyen To: Thinh Nguyen , Felipe Balbi , "linux-usb@vger.kernel.org" CC: John Youn , "stable@vger.kernel.org" Subject: Re: [PATCH 2/3] usb: dwc3: gadget: Fix early exit in set/clear ep halt Date: Tue, 23 May 2017 20:35:18 +0000 Message-ID: <30102591E157244384E984126FC3CB4F30071006@us01wembx1.internal.synopsys.com> References: <8760icg2zx.fsf@linux.intel.com> <8737dgg2w8.fsf@linux.intel.com> <20e1cce1-d3b7-e126-88aa-b2c823cc4b8b@synopsys.com> <87o9w3cs6b.fsf@linux.intel.com> <1f5a4ed8-fa79-e644-ce34-1125023d69ba@synopsys.com> <87tw5ub1ti.fsf@linux.intel.com> <30102591E157244384E984126FC3CB4F3005E00F@us01wembx1.internal.synopsys.com> Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 Sender: stable-owner@vger.kernel.org List-ID: Hi Felipe, On 5/11/2017 6:12 PM, Thinh Nguyen wrote: > On 4/11/2017 11:03 PM, Felipe Balbi wrote: >> 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. >> > > 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