Re: [PATCH 06/27] misc: eeprom: Export I2C module alias information in missing drivers

[Jean Delvare <jdelvare@suse.de>]

Le Monday 03 August 2015 à 16:07 +0200, Javier Martinez Canillas a
écrit :
> Hello Jean,
> 
> On 08/03/2015 01:05 PM, Jean Delvare wrote:
> > Hi Javier,
> > 
> > On Thu, 30 Jul 2015 18:18:31 +0200, Javier Martinez Canillas wrote:
> >> The I2C core always reports the MODALIAS uevent as "i2c:<client name"
> >> regardless if the driver was matched using the I2C id_table or the
> >> of_match_table. So the driver needs to export the I2C table and this
> >> be built into the module or udev won't have the necessary information
> >> to auto load the correct module when the device is added.
> >>
> >> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> >>
> >> ---
> >>
> >>  drivers/misc/eeprom/eeprom.c  | 1 +
> >>  drivers/misc/eeprom/max6875.c | 1 +
> >>  2 files changed, 2 insertions(+)
> >>
> >> diff --git a/drivers/misc/eeprom/eeprom.c b/drivers/misc/eeprom/eeprom.c
> >> index b432873def96..4bb54e1c40a7 100644
> >> --- a/drivers/misc/eeprom/eeprom.c
> >> +++ b/drivers/misc/eeprom/eeprom.c
> >> @@ -203,6 +203,7 @@ static const struct i2c_device_id eeprom_id[] = {
> >>  	{ "eeprom", 0 },
> >>  	{ }
> >>  };
> >> +MODULE_DEVICE_TABLE(i2c, eeprom_id);
> >>  
> >>  static struct i2c_driver eeprom_driver = {
> >>  	.driver = {
> > 
> > I seem to recall this one is missing on purpose. The legacy eeprom
> > driver is deprecated in favor of the at24 driver, so no one should
> > declare "eeprom" i2c devices and thus the module alias is useless. So I
> > would leave the legacy eeprom driver alone.
> >
> > The only feature the at24 driver is missing is device auto-detection as
> > far as I know. Maybe it should be added to ease the transition. Or
> > maybe not, I admit I'm not sure.
> >
> 
> It's up to you but since the driver is still in mainline and it has an
> i2c_device_id table, an "eeprom" I2C device can be registered and matched
> by the I2C core but if built as a module, it won't be autoloaded.

The eeprom driver instantiates its own devices, so auto-loading is not
needed. "eeprom" devices should not be found in device trees, that would
be a bug. Adding an alias is an invitation to introduce such bugs thus
my request to not add such an alias.

> I'm not familiar with the at24 platform so feel free to discard the patch
> if you think that it is not needed and no one is really using this driver
> (although in that case I think we the driver should just be removed).

I'm really talking about the at24 eeprom driver
(drivers/misc/eeprom/at24.c) which has nothing to to with the at91
platform, if that's what you are confusing with.

Yes, the long-term plan is to get rid of the legacy eeprom driver. But
we need a transition path for users. Either the at24 driver should be
able to instantiate SPD and EDID devices as the eeprom driver does, or
we need a user-space helper to do that kind of detection, so that
consumer scripts such as decode-dimms keep working. The former is a
smaller change, I just hope it won't have any drawback.

-- 
Jean Delvare
SUSE L3 Support