All of lore.kernel.org
 help / color / mirror / Atom feed
From: Baolin Wang <baolin.wang@linaro.org>
To: Felipe Balbi <balbi@kernel.org>
Cc: Alan Stern <stern@rowland.harvard.edu>,
	Greg KH <gregkh@linuxfoundation.org>,
	USB <linux-usb@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Linaro Kernel Mailman List <linaro-kernel@lists.linaro.org>,
	Mark Brown <broonie@kernel.org>
Subject: Re: [PATCH] usb: dwc3: ep0: Fix the possible missed request for handling delay STATUS phase
Date: Mon, 20 Feb 2017 10:27:32 +0800	[thread overview]
Message-ID: <CAMz4kuJaq7fYpe=qNLyzujvLFwd8DvgmN-_OY80LqFNrU4zvkA@mail.gmail.com> (raw)
In-Reply-To: <87o9y1medz.fsf@linux.intel.com>

On 17 February 2017 at 16:04, Felipe Balbi <balbi@kernel.org> wrote:
>
> Hi,
>
> Baolin Wang <baolin.wang@linaro.org> 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.
>
>> 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 ;-)

OK, I will take some time to check and test again. It will be better
if I send out one RFC patch to review.

-- 
Baolin.wang
Best Regards

  reply	other threads:[~2017-02-20  2:27 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-14  8:40 [PATCH] usb: dwc3: ep0: Fix the possible missed request for handling delay STATUS phase Baolin Wang
2017-01-16 10:56 ` Felipe Balbi
2017-01-16 11:29   ` Baolin Wang
2017-01-16 11:29     ` Felipe Balbi
2017-01-16 12:00       ` Baolin Wang
2017-01-16 12:06         ` Felipe Balbi
2017-01-16 17:53           ` Alan Stern
2017-01-16 19:18             ` Felipe Balbi
2017-01-17 15:54               ` Alan Stern
2017-01-23 11:57                 ` Felipe Balbi
2017-02-17  5:41                   ` Baolin Wang
2017-02-17  8:04                     ` Felipe Balbi
2017-02-20  2:27                       ` Baolin Wang [this message]
2017-02-21  9:18                       ` Baolin Wang
2017-02-27 22:11                         ` Alan Stern
2017-02-28 11:56                           ` Felipe Balbi
2017-02-28 18:34                             ` Alan Stern
2017-03-02 10:43                               ` Felipe Balbi
2017-03-02 10:15                           ` Baolin Wang
2017-03-02 10:48                             ` Felipe Balbi
2017-01-17  7:02           ` Baolin Wang
2017-01-17 10:39             ` Felipe Balbi
2017-01-17 11:40               ` Baolin Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAMz4kuJaq7fYpe=qNLyzujvLFwd8DvgmN-_OY80LqFNrU4zvkA@mail.gmail.com' \
    --to=baolin.wang@linaro.org \
    --cc=balbi@kernel.org \
    --cc=broonie@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linaro-kernel@lists.linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=stern@rowland.harvard.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.