On Wednesday, September 09, 2015 at 06:30:20 AM, Peng Fan wrote: > Hi Marek,
Hi! > On Wed, Sep 09, 2015 at 11:24:37AM +0800, Peng Fan wrote: > >On Tue, Sep 08, 2015 at 07:17:22PM -0700, Stephen Warren wrote: > >>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->op > >>>>>>>>> s->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? > > > >Since this is a bug fix, I think this patch should be in current release. > >Without this patch, boards which want to enable fastboot using ci_udc > >driver will hang. > > If you have plan to pick the bug fix, please pick V2. > https://patchwork.ozlabs.org/patch/511739/ Aye aye, sir! One applied patch, coming right up! _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot