Date: Mon, 1 Jul 2013 21:48:03 -0700 From: Oleksandr Tymoshenko <gonzo@bluezbox.com> To: Hans Petter Selasky <hps@bitfrost.no> Cc: arm@freebsd.org, usb@freebsd.org Subject: Re: Beaglebone USB driver (Mentor Graphics OTG) Message-ID: <A9072D24-1E28-47D2-A152-D962C74D1811@bluezbox.com> In-Reply-To: <51CBDFEA.7050203@bitfrost.no> References: <51608AA4.2020804@bluezbox.com> <51611A7B.2010105@bitfrost.no> <0927BB4C-6917-408D-B102-AB98F72314B6@bluezbox.com> <51CBDFEA.7050203@bitfrost.no>
next in thread | previous in thread | raw e-mail | index | archive | help
On 2013-06-26, at 11:47 PM, Hans Petter Selasky <hps@bitfrost.no> wrote: > On 06/27/13 02:53, Oleksandr Tymoshenko wrote: >>=20 >> On 2013-04-07, at 12:04 AM, Hans Petter Selasky <hps@bitfrost.no> = 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=
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?A9072D24-1E28-47D2-A152-D962C74D1811>