From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751558AbdCBKPc (ORCPT ); Thu, 2 Mar 2017 05:15:32 -0500 Received: from mail-ot0-f180.google.com ([74.125.82.180]:35759 "EHLO mail-ot0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751497AbdCBKPT (ORCPT ); Thu, 2 Mar 2017 05:15:19 -0500 MIME-Version: 1.0 In-Reply-To: References: From: Baolin Wang Date: Thu, 2 Mar 2017 18:15:17 +0800 Message-ID: Subject: Re: [PATCH] usb: dwc3: ep0: Fix the possible missed request for handling delay STATUS phase To: Alan Stern Cc: Felipe Balbi , Greg KH , USB , LKML , Linaro Kernel Mailman List , Mark Brown Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On 28 February 2017 at 06:11, Alan Stern wrote: > On Tue, 21 Feb 2017, Baolin Wang wrote: > >> On 17 February 2017 at 16:04, Felipe Balbi wrote: >> > >> > Hi, >> > >> > Baolin Wang writes: >> >>>> (One possible approach would be to have the setup routine return >> >>>> different values for explicit and implicit status stages -- for >> >>>> example, return 1 if it wants to submit an explicit status request. >> >>>> That wouldn't be very different from the current >> >>>> USB_GADGET_DELAYED_STATUS approach.) >> >>> >> >>> not really, no. The idea was for composite.c and/or functions to support >> >>> both methods (temporarily) and use "gadget->wants_explicit_stages" to >> >>> explicitly queue DATA and STATUS. That would mean that f_mass_storage >> >>> wouldn't have to return DELAYED_STATUS if >> >>> (gadget->wants_explicit_stages). >> >>> >> >>> After all UDCs are converted over and set wants_explicit_stages (which >> >>> should all be done in a single series), then we get rid of the flag and >> >>> the older method of DELAYED_STATUS. >> >> >> >> (Sorry for late reply due to my holiday) >> >> I also met the problem pointed by Alan, from my test, I still want to >> >> need one return value to indicate if it wants to submit an explicit >> >> status request. Think about the Control-IN with a data stage, we can >> >> not get the STATUS phase request from usb_ep_queue() call, and we need >> > >> > why not? wLength tells you that this is a 3-stage transfer. Gadget >> > driver should be able to figure out that it needs to usb_ep_queue() >> > another request for status stage. >> >> I tried again, but still can not work. Suppose the no-data control: >> (1) SET_ADDRESS request: function driver will not queue one request >> for status phase by usb_ep_queue() call. > > Function drivers do not handle Set-Address requests at all. The UDC > driver handles these requests without telling the gadget driver about > them. Correct. What I mean is it will not queue one request for status phase by usb_ep_queue() call, UDC driver will do that. > >> (2) SET_CONFIGURATION request: function driver will queue one 0-length >> request for status phase by usb_ep_queue() call, especially for >> mass_storage driver, it will queue one request for status phase >> later. >> >> So I am not sure how the Gadget driver can figure out that it needs to >> usb_ep_queue() another request for status stage when handling the >> no-data control? > > Gadget drivers already queue status-stage requests for no-data > control-OUT requests. The difficulty comes when you want to handle an > IN request or an OUT request with a data stage. > I try to explain that explicitly, In dwc3 driver, we can handle the STATUS phase request in 2 places: (1) in usb_ep_queue(), (2) in dwc3_ep0_xfernotready() For no-data control-OUT requests: (1) SET_ADDRESS request: no request queued for status phase by usb_ep_queue(), dwc3 driver need handle the STATUS phase request when one not-ready-event comes in dwc3_ep0_xfernotready() function. (2) SET_CONFIGURATION request: function driver will queue one 0-length request for status phase by usb_ep_queue(), but we can handle this request in usb_ep_queue() or dwc3_ep0_xfernotready(). When the function driver queued one 0-length request for status phase before the not-ready-event comes, we need handle this request in dwc3_ep0_xfernotready() when the not-ready-event comes. When the function driver queued one 0-length request for status phase after the not-ready-event comes, we can handle this request in usb_ep_queue(). So in dwc3_ep0_xfernotready(), we need to check if the request for status phase has been queued into pending request list, but if the pending request list is NULL, which means the function driver have not queued one 0-length request until now (then we can handle it in usb_ep_queue()), or situation (1) (no request queued for status phase), then I can not identify this 2 situations to determine where I can handle the status request. Hope I make it clear. Your concern about an IN request or an OUT request with a data stage, I agree with Felipe and we can identify. Thanks. -- Baolin.wang Best Regards