Subject: Re: USB stack needs early review (Re: Someone should fix our USB stack...)
To: None <tech-kern@netbsd.org>
From: ITOH Yasufumi <itohy@netbsd.org>
List: tech-kern
Date: 04/27/2007 00:48:33
Hello,

I rewrote DMA related code and implemented following API addition/change
(-: removed, +: added).


+usbd_status usbd_map_alloc(usbd_xfer_handle xfer)
	[need context]
	Pre-allocate resources need for mapping buffer/mbuf without sleep.

+void usbd_map_free(usbd_xfer_handle xfer)
	[need context]
	Release pre-allocated resources.

+usbd_status usbd_map_buffer(usbd_xfer_handle xfer, void *buf, u_int32_t size)
	[nocontext ok] map resources must have be allocated by usbd_map_alloc()
	Map buf (in kernel space) into DMA space.

+usbd_status usbd_map_buffer_mbuf(usbd_xfer_handle xfer, struct mbuf *chain)
	[nocontext ok] map resources must have be allocated by usbd_map_alloc()
	Map an mbuf into DMA space.

+void usbd_unmap_buffer(usbd_xfer_handle xfer)
	[nocontext ok] map resources must have be allocated by usbd_map_alloc()
	Undo mapping of usbd_map_buffer() or usbd_map_buffer_mbuf().

-usbd_xfer_handle usbd_alloc_xfer(usbd_device_handle dev)
+usbd_xfer_handle usbd_alloc_xfer(usbd_device_handle dev, usbd_pipe_handle pipe)
	[need context]
	Allocate an xfer.

+usbd_xfer_handle usbd_alloc_default_xfer(usbd_device_handle dev)
	[need context]
	Allocate an xfer with default pipe.


Now umass(9) uses the buffer mapping APIs for data transfer.
It also implements pre-allocation of resources.

ftp://ftp.netbsd.org/pub/NetBSD/misc/itohy/usbtest-20070426.tar.gz

This applies to NetBSD 4.99.13 (before caddr_t removal).

usbdi(9) interface is based on FreeBSD version, excluding
 - removal of portability code

Patch most NetBSD changes, excluding
 - DMA memory "reserve"
 - caddr_t removal, expecting it will soon be reverted :D
 - volatiles in DMA structure, since it should not be needed
   with proper bus_dmamap_sync(9)s

DMA/non-DMA memory management overhaul
 - Move all DMA related code to usb_mem.[ch]
   (add usb_alloc_buffer_dma(), usb_free_buffer_dma(), etc.).
   XXX Should usb_mem.[ch] be renamed as usb_mem_dma.[ch] ?
 - Add corresponding non-DMA code to usb_mem_nodma.[ch] .
   Currently just use malloc(9).
 - Above files are conditionally used by config framework (added
   attributes to conf/files and dev/usb/files.usb).
 - Add diagnostic panics when resource allocation is requested
   on interrupt context.

Allocate DMA/non-DMA buffer per host interface, not globally.
 advantage:	Buffers can be freed on detaching host interface.
		Activity of a host interface does not affect others.
 disadvantages:	It possibly consumes more memory.

API changes
 - usbd_alloc_xfer() is changed:
    old: usbd_xfer_handle usbd_alloc_xfer(usbd_device_handle dev);
    new: usbd_xfer_handle usbd_alloc_xfer(usbd_device_handle dev,
		usbd_pipe_handle pipe);
 - pipe argument of usbd_setup_*xfer() are now unused
   XXX the pipe argument should be removed?
 - add mapping APIs
 - async request will be processed as a task (kernel thread context),
   and delayed to some extent
 - usbdivar.h: struct usbd_xfer: renamed a member "allocbuf" to "hcbuffer"
   (mapped/allocated/refered buffer for HCI driver)
 - usb_port.h: change usb_proc_ptr from  struct ptoc *  to struct lwp *
 - usb_port.h: add usb_sigproc_ptr for psignal(9) (struct proc *)
 - usb.h: add UE_MAXPKTSZ(ep) and UE_MAXPKTSZ_MASK macros for USB 2.0

changes to USB device drivers
 - atu, aue, axe, cdce, cue, kue, rum, udav, upl, ural, url,
   uaudio, ubt, ucom, ugen, uhidev, uirda, ulpt, umidi, urio,
   uscanner, ustir, utoppy:
    * catch up API change of usbd_alloc_xfer()
 - umass, usscanner:
    * catch up API change of usbd_alloc_xfer()
    * eliminate memory copy for large transfer

ohci
 - free resources on detach
 - add lots of bus_dmamap_sync() operations
 - simplify the code of loading std chain
 - rewrite code of looking up TD/ITD from DMA addr by using allocation chunk
 - add workaround for CMD Tech 670 and 673 chipsets
 - make sure resources are not allocated in interrupt context
 - add support for mapping buffer and mbuf

slhci
 - allocate xfer and slhci_xfer at once, and simplify relevant code
 - add slhci_detach()
 - remove second arg of slhci_attach() since it is the same as the first arg.
 - add support for "mapping" (no, it doesn't map since it doesn't do DMA)
   buffer and mbuf
 - NOT TESTED, missing hardware

ehci
 - add lots of bus_dmamap_sync() operations, possibly too many
 - make sure resources are not allocated in interrupt context
 - add support for mapping buffer and mbuf
 - NOT TESTED, missing hardware

uhci
 - add lots of bus_dmamap_sync() operations, possibly too many
 - make sure resources are not allocated in interrupt context
 - add support for mapping buffer and mbuf

To do
 - review, test, debug
 - rewrite network drivers to utilize usbd_map_buffer_mbuf()


In article <200703310736127560070004MA26@nm04mta.dion.ne.jp>
isaki@NetBSD.org writes:
> Could you separate sl811hs.c into two parts?
>  1) a lot of your improvement including import from FreeBSD,
>     like sl11_detach(), memory allocation, etc...
>  2) synchronization with logic of FreeBSD, like sl11_speed(),
>     sl11_reset(), (and also sl11read()/sl11write()??)
> 
> At least, "#if 1"-ed part in sl11_speed(), sl11_reset() does
> not work at my SL811HS/T on x68k.  I'm not sure, but it is
> caused by difference of chip revision.
> (I didn't test with the entire patch of you, sorry...)

OK, I "#if 0"-out the FreeBSD-derived portions.

In article <68E9CD46-B1AF-4930-A7E5-302F82BFB670@shagadelic.org>
thorpej@shagadelic.org writes:
> I would suggest checking this code into a branch in the CVS tree.

Mmm, I'll try to...

In article <200704251303.16418.hselasky@c2i.net>
hselasky@c2i.net writes:
> When you pre-allocate resources you need an upper bound on the data-buffer 
> also?

I'm using 64KB (MAXPHYS) and 17 DMA segments (USB_DMA_NSEG) for
the upper bound.

Regards,
-- 
ITOH Yasufumi