From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com ([192.55.52.43]:12621 "EHLO mga05.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753179AbdDLGDP (ORCPT ); Wed, 12 Apr 2017 02:03:15 -0400 From: Felipe Balbi To: Thinh Nguyen , Thinh Nguyen , 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 In-Reply-To: <1f5a4ed8-fa79-e644-ce34-1125023d69ba@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> Date: Wed, 12 Apr 2017 09:02:49 +0300 Message-ID: <87tw5ub1ti.fsf@linux.intel.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" Sender: stable-owner@vger.kernel.org List-ID: --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable 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 ret= urn >>>>>> 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) reque= st. >>>>>> This was to workaround the issue for macOS where the device hangs fr= om >>>>>> 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 depen= ds >>>>>> on this feature for its protocol. For instance, in mass storage clas= s, >>>>>> 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 t= he >>>>>> 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 protoco= l. >>>>>> >>>>>> Fixes: ffb80fc672c3 ("usb: dwc3: gadget: skip Set/Clear Halt when in= valid") >>>>>> 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, >>=20 >> 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. >>=20 >>> but maybe there is still pending transfer when it tries to send the >>> request? (There shouldn't be any before issuing ClearStall command). Do >>=20 >> this could be, I don't remember if I checked this or not :-) >>=20 >> 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. >>=20 > > 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. =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEElLzh7wn96CXwjh2IzL64meEamQYFAljtwwkACgkQzL64meEa mQaQZA//TOALPOrkCiKyt7Db/955VNI1f2CPFtqfTF8zKZSQRdk7sMeBE3w4+nry 1uXVG+TlIxMLN2g+b/+0y9J8RiYckGxXoRln1MDElyTCt1N9H6cAp90Oka3V3lPE xrzb28oh5aNnlptQ7zXCmCKrdUX46WSlQDUAEDOQzmD/fEb5ipT5Hsvy+5HbQO+I Cnj2fbf8RD3WgB3PYgxi2vdGMcEE0QFiRkog7xihU27CuBZc3vOMnMW8V9UluUKH 38VDHOzhyzrTIMHqO9S/dzeVwbgZIzpxLgso4rk5MAQYiO4Urgu7E2EuaVLo9mcy s9dfEeFpY+D7uNp1h7rCNGdslbesrPg9xYrFVTtrnW45eo12+tVQgzsbNcoVsKJ6 5Uhnjojj0fKZGewmzEQikembQOQ55SINuG3gdL9AJcHpjvD/+whzsUewOeFVg9Xg uM3McP0w5Zyx5+zDfjivE8PiVuRdq8kcC7KaPzzuyWch7DRwev2wfzVU4Wyj/tYo WkqIX8mVR4Nyn+uzLml/j/VnX31J9wY4qnzWh0IZxUm1RfkthaWCtqUWxomf32KG QjmXawUE7MJysqxe1A5YovaZFvFdcWaVSbk7PVQh4rrfSEV2cX4KW50HIRkVAucw xEOZQ7VFd4TloqXfRM8JNu655nLq1m305KI8pGzcZ/KUm0/DoQ8= =SJAE -----END PGP SIGNATURE----- --=-=-=--