cvs commit: src/sys/dev/usb usbdi.c

[Daan Vreeken [PA4DAN]]

Hi Ian,

On Wednesday 06 December 2006 03:43, you wrote:
> In message <200611291204.43704.Danovitsch at vitsch.net>, "Daan Vreeken
> [PA4DAN]"
>
> writes:
> >Hi Ian,
> >
> >On Wednesday 29 November 2006 11:58, you wrote:
> >> Thanks for all the details - I'll try to put together a patch in the
> >> next few days that adds bus_dmamap_sync() calls whereever there are
> >> shared access ordering requirements in the host controller drivers.
> >> As Olivier mentioned, bus_dma(9) says that this should be done even
> >> for BUS_DMA_COHERENT allocations, so adding them may fix problems on
> >> other platforms too.
> >
> >Ok, thanks for looking into this!
> >At the moment I'm working on a driver for the USB device port that's on
> > the ARM board. The driver is about half way being usefull. Once it's
> > finished I can debug USB problems from the device perspective and see
> > what data actually goes over the wire. That should show us what exactly
> > goes wrong without the bus_dmapmap_sync() calls.
>
> I haven't yet found the time to look into this unfortunately - one
> thing that did become obvious however is that the interface to the
> USB host controller hardware does need something very close to
> BUS_DMA_COHERENT mappings. For example, the host controllers allow
> you to link certain structures into linked lists that the hardware
> is constantly traversing, without ever halting the hardware (you
> first set up the "next" pointer in the new entry and then change a
> "next" pointer in a live entry to point at the new entry). This
> kind of update wouldn't be possible with bounce buffers, for example,
> because a BUS_DMASYNC_PREWRITE call to update the live "next" pointer
> could inadvertently clobber other state in the live object.

I have done some digging and I think the _PREWRITE calls aren't needed for the 
status registers of the controller. The documentation of the OHCI-controller 
inside the ARM processor can be found here :
http://www.atmel.com/dyn/resources/prod_documents/doc1768.pdf (Page 590)

The documentation talks about two memory regions ("Device registers in memory 
space" and "Shared ram"). The pointers pointing to the chain of transfers 
reside in the device registers. Correct me if I'm wrong, but I believe the 
ohci registers are directly mapped into memory on the ARM. (Meaning that a 
write to a register will go directly into the register instead of into the 
cache.)
If this is the case, syncing the data in the "shared ram" once before 
adjusting the 'next' pointers would be enough.

(Disclaimer: I'm still rather new to this whole cache-has-to-be-flushed thing, 
so please correct me if I'm wrong :)

> Assuming that the mapping is "coherent enough" for BUS_DMASYNC_PREWRITE
> to only write what needs to be written, then the next issue is
> whether it is really necessary to go through a potentally large
> number of transfer descriptors, calling BUS_DMASYNC_PREWRITE on
> each one? Is there any hardware we support where coherent mappings
> require a per-region flush/barrier call to ensure writes have taken
> place? On the arm platform it seemed that just a global flush/barrier
> was enough, given the effect of the usbdi.c patch, and on i386/amd64
> no flush/barrier seems to be required at all.

Can't help you there...

I can help debug the problems we're seeing on the ARM platform though. During 
the past week I have written a driver for the USB Device Port of the ARM. It 
now provides a file in /dev/ that can be used with a simple userland 
application to create a USB device. I've ported the firmware I once wrote for 
a custom USB embedded device to use this userland interface. With this I can 
see the actual data that moves over the USB cable to and from the device.
Because I still wasn't sure what was really going wrong without the 
SYNC_PREWRITE patch I connected the USB device port of the board to the USB 
host port. (The ARM board can now be used to debug itself ;-)

The userland application generates logs and I have collected the logs of a 
couple of failed (no patch) and successfull (with patch) enumerations of the 
device. The logs can be found here :
http://vitsch.net/bsd/patches/2006-12-07_usb_arm/

Using "diff" to extract the differences between a failed and a successfull 
enumeration gives the following :

--- log.ok1     Thu Dec  7 19:09:36 2006
+++ log.fail4   Thu Dec  7 21:49:48 2006
@@ -35,28 +35,26 @@
    send str manuf
 going to send 2 bytes
 <-- sending 16 03
---> SETUP: 80 06 01 03 09 04 16 00
+--> SETUP: 80 06 01 03 09 04 04 00
  get descriptor
   string descriptor
    send str manuf
-going to send 22 bytes
-<-- sending 16 03 44 00 61 00 6E 00
-<-- sending 6F 00 76 00 69 00 74 00
-<-- sending 73 00 63 00 68 00
+going to send 4 bytes
+<-- sending 16 03 44 00

As you can see, the successfull attempt requests 0x16 bytes of the 	
manufacturer string (which also is 0x16 bytes long), while the failed attempt 
only requests 0x04 bytes (or some other random number)...

Different runs give different numbers as can be seen in the dmesg output :
usbd_get_string_desc: expected 24, got 22
usbd_get_string_desc: expected 24, got 22
usbd_get_string_desc: expected 22, got 4

So now I'm pretty sure that the problem is indeed with the content of the 
"SETUP" packets. The intended content isn't making it to the device, while 
the associated data seems to be correct every single time.

Hope this helps.
-- 
Daan