On 12/13/2010 1:36 PM, Artem Leonenko wrote:
> There is one nasty scenario when CI13xxx driver fails:
> a) two or more rx requests are queued (g_ether does that)
> b) rx request completed, interrupt fires and ci13xxx dequeues rq
> c) request complete() callback gets called and in turn it calls
> ep_queue()
> c1) in ep_queue() request gets added to the TAIL of the
> rx queue list
> d) ep gets primed with rq from (b)
> e) interrupt fires
> f) request gets popped from queue head for hw dequeue
> G) requets from queue head wasn't enqueued
> g1) isr_tr_complete_low() doesn't
> enqueue more requests and it doesn't prime EP,
> rx traffic stalls
>
> Solution:
> a) enque queued requests ASAP, i.e. before calling complete()
> callback.
> b) don't HW enqueue and prime endpoint with recently added
> request and
> use the oldest request in the queue.
>
> Fixed issues:
> a) ep_queue() may return an error code despite request was
> successfully
> added to the queue (if _hardware_enqueue() fails)
> b) Added requests are always processed in LIFO order, even if
> they are
> added in complete() callback
> c) Finally more than two and more queued requests are processed
> consistently,
> even if they were added in complete() callback
>
> The fix was successfully tested on MIPS based SoC with 4KEc CPU core and
> CI13612 USB core. Board successfully boots with NFS root using g_ether
> on ci13xxx udc.
>
> Signed-off-by: Artem Leonenko <tikkeri@xxxxxxxxx>
> ---
> drivers/usb/gadget/ci13xxx_udc.c | 14 ++++++++------
> 1 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/gadget/ci13xxx_udc.c
> b/drivers/usb/gadget/ci13xxx_udc.c
> index 03505cb..0164a81 100644
> --- a/drivers/usb/gadget/ci13xxx_udc.c
> +++ b/drivers/usb/gadget/ci13xxx_udc.c
> @@ -1794,18 +1794,20 @@ __acquires(mEp->lock)
>
> dbg_done(_usb_addr(mEp), mReq->ptr->token, retval);
>
> + if (!list_empty(&mEp->qh[mEp->dir].queue)) {
> + struct ci13xxx_req* mReqEnq;
> +
> + mReqEnq = list_entry(mEp->qh[mEp->dir].queue.next,
> + struct ci13xxx_req, queue);
> + _hardware_enqueue(mEp, mReqEnq);
> + }
> +
> if (!mReq->req.no_interrupt && mReq->req.complete != NULL) {
> spin_unlock(mEp->lock);
> mReq->req.complete(&mEp->ep, &mReq->req);
> spin_lock(mEp->lock);
> }
>
> - if (!list_empty(&mEp->qh[mEp->dir].queue)) {
> - mReq = list_entry(mEp->qh[mEp->dir].queue.next,
> - struct ci13xxx_req, queue);
> - _hardware_enqueue(mEp, mReq);
> - }
> -
> done:
> return retval;
> }
You did not add the fix to check the list status before calling
_hardware_enqueue in ep_queue function in this patch set.
--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--
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
