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

[Marek Vasut]

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.

> 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 ?

Best regards,
Marek Vasut