Hi,
Brian Swetland a Ãcrit :
On Tue, Nov 9, 2010 at 6:54 PM, David Brownell <david-b@xxxxxxxxxxx> wrote:
--- On Tue, 11/9/10, Pavan Kondeti <pkondeti@xxxxxxxxxxxxxx> wrote:
Hi Matthieu,
This look like the arc/chipidea/mips ehci otg
core.
Yes. It is chipidea core for ARM.
Why can't you reuse the ci13xxx_udc.c driver
That basic approach is FAR PREFERABLE. Fix
the bugs once, tune once, and so forth, reuse
the ULPI support, etc. Work on more
platforms, since the silicon IP is reused.
You'll end up with more folk who can help
maintain the driver too, since the pool of
potential helpers won't be limited to those
who have/use MSM hardware.
Just be sure to cleanly factor the bus
(PCI vs MSM-s ARM platform flavor and
SoC glues (bus-related). That factoring
will likely be the hardest part; but there
are examples of similar stuff in Linux today.
The main headache is that this particular IP has different bugs in
different instantiations (I know, for example, it exists in Tegra with
a different set of issues around fetching descriptor heads and cache
alignment, on MSM7201A after extensive testing we discovered there was
no reliable way of adding a descriptor to a list of transactions once
that queue was active, etc...), so things that work in one SoC may
break another, etc, etc, but that's part of the adventure I suppose.
I certainly agree that one unified driver is the way to go if you can
make it all work.
The best way to handle this is to introduce flags in the driver. For
example look at drivers/mmc/host/sdhci.c (quirk flags).
But for now, let's make work msm version. We can add workaround for
other controller later.
Now you should check if it is better to start on ci13xxx_udc or make
generic your driver.
I had worked a bit on ci13xxx_udc, and the code is sometimes messy, hard
to understand.
Also to making work on our core, we need to the attached patch.
Matthieu
diff --git a/drivers/usb/gadget/ci13xxx_udc.c b/drivers/usb/gadget/ci13xxx_udc.c
index 0252bbc..5a65cda 100644
--- a/drivers/usb/gadget/ci13xxx_udc.c
+++ b/drivers/usb/gadget/ci13xxx_udc.c
@@ -2140,6 +2140,7 @@ static int ep_queue(struct usb_ep *ep, struct usb_request *req,
struct ci13xxx_req *mReq = container_of(req, struct ci13xxx_req, req);
int retval = 0;
unsigned long flags;
+ int empty;
trace("%p, %p, %X", ep, req, gfp_flags);
@@ -2170,15 +2171,18 @@ static int ep_queue(struct usb_ep *ep, struct usb_request *req,
dbg_queue(_usb_addr(mEp), req, retval);
+ empty = list_empty(&mEp->qh[mEp->dir].queue);
/* push request */
mReq->req.status = -EINPROGRESS;
mReq->req.actual = 0;
list_add_tail(&mReq->queue, &mEp->qh[mEp->dir].queue);
- retval = _hardware_enqueue(mEp, mReq);
- if (retval == -EALREADY || retval == -EBUSY) {
- dbg_event(_usb_addr(mEp), "QUEUE", retval);
- retval = 0;
+ if (empty) {
+ retval = _hardware_enqueue(mEp, mReq);
+ if (retval == -EALREADY || retval == -EBUSY) {
+ dbg_event(_usb_addr(mEp), "QUEUE", retval);
+ retval = 0;
+ }
}
done:
