Alan Stern wrote:

So while this is a good bug to fix, I don't really like this
particular fix because of those related issues.


I gather then that you would like to keep disable_interface() but not enable_interface(). Instead, after unbinding the driver usb_unbind_interface() should call usb_set_interface() to restore altsetting 0. Obviously that shouldn't be done _before_ unbinding the driver.

Maybe keep neither -- just usb_set_interface(), which should in sequence (a) disable all the endpoints, canceling urbs, (b) try to reset the device state, with clean fault if the hardware's really gone away, and (c) re-enable the endpoints.

All of that is stuff that should be done when drivers get
unbound from the interface.  And it's more or less what
you're doing with disable()/enable() -- except that it also
resets the device state to something sane, not just the
endpoint state for usbcore and the hcd.

The only _additional_ things that need doing in the case
of driver unbinding are to scrub out the driver state,
by calling driver->disconnect().   Arguably that should be
done immediately before (b), but before (a) should work
just as well in many cases.


Finally, usb_set_interface() needs to be modified to remove the
special-case logic for interfaces with only 1 altsetting.  If the
SET_INTERFACE control request stalls, the function should manually clear
the HALT feature for each endpoint in the new altsetting.  I guess it
wouldn't hurt to include endpoint 0 as well.

I'd made that point about the special casing ... but I'm not sure what you're trying to say about ep0. (Sure, it'd be nice to have urbs associated with interfaces ... although fast-pathing argues to have them associate directly with endpoints. Either sort of change would be too major for this point in 2.6, IMO.)


usb_set_interface() does have the disadvantage of duplicating the work
done by disable_interface() -- does that really have to be in there?

Absolutely. Remember that the HC may know about the previous endpoint settings, which is what SET_INTERFACE is changing. So the endpoint disable logic needs to be called, so the HCD can clear out that state.

Or you might want to have the usb_set_interface() code call
your new routines to disable()/enable() the relevant setting.
That was another reason to look at this approach:  no point
in duplicating code.

- Dave





-------------------------------------------------------
This SF.net email is sponsored by: VM Ware
With VMware you can run multiple operating systems on a single machine.
WITHOUT REBOOTING! Mix Linux / Windows / Novell virtual machines at the
same time. Free trial click here: http://www.vmware.com/wl/offer/345/0
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to