Re: [RESEND PATCH v3 1/2] usb: dwc3: gadget: Add disconnect checking when changing function dynamically

[Baolin Wang]

On 13 October 2016 at 16:28, Janusz Dziedzic <janusz.dziedzic@xxxxxxxxx> wrote:
> On 13 October 2016 at 10:21, Baolin Wang <baolin.wang@xxxxxxxxxx> wrote:
>> Hi,
>>
>> On 13 October 2016 at 16:16, Janusz Dziedzic <janusz.dziedzic@xxxxxxxxx> wrote:
>>> On 13 October 2016 at 09:37, Baolin Wang <baolin.wang@xxxxxxxxxx> wrote:
>>>> Hi,
>>>>
>>>> On 13 October 2016 at 15:06, Felipe Balbi <balbi@xxxxxxxxxx> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> Baolin Wang <baolin.wang@xxxxxxxxxx> writes:
>>>>>> When system has stpped the gadget, we should avoid queuing any requests
>>>>>
>>>>> queueing is *not* a problem. Starting is. In fact, that's what your
>>>>> patch is doing.
>>>>
>>>> OK.
>>>>
>>>>>
>>>>>> which will cause tranfer failed. Thus adding some disconnect checking to
>>>>>                    ^^^^^^^
>>>>>                    transfer
>>>>
>>>> Sorry for spelling mistake, will fix it.
>>>>
>>>>>
>>>>>> avoid this situation.
>>>>>>
>>>>>> Signed-off-by: Baolin Wang <baolin.wang@xxxxxxxxxx>
>>>>>> ---
>>>>>> Changes since v2:
>>>>>>  - Move disconnect checking into dwc3_send_gadget_ep_cmd().
>>>>>>  - Rename completion name and issue complete() at one place.
>>>>>>  - Move completion initialization into dwc3_gadget_init().
>>>>>>
>>>>>> Changes since v1:
>>>>>>  - Split into 2 separate ptaches.
>>>>>>  - Choose complete mechanism instead of polling.
>>>>>> ---
>>>>>>  drivers/usb/dwc3/gadget.c |    3 +++
>>>>>>  1 file changed, 3 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>>>>>> index 1783406..ca2ae5b 100644
>>>>>> --- a/drivers/usb/dwc3/gadget.c
>>>>>> +++ b/drivers/usb/dwc3/gadget.c
>>>>>> @@ -241,6 +241,9 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, unsigned cmd,
>>>>>>       int                     susphy = false;
>>>>>>       int                     ret = -EINVAL;
>>>>>>
>>>>>> +     if (!dwc->pullups_connected)
>>>>>> +             return -ESHUTDOWN;
>>>>>> +
>>>
>>> you skip trace_dwc3_gadget_ep_cmd()
>>
>> Yes, we did not need trace here since we did not send out the command.
>>
> What in such case: enumeration will not work and this will be because
> this ESHUTDOWN or wrong pullups_connected usage. Without a trace you
> will not know where the problem is.
> In my opinion this trace could be useful.

We have returned the '-ESHUTDOWN' error number for user to know what happened.

>
> BR
> Janusz
>
>>>
>>> BR
>>> Janusz
>>>
>>>>>>       /*
>>>>>>        * Synopsys Databook 2.60a states, on section 6.3.2.5.[1-8], that if
>>>>>>        * we're issuing an endpoint command, we must check if
>>>>>> --
>>>>>> 1.7.9.5
>>>>>>
>>>>>> --
>>>>>> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
>>>>>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>>>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>>>
>>>>> --
>>>>> balbi
>>>>
>>>>
>>>>
>>>> --
>>>> Baolin.wang
>>>> Best Regards
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
>>>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>
>>
>> --
>> Baolin.wang
>> Best Regards



-- 
Baolin.wang
Best Regards