From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from co202.xi-lite.net ([149.6.83.202]:34450 "EHLO co202.xi-lite.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755309Ab0KSRQV (ORCPT ); Fri, 19 Nov 2010 12:16:21 -0500 Message-ID: <4CE6B0E0.2030900@parrot.com> Date: Fri, 19 Nov 2010 18:16:16 +0100 From: Matthieu CASTET MIME-Version: 1.0 Subject: Re: [PATCH v2] USB: Add MSM USB Device Controller driver References: <20101110021259.GA16558@codeaurora.org> <320241.1412.qm@web180311.mail.gq1.yahoo.com> In-Reply-To: Content-Type: multipart/mixed; boundary="------------060902050702050004060401" Sender: linux-arm-msm-owner@vger.kernel.org List-ID: To: Brian Swetland Cc: David Brownell , Pavan Kondeti , "greg@kroah.com" , "linux-usb@vger.kernel.org" , "linux-arm-msm@vger.kernel.org" , Mike Lockwood --------------060902050702050004060401 Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit Hi, Brian Swetland a écrit : > On Tue, Nov 9, 2010 at 6:54 PM, David Brownell wrote: >> >> --- On Tue, 11/9/10, Pavan Kondeti 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 --------------060902050702050004060401 Content-Type: text/plain; name="diff" Content-Transfer-Encoding: base64 Content-Disposition: inline; filename="diff" ZGlmZiAtLWdpdCBhL2RyaXZlcnMvdXNiL2dhZGdldC9jaTEzeHh4X3VkYy5jIGIvZHJpdmVy cy91c2IvZ2FkZ2V0L2NpMTN4eHhfdWRjLmMKaW5kZXggMDI1MmJiYy4uNWE2NWNkYSAxMDA2 NDQKLS0tIGEvZHJpdmVycy91c2IvZ2FkZ2V0L2NpMTN4eHhfdWRjLmMKKysrIGIvZHJpdmVy cy91c2IvZ2FkZ2V0L2NpMTN4eHhfdWRjLmMKQEAgLTIxNDAsNiArMjE0MCw3IEBAIHN0YXRp YyBpbnQgZXBfcXVldWUoc3RydWN0IHVzYl9lcCAqZXAsIHN0cnVjdCB1c2JfcmVxdWVzdCAq cmVxLAogCXN0cnVjdCBjaTEzeHh4X3JlcSAqbVJlcSA9IGNvbnRhaW5lcl9vZihyZXEsIHN0 cnVjdCBjaTEzeHh4X3JlcSwgcmVxKTsKIAlpbnQgcmV0dmFsID0gMDsKIAl1bnNpZ25lZCBs b25nIGZsYWdzOworCWludCBlbXB0eTsKIAogCXRyYWNlKCIlcCwgJXAsICVYIiwgZXAsIHJl cSwgZ2ZwX2ZsYWdzKTsKIApAQCAtMjE3MCwxNSArMjE3MSwxOCBAQCBzdGF0aWMgaW50IGVw X3F1ZXVlKHN0cnVjdCB1c2JfZXAgKmVwLCBzdHJ1Y3QgdXNiX3JlcXVlc3QgKnJlcSwKIAog CWRiZ19xdWV1ZShfdXNiX2FkZHIobUVwKSwgcmVxLCByZXR2YWwpOwogCisJZW1wdHkgPSBs aXN0X2VtcHR5KCZtRXAtPnFoW21FcC0+ZGlyXS5xdWV1ZSk7CiAJLyogcHVzaCByZXF1ZXN0 ICovCiAJbVJlcS0+cmVxLnN0YXR1cyA9IC1FSU5QUk9HUkVTUzsKIAltUmVxLT5yZXEuYWN0 dWFsID0gMDsKIAlsaXN0X2FkZF90YWlsKCZtUmVxLT5xdWV1ZSwgJm1FcC0+cWhbbUVwLT5k aXJdLnF1ZXVlKTsKIAotCXJldHZhbCA9IF9oYXJkd2FyZV9lbnF1ZXVlKG1FcCwgbVJlcSk7 Ci0JaWYgKHJldHZhbCA9PSAtRUFMUkVBRFkgfHwgcmV0dmFsID09IC1FQlVTWSkgewotCQlk YmdfZXZlbnQoX3VzYl9hZGRyKG1FcCksICJRVUVVRSIsIHJldHZhbCk7Ci0JCXJldHZhbCA9 IDA7CisJaWYgKGVtcHR5KSB7CisJCXJldHZhbCA9IF9oYXJkd2FyZV9lbnF1ZXVlKG1FcCwg bVJlcSk7CisJCWlmIChyZXR2YWwgPT0gLUVBTFJFQURZIHx8IHJldHZhbCA9PSAtRUJVU1kp IHsKKwkJCWRiZ19ldmVudChfdXNiX2FkZHIobUVwKSwgIlFVRVVFIiwgcmV0dmFsKTsKKwkJ CXJldHZhbCA9IDA7CisJCX0KIAl9CiAKICBkb25lOgo= --------------060902050702050004060401--