[i2c] [patch 2.6.19-rc6 7/11] i2c driver remove()

[David Brownell]

On Monday 04 December 2006 3:05 pm, Scott Wood wrote:
> David Brownell wrote:
> > On Monday 04 December 2006 1:57 pm, Scott Wood wrote:
> >>Is client->driver supposed to be set for new-style devices? 
> > 
> > 
> > Not always ... one of the canonical operating modes in the driver
> > model is to have a device nodescreated, and then have its driver
> > bound *MUCH* later, e.g. by "hotplug" event processing.  That
> > requires such a "device may not have a driver" state.
> 
> Sure...  I meant once the driver was bound to the device (or more 
> generally, whether client->driver was *ever* supposed to be set).  The 
> core doesn't seem to set it, nor does the tps65010 driver, but it looks 
> like the code expects that it *could* happen with a new-style device in 
> some parts, and not others.

I could believe that.  It would be a Good Thing to get rid of client->driver.
Meanwhile, I believe I'll update the probe() and remove() support so that
it manages that field.  Drivers certainly shouldn't need to muck around
like that, although all the "legacy" i2c drivers do.


> >>The above 
> >>(along with several other places like it) indicates it's at least a
> >>possibility, but i2c_del_driver will call a NULL detach_client() if
> >>client->driver is set.
> > 
> > 
> > Clearly I didn't see that particuar problem.  How would that happen?
> 
> It happened in my driver when rmmoding, as a result of setting 
> client->driver in its probe() method.  I've removed that, and it now 
> works, but I was curious as to whether it should be set and 
> i2c_del_driver() needs to be fixed to set client->driver to NULL, or 
> whether client->driver is always supposed to be NULL for new-style devices.

There "should be" no difference, but this is also something that any
new-style driver should not pay attention to.


> If the latter is true, though, there are other issues; besides the 
> misleading code checking for new-style methods in client->driver (and 
> not considering it an error) when they should never be there, remove 
> events will be missed if i2c_unregister_device() is called, since it 
> won't call i2c_device_remove() if client->driver is NULL (even though 
> i2c_device_remove() doesn't seem to depend on client->device).

Hmm, well see if you notice any places there'd be trouble if that field
were set by i2c_device_probe(), and cleared by i2c_device_remove(), right
before delegating to the "new style" driver's own method.

- Dave