From owner-freebsd-arm@FreeBSD.ORG Tue Jul 2 04:48:26 2013 Return-Path: Delivered-To: arm@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 9AA33AED; Tue, 2 Jul 2013 04:48:26 +0000 (UTC) (envelope-from gonzo@id.bluezbox.com) Received: from id.bluezbox.com (id.bluezbox.com [88.198.91.248]) by mx1.freebsd.org (Postfix) with ESMTP id 1D05715BD; Tue, 2 Jul 2013 04:48:25 +0000 (UTC) Received: from [207.6.254.8] (helo=[192.168.1.65]) by id.bluezbox.com with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.77 (FreeBSD)) (envelope-from ) id 1UtsVp-000FRk-TP; Mon, 01 Jul 2013 21:48:24 -0700 Content-Type: text/plain; charset=iso-8859-1 Mime-Version: 1.0 (Mac OS X Mail 6.3 \(1503\)) Subject: Re: Beaglebone USB driver (Mentor Graphics OTG) From: Oleksandr Tymoshenko In-Reply-To: <51CBDFEA.7050203@bitfrost.no> Date: Mon, 1 Jul 2013 21:48:03 -0700 Content-Transfer-Encoding: quoted-printable Message-Id: References: <51608AA4.2020804@bluezbox.com> <51611A7B.2010105@bitfrost.no> <0927BB4C-6917-408D-B102-AB98F72314B6@bluezbox.com> <51CBDFEA.7050203@bitfrost.no> To: Hans Petter Selasky X-Mailer: Apple Mail (2.1503) Sender: gonzo@id.bluezbox.com X-Spam-Level: -- X-Spam-Report: Spam detection software, running on the system "id.bluezbox.com", has identified this incoming email as possible spam. The original message has been attached to this so you can view it (if it isn't spam) or label similar future email. If you have any questions, see The administrator of that system for details. Content preview: On 2013-06-26, at 11:47 PM, Hans Petter Selasky wrote: > On 06/27/13 02:53, Oleksandr Tymoshenko wrote: >> >> On 2013-04-07, at 12:04 AM, Hans Petter Selasky wrote: >> >>> On 04/06/13 22:50, Oleksandr Tymoshenko wrote: >>>> Hello, >>>> >>>> This is first iteration of Host Mode support for Mentor Graphics >>>> OTG USB controller. I tested it by building kernel with USB memory >>>> stick mounted as /usr/obj, resulting kernel was bootable and worked fine. >>>> I reused some ideas (mostly for channel-management) from >>>> DWT OTG driver. >>>> >>>> Some pieces are still missing: >>>> - Support for SPLIT transactions, I don not have high speed hub >>>> right now to test it, but implementing it should be really >>>> straighforward. >>>> - Isochronous transfers. I do not have hardware to test this. Does >>>> anybody have any suggestion about simple use case? >>>> - Control Data OUT transaction >>>> - Wrapper for atmel HW has not ben synced with new core logic requirements >>>> yet >>>> >>>> Please review and test. I tested it only with gcc-built kernel/world. >>>> Now when >>>> first iteration is finished I'm going to update all my boards to new >>>> world order >>>> (clang/EABI) and re-test this stuff. >>>> >>>> Patch: >>>> http://people.freebsd.org/~gonzo/arm/patches/beaglebone-musb.diff >>> >>> Hi, >>> >>> Looks like you've got the grasp of the USB controller stuff :-) >>> >>> Some comments: >>> >>> 1) Use DPRINTFN(-1, ...) instead of printf() for all printf() that are not part of boot dmesg. >>> >>> + break; >>> + default: >>> + td->transfer_type = 0; >>> + printf("Invalid USB speed: %d\n", speed); >>> + break; >>> + } >>> >>> >>> 2) You should implement if HOST mode, support for SUSPEND and RESUME. See EHCI driver. Basically what you need is: >>> >>> a) USB transfers are stopped/paused. I know there is a hack you need if the host transfer cancel hangs, and that is to write a dummy device address and wait for the USB transfer to error out after 250 us max. >>> >>> b) switch on USB suspend signalling. >>> >>> >>> At resume: >>> >>> c) do resume [...] Content analysis details: (-2.9 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP 0.0 URIBL_BLOCKED ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [URIs: totalphase.com] -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] Cc: arm@freebsd.org, usb@freebsd.org X-BeenThere: freebsd-arm@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Porting FreeBSD to the StrongARM Processor List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 02 Jul 2013 04:48:26 -0000 On 2013-06-26, at 11:47 PM, Hans Petter Selasky wrote: > On 06/27/13 02:53, Oleksandr Tymoshenko wrote: >>=20 >> On 2013-04-07, at 12:04 AM, Hans Petter Selasky = wrote: >>=20 >>> On 04/06/13 22:50, Oleksandr Tymoshenko wrote: >>>> Hello, >>>>=20 >>>> This is first iteration of Host Mode support for Mentor Graphics >>>> OTG USB controller. I tested it by building kernel with USB memory >>>> stick mounted as /usr/obj, resulting kernel was bootable and worked = fine. >>>> I reused some ideas (mostly for channel-management) from >>>> DWT OTG driver. >>>>=20 >>>> Some pieces are still missing: >>>> - Support for SPLIT transactions, I don not have high speed hub >>>> right now to test it, but implementing it should be really >>>> straighforward. >>>> - Isochronous transfers. I do not have hardware to test this. Does >>>> anybody have any suggestion about simple use case? >>>> - Control Data OUT transaction >>>> - Wrapper for atmel HW has not ben synced with new core logic = requirements >>>> yet >>>>=20 >>>> Please review and test. I tested it only with gcc-built = kernel/world. >>>> Now when >>>> first iteration is finished I'm going to update all my boards to = new >>>> world order >>>> (clang/EABI) and re-test this stuff. >>>>=20 >>>> Patch: >>>> http://people.freebsd.org/~gonzo/arm/patches/beaglebone-musb.diff >>>=20 >>> Hi, >>>=20 >>> Looks like you've got the grasp of the USB controller stuff :-) >>>=20 >>> Some comments: >>>=20 >>> 1) Use DPRINTFN(-1, ...) instead of printf() for all printf() that = are not part of boot dmesg. >>>=20 >>> + break; >>> + default: >>> + td->transfer_type =3D 0; >>> + printf("Invalid USB speed: %d\n", = speed); >>> + break; >>> + } >>>=20 >>>=20 >>> 2) You should implement if HOST mode, support for SUSPEND and = RESUME. See EHCI driver. Basically what you need is: >>>=20 >>> a) USB transfers are stopped/paused. I know there is a hack you need = if the host transfer cancel hangs, and that is to write a dummy device = address and wait for the USB transfer to error out after 250 us max. >>>=20 >>> b) switch on USB suspend signalling. >>>=20 >>>=20 >>> At resume: >>>=20 >>> c) do resume signalling, similar to EHCI/UHCI I think. >>>=20 >>> d) switch on channel tokens. >>>=20 >>> case UHF_PORT_SUSPEND: >>> + if (sc->sc_mode =3D=3D MUSB2_HOST_MODE) >>> + printf("TODO: Set UHF_PORT_SUSPEND\n"); >>> + break; >>>=20 >>>=20 >>>=20 >>> 3) Make sure that channels are not generating tokens if they are = aborted / cancelled / timedout. This can not be verified using a USB = mass storage device. Verify this by connecting a USB serial adapter. Try = to open/close /dev/cuaU0. Make sure it does not loose any bytes and that = channel cancel does not hang forever. >>=20 >>=20 >=20 > Hi, >=20 >> Thanks for review. Took me quite some time to get back >> to the driver but here is updated version that addresses some >> of the issues you've mentioned: >> = http://people.freebsd.org/~gonzo/arm/patches/beaglebone-usb-20130626.diff >>=20 >> It fixes several bugs, adds proper SPLIT transactions support and >> suspend/resume signalling. I tested it with urtwn-based WiFi chip, >> mass storage device, USB keyboard connected directly and using >> high-speed hub. >>=20 >> Suspend/resume is not 100% complete though. I can use USB serial >> port adapter if it's suspended/resumed unconnected. But it stuck if I = do >> the test while connected. Is it the right way to test it? >=20 > Which suspend you mean system/resume suspend or USB suspend/resume? I was talking about USB suspend/resume. I do not thin we have working=20 suspend/resume for ARM yet so I don't have reliable way to test it. >=20 > You should implement a musb_set_hw_power_sleep() too. This handles = system going into suspend. You should probably reset that adapter at = this point. >=20 > static void > musb_set_hw_power_sleep(struct usb_bus *bus, uint32_t state) > { > struct ehci_softc *sc =3D EHCI_BUS2SC(bus); >=20 > switch (state) { > case USB_HW_POWER_SUSPEND: > case USB_HW_POWER_SHUTDOWN: > ehci_suspend(sc); > break; > case USB_HW_POWER_RESUME: > ehci_resume(sc); > break; > default: > break; > } > } >=20 > If the musb requires that you stop tokens before going in and out of = suspend, you need to add functions like this: >=20 >=20 > ehci_device_resume/ehci_device_suspend >=20 >>=20 >> On the related note: can somebody suggest budget USB protocol = analyzer >> with support for high-speed bus? >>=20 >=20 > http://www.totalphase.com/products/beagle_usb480/ >=20 > There are more, but I don't have the list right now. >=20 > You can probably just remove this check. We don't support LOW speed in = device mode. >=20 > - > - if ((udev->speed !=3D USB_SPEED_FULL) && > - (udev->speed !=3D USB_SPEED_HIGH)) { > - /* not supported */ > - return; > + if (sc->sc_mode !=3D MUSB2_HOST_MODE) { > + if ((udev->speed !=3D USB_SPEED_FULL) && > + (udev->speed !=3D USB_SPEED_HIGH)) { > + /* not supported */ > + return; > + } > } Removed > The musbotg_channel_free function is not sufficient! You need to = program a non-existing device address like 127 and wait for 2 ms I = recommend. The ABORT bits don't work with this controller last time I = tried them! You can verify this by loading and unloading the wireless = driver. I guess that IN tokens don't stop when you unload the driver. = You will see this using an USB analyzer. >=20 > +#define MUSB2_MAX_DEVICES (USB_MAX_DEVICES - 1) >=20 > +static void=09 > +musbotg_channel_free(struct musbotg_softc *sc, struct musbotg_td *td) > +{ > + > + DPRINTFN(1, "ep_no=3D%d\n", td->channel); > + > + if (sc->sc_mode =3D=3D MUSB2_DEVICE_MODE) > + return; > + > + if (td =3D=3D NULL) > + return; > + if (td->channel =3D=3D -1) > + return; > + > + musbotg_ep_int_set(sc, td->channel, 0); > + sc->sc_channel_mask &=3D ~(1 << td->channel); > + td->channel =3D -1; >=20 > /* force transfer failure */ > MUSB2_WRITE_1(sc, MUSB2_REG_RXFADDR(0), 127); > XXX channel should not be re-used until after 2*125us. Can probably = use ticks for this! >=20 > +} >=20 > The 2ms can be done asynchronosly by implementing this function. See = EHCI driver. >=20 > static void > musbotg_get_dma_delay(struct usb_device *udev, uint32_t *pus) > { >=20 > if (host_mode) > *pus =3D 2000; /* microseconds */ > else > *pus =3D 0; > } Writing 127 to FADDR breaks driver. It can't even attach device = properly.=20 I do not completely understand the scenario behind this requirement. My=20= recollection is: when we cancel IN transaction somehow we should ensure=20= that function that currently handles it does not stuck forever. =46rom = what=20 I see it's just impossible. There is internal NAK timer that fails the = transaction once it reached configured value (or 3 by default AFAIR). Isn't it = enough? I tried unloading active uwrtn driver - it unloads with some delay but = as=20 far as I can see from logs delay is not due to USB internal transactions = code it must be upper layer.=20 Could you, please, elaborate on the matter? Here is updated version of the patch:=20 = http://people.freebsd.org/~gonzo/arm/patches/beaglebone-usb-20130701.diff Besides cosmetic fixes I also synced = dev/usb/controller/musb_otg_atmelarm.c to the latest changes of core logic: added required wrappers and some=20 initializations.=20 I do not longer have access to USB analyzer so my debugging abilities is somewhat limited now.=20=