i2c chips: add adt7461 support to lm90 driver

PATCH Applied James Chapman 3 participants 9 messages Feb 28, 2005 – May 19, 2005

lore

Share & tools

lore

James Chapman

author

2005-02-28 17:14 UTC #0 lore raw

PATCH 1/1 i2c chips: add adt7461 support to lm90 driver

Add ADT7461 (temperature sensor) support to LM90 driver.

Signed-off-by James Chapman <[email protected]>

drivers/i2c/chips/lm90.c +7 -2

diff -Nru a/drivers/i2c/chips/lm90.c b/drivers/i2c/chips/lm90.c
@@ -85,7 +85,7 @@
  * Insmod parameters
  */
 
-SENSORS_INSMOD_5(lm90, adm1032, lm99, lm86, max6657);
+SENSORS_INSMOD_6(lm90, adm1032, lm99, lm86, max6657, adt7461);
 
 /*
  * The LM90 registers

@@ -386,7 +386,10 @@
 			 && (reg_config1 & 0x3F) == 0x00
 			 && reg_convrate <= 0x0A) {
 				kind = adm1032;
-			}
+			} else
+			if (address == 0x4c
+			 && chip_id == 0x51) /* ADT7461 */
+				kind = adt7461;
 		} else
 		if (man_id == 0x4D) { /* Maxim */
 			/*

@@ -423,6 +426,8 @@
 		name = "lm86";
 	} else if (kind == max6657) {
 		name = "max6657";
+	} else if (kind == adt7461) {
+		name = "adt7461";
 	}
 
 	/* We can fill in the remaining client fields */

Greg KH

Applied

2005-03-02 17:03 UTC #1 lore raw

Quoted 3 lines · James Chapman

Add ADT7461 (temperature sensor) support to LM90 driver.
Signed-off-by James Chapman <[email protected]>

Applied, thanks.

greg k-h

Jean Delvare

2005-03-02 19:37 UTC #2 lore raw

Hi James,

Quoted 5 lines

Quoted 3 lines
Add ADT7461 (temperature sensor) support to LM90 driver.
Signed-off-by James Chapman <[email protected]>
Applied, thanks.

I had Greg drop this patch because it doesn't seem to be correct to me. The patch assumes that the ADT7461 is 100% compatible with the ADM1032. The datasheets mentions that, but only in the default confiuration of the chip. If I read it correctly, the ADT7461 can also be switched to an extended temperature range mode, where the register formats are different. In this mode, the chip is NOT compatible with the ADM1032 and needs specific handling.

I wonder how the limits are affected by the extended mode. The datasheet doesn't give much details about this. I would assume that they are affected too though, or it wouldn't make much sense.

I also see that the ADT7461 datasheet explicitely says that temperature measurements below 0 return 0 and over 127 return 127, in compatibility mode. Then we probably should not allow limits to be set outside of this range, as it might fool the user into thinking that the device can actually measure this. The ADM1032 datasheet isn't as clear on the topic, so I'm not sure what should be done for this one.

Please modify your patch so that it works properly for both modes. I do not request that you implement mode change, nor even that you support extended range mode if you don't need it. But I want you to ensure that the driver will behave properly on an ADT7461 found in this non-compatible range. This might be as easy as to not recognize the chip as supported if found in that mode, providing you don't care about the extended range.

I would also like you do update the documentation located on top of the lm90 driver source file. I listed all supported chips with links to datasheet and some additional per-chip information. The ADT7461 definitely needs something similar. Then please update Kconfig to mention the new chip.

I would also appreciate a patch to lm_sensors' sensors-detect script for the ADT7461, and possibly a patch for libsensors and sensors as well, unless you are not interested in these (in which case you would need to explicitely mention in Kconfig that the ADT7461 has no user-space support at the moment).

Thanks,

Signature · 1 line

Jean Delvare

James Chapman

author

2005-03-03 17:49 UTC #3 lore raw

A revised adt7461 patch addressing all of Jean's comments is attached.

This driver will detect the adt7461 chip only if boot firmware has left the chip in its default lm90-compatible mode.

i2c: add adt7461 chip support

Signed-off-by James Chapman <[email protected]>

The Analog Devices ADT7461 temperature sensor chip is compatible with the lm90 device provided its extended temperature range is not enabled. The chip will be ignored if the boot firmware enables extended temperature range.

Also, since the adt7461 treats temp values <0 as 0 and >127 as 127, the driver prevents temperature values outside the supported range from being set.

drivers/i2c/chips/lm90.c +22 -1

Index: linux-2.6.i2c/drivers/i2c/chips/lm90.c
===================================================================
@@ -43,6 +43,12 @@
  * variants. The extra address and features of the MAX6659 are not
  * supported by this driver.
  *
+ * This driver also supports the ADT7461 chip from Analog Devices but
+ * only in its "compatability mode". If an ADT7461 chip is found but
+ * is configured in non-compatible mode (where its temperature
+ * register values are decoded differently) it is ignored by this
+ * driver.
+ *
  * Since the LM90 was the first chipset supported by this driver, most
  * comments will refer to this chipset, but are actually general and
  * concern all supported chipsets, unless mentioned otherwise.

@@ -76,6 +82,7 @@
  * LM86, LM89, LM90, LM99, ADM1032, MAX6657 and MAX6658 have address 0x4c.
  * LM89-1, and LM99-1 have address 0x4d.
  * MAX6659 can have address 0x4c, 0x4d or 0x4e (unsupported).
+ * ADT7461 always has address 0x4c.
  */
 
 static unsigned short normal_i2c[] = { 0x4c, 0x4d, I2C_CLIENT_END };

@@ -85,7 +92,7 @@
  * Insmod parameters
  */
 
-SENSORS_INSMOD_5(lm90, adm1032, lm99, lm86, max6657);
+SENSORS_INSMOD_6(lm90, adm1032, lm99, lm86, max6657, adt7461);
 
 /*
  * The LM90 registers

@@ -180,6 +187,7 @@
 	struct semaphore update_lock;
 	char valid; /* zero until following fields are valid */
 	unsigned long last_updated; /* in jiffies */
+	int kind;
 
 	/* registers values */
 	s8 temp_input1, temp_low1, temp_high1; /* local */

@@ -221,6 +229,8 @@
 	struct i2c_client *client = to_i2c_client(dev); \
 	struct lm90_data *data = i2c_get_clientdata(client); \
 	long val = simple_strtol(buf, NULL, 10); \
+	if ((data->kind == adt7461) && ((val < 0) || (val > 127000))) \
+		return -EINVAL; \
 	data->value = TEMP1_TO_REG(val); \
 	i2c_smbus_write_byte_data(client, reg, data->value); \
 	return count; \

@@ -232,6 +242,8 @@
 	struct i2c_client *client = to_i2c_client(dev); \
 	struct lm90_data *data = i2c_get_clientdata(client); \
 	long val = simple_strtol(buf, NULL, 10); \
+	if ((data->kind == adt7461) && ((val < 0) || (val > 127000))) \
+		return -EINVAL; \
 	data->value = TEMP2_TO_REG(val); \
 	i2c_smbus_write_byte_data(client, regh, data->value >> 8); \
 	i2c_smbus_write_byte_data(client, regl, data->value & 0xff); \

@@ -386,6 +398,12 @@
 			 && (reg_config1 & 0x3F) == 0x00
 			 && reg_convrate <= 0x0A) {
 				kind = adm1032;
+			} else
+			if (address == 0x4c
+			 && chip_id == 0x51 /* ADT7461 */
+			 && (reg_config1 & 0x3F) == 0x00
+			 && reg_convrate <= 0x0A) {
+				kind = adt7461;
 			}
 		} else
 		if (man_id == 0x4D) { /* Maxim */

@@ -423,12 +441,15 @@
 		name = "lm86";
 	} else if (kind == max6657) {
 		name = "max6657";
+	} else if (kind == adt7461) {
+		name = "adt7461";
 	}
 
 	/* We can fill in the remaining client fields */
 	strlcpy(new_client->name, name, I2C_NAME_SIZE);
 	new_client->id = lm90_id++;
 	data->valid = 0;
+	data->kind = kind;
 	init_MUTEX(&data->update_lock);
 
 	/* Tell the I2C layer a new client has arrived */

Jean Delvare

2005-03-03 21:36 UTC #4 lore raw

Hi James,

Quoted 5 lines

A revised adt7461 patch addressing all of Jean's comments is attached.
This driver will detect the adt7461 chip only if boot firmware has left the chip in its default lm90-compatible mode.

I'm fine with the idea but not quite with your implementation:

Quoted 10 lines

drivers/i2c/chips/lm90.c +2 -0

@@ -221,6 +229,8 @@
 	struct i2c_client *client = to_i2c_client(dev); \
 	struct lm90_data *data = i2c_get_clientdata(client); \
 	long val = simple_strtol(buf, NULL, 10); \
+	if ((data->kind == adt7461) && ((val < 0) || (val > 127000))) \
+		return -EINVAL; \
 	data->value = TEMP1_TO_REG(val); \
 	i2c_smbus_write_byte_data(client, reg, data->value); \
 	return count; \

drivers/i2c/chips/lm90.c line 249

Quoted 8 lines

drivers/i2c/chips/lm90.c +2 -0

@@ -232,6 +242,8 @@
 	struct i2c_client *client = to_i2c_client(dev); \
 	struct lm90_data *data = i2c_get_clientdata(client); \
 	long val = simple_strtol(buf, NULL, 10); \
+	if ((data->kind == adt7461) && ((val < 0) || (val > 127000))) \
+		return -EINVAL; \
 	data->value = TEMP2_TO_REG(val); \
 	i2c_smbus_write_byte_data(client, regh, data->value >> 8); \
 	i2c_smbus_write_byte_data(client, regl, data->value & 0xff); \

This is inconsistent with the rest of the interface. For continuous values, we do not return errors on out-of-range writes. Instead, we force the value to whatever range boundary makes sense. See TEMP1_TO_REG and TEMP2_TO_REG. So I would suggest that you implement TEMP1_TO_REG_ADT7461 and TEMP2_TO_REG_ADT7461 the same way with different boundaries, and call them.

drivers/i2c/chips/lm90.c line 404

Quoted 3 lines

drivers/i2c/chips/lm90.c +3 -0

+			if (address == 0x4c
+			 && chip_id == 0x51 /* ADT7461 */
+			 && (reg_config1 & 0x3F) == 0x00

That could be broaden to "& 0x1F" is I am not mistaken. We don't really care about bit 5, do we? And maybe put a comment explaining that we check for compatibility at this point (similar checks for the other chips are only for unused bits, not for specific configuration).

Thanks,

Signature · 1 line

Jean Delvare

Jean Delvare

2005-05-19 06:25 UTC #5 lore raw

PATCH 1/1 i2c chips: add adt7461 support to lm90

In reply to James Chapman

Hi James,

Quoted 1 line

ADT7461 will only be supported in lm90-compat mode.

Sounds like the best solution to me too. We can still support the other mode later if someone really needs it.

Quoted 8 lines

Quoted 5 lines
I would also appreciate a patch to lm_sensors' sensors-detect script for the ADT7461, and possibly a patch for libsensors and sensors as well, unless you are not interested in these (in which case you would need to explicitely mention in Kconfig that the ADT7461 has no user-space support at the moment).
Patch to add adt7461 support to lmsensors-2.9.0 is attached. I'm new to the internals of lmsensors so I may not have done all I need to.

Patch is good but incomplete. You'd need to also "add support" in prog/sensors/main.c and etc/sensors.conf.eg. Please provide a new patch that does everything or a diffenet patch for me to apply additionally to this one.

Thanks,

Signature · 1 line

Jean Delvare

Jean Delvare

Applied

2005-05-19 06:25 UTC #6 lore raw

PATCH 1/1 i2c chips: add adt7461 support to lm90

In reply to James Chapman

Hi James,

Quoted 1 line

Attached is a new version of the adt7461 sensors patch, as requested.

Applied, thanks.

Signature · 1 line

Jean Delvare

James Chapman

author

2005-05-19 06:25 UTC #7 lore raw

PATCH 1/1 i2c chips: add adt7461 support to lm90 driver

In reply to James Chapman

Hi Jean,

Quoted 1 line · Jean Delvare

Quoted 1 line
Quoted 1 line
Add ADT7461 (temperature sensor) support to LM90 driver.

Thanks for your good comments. I'll post a revised kernel patch soon.

ADT7461 will only be supported in lm90-compat mode.

Quoted 5 lines

I would also appreciate a patch to lm_sensors' sensors-detect script for the ADT7461, and possibly a patch for libsensors and sensors as well, unless you are not interested in these (in which case you would need to explicitely mention in Kconfig that the ADT7461 has no user-space support at the moment).

Patch to add adt7461 support to lmsensors-2.9.0 is attached. I'm new to the internals of lmsensors so I may not have done all I need to.

I'm unable to test it since I can't run sensors on my target which is an embedded ppc board and lmsensors won't cross-compile. I don't have enough space to run perl anyway! :)

/james
-------------- next part --------------
Index: lm_sensors-2.9.0/prog/detect/sensors-detect
=================================--- lm_sensors-2.9.0.orig/prog/detect/sensors-detect 2004-12-10 20:10:09.000000000 +0000
+++ lm_sensors-2.9.0/prog/detect/sensors-detect 2005-03-03 16:46:08.000000000 +0000
@@ -1293,7 +1293,7 @@

      },
      {
        name => "Analog Devices ADT7461",
-       driver => "to-be-written",
+       driver => "lm90",
        i2c_addrs => [0x4c],
        i2c_detect => sub { lm90_detect 5, @_ },
      },

Index: lm_sensors-2.9.0/lib/chips.c
=================================--- lm_sensors-2.9.0.orig/lib/chips.c 2004-11-24 04:31:40.000000000 +0000
+++ lm_sensors-2.9.0/lib/chips.c 2005-03-03 16:51:41.000000000 +0000
@@ -5350,6 +5350,7 @@
{ SENSORS_LM99_PREFIX, lm90_features },
{ SENSORS_LM86_PREFIX, lm90_features },
{ SENSORS_MAX6657_PREFIX, lm90_features },
+ { SENSORS_ADT7461_PREFIX, lm90_features },
{ SENSORS_LM63_PREFIX, lm63_features },
{ SENSORS_MAX1619_PREFIX, max1619_features },
{ SENSORS_XEONTEMP_PREFIX, xeontemp_features },
Index: lm_sensors-2.9.0/lib/chips.h
=================================--- lm_sensors-2.9.0.orig/lib/chips.h 2004-11-24 04:31:40.000000000 +0000
+++ lm_sensors-2.9.0/lib/chips.h 2005-03-03 16:50:42.000000000 +0000
@@ -475,13 +475,14 @@

 #define SENSORS_LM85_ZONE3_RANGE           100  /* RW -- zone3_range  */
 #define SENSORS_LM85_ZONE3_SMOOTH          101  /* RW -- zone3_smooth  */

-/* LM86/LM89/LM90/LM99/ADM1032/MAX6657 chips */ +/* LM86/LM89/LM90/LM99/ADM1032/MAX6657/ADT7461 chips */

#define SENSORS_LM90_PREFIX "lm90"
#define SENSORS_ADM1032_PREFIX "adm1032"
#define SENSORS_LM99_PREFIX "lm99"
#define SENSORS_LM86_PREFIX "lm86"
#define SENSORS_MAX6657_PREFIX "max6657"
+#define SENSORS_ADT7461_PREFIX "adt7461"

#define SENSORS_LM90_LOCAL_TEMP 51 /* R */
#define SENSORS_LM90_LOCAL_HIGH 52 /* RW */

James Chapman

author

2005-05-19 06:25 UTC #8 lore raw

PATCH 1/1 i2c chips: add adt7461 support to lm90 driver

In reply to James Chapman

Hi Jean,

Attached is a new version of the adt7461 sensors patch, as requested.

Quoted 7 lines · Jean Delvare

Quoted 2 lines
Patch to add adt7461 support to lmsensors-2.9.0 is attached. I'm new to the internals of lmsensors so I may not have done all I need to.
Patch is good but incomplete. You'd need to also "add support" in prog/sensors/main.c and etc/sensors.conf.eg. Please provide a new patch that does everything or a diffenet patch for me to apply additionally to this one.

-------------- next part --------------
Add adt7461 temp sensor support as an lm90-compatible device. Requires lm90 kernel driver changes archived at
http://archives.andrew.net.au/lm-sensors/msg29763.html

Index: lm_sensors-2.9.0/prog/detect/sensors-detect
=================================--- lm_sensors-2.9.0.orig/prog/detect/sensors-detect 2004-12-10 20:10:09.000000000 +0000
+++ lm_sensors-2.9.0/prog/detect/sensors-detect 2005-03-03 16:46:08.000000000 +0000
@@ -1293,7 +1293,7 @@

      },
      {
        name => "Analog Devices ADT7461",
-       driver => "to-be-written",
+       driver => "lm90",
        i2c_addrs => [0x4c],
        i2c_detect => sub { lm90_detect 5, @_ },
      },

 #define SENSORS_LM85_ZONE3_RANGE           100  /* RW -- zone3_range  */
 #define SENSORS_LM85_ZONE3_SMOOTH          101  /* RW -- zone3_smooth  */

-/* LM86/LM89/LM90/LM99/ADM1032/MAX6657 chips */ +/* LM86/LM89/LM90/LM99/ADM1032/MAX6657/ADT7461 chips */

#define SENSORS_LM90_LOCAL_TEMP 51 /* R */
#define SENSORS_LM90_LOCAL_HIGH 52 /* RW */
Index: lm_sensors-2.9.0/prog/sensors/main.c
=================================--- lm_sensors-2.9.0.orig/prog/sensors/main.c 2004-11-24 04:31:40.000000000 +0000
+++ lm_sensors-2.9.0/prog/sensors/main.c 2005-03-03 20:32:25.000000000 +0000
@@ -392,6 +392,7 @@
{ "lm99", print_lm90 },
{ "lm86", print_lm90 },
{ "max6657", print_lm90 },
+ { "adt7461", print_lm90 },
{ "lm63", print_lm63 },
{ "xeontemp", print_xeontemp },
{ "max6650", print_max6650 },
Index: lm_sensors-2.9.0/etc/sensors.conf.eg
=================================--- lm_sensors-2.9.0.orig/etc/sensors.conf.eg 2004-11-25 19:40:09.000000000 +0000
+++ lm_sensors-2.9.0/etc/sensors.conf.eg 2005-03-03 20:31:43.000000000 +0000
@@ -1750,7 +1750,7 @@
set temp2_hyst 40

-chip "lm90-*" "adm1032-*" "lm86-*" "max6657-*" +chip "lm90-*" "adm1032-*" "lm86-*" "max6657-*" "adt7461-*"

label temp1 "M/B Temp"
label temp2 "CPU Temp"