Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 30 Dec 2004 11:16:31 -0700 (MST)
From:      "M. Warner Losh" <imp@bsdimp.com>
To:        jb@cimlogic.com.au
Cc:        current@freebsd.org
Subject:   Re: USB problems
Message-ID:  <20041230.111631.13597845.imp@bsdimp.com>
In-Reply-To: <20041228010938.GA39686@freebsd3.cimlogic.com.au>
References:  <20041228010938.GA39686@freebsd3.cimlogic.com.au>

next in thread | previous in thread | raw e-mail | index | archive | help
In message: <20041228010938.GA39686@freebsd3.cimlogic.com.au>
            John Birrell <jb@cimlogic.com.au> writes:
: 1. The USB sub-system doesn't handle loading and unloading drivers properly.
:    If a driver is unloaded when a USB device is still attached, the next
:    time the driver is loaded, the kernel panics. This might not be such
:    a problem to normal users because they don't have a need to do that, but
:    during driver development when you want to load and unload repeatedly,
:    it's a pain.

I don't see this.  Can you give a more concrete example?  I've done
work in this area and I believe that it is almost working correctly.

: 3. The usbd_bulk_transfer function calls tsleep() to wait for streaming
:    data to become available. On current, this bumps into a KASSERT in
:    msleep because Giant is not locked and no mutex has been supplied.
:    In my driver, I need to run an 'encoder' thread which calls
:    usbd_bulk_transfer() to gobble the incoming MPEG data stream. While
:    this is going on, there is no syscall in progress because the
:    application is off doing other things. It might be looking at the
:    mmaped buffer or it may not.
: 
:    For FreeBSD, usbd_bulk_transfer() needs to change to allow the driver
:    to specify it's mutex. I don't know what the implications are for
:    uhci given that it hasn't been converted to use mutexes. Can anyone
:    comment on that?

I have changes to make sure this can't happen as well.  But you must
hold Giant whenever you call into the usb subsystem.

: I'd love to get rid of the attach_args structure and just pass a
: usbd_device_handle into the drivers, with struct usbd_device containing
: a couple of extra variables for use during matching.

I'd love to remove that attach_args and replace it with nothing.

: I'd like to remove the subdevs array from struct usbd_device under
: FreeBSD because the parent/child device tree should only be managed by
: bus routines. The relationship between a USB device and it's device_t
: should just be a field in struct usbd_device and it should be cleared
: when there is no device_t. Leaving a device_t hanging around when a
: driver had been unloaded is what is causing the panics I am seeing.

I'm working on this.  However, many of the panics around this area
were fixed just before 5.3.

: I think that the uhub driver should have a uhub_driver_added routine rather
: than using the generic one. I'd really like to see the uhub code re-match
: vendor/product codes when a new driver is added, and if the new driver
: matches, then uhub should detach ugen and use the new driver. Similarly,
: when a driver is unloaded, if a USB device is still present, it should go
: back to ugen.

I already deal with these things, are you sure you're looking at the
current code?  Well, I Don't deal with ugen, but when I have to load
drivers I don't include it in my kernel.  There's no harm in that.
I'm working on a generic newbus way of dealing with this situation,
but acpi does non-idempotent things in its probe routines :-(.

Warner



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20041230.111631.13597845.imp>