[i2c] [patch 2.6.22-rc1] i2c legacy drivers shouldn't issue uevents

[David Brownell]

On Tuesday 15 May 2007, Jean Delvare wrote:
> Hi David,
> 
> On Mon, 14 May 2007 19:32:55 -0700, David Brownell wrote:
> > Prevent legacy drivers from issuing uevents for device creation/removal,
> > so that userspace can't cause modprobing loops for them.  This became a
> > problem for some legacy PC drivers.  I can't easily see it becoming an
> > issue with I2C legacy drivers, but consistency-in-paranoia seems likely
> > to be a good thing here.  For usable i2c-level driver model uevents, just
> > switch to a new-style driver.
> 
> I'm curious how this could cause a loop. As far as I know, module
> loading is serialized, so the uevent would call modprobe which would be
> delayed until the module loading that caused the uevent is finished -

Legacy platform drivers often have bad habits like creating the device
before exiting on error (since it turns out, after probing, that the
device isn't really there).  So they'd cause two uevents.


> at which point the new modprobe would bail out without doing anything,
> as its target module is already loaded. What am I missing?

Probably that error path, whereby the "new" modprobe would progress
after the target module load aborted.  Like the new one would, and
the third modprobe it triggered, ad infinitum.

Root cause being that automagic modprobing relies on correctly
factored driver model support:  device node creation goes in a
different module than driver probing/binding.  Ergo the paranoia,
whereby all such unfactored "legacy" drivers should forgo use of
the hotplug/uevent mechanisms.

- Dave


> > Signed-off-by: David Brownell <dbrownell at users.sourceforge.net>
> > 
> > ---
> >  drivers/i2c/i2c-core.c |    5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > --- g26.orig/drivers/i2c/i2c-core.c	2007-05-08 07:45:06.000000000 -0700
> > +++ g26/drivers/i2c/i2c-core.c	2007-05-08 07:47:34.000000000 -0700
> > @@ -710,9 +710,10 @@ int i2c_attach_client(struct i2c_client 
> >  	if (client->driver)
> >  		client->dev.driver = &client->driver->driver;
> >  
> > -	if (client->driver && !is_newstyle_driver(client->driver))
> > +	if (client->driver && !is_newstyle_driver(client->driver)) {
> >  		client->dev.release = i2c_client_release;
> > -	else
> > +		client->dev.uevent_suppress = 1;
> > +	} else
> >  		client->dev.release = i2c_client_dev_release;
> >  
> >  	snprintf(&client->dev.bus_id[0], sizeof(client->dev.bus_id),
> > 
> 
> Still, this seems sane. Patch applied, thanks.
> 
> -- 
> Jean Delvare
>