* [PATCH 2/3] usb: dwc3: gadget: Fix early exit in set/clear ep halt [not found] <a994bd6ad75720559bd43b7c777b3135dfef9d50.1491609187.git.thinhn@synopsys.com> @ 2017-04-07 23:57 ` Thinh Nguyen 2017-04-10 7:01 ` Felipe Balbi 0 siblings, 1 reply; 10+ messages in thread From: Thinh Nguyen @ 2017-04-07 23:57 UTC (permalink / raw) To: Felipe Balbi, linux-usb; +Cc: John Youn, Thinh Nguyen, stable 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@synopsys.com> --- drivers/usb/dwc3/gadget.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 93d98fb7215e..59385eadd8b4 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -1442,7 +1442,7 @@ int __dwc3_gadget_ep_set_halt(struct dwc3_ep *dep, int value, int protocol) unsigned transfer_in_flight; unsigned started; - if (dep->flags & DWC3_EP_STALL) + if (!protocol && (dep->flags & DWC3_EP_STALL)) return 0; if (dep->number > 1) @@ -1466,7 +1466,7 @@ int __dwc3_gadget_ep_set_halt(struct dwc3_ep *dep, int value, int protocol) else dep->flags |= DWC3_EP_STALL; } else { - if (!(dep->flags & DWC3_EP_STALL)) + if (!protocol && !(dep->flags & DWC3_EP_STALL)) return 0; ret = dwc3_send_clear_stall_ep_cmd(dep); -- 2.11.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] usb: dwc3: gadget: Fix early exit in set/clear ep halt 2017-04-07 23:57 ` [PATCH 2/3] usb: dwc3: gadget: Fix early exit in set/clear ep halt Thinh Nguyen @ 2017-04-10 7:01 ` Felipe Balbi 2017-04-10 7:03 ` Felipe Balbi 0 siblings, 1 reply; 10+ messages in thread From: Felipe Balbi @ 2017-04-10 7:01 UTC (permalink / raw) To: Thinh Nguyen, linux-usb; +Cc: John Youn, Thinh Nguyen, stable [-- Attachment #1: Type: text/plain, Size: 1952 bytes --] Hi, Thinh Nguyen <Thinh.Nguyen@synopsys.com> 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@synopsys.com> this will regress the macOS case I wrote that commit for. We need to FIRST figure out why ClearStall command times out. Have you done that? Do you have a testcase which constantly issues ClearFeature(ENDPOINT_HALT) to a single endpoint and can verify that it doesn't time out? Why does it timeout on macOS? -- balbi [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] usb: dwc3: gadget: Fix early exit in set/clear ep halt 2017-04-10 7:01 ` Felipe Balbi @ 2017-04-10 7:03 ` Felipe Balbi 2017-04-11 3:21 ` Thinh Nguyen 0 siblings, 1 reply; 10+ messages in thread From: Felipe Balbi @ 2017-04-10 7:03 UTC (permalink / raw) To: Thinh Nguyen, linux-usb; +Cc: John Youn, Thinh Nguyen, stable [-- Attachment #1: Type: text/plain, Size: 1930 bytes --] Hi again, Felipe Balbi <balbi@kernel.org> writes: > Thinh Nguyen <Thinh.Nguyen@synopsys.com> 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@synopsys.com> > > 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. -- balbi [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] usb: dwc3: gadget: Fix early exit in set/clear ep halt 2017-04-10 7:03 ` Felipe Balbi @ 2017-04-11 3:21 ` Thinh Nguyen 2017-04-11 7:35 ` Felipe Balbi 0 siblings, 1 reply; 10+ messages in thread From: Thinh Nguyen @ 2017-04-11 3:21 UTC (permalink / raw) To: Felipe Balbi, Thinh Nguyen, linux-usb; +Cc: John Youn, stable Hi Felipe, On 4/10/2017 12:04 AM, Felipe Balbi wrote: > > Hi again, > > Felipe Balbi <balbi@kernel.org> writes: >> Thinh Nguyen <Thinh.Nguyen@synopsys.com> 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@synopsys.com> >> >> 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, but maybe there is still pending transfer when it tries to send the request? (There shouldn't be any before issuing ClearStall command). Do you see Starttransfer command after clear transfer? BR, Thinh Nguyen ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] usb: dwc3: gadget: Fix early exit in set/clear ep halt 2017-04-11 3:21 ` Thinh Nguyen @ 2017-04-11 7:35 ` Felipe Balbi 2017-04-11 23:06 ` Thinh Nguyen 0 siblings, 1 reply; 10+ messages in thread From: Felipe Balbi @ 2017-04-11 7:35 UTC (permalink / raw) To: Thinh Nguyen, Thinh Nguyen, linux-usb; +Cc: John Youn, stable [-- Attachment #1: Type: text/plain, Size: 2702 bytes --] Hi, Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes: >> Felipe Balbi <balbi@kernel.org> writes: >>> Thinh Nguyen <Thinh.Nguyen@synopsys.com> 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@synopsys.com> >>> >>> 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. -- balbi [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] usb: dwc3: gadget: Fix early exit in set/clear ep halt 2017-04-11 7:35 ` Felipe Balbi @ 2017-04-11 23:06 ` Thinh Nguyen 2017-04-12 6:02 ` Felipe Balbi 0 siblings, 1 reply; 10+ messages in thread From: Thinh Nguyen @ 2017-04-11 23:06 UTC (permalink / raw) To: Felipe Balbi, Thinh Nguyen, linux-usb; +Cc: John Youn, stable On 4/11/2017 12:36 AM, Felipe Balbi wrote: > > Hi, > > Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes: >>> Felipe Balbi <balbi@kernel.org> writes: >>>> Thinh Nguyen <Thinh.Nguyen@synopsys.com> 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@synopsys.com> >>>> >>>> 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? Thanks, Thinh ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] usb: dwc3: gadget: Fix early exit in set/clear ep halt 2017-04-11 23:06 ` Thinh Nguyen @ 2017-04-12 6:02 ` Felipe Balbi 2017-05-12 1:12 ` Thinh Nguyen 0 siblings, 1 reply; 10+ messages in thread From: Felipe Balbi @ 2017-04-12 6:02 UTC (permalink / raw) To: Thinh Nguyen, Thinh Nguyen, linux-usb; +Cc: John Youn, stable [-- Attachment #1: Type: text/plain, Size: 3060 bytes --] Hi, Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes: >>>> Felipe Balbi <balbi@kernel.org> writes: >>>>> Thinh Nguyen <Thinh.Nguyen@synopsys.com> 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@synopsys.com> >>>>> >>>>> 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 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] usb: dwc3: gadget: Fix early exit in set/clear ep halt 2017-04-12 6:02 ` Felipe Balbi @ 2017-05-12 1:12 ` Thinh Nguyen 2017-05-23 20:35 ` Thinh Nguyen 0 siblings, 1 reply; 10+ messages in thread From: Thinh Nguyen @ 2017-05-12 1:12 UTC (permalink / raw) To: Felipe Balbi, linux-usb; +Cc: John Youn, stable Hi Felipe, On 4/11/2017 11:03 PM, Felipe Balbi wrote: > > Hi, > > Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes: >>>>> Felipe Balbi <balbi@kernel.org> writes: >>>>>> Thinh Nguyen <Thinh.Nguyen@synopsys.com> 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@synopsys.com> >>>>>> >>>>>> 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? BR, Thinh ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] usb: dwc3: gadget: Fix early exit in set/clear ep halt 2017-05-12 1:12 ` Thinh Nguyen @ 2017-05-23 20:35 ` Thinh Nguyen 2017-06-02 8:24 ` Felipe Balbi 0 siblings, 1 reply; 10+ messages in thread From: Thinh Nguyen @ 2017-05-23 20:35 UTC (permalink / raw) To: Thinh Nguyen, Felipe Balbi, linux-usb; +Cc: John Youn, stable 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@synopsys.com> writes: >>>>>> Felipe Balbi <balbi@kernel.org> writes: >>>>>>> Thinh Nguyen <Thinh.Nguyen@synopsys.com> 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@synopsys.com> >>>>>>> >>>>>>> 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] usb: dwc3: gadget: Fix early exit in set/clear ep halt 2017-05-23 20:35 ` Thinh Nguyen @ 2017-06-02 8:24 ` Felipe Balbi 0 siblings, 0 replies; 10+ messages in thread From: Felipe Balbi @ 2017-06-02 8:24 UTC (permalink / raw) To: Thinh Nguyen, Thinh Nguyen, linux-usb; +Cc: John Youn, stable [-- Attachment #1: Type: text/plain, Size: 1062 bytes --] Hi, Thinh Nguyen <Thinh.Nguyen@synopsys.com> writes: >>>>> 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? I just came back from vacations ;-) I'll get back to this. Reverting that commit won't do any good as we'd be exchanging one regression for another. We really need to understand what's going on. -- balbi [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-06-02 8:25 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <a994bd6ad75720559bd43b7c777b3135dfef9d50.1491609187.git.thinhn@synopsys.com> 2017-04-07 23:57 ` [PATCH 2/3] usb: dwc3: gadget: Fix early exit in set/clear ep halt Thinh Nguyen 2017-04-10 7:01 ` Felipe Balbi 2017-04-10 7:03 ` Felipe Balbi 2017-04-11 3:21 ` Thinh Nguyen 2017-04-11 7:35 ` Felipe Balbi 2017-04-11 23:06 ` Thinh Nguyen 2017-04-12 6:02 ` Felipe Balbi 2017-05-12 1:12 ` Thinh Nguyen 2017-05-23 20:35 ` Thinh Nguyen 2017-06-02 8:24 ` Felipe Balbi
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.