From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751219AbdBUJSL (ORCPT ); Tue, 21 Feb 2017 04:18:11 -0500 Received: from mail-ot0-f176.google.com ([74.125.82.176]:34010 "EHLO mail-ot0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750954AbdBUJSE (ORCPT ); Tue, 21 Feb 2017 04:18:04 -0500 MIME-Version: 1.0 In-Reply-To: <87o9y1medz.fsf@linux.intel.com> References: <87h94q6lai.fsf@linux.intel.com> <87o9y1medz.fsf@linux.intel.com> From: Baolin Wang Date: Tue, 21 Feb 2017 17:18:02 +0800 Message-ID: Subject: Re: [PATCH] usb: dwc3: ep0: Fix the possible missed request for handling delay STATUS phase To: Felipe Balbi Cc: Alan Stern , 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 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. (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? >> to handle this STATUS phase request in dwc3_ep0_xfernotready(). But >> Control-OUT will get one 0-length IN request for the status stage from >> usb_ep_queue(), so we need one return value from setup routine to > > no we don't :-) > >> distinguish these in dwc3_ep0_xfernotready(), or we can not handle >> status request correctly. Maybe I missed something else. >>> >>>> On the other hand, I am very doubtful about requiring explicit setup >>>> requests. >>> >>> right, me too ;-) >> >> So do you suggest me continue to try to do this? Thanks. > > explicit setup? no > explicit status? yes > > If you don't wanna do it, it's fine :-) I'll just add to my TODO > list. It just depends on how much other tasks you have on your end ;-) > > -- > balbi -- Baolin.wang Best Regards