[lm-sensors] [PATCH] hwmon: (jc42) Change I2C detection class

[lm-sensors] [PATCH] hwmon: (jc42) Change I2C detection class

[Jean Delvare]

While the JC42-compatible chips are temperature sensors, I2C_CLASS_SPD
makes more sense because these chips always live on memory modules.

Signed-off-by: Jean Delvare <khali@linux-fr.org>
Cc: Guenter Roeck <linux@roeck-us.net>
---
Guenter, what do you think?

 drivers/hwmon/jc42.c |    2 +-
 include/linux/i2c.h  |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

--- linux-2.6.39-rc3.orig/drivers/hwmon/jc42.c	2011-03-15 22:52:51.000000000 +0100
+++ linux-2.6.39-rc3/drivers/hwmon/jc42.c	2011-04-16 09:31:47.000000000 +0200
@@ -213,7 +213,7 @@ static const struct dev_pm_ops jc42_dev_
 
 /* This is the driver that will be inserted */
 static struct i2c_driver jc42_driver = {
-	.class		= I2C_CLASS_HWMON,
+	.class		= I2C_CLASS_SPD,
 	.driver = {
 		.name	= "jc42",
 		.pm = JC42_DEV_PM_OPS,
--- linux-2.6.39-rc3.orig/include/linux/i2c.h	2011-04-15 18:08:30.000000000 +0200
+++ linux-2.6.39-rc3/include/linux/i2c.h	2011-04-16 09:48:50.000000000 +0200
@@ -400,7 +400,7 @@ void i2c_unlock_adapter(struct i2c_adapt
 /* i2c adapter classes (bitmask) */
 #define I2C_CLASS_HWMON		(1<<0)	/* lm_sensors, ... */
 #define I2C_CLASS_DDC		(1<<3)	/* DDC bus on graphics adapters */
-#define I2C_CLASS_SPD		(1<<7)	/* SPD EEPROMs and similar */
+#define I2C_CLASS_SPD		(1<<7)	/* Memory modules */
 
 /* Internal numbers to terminate lists */
 #define I2C_CLIENT_END		0xfffeU


-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (jc42) Change I2C detection class

[Guenter Roeck]

On Sat, Apr 16, 2011 at 08:17:13AM -0400, Jean Delvare wrote:
> While the JC42-compatible chips are temperature sensors, I2C_CLASS_SPD
> makes more sense because these chips always live on memory modules.
> 
> Signed-off-by: Jean Delvare <khali@linux-fr.org>
> Cc: Guenter Roeck <linux@roeck-us.net>
> ---
> Guenter, what do you think?
> 
I don't know. Seems odd to define a device based on its location instead of its functionality.

What is the device class used for anyway ? I browsed through the code, but did not find it.

Guenter

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (jc42) Change I2C detection class

[Guenter Roeck]

On Sat, Apr 16, 2011 at 10:58:24AM -0400, Guenter Roeck wrote:
> On Sat, Apr 16, 2011 at 08:17:13AM -0400, Jean Delvare wrote:
> > While the JC42-compatible chips are temperature sensors, I2C_CLASS_SPD
> > makes more sense because these chips always live on memory modules.
> > 
> > Signed-off-by: Jean Delvare <khali@linux-fr.org>
> > Cc: Guenter Roeck <linux@roeck-us.net>
> > ---
> > Guenter, what do you think?
> > 
> I don't know. Seems odd to define a device based on its location instead of its functionality.
> 
> What is the device class used for anyway ? I browsed through the code, but did not find it.
> 
Never mind, I found it, and understand your reasoning now. Ok with me.

Acked-by: Guenter Roeck <guenter.roeck@ericsson.com>


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (jc42) Change I2C detection class

[Jean Delvare]

Hi Guenter,

On Sat, 16 Apr 2011 07:58:24 -0700, Guenter Roeck wrote:
> On Sat, Apr 16, 2011 at 08:17:13AM -0400, Jean Delvare wrote:
> > While the JC42-compatible chips are temperature sensors, I2C_CLASS_SPD
> > makes more sense because these chips always live on memory modules.
> > 
> > Signed-off-by: Jean Delvare <khali@linux-fr.org>
> > Cc: Guenter Roeck <linux@roeck-us.net>
> > ---
> > Guenter, what do you think?
> > 
> I don't know. Seems odd to define a device based on its location instead of its functionality.

The class isn't "defining a device". It should really be called
"probing class" or "auto-detection class". As a matter of fact, many
hwmon devices don't set their class to I2C_CLASS_HWMON because they
don't detect devices in the first place.

> What is the device class used for anyway ? I browsed through the code, but did not find it.

In i2c-core.c:i2c_detect():

	/* Stop here if the classes do not match */
	if (!(adapter->class & driver->class))
		return 0;

I2C classes are used to limit the probing scope of i2c device drivers.
They are pretty much arbitrary as long as things work (i.e. common I2C
chips as hardware monitoring and SPD EEPROMs on PC motherboards are
properly detected, and no misdetections happen.)

Back to my patch: I am currently working on implementing SMBus
multiplexing for Asus Z8NA boards. On these boards, all I2C devices
except the memory modules are on the SMBus directly, while memory module
slots are all behind a multiplexer. They are split in 2 or 3 groups,
each of which corresponds to one multiplexer position.

I am considering several options to support this topology. One option
is to set the class of the trunk to I2C_CLASS_HWMON only (i.e. remove
I2C_CLASS_SPD), and enable auto-detection with class = I2C_CLASS_SPD on
segments behind the multiplexer. This will work fine for SPD EEPROMs,
but not for JC42 thermal sensors, unless the jc42 driver switches to
class I2C_CLASS_SPD. Thus my proposal. This option is the most simple
to implement and thus has my preference (unless I find a problem with
it at some point...)

Note that this isn't only needed for these Asus boards. There are other
Supermicro [1] boards which need exactly the same. And Tyan boards have
used multiplexing in the past, this should help them too. The SMBus
multiplexing approach is mandatory for any board with more than 8
memory slots (AT24C02 chips have only 3 2-state pins for I2C address
selection), and many server boards these days have 12 or even 18.

[1] http://marc.info/?l=linux-i2c&m\x130260345621586&w=2

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors