Hi,
Felipe Balbi wrote:
> Hi,
>
> Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> writes:
>> Felipe Balbi wrote:
>>> Hi,
>>>
>>> Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> writes:
>>>> @@ -1627,13 +1633,37 @@ int __dwc3_gadget_ep_set_halt(struct dwc3_ep *dep, int value, int protocol)
>>>> else
>>>> dep->flags |= DWC3_EP_STALL;
>>>> } else {
>>>> + /*
>>>> + * Don't issue CLEAR_STALL command to control endpoints. The
>>>> + * controller automatically clears the STALL when it receives
>>>> + * the SETUP token.
>>>> + */
>>> we have a separate dwc3_gadget_ep0_set_halt() to handle that. How was
>>> this triggered?
>>>
>> I think it's a bit confusing here that the dwc3_gadget_ep0_set_halt()
>> has the similar name as __dwc3_gadget_ep_set_halt(). However, that
>> function only calls dwc3_ep0_stall_and_restart() and not handled through
>> SET/CLEAR_FEATURE(halt) request.
>>
>> If host issues SET_FEATURE(halt) or CLEAR_FEATURE(halt) to control
>> endpoints, it still goes through __dwc3_gadget_ep_set_halt().
> Perhaps that should be fixed, then?
>
If we want to add a patch to make this clear, we can add a separate
patch to rename dwc3_gadget_ep0_set_halt() to something along the line
of dwc3_ep0_stall_and_restart().
Everything else looks fine to me. Let me know if you have any other concern.
Thanks,
Thinh
