From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com ([134.134.136.31]:42161 "EHLO mga06.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932227AbdDEIbn (ORCPT ); Wed, 5 Apr 2017 04:31:43 -0400 From: Felipe Balbi To: John Youn , Linux USB , John Youn Cc: "stable\@vger.kernel.org" Subject: Re: [PATCH] usb: dwc3: gadget: skip Set/Clear Halt when invalid In-Reply-To: <2B3535C5ECE8B5419E3ECBE300772909021B4A8ED8@US01WEMBX2.internal.synopsys.com> References: <20170119114034.15184-1-felipe.balbi@linux.intel.com> <877f30iogv.fsf@linux.intel.com> <2B3535C5ECE8B5419E3ECBE300772909021B4A8ED8@US01WEMBX2.internal.synopsys.com> Date: Wed, 05 Apr 2017 11:31:30 +0300 Message-ID: <87r317gsrh.fsf@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain Sender: stable-owner@vger.kernel.org List-ID: Hi, John Youn writes: >> Felipe Balbi writes: >>> At least macOS seems to be sending >>> ClearFeature(ENDPOINT_HALT) to endpoints which >>> aren't Halted. This makes DWC3's CLEARSTALL command >>> time out which causes several issues for the driver. >>> >>> Instead, let's just return 0 and bail out early. >>> >>> Cc: >>> Signed-off-by: Felipe Balbi >>> --- >>> >>> this falls into "has never worked before" category, so I'll be sending >>> it together with other patches for v4.11 merge window. Still, it's a >>> valid bug that's likely needed for stable trees. >>> >>> drivers/usb/dwc3/gadget.c | 5 +++++ >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c >>> index 6faf484e5dfc..0a664d8eba3f 100644 >>> --- a/drivers/usb/dwc3/gadget.c >>> +++ b/drivers/usb/dwc3/gadget.c >>> @@ -1379,6 +1379,9 @@ 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) >>> + return 0; >>> + >>> if (dep->number > 1) >>> trb = dwc3_ep_prev_trb(dep, dep->trb_enqueue); >>> else >>> @@ -1400,6 +1403,8 @@ 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)) >>> + return 0; >>> >>> ret = dwc3_send_clear_stall_ep_cmd(dep); >>> if (ret) >> >> Reviving this old thread here. While $subject allowed dwc3 to work when >> attached to macOS Host, I fear that we might have more issues than not >> in the future. The reason is that USB20 spec allows hosts to use >> ClearFeature(ENDPOINT_HALT) as a "Reset Data Toggle/SeqN" hint. >> >> With this, we're basically blocking that possibility. Still, without >> $subject, ClearStall commands were timing out. I'll try to do a local >> revert here and check what happens in this case, but would you have any >> idea why ClearStall would time out like that? >> > > Hi Felipe, > > I think Thinh Nguyen e-mailed you about this before saying it caused a > regression for us. But he has not had time to look into it further and > follow-up yet. > > No idea about the timing out on Mac though. We can try to reproduce > this when we have some time and take a look. Do you have a USB trace? not anymore, but as soon I have some free time, I can revert the patch locally and try to reproduce. I'll also implement a little libusb-based test to issue that request hundreds of times in a row and see if I can force the problem to happen ;-) Thanks for responding so quickly :-) -- balbi