On Tue, Apr 26, 2005 at 11:57:34AM -0400, Alan Stern wrote:
> On Tue, 26 Apr 2005, Roman Kagan wrote:
> 
> > On Mon, Apr 25, 2005 at 04:01:07PM -0400, Alan Stern wrote:
> > > I think usb_driver_claim_interface is correct as it stands.  It was a 
> > > mistake to leave out from usb_driver_release_interface originally the 
> > > line 
> > > setting iface->condition to USB_INTERFACE_UNBINDING.
> > 
> > But it would be asymmetric then.
> 
> In what sense?  You mean because we don't also protect against a driver
> calling usb_driver_claim_interface from within that interface's probe?  
> We don't need to check for that, do we?

Well, I'm not sure, but we do it now (from 2.6.12-rc2):

int usb_driver_claim_interface(struct usb_driver *driver,
                                struct usb_interface *iface, void* priv)
{
        struct device *dev = &iface->dev;

        if (dev->driver)
                return -EBUSY;

        dev->driver = &driver->driver;
        usb_set_intfdata(iface, priv);
        iface->condition = USB_INTERFACE_BOUND;

        /* if interface was already added, bind now; else let
         * the future device_add() bind it, bypassing probe()
         */
        if (!list_empty (&dev->bus_list))
                device_bind_driver(dev);

        return 0;
}


> 
> >  FWIW my personal preference would be
> > to get rid of usb_interface.condition altogether, and always use the
> > driver base facilities to deduce the current interface bound state.
> > Otherwise maintaining a private field in sync with what the driver base
> > thinks is always error prone.
> 
> There are other reasons for keeping usb_interface.condition.  For one 
> thing, the driver-model data doesn't tell when a bind or unbind is in 
> progress.  That information is used elsewhere in usbcore.

AFAICT this "elsewhere" is only usb_lock_device_for_reset().  And it
looks like all its users (scsi reset in storage/scsiglue.c and
image/microtek.c) don't really care if it is transitioning or already in
an established bound state.

> Unfortunately there are several spots where a device can be bound to a
> driver, and the code generally doesn't check for errors.  This makes it
> difficult to check klist_node_attached at each spot.  The best solution
> would be always to have device_release_driver wait for the klist_node's
> completion before releasing the semaphore.  But there's no simple way to
> do that, given how driver_detach works.

Agreed.

Cheers,
  Roman.


-------------------------------------------------------
This SF.Net email is sponsored by: NEC IT Guy Games.
Get your fingers limbered up and give it your best shot. 4 great events, 4
opportunities to win big! Highest score wins.NEC IT Guy Games. Play to
win an NEC 61 plasma display. Visit http://www.necitguy.com/?r=20
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to