How about this instead: ... By the way, hopefully the infinite recursion bit will be dealt with in the driver core, at which point that part of the comment can go away.
Seems fair to me. That was your text!
Will you take care of it then? My plan (which I hope you agree with) is that you push the core changes to Greg, and I send the usbfs changes
But usbfs is (unfortunately) part of usbcore ... which is why I was thinking of this in structural terms: one patch to finish the changes related to the driver model (remove interface.driver, unbork locking), and the other stuff (mostly related to dev->serialize). Plus the driver patches (see below).
Here's my update to usb_driver_release_interface() by the way. It should terminate the recursion without needing a driver model core change (which, however good an idea, may be harder to merge). Compiles, otherwise untested ... does it look right?
(= things in devio.c, including your changes, except for the ->driver to ->dev->driver changes). By the way, my current patch for fixing up drivers looks like this:
This makes me count three patches, which (seems to me) should be merged in about this order:
- Most drivers that use usb_driver_{claim,release}_interface()
need changes to cope with the "half bound" state going away.
Basically a version of that patch ... unless the attached
patch removes the requirement for such changes?- Actually make usb_interface.driver vanish. That changes the semantics of the claim/release calls ... so in a way it'd be better to change at least some call signature (release would be simple) to force compile-time errors for unmodified drivers.
- Other usbfs changes, relying on those updated semantics. And at least the reset() changes depend on all four of those reset/enumeration patches merging...
Somehow I don't see any of that getting into 2.6.2!
Greg -- got any suggestions about how to merge this stuff?
- Dave
--- 1.151/drivers/usb/core/usb.c Thu Jan 22 12:29:38 2004
+++ edited/drivers/usb/core/usb.c Mon Jan 26 11:04:12 2004
@@ -289,16 +289,16 @@
* @iface: the interface from which it will be unbound
*
* This can be used by drivers to release an interface without waiting
- * for their disconnect() methods to be called. Since it calls disconnect,
- * drivers that use it from their disconnect method need to protect
- * themselves against infinite recursion.
+ * for their disconnect() methods to be called. In most cases this also
+ * causes the driver disconnect() method to be called.
*
* This call is synchronous, and may not be used in an interrupt context.
* Callers must own the usb_device serialize semaphore and the driver model's
* usb bus writelock. So driver disconnect() entries don't need extra locking,
* but other call contexts may need to explicitly claim those locks.
*/
-void usb_driver_release_interface(struct usb_driver *driver, struct usb_interface
*iface)
+void usb_driver_release_interface(struct usb_driver *driver,
+ struct usb_interface *iface)
{
struct device *dev = &iface->dev;
@@ -306,8 +306,8 @@
if (!dev->driver || dev->driver != &driver->driver)
return;
- /* in case we're called before dev_add() */
- if (!list_empty (&dev->bus_list))
+ /* don't disconnect from disconnect(), or before dev_add() */
+ if (!list_empty (&dev->driver_list) && !list_empty (&dev->bus_list))
device_release_driver(dev);
dev->driver = NULL;
