Hi,
On 28 February 2017 at 06:11, Alan Stern <[email protected]> wrote:
> On Tue, 21 Feb 2017, Baolin Wang wrote:
>
>> On 17 February 2017 at 16:04, Felipe Balbi <[email protected]> wrote:
>> >
>> > Hi,
>> >
>> > Baolin Wang <[email protected]> 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
Hi,
Baolin Wang <[email protected]> writes:
>>> > Baolin Wang <[email protected]> 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.
how the UDC driver handles this case, is up to the UDC driver. In DWC3 I
chose to rely on the same ep_queue mechanism; but that's an arbitrary
choice.
>>> (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()
this is the very detail that what I proposed will change. After what I
proposed is implemented, status stage will *always* be done in response
to a usb_ep_queue().
> 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.
or we change dwc3 to prepare an internal request and queue it to its own
enpdoint.
> (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
for DWC3, status stage *must* be done after XFER_NOT_READY event. That's
required by the databook. What you're claiming is not correct.
The only situation where we start status stage from usb_ep_queue() is
for the case when XFER_NOT_READY already triggered and we set
PENDING_REQUEST flag for the endpoint.
> 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().
already implemented. Nothing will change for this case.
> 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.
this is already implemented. There's nothing new coming to this case.
--
balbi