From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peng Fan Date: Wed, 9 Sep 2015 12:30:20 +0800 Subject: [U-Boot] [PATCH] usb: gadget: ci_udc: implement usb_ep_ops dequeue callback In-Reply-To: <20150909032434.GA7343@shlinux2> References: <1440673250-17589-1-git-send-email-Peng.Fan@freescale.com> <20150830072555.GA15009@shlinux2> <55E60048.3090406@wwwdotorg.org> <201509040011.45064.marex@denx.de> <55EF96B2.4000903@wwwdotorg.org> <20150909032434.GA7343@shlinux2> Message-ID: <20150909043016.GA13414@shlinux2> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Marek, 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->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? > >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/ Thanks, Peng. > >Regards, >Peng. >> > >-- --