[i2c] [patch 2.6.21-rc3-git +i2c] switch at91 to new-style i2c drivers

[Jean Delvare]

Hi David,

On Sat, 10 Mar 2007 20:56:03 -0800, David Brownell wrote:
> On Saturday 10 March 2007 12:33 pm, Jean Delvare wrote:
> 
> > >  
> > > +/* work with "modprobe at91_i2c" from hotplugging or coldplugging */
> > > +MODULE_ALIAS("at91_i2c");
> > 
> > What is it useful for?
> 
> Mostly coldplugging.  The general rule has some startup agent
> scanning /sys/devices and then doing a "modprobe $(cat modalias)"
> for any device that doesn't have a driver link.  Since the
> name of that module is "i2c-at91", which doesn't match the
> driver name (which does match the device name) such a modprobe
> would fail without the MODULE_ALIAS directive.

Why were the driver and module given different names in the first
place? ;)

> > > --- at91.orig/arch/arm/mach-at91/board-dk.c	2007-03-04 15:40:41.000000000 -0800
> > > +++ at91/arch/arm/mach-at91/board-dk.c	2007-03-08 09:21:56.000000000 -0800
> > > @@ -99,6 +99,14 @@ static struct at91_mmc_data __initdata d
> > >  	.wire4		= 1,
> > >  };
> > >  
> > > +static struct i2c_board_info __initdata dk_i2c_devices[] = { {
> > > +	I2C_BOARD_INFO("ics1523", 0x26, ""),
> > 
> > Why not simply NULL?
> 
> Does that even work when initializing an array?  I know that "" does.

Oops, you're totally right, I had forgotten that it was an array of
chars and not a pointer, sorry. NULL is unlikely to work.

> Better would be to not include the type name in that macro.  Judging
> by that small sample, it's not needed often enough.

3/7 in this patch, so indeed it's questionable whether we want to
include it the macro. It's up to you - but we have to decide ourselves
before people start converting their platforms. Not including it has
the advantage that we get rid of these "" which confused me.

> > Also, I guess it's a matter of personal taste, but:
> > 
> > static struct i2c_board_info __initdata dk_i2c_devices[] = {
> > 	{ I2C_BOARD_INFO("ics1523", 0x26, "") },
> > 	{ I2C_BOARD_INFO("x9429", 0x28, "") },
> > 	{ I2C_BOARD_INFO("at24c", 0x50, "24c1024") },
> 
> Except that the at24c lines are going to need to pass
> some platform_data, so it won't fit on one line; there
> will be the basic info, then another line.

Good point again...

-- 
Jean Delvare