All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [lm-sensors] [PATCH] hwmon: (jc42) Change I2C detection class
@ 2011-04-16 12:17 Jean Delvare
  2011-04-16 14:58 ` Guenter Roeck
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Jean Delvare @ 2011-04-16 12:17 UTC (permalink / raw
  To: lm-sensors

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [lm-sensors] [PATCH] hwmon: (jc42) Change I2C detection class
  2011-04-16 12:17 [lm-sensors] [PATCH] hwmon: (jc42) Change I2C detection class Jean Delvare
@ 2011-04-16 14:58 ` Guenter Roeck
  2011-04-16 15:45 ` Guenter Roeck
  2011-04-16 16:06 ` Jean Delvare
  2 siblings, 0 replies; 4+ messages in thread
From: Guenter Roeck @ 2011-04-16 14:58 UTC (permalink / raw
  To: lm-sensors

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [lm-sensors] [PATCH] hwmon: (jc42) Change I2C detection class
  2011-04-16 12:17 [lm-sensors] [PATCH] hwmon: (jc42) Change I2C detection class Jean Delvare
  2011-04-16 14:58 ` Guenter Roeck
@ 2011-04-16 15:45 ` Guenter Roeck
  2011-04-16 16:06 ` Jean Delvare
  2 siblings, 0 replies; 4+ messages in thread
From: Guenter Roeck @ 2011-04-16 15:45 UTC (permalink / raw
  To: lm-sensors

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [lm-sensors] [PATCH] hwmon: (jc42) Change I2C detection class
  2011-04-16 12:17 [lm-sensors] [PATCH] hwmon: (jc42) Change I2C detection class Jean Delvare
  2011-04-16 14:58 ` Guenter Roeck
  2011-04-16 15:45 ` Guenter Roeck
@ 2011-04-16 16:06 ` Jean Delvare
  2 siblings, 0 replies; 4+ messages in thread
From: Jean Delvare @ 2011-04-16 16:06 UTC (permalink / raw
  To: lm-sensors

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2011-04-16 16:06 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-04-16 12:17 [lm-sensors] [PATCH] hwmon: (jc42) Change I2C detection class Jean Delvare
2011-04-16 14:58 ` Guenter Roeck
2011-04-16 15:45 ` Guenter Roeck
2011-04-16 16:06 ` Jean Delvare

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.