[U-Boot] [PATCH] usb: gadget: ci_udc: implement usb_ep_ops dequeue callback

[Stephen Warren <swarren@wwwdotorg.org>]

On 09/03/2015 03:11 PM, Marek Vasut wrote:
> On Tuesday, September 01, 2015 at 09:45:12 PM, Stephen Warren wrote:
>> On 08/30/2015 12:26 AM, Peng Fan wrote:
>>> Hi Stephen,
> 
> Hi,
> 
> sorry for the delayed reply, I had to dig into the code myself.
> 
>>> On Fri, Aug 28, 2015 at 08:05:36AM +0800, Peng Fan wrote:
>>>> Hi Stephen,
>>>>
>>>> On Thu, Aug 27, 2015 at 10:06:14AM -0600, Stephen Warren wrote:
>>>>> On 08/27/2015 05:08 AM, Marek Vasut wrote:
>>>>>> On Thursday, August 27, 2015 at 01:00:50 PM, Peng Fan wrote:
>>>>>>> Implement endpoint dequeue callback function.
>>>>>>>
>>>>>>> Without this function, uboot will hang when executing fastboot
>>>>>>> comamnd. See following flow:
>>>>>>> "fastboot_tx_write_str->fastboot_tx_write->usb_ep_dequeue->ep->ops->d
>>>>>>> equeue " without implement ci_udc dequeue function, ep->ops->dequeue
>>>>>>> is NULL, then uboot will hang.
>>>>>>>
>>>>>>> Tested on mx6qsabresd board with fastboot enabled.
>>>>>>>
>>>>>>> diff --git a/drivers/usb/gadget/ci_udc.c
>>>>>>> b/drivers/usb/gadget/ci_udc.c
>>>>>>>
>>>>>>> +static int ci_ep_dequeue(struct usb_ep *_ep, struct usb_request
>>>>>>> *_req)
>>>>>>>
>>>>>>> +	if (ci_req->req.status == -EINPROGRESS) {
>>>>>>> +		ci_req->req.status = -ECONNRESET;
>>>>>>> +		if (ci_req->req.complete)
>>>>>>> +			ci_req->req.complete(_ep, _req);
>>>>>>> +	}
>>>>>
>>>>> Is there no need to reprogram the HW to abort the transfer?
>>>>
>>>> I checked linux udc driver drivers/usb/gadget/udc/fsl_qe_udc.c
>>>> qe_ep_dequeue->done->usb_gadget_giveback_request->"req->complete(ep,
>>>> req)" I did not see code to reprogram the HW to abort the transfer.
>>>
>>> Do you have further comments?
>>> I checked other gadget drivers in drivers/usb/gadget/, I did not see
>>> drivers that reprogram the HW to abort the transfer. For now, I do not
>>> think out a scenario to reprogram the HW to abort the transfer
>>
>> Marek, what are the semantics of this function? Is it supposed to simply
>> update SW state to make U-Boot not care about the transaction
> 
> Yes, that's correct.
> 
>> or is it supposed to actually stop the HW performing the transaction on
>> the USB bus?
> 
> No, it's not supposed to kill the transaction in hardware.

OK, the patch seems fine then.

>> If it's the former, then the patch is likely fine. If it's the latter,
>> then I think the function does need actually need to do something to
>> make the HW stop, or we can't implement this particular function.
> 
> Do we need this for the current release or is this for -next ?

I assume that's a question for Peng?