USB ugen_set_interface issue (+ possible patch)

[Jay Cornwall]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi

First of all, my humblest apologies for this perhaps slightly offtopic posting 
- - I'm using and testing with 5.0-RELEASE at the moment, but I couldn't find a 
more appropriate list to post my questions.

I'm currently investigating a problem with the Speedtouch software 
(http://speedtouch.sf.net/) for the Speedtouch USB ADSL modem, and FreeBSD 
5.0. The software has worked up until FreeBSD 4.8, but causes kernel panics 
when initialising the modem in 5.0. I've done some investigative work into 
this, and think I have a partial solution.

The problem appears to be in /sys/dev/usb/ugen.c, within the 
ugen_set_interface function. After setting the interface, subsequent calls to 
ugen_destroy_devnodes result in a kernel panic, after calling destroy_dev 
within /sys/dev/kern/kern_conf.c:

    if (!(dev->si_flags & SI_NAMED)) {
        printf( "WARNING: Driver mistake: destroy_dev on %d/%d\n",
            major(dev), minor(dev));
        panic("don't do that");
        return;
    }

This code had been introduced since FreeBSD 4.8 for use with the new DevFS 
system; it appears that in previous versions this condition was not checked, 
and caused no problems.

I placed assertions at the start and end of ugen_set_interface to check this 
condition was true for all dev_t's that ugen_destroy_devnodes would call 
destroy_dev on. The assertion was true at the start of the function, but 
false at the end.

A comment in the Speedtouch source code seems to confirm this:
    /* BUG: at least in FreeBSD 4.3, USB_SET_ALTINTERFACE does not
     * make the necessary devices for the interface.  So we call
     * USB_SET_CONFIG to ensure they are created.  But this will
     * fail if any endpoints (other than the control) are open.
     * Of course, the devices might already exist, so failure
     * to set the configuration does not mean it will not work.
     */

It goes on to use a USB_SET_CONFIG ioctl to create the devices, but this no 
longer works since the introduction of a ugen_destroy_devnodes call to 
ugen_set_config (which causes the above-described kernel panic).

I can't see any reason why ugen_set_interface shouldn't destroy the old 
devices, and create new ones, but I'm inexperienced with this code (and USB 
in general). If I've made a false assumption, please correct me, as I'm 
always willing to learn more. :)

My proposal for a patch to fix this is fairly simple, and is attached to this 
email. It simply adds a ugen_destroy_devnodes/ugen_make_devnodes pair to the 
call, and it eliminates the kernel panic I'm seeing with the Speedtouch 
software.

I'm open to your comments. :o)

(BTW, there are still further issues with the Speedtouch software, but I'm 
working on that :))

Cheers,
Jay

- -- 
http://www.evilrealms.net/ - Systems Administrator & Developer
http://www.ic.ac.uk/ - Imperial College, 2nd year CS student
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.1 (GNU/Linux)

iD8DBQE+uGqqfJLn3O/2GbERAtBzAJ9I9w66+D2l65GXbdDcrDFjZ11idwCg5/0v
tWE8IiPLawsnEoGjOk0ylGo=
=cqqZ
-----END PGP SIGNATURE-----