Re: [PATCH] usb: dwc3: ep0: Fix the possible missed request for handling delay STATUS phase

[Baolin Wang <baolin.wang@linaro.org>]

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.

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