On 12/11/2010 1:35 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
>
> And as a part of the fix to make g_ether (and especially booting with
> NFS root) I had
> to increase NET_IP_ALIGN to mips32 natural value (4).
>
> The fix was successfully tested on MIPS based SoC with 4KEc core and
> CI13612 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 | 15 ++++++++-------
> drivers/usb/gadget/u_ether.h | 5 +++++
> 2 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/usb/gadget/ci13xxx_udc.c
> b/drivers/usb/gadget/ci13xxx_udc.c
> index d788c7d..2c06cad 100644
> --- a/drivers/usb/gadget/ci13xxx_udc.c
> +++ b/drivers/usb/gadget/ci13xxx_udc.c
> @@ -1789,18 +1789,17 @@ __acquires(mEp->lock)
>
> dbg_done(_usb_addr(mEp), mReq->ptr->token, retval);
>
> - 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);
You should use another variable for storing the request pointer. Because
mReq is used below again to call the request complete function.
This patch worked fine (ping works with g_ether) for me after making
this change. I did not modify NET_IP_ALIGN.
> _hardware_enqueue(mEp, mReq);
> }
>
> + if (!mReq->req.no_interrupt && mReq->req.complete != NULL) {
> + spin_unlock(mEp->lock);
> + mReq->req.complete(&mEp->ep, &mReq->req);
> + spin_lock(mEp->lock);
> + }
> done:
> return retval;
> }
> @@ -2170,7 +2169,9 @@ static int ep_queue(struct usb_ep *ep, struct
> usb_request *req,
> mReq->req.actual = 0;
> list_add_tail(&mReq->queue, &mEp->qh[mEp->dir].queue);
>
> - retval = _hardware_enqueue(mEp, mReq);
> + if (list_is_singular(&mEp->qh[mEp->dir].queue))
> + retval = _hardware_enqueue(mEp, mReq);
> +
> if (retval == -EALREADY || retval == -EBUSY) {
> dbg_event(_usb_addr(mEp), "QUEUE", retval);
> retval = 0;
Given that _hardware_enqueue is called only when endpoint is in
un-primed state, may be we should pass the value returned by
_hardware_enqueue back to usb_ep_queue() caller.
> diff --git a/drivers/usb/gadget/u_ether.h b/drivers/usb/gadget/u_ether.h
> index 3c8c0c9..c4bb92e 100644
> --- a/drivers/usb/gadget/u_ether.h
> +++ b/drivers/usb/gadget/u_ether.h
> @@ -23,6 +23,11 @@
> #ifndef __U_ETHER_H
> #define __U_ETHER_H
>
> +#if defined(CONFIG_USB_GADGET_CI13XXX)
> +#define NET_IP_ALIGN 4
> +#endif
> +
> +
> #include <linux/err.h>
> #include <linux/if_ether.h>
> #include <linux/usb/composite.h>
> --
> 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
--
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
