Subject: Re: USB stack needs early review (Re: Someone should fix our USB stack...)
To: ITOH Yasufumi <itohy@netbsd.org>
From: Jason Thorpe <thorpej@shagadelic.org>
List: tech-kern
Date: 03/28/2007 10:22:13
On Mar 28, 2007, at 7:07 AM, ITOH Yasufumi wrote:

> Hello,
>
> Here's today's version waiting for review.

I would suggest checking this code into a branch in the CVS tree.

>
> What's new:
>  - test uhci(4) and fix some bugs
>  - add memory alloc/dealloc tracing code (USBMEM_DEBUG: enabled by  
> default)
>  - fix memory leaks in ohci(4), probably also in ehci(4) and uhci(4),
>    which are also fixed
>  - add diagnostic panics for resource allocation on interrupt context.
>  - fix umass(4) to allocate resources beforehand (found by the diag  
> panic
>    above)
>  - usb.h: add UE_MAXPKTSZ(ep) and UE_MAXPKTSZ_MASK macros (for high- 
> speed
>    isochronous devices)
>  - still it sometimes panics, however
>
> ftp://ftp.netbsd.org/pub/NetBSD/misc/itohy/usbtest-20070328.tar.gz
>
> This applies to NetBSD 4.99.13 (before caddr_t removal).
>
> Started 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
>  - no visible changes to USB driver interface
>  - 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
>
> changed to USB device drivers
>  - umass: allocate memory beforehand to avoid resource allocation on
>    callback (interrupt context)
>
> 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
>
> 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.
>  - NOT TESTED, missing hardware
>
> ehci
>  - add lots of bus_dmamap_sync() operations, possibly too many
>  - NOT TESTED, missing hardware
>
> uhci
>  - add lots of bus_dmamap_sync() operations, possibly too many
>
> To do
>  - review, test, debug
>  - still possibility left to allocate memory in interrupt context  
> --- should
>    be fixed (by ad hoc manner or whole redesign?)
>
>
> Christos Zoulas writes:
>> In article <200703221525.l2MFP1GX013433@v057181.ppp.asahi-net.or.jp>,
>> ITOH Yasufumi <itohy@netbsd.org> wrote:
>>
>>> Patch most NetBSD changes, excluding
>>> - DMA memory "reserve"
>>> - caddr_t removal, expecting it will soon be reverted :D
>>
>> It will be a cold day in hell :-)
>
> No one shall be against to have caddr_t since Solaris has it.
>
> :-)
> -- 
> ITOH Yasufumi

-- thorpej