[v3,4/4] eeprom: Add a MODULE_DEVICE_TABLE

The new DIMM-bus code can detect eeproms, so give the eeprom driver
an appropriate modalias.

Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
 drivers/misc/eeprom/eeprom.c | 1 +
 1 file changed, 1 insertion(+)

Hi Andy,

On Wed, 17 Jul 2013 13:53:08 -0700, Andy Lutomirski wrote:
> The new DIMM-bus code can detect eeproms, so give the eeprom driver
> an appropriate modalias.
> 
> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
> ---
>  drivers/misc/eeprom/eeprom.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/misc/eeprom/eeprom.c b/drivers/misc/eeprom/eeprom.c
> index c169e07..1ec85b8 100644
> --- a/drivers/misc/eeprom/eeprom.c
> +++ b/drivers/misc/eeprom/eeprom.c
> @@ -215,6 +215,7 @@ static const struct i2c_device_id eeprom_id[] = {
>  	{ "eeprom", 0 },
>  	{ }
>  };
> +MODULE_DEVICE_TABLE(i2c, eeprom_id);
>  
>  static struct i2c_driver eeprom_driver = {
>  	.driver = {

Nack. The eeprom driver will eventually die, I don't want to see
anything like that added to it. Please use the at24 driver for anything
new.

Thanks,

On Thu, Jul 18, 2013 at 12:11 AM, Jean Delvare <khali@linux-fr.org> wrote:
> Hi Andy,
>
> On Wed, 17 Jul 2013 13:53:08 -0700, Andy Lutomirski wrote:
>> The new DIMM-bus code can detect eeproms, so give the eeprom driver
>> an appropriate modalias.
>>
>> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
>> ---
>>  drivers/misc/eeprom/eeprom.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/misc/eeprom/eeprom.c b/drivers/misc/eeprom/eeprom.c
>> index c169e07..1ec85b8 100644
>> --- a/drivers/misc/eeprom/eeprom.c
>> +++ b/drivers/misc/eeprom/eeprom.c
>> @@ -215,6 +215,7 @@ static const struct i2c_device_id eeprom_id[] = {
>>       { "eeprom", 0 },
>>       { }
>>  };
>> +MODULE_DEVICE_TABLE(i2c, eeprom_id);
>>
>>  static struct i2c_driver eeprom_driver = {
>>       .driver = {
>
> Nack. The eeprom driver will eventually die, I don't want to see
> anything like that added to it. Please use the at24 driver for anything
> new.

at24 doesn't have .class=I2C_CLASS_SPD (and I still am not really a
fan of the i2c class mechanism).  Shall I play with ways to get known
DIMM-only busses to probe these things directly and instantiate with
i2c_new_device(..., .type="spd")?  Do you have other plans for how
probing should work?

(For background in case you haven't been following the rest of the
thread: the Intel iMC (integrated memory controller) bus appears to
have only DIMMs attached, so everything on the bus is either the host
or is something attached to a DIMM.  The latter devices have
well-defined addresses: four bits of type and three bits of slot
number.  This makes it easy to probe accurately.  I'm not sure that
the i2c core class mechanism is well-suited to this, it's easy to do
manually.)

--Andy

>
> Thanks,
> --
> Jean Delvare

Hi Andy,

On Thu, 18 Jul 2013 09:15:47 -0700, Andy Lutomirski wrote:
> at24 doesn't have .class=I2C_CLASS_SPD (and I still am not really a
> fan of the i2c class mechanism).

The class-based auto-detection has its merits but it's not the solution
to all problems.

.class=I2C_CLASS_SPD could be added to the at24 driver. If we ever want
to kill the legacy eeprom driver, we will have to do exactly that.

That being said this won't help in your case, as your bus driver does
not support the SMBus transaction (Receive Byte) used to probe for
EEPROM chips in address range 0x50-0x57.

> Shall I play with ways to get known
> DIMM-only busses to probe these things directly and instantiate with
> i2c_new_device(..., .type="spd")?  Do you have other plans for how
> probing should work?

You could indeed call i2c_new_probed_device() on addresses 0x50-0x57
at the end of your new i2c bus driver's probe function, in order to
instantiate spd devices. This function takes a probe function as a
parameter so you could use a transaction type supported (Read Byte.)

> (For background in case you haven't been following the rest of the
> thread: the Intel iMC (integrated memory controller) bus appears to
> have only DIMMs attached, so everything on the bus is either the host
> or is something attached to a DIMM.  The latter devices have
> well-defined addresses: four bits of type and three bits of slot
> number.  This makes it easy to probe accurately.  I'm not sure that
> the i2c core class mechanism is well-suited to this, it's easy to do
> manually.)

Ideally the BIOS/firmware/whatever would tell the OS which memory slots
are used so that the kernel can instantiate all I2C/SMBus slave devices
without even probing for them. I really wonder how many decades it will
take to Intel and co to come up with a complete device typology
description on the PC/x86_64 platfor, so that probing is finally
history.

On Thu, Jul 18, 2013 at 1:31 PM, Jean Delvare <khali@linux-fr.org> wrote:
> On Thu, 18 Jul 2013 09:15:47 -0700, Andy Lutomirski wrote:
>> at24 doesn't have .class=I2C_CLASS_SPD (and I still am not really a
>> fan of the i2c class mechanism).
>
> The class-based auto-detection has its merits but it's not the solution
> to all problems.
>
> .class=I2C_CLASS_SPD could be added to the at24 driver. If we ever want
> to kill the legacy eeprom driver, we will have to do exactly that.
>
> That being said this won't help in your case, as your bus driver does
> not support the SMBus transaction (Receive Byte) used to probe for
> EEPROM chips in address range 0x50-0x57.
>
>> Shall I play with ways to get known
>> DIMM-only busses to probe these things directly and instantiate with
>> i2c_new_device(..., .type="spd")?  Do you have other plans for how
>> probing should work?
>
> You could indeed call i2c_new_probed_device() on addresses 0x50-0x57
> at the end of your new i2c bus driver's probe function, in order to
> instantiate spd devices. This function takes a probe function as a
> parameter so you could use a transaction type supported (Read Byte.)
>

Can you take a look at patch v4 1/2 and let me know if you think it's
any good?  It does more or less that, albeit using i2c_new_device and
a bit fancier logic.  It's also factored out to make it (in theory)
reusable by any other bus with similar properties, but I can easily
move it somewhere else.

http://news.gmane.org/find-root.php?message_id=%3c0087fdcb080b40f5eaf3abbfd35ee107ca713d52.1374171757.git.luto%40amacapital.net%3e

(And apologies for my newbishness in poking around the i2c code.)

>> (For background in case you haven't been following the rest of the
>> thread: the Intel iMC (integrated memory controller) bus appears to
>> have only DIMMs attached, so everything on the bus is either the host
>> or is something attached to a DIMM.  The latter devices have
>> well-defined addresses: four bits of type and three bits of slot
>> number.  This makes it easy to probe accurately.  I'm not sure that
>> the i2c core class mechanism is well-suited to this, it's easy to do
>> manually.)
>
> Ideally the BIOS/firmware/whatever would tell the OS which memory slots
> are used so that the kernel can instantiate all I2C/SMBus slave devices
> without even probing for them. I really wonder how many decades it will
> take to Intel and co to come up with a complete device typology
> description on the PC/x86_64 platfor, so that probing is finally
> history.

Intel almost did it.  They give status bits indicating whether they
(where "they" means either BIOS or the chipset -- I'm not sure) think
that a given slot contains a TSOD.  I think that those bits actually
control the periodic hardware TSOD polling.  On my i7 Extreme, they're
all zero.

I suppose that one could query the EDAC infrastructure to see which
slots are mapped to which RAM addresses, but that would be a giant
mess.

--Andy