[lm-sensors] MAX6642 chip detection and other stuff

[lm-sensors] MAX6642 chip detection and other stuff

[Guenter Roeck]

[-- Attachment #1: Type: text/plain, Size: 2213 bytes --]

Hi all,

I finally got MAX6642 samples, so I am able to play around with the chip.

The attached patch (on top of Per's most recent patch) works quite nicely.
Per, maybe you can just merge it with your patch and resubmit it.

While testing the chip, I found another little problem: the fault attribute is named
temp_fault. That will have to be renamed to temp2_fault, first because it reflects
a fault with the external diode and second to match the ABI. We will need a separate
patch to fix this problem.

Here is the output of i2cdump for the MAX6642 (with open/unconnected external sensor).

root@groeck-desktop:/home/groeck# i2cdump -y 5 0x48
No size specified (using byte-data access)
     0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f    0123456789abcdef
00: 18 ff 84 10 10 46 46 78 78 78 78 78 78 78 78 78    ?.???FFxxxxxxxxx
10: c0 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40    ?@@@@@@@@@@@@@@@
20: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40    @@@@@@@@@@@@@@@@
30: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40    @@@@@@@@@@@@@@@@
40: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40    @@@@@@@@@@@@@@@@
50: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40    @@@@@@@@@@@@@@@@
60: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40    @@@@@@@@@@@@@@@@
70: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40    @@@@@@@@@@@@@@@@
80: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40    @@@@@@@@@@@@@@@@
90: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40    @@@@@@@@@@@@@@@@
a0: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40    @@@@@@@@@@@@@@@@
b0: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40    @@@@@@@@@@@@@@@@
c0: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40    @@@@@@@@@@@@@@@@
d0: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40    @@@@@@@@@@@@@@@@
e0: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40    @@@@@@@@@@@@@@@@
f0: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 4d 4d    @@@@@@@@@@@@@@MM
root@groeck-desktop:/home/groeck# modprobe max6642
root@groeck-desktop:/home/groeck# sensors
max6642-i2c-5-48
Adapter: i2c-devantech-iss at bus 001 device 007
temp1:        +24.0°C  (high = +70.0°C)
temp2:          FAULT  (high = +120.0°C)

Thanks,
Guenter

[-- Attachment #2: 0001-hwmon-max6642-Improve-chip-detection.patch --]
[-- Type: text/x-diff, Size: 2733 bytes --]

From 197b9b4fb23c372de6d08afad4a0ebd72cfaadc6 Mon Sep 17 00:00:00 2001
From: Guenter Roeck <guenter.roeck@ericsson.com>
Date: Fri, 27 May 2011 21:39:59 -0700
Subject: [PATCH] hwmon: (max6642) Improve chip detection

Signed-off-by: Guenter Roeck <guenter.roeck@ericsson.com>
---
 drivers/hwmon/max6642.c |   35 ++++++++++++++++++-----------------
 1 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/hwmon/max6642.c b/drivers/hwmon/max6642.c
index 872155e..160853c 100644
--- a/drivers/hwmon/max6642.c
+++ b/drivers/hwmon/max6642.c
@@ -64,14 +64,6 @@ static const unsigned short normal_i2c[] = {
 #define MAX6642_REG_W_REMOTE_HIGH	0x0D
 
 /*
- * Registers for detection tests. These registers are not present and,
- * when read, will only return the last valid register read.
- */
-#define MAX6642_REG_R_DUMMY_1          0x04
-#define MAX6642_REG_R_DUMMY_2          0x06
-#define MAX6642_REG_R_DUMMY_3          0xFF
-
-/*
  * Conversions
  */
 
@@ -134,7 +126,7 @@ static int max6642_detect(struct i2c_client *client,
 			  struct i2c_board_info *info)
 {
 	struct i2c_adapter *adapter = client->adapter;
-	u8 reg_config, reg_status, man_id, dummy_reg;
+	u8 reg_config, reg_status, man_id;
 
 	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA))
 		return -ENODEV;
@@ -145,16 +137,13 @@ static int max6642_detect(struct i2c_client *client,
 		return -ENODEV;
 
 	/* sanity check */
-	dummy_reg = i2c_smbus_read_byte_data(client, MAX6642_REG_R_DUMMY_1);
-	if (dummy_reg != 0x4D)
+	if (i2c_smbus_read_byte_data(client, 0x04) != 0x4D)
 		return -ENODEV;
 
-	dummy_reg = i2c_smbus_read_byte_data(client, MAX6642_REG_R_DUMMY_2);
-	if (dummy_reg != 0x4D)
+	if (i2c_smbus_read_byte_data(client, 0x06) != 0x4D)
 		return -ENODEV;
 
-	dummy_reg = i2c_smbus_read_byte_data(client, MAX6642_REG_R_DUMMY_3);
-	if (dummy_reg != 0x4D)
+	if (i2c_smbus_read_byte_data(client, 0xff) != 0x4D)
 		return -ENODEV;
 
 	/*
@@ -163,9 +152,21 @@ static int max6642_detect(struct i2c_client *client,
 	 * zero in the status register.
 	 */
 	reg_config = i2c_smbus_read_byte_data(client, MAX6642_REG_R_CONFIG);
+	if ((reg_config & 0x0f) != 0x00)
+		return -ENODEV;
+
+	/* in between, another round of sanity checks */
+	if (i2c_smbus_read_byte_data(client, 0x04) != reg_config)
+		return -ENODEV;
+
+	if (i2c_smbus_read_byte_data(client, 0x06) != reg_config)
+		return -ENODEV;
+
+	if (i2c_smbus_read_byte_data(client, 0xff) != reg_config)
+		return -ENODEV;
+
 	reg_status = i2c_smbus_read_byte_data(client, MAX6642_REG_R_STATUS);
-	if (((reg_config & 0x0f) != 0x00) ||
-	    ((reg_status & 0x2b) != 0x00))
+	if ((reg_status & 0x2b) != 0x00)
 		return -ENODEV;
 
 	strlcpy(info->type, "max6642", I2C_NAME_SIZE);
-- 
1.7.3.1


[-- Attachment #3: Type: text/plain, Size: 153 bytes --]

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

Re: [lm-sensors] MAX6642 chip detection and other stuff

[Jean Delvare]

Hi Guenter,

On Fri, 27 May 2011 21:51:13 -0700, Guenter Roeck wrote:
> The attached patch (on top of Per's most recent patch) works quite nicely.
> Per, maybe you can just merge it with your patch and resubmit it.

Looks good, pretty much what I had in mind. But I think you could make
the code even more compact:

	/* sanity check */
	if (i2c_smbus_read_byte_data(client, 0x04) != 0x4D
	 || i2c_smbus_read_byte_data(client, 0x06) != 0x4D
	 || i2c_smbus_read_byte_data(client, 0xff) != 0x4D)
		return -ENODEV;

-- 
Jean Delvare

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

Re: [lm-sensors] MAX6642 chip detection and other stuff

[Guenter Roeck]

Hi Jean,

On Sat, May 28, 2011 at 04:36:31AM -0400, Jean Delvare wrote:
> Hi Guenter,
> 
> On Fri, 27 May 2011 21:51:13 -0700, Guenter Roeck wrote:
> > The attached patch (on top of Per's most recent patch) works quite nicely.
> > Per, maybe you can just merge it with your patch and resubmit it.
> 
> Looks good, pretty much what I had in mind. But I think you could make
> the code even more compact:
> 
> 	/* sanity check */
> 	if (i2c_smbus_read_byte_data(client, 0x04) != 0x4D
> 	 || i2c_smbus_read_byte_data(client, 0x06) != 0x4D
> 	 || i2c_smbus_read_byte_data(client, 0xff) != 0x4D)
> 		return -ENODEV;
> 
Yes, you are right. I didn't do it to avoid a checkpatch warning, but forgot
that I don't use a variable anymore.

Guenter

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

Re: [lm-sensors] MAX6642 chip detection and other stuff

[Per Dalén]

[-- Attachment #1: Type: text/plain, Size: 905 bytes --]

Hi Guenter and Jean,

Sorry for the delay. Much stuff at work...
Here's (Jean's ;) patch.

BR
Per

On 05/28/2011 04:04 PM, Guenter Roeck wrote:
> Hi Jean,
> 
> On Sat, May 28, 2011 at 04:36:31AM -0400, Jean Delvare wrote:
>> Hi Guenter,
>>
>> On Fri, 27 May 2011 21:51:13 -0700, Guenter Roeck wrote:
>>> The attached patch (on top of Per's most recent patch) works quite nicely.
>>> Per, maybe you can just merge it with your patch and resubmit it.
>>
>> Looks good, pretty much what I had in mind. But I think you could make
>> the code even more compact:
>>
>> 	/* sanity check */
>> 	if (i2c_smbus_read_byte_data(client, 0x04) != 0x4D
>> 	 || i2c_smbus_read_byte_data(client, 0x06) != 0x4D
>> 	 || i2c_smbus_read_byte_data(client, 0xff) != 0x4D)
>> 		return -ENODEV;
>>
> Yes, you are right. I didn't do it to avoid a checkpatch warning, but forgot
> that I don't use a variable anymore.
> 
> Guenter


[-- Attachment #2: 0002-hwmon-max6642-Improve-chip-detection.patch --]
[-- Type: text/plain, Size: 846 bytes --]

Improve the detection of MAX6642 by reading non exciting registers (0x04, 0x06 and 0xff). The value of those registers should be the same as the last valid resister read.

Signed-off-by: Per Dalen <per.dalen@appeartv.com>
---
diff --git a/drivers/hwmon/max6642.c b/drivers/hwmon/max6642.c
index 0f9fc40..4fb0564 100644
--- a/drivers/hwmon/max6642.c
+++ b/drivers/hwmon/max6642.c
@@ -136,6 +136,12 @@ static int max6642_detect(struct i2c_client *client,
 	if (man_id != 0x4D)
 		return -ENODEV;
 
+	/* sanity check */
+	if (i2c_smbus_read_byte_data(client, 0x04) != 0x4D
+	    || i2c_smbus_read_byte_data(client, 0x06) != 0x4D
+	    || i2c_smbus_read_byte_data(client, 0xff) != 0x4D)
+		return -ENODEV;
+
 	/*
 	 * We read the config and status register, the 4 lower bits in the
 	 * config register should be zero and bit 5, 3, 1 and 0 should be

[-- Attachment #3: Type: text/plain, Size: 153 bytes --]

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

Re: [lm-sensors] MAX6642 chip detection and other stuff

[Per Dalén]

[-- Attachment #1: Type: text/plain, Size: 2438 bytes --]

Hi,

Thanks for finding this error. I have attached the patch you requested.

BR
Per

On 05/28/2011 06:51 AM, Guenter Roeck wrote:
> Hi all,
> 
> I finally got MAX6642 samples, so I am able to play around with the chip.
> 
> The attached patch (on top of Per's most recent patch) works quite nicely.
> Per, maybe you can just merge it with your patch and resubmit it.
> 
> While testing the chip, I found another little problem: the fault attribute is named
> temp_fault. That will have to be renamed to temp2_fault, first because it reflects
> a fault with the external diode and second to match the ABI. We will need a separate
> patch to fix this problem.
> 
> Here is the output of i2cdump for the MAX6642 (with open/unconnected external sensor).
> 
> root@groeck-desktop:/home/groeck# i2cdump -y 5 0x48
> No size specified (using byte-data access)
>      0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f    0123456789abcdef
> 00: 18 ff 84 10 10 46 46 78 78 78 78 78 78 78 78 78    ?.???FFxxxxxxxxx
> 10: c0 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40    ?@@@@@@@@@@@@@@@
> 20: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40    @@@@@@@@@@@@@@@@
> 30: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40    @@@@@@@@@@@@@@@@
> 40: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40    @@@@@@@@@@@@@@@@
> 50: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40    @@@@@@@@@@@@@@@@
> 60: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40    @@@@@@@@@@@@@@@@
> 70: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40    @@@@@@@@@@@@@@@@
> 80: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40    @@@@@@@@@@@@@@@@
> 90: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40    @@@@@@@@@@@@@@@@
> a0: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40    @@@@@@@@@@@@@@@@
> b0: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40    @@@@@@@@@@@@@@@@
> c0: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40    @@@@@@@@@@@@@@@@
> d0: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40    @@@@@@@@@@@@@@@@
> e0: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40    @@@@@@@@@@@@@@@@
> f0: 40 40 40 40 40 40 40 40 40 40 40 40 40 40 4d 4d    @@@@@@@@@@@@@@MM
> root@groeck-desktop:/home/groeck# modprobe max6642
> root@groeck-desktop:/home/groeck# sensors
> max6642-i2c-5-48
> Adapter: i2c-devantech-iss at bus 001 device 007
> temp1:        +24.0°C  (high = +70.0°C)
> temp2:          FAULT  (high = +120.0°C)
> 
> Thanks,
> Guenter


[-- Attachment #2: 0001-hwmon-max6642-wrong-temp_fault.patch --]
[-- Type: text/plain, Size: 1475 bytes --]

BUG: The temp_fault is wrong, it should be temp2_fault insteedd. 

Guenter: "While testing the chip, I found another little problem: the fault attribute is named temp_fault. That will have to be renamed to temp2_fault, first because it reflects a fault with the external diode and second to match the ABI."
Thanks Guenter for finding the bug.


Signed-off-by: Per Dalen <per.dalen@appeartv.com>
---
diff --git a/drivers/hwmon/max6642.c b/drivers/hwmon/max6642.c
index 0f9fc40..0f30e1b 100644
--- a/drivers/hwmon/max6642.c
+++ b/drivers/hwmon/max6642.c
@@ -246,7 +246,7 @@ static SENSOR_DEVICE_ATTR_2(temp1_max, S_IWUSR | S_IRUGO, show_temp_max,
 			    set_temp_max, 0, MAX6642_REG_W_LOCAL_HIGH);
 static SENSOR_DEVICE_ATTR_2(temp2_max, S_IWUSR | S_IRUGO, show_temp_max,
 			    set_temp_max, 1, MAX6642_REG_W_REMOTE_HIGH);
-static SENSOR_DEVICE_ATTR(temp_fault, S_IRUGO, show_alarm, NULL, 2);
+static SENSOR_DEVICE_ATTR(temp2_fault, S_IRUGO, show_alarm, NULL, 2);
 static SENSOR_DEVICE_ATTR(temp1_max_alarm, S_IRUGO, show_alarm, NULL, 6);
 static SENSOR_DEVICE_ATTR(temp2_max_alarm, S_IRUGO, show_alarm, NULL, 4);
 
@@ -256,7 +256,7 @@ static struct attribute *max6642_attributes[] = {
 	&sensor_dev_attr_temp1_max.dev_attr.attr,
 	&sensor_dev_attr_temp2_max.dev_attr.attr,
 
-	&sensor_dev_attr_temp_fault.dev_attr.attr,
+	&sensor_dev_attr_temp2_fault.dev_attr.attr,
 	&sensor_dev_attr_temp1_max_alarm.dev_attr.attr,
 	&sensor_dev_attr_temp2_max_alarm.dev_attr.attr,
 	NULL

[-- Attachment #3: Type: text/plain, Size: 153 bytes --]

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

Re: [lm-sensors] MAX6642 chip detection and other stuff

[Jean Delvare]

On Wed, 01 Jun 2011 09:42:59 +0200, Per Dalén wrote:
> Thanks for finding this error. I have attached the patch you requested.

Acked-by: Jean Delvare <khali@linux-fr.org>

I'll let Guenter pick the patch.

Thanks,
-- 
Jean Delvare

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

Re: [lm-sensors] MAX6642 chip detection and other stuff

[Guenter Roeck]

Hi Per,

On Wed, Jun 01, 2011 at 02:44:19AM -0400, Per Dalén wrote:
> Hi Guenter and Jean,
> 
> Sorry for the delay. Much stuff at work...
> Here's (Jean's ;) patch.
> 
> BR
> Per
> 
[ ... ]

> Improve the detection of MAX6642 by reading non exciting registers (0x04, 0x06 and 0xff). The value of those registers should be the same as the last valid resister read.

> Signed-off-by: Per Dalen <per.dalen@appeartv.com>
> ---
> diff --git a/drivers/hwmon/max6642.c b/drivers/hwmon/max6642.c
> index 0f9fc40..4fb0564 100644
> --- a/drivers/hwmon/max6642.c
> +++ b/drivers/hwmon/max6642.c
> @@ -136,6 +136,12 @@ static int max6642_detect(struct i2c_client *client,
>  	if (man_id != 0x4D)
>  		return -ENODEV;
>  
> +	/* sanity check */
> +	if (i2c_smbus_read_byte_data(client, 0x04) != 0x4D
> +	    || i2c_smbus_read_byte_data(client, 0x06) != 0x4D
> +	    || i2c_smbus_read_byte_data(client, 0xff) != 0x4D)
> +		return -ENODEV;
> +
>  	/*
>  	 * We read the config and status register, the 4 lower bits in the
>  	 * config register should be zero and bit 5, 3, 1 and 0 should be

Looks good, only the second part of my suggested changes got lost.

        reg_config = i2c_smbus_read_byte_data(client, MAX6642_REG_R_CONFIG);
+       if ((reg_config & 0x0f) != 0x00)
+               return -ENODEV;
+
+       /* in between, another round of sanity checks */
+       if (i2c_smbus_read_byte_data(client, 0x04) != reg_config
+	    || i2c_smbus_read_byte_data(client, 0x06) != reg_config
+	    || i2c_smbus_read_byte_data(client, 0xff) != reg_config)
+               return -ENODEV;
+

Please add those, and we should be ready to go.

Thanks,
Guenter

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

Re: [lm-sensors] MAX6642 chip detection and other stuff

[Guenter Roeck]

On Wed, Jun 01, 2011 at 05:47:38AM -0400, Jean Delvare wrote:
> On Wed, 01 Jun 2011 09:42:59 +0200, Per Dalén wrote:
> > Thanks for finding this error. I have attached the patch you requested.
> 
> Acked-by: Jean Delvare <khali@linux-fr.org>
> 
> I'll let Guenter pick the patch.
> 
Thanks, applied.

Guenter

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

Re: [lm-sensors] MAX6642 chip detection and other stuff

[Guenter Roeck]

Hi Per,

On Wed, Jun 01, 2011 at 02:44:19AM -0400, Per Dalén wrote:
> Hi Guenter and Jean,
> 
> Sorry for the delay. Much stuff at work...
> Here's (Jean's ;) patch.
> 
> BR
> Per
> 
> On 05/28/2011 04:04 PM, Guenter Roeck wrote:
> > Hi Jean,
> > 
> > On Sat, May 28, 2011 at 04:36:31AM -0400, Jean Delvare wrote:
> >> Hi Guenter,
> >>
> >> On Fri, 27 May 2011 21:51:13 -0700, Guenter Roeck wrote:
> >>> The attached patch (on top of Per's most recent patch) works quite nicely.
> >>> Per, maybe you can just merge it with your patch and resubmit it.
> >>
> >> Looks good, pretty much what I had in mind. But I think you could make
> >> the code even more compact:
> >>
> >> 	/* sanity check */
> >> 	if (i2c_smbus_read_byte_data(client, 0x04) != 0x4D
> >> 	 || i2c_smbus_read_byte_data(client, 0x06) != 0x4D
> >> 	 || i2c_smbus_read_byte_data(client, 0xff) != 0x4D)
> >> 		return -ENODEV;
> >>
> > Yes, you are right. I didn't do it to avoid a checkpatch warning, but forgot
> > that I don't use a variable anymore.
> > 
> > Guenter
> 

> Improve the detection of MAX6642 by reading non exciting registers (0x04, 0x06 and 0xff). The value of those registers should be the same as the last valid resister read.
> 
> Signed-off-by: Per Dalen <per.dalen@appeartv.com>

I want to send this off to Linus with my next set of patches, so I added the second set
of checks myself. No need to resubmit.

Thanks,
Guenter

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