Skip to contentSkip to conversation

MAX6642: Better detection schema

PATCH Applied Per Dalén 3 participants 3 messages May 26, 2011 Hardware monitoring

lore
Share & tools
lore

Per Dalén

author
#0 lore raw

PATCH v2 1/1 MAX6642: Better detection schema

Changed according to the suggestions from Guenter Roeck:

* Check the manufacturer ID directly. Fastens the test if it's not a Maxim chip.
* Read the other non-existing registers after the manufacturer ID.

---
Check for non-existing registers, registers 0x04, 0x06 and 0xff, which are supported by other Maxim chips. Reading such registers returns the previously read value.

Signed-off-by Per Dalen <[email protected]>

---

drivers/hwmon/max6642.c +22 -1
index 0f9fc40..565b507 100644
@@ -64,6 +64,14 @@ static const unsigned short normal_i2c[] = {
 #define MAX6642_REG_W_REMOTE_HIGH	0x0D
 
 /*
+ * Registers for detection tests, these registers isn't pressent and
+ * will only show 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
  */
 

@@ -126,7 +134,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;
+	u8 reg_config, reg_status, man_id, dummy_reg;
 
 	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA))
 		return -ENODEV;

@@ -136,6 +144,19 @@ static int max6642_detect(struct i2c_client *client,
 	if (man_id != 0x4D)
 		return -ENODEV;
 
+	/* sanity check */
+	dummy_reg = i2c_smbus_read_byte_data(client, MAX6642_REG_R_DUMMY_1);
+	if (dummy_reg != 0x4D)
+		return -ENODEV;
+
+	dummy_reg = i2c_smbus_read_byte_data(client, MAX6642_REG_R_DUMMY_2);
+	if (dummy_reg != 0x4D)
+		return -ENODEV;
+
+	dummy_reg = i2c_smbus_read_byte_data(client, MAX6642_REG_R_DUMMY_3);
+	if (dummy_reg != 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

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Applied
#1 lore raw
Quoted 6 lines · Per Dalén

Changed according to the suggestions from Guenter Roeck:

* Check the manufacturer ID directly. Fastens the test if it's not a Maxim chip.
* Read the other non-existing registers after the manufacturer ID.

Quoted 6 lines

---
Check for non-existing registers, registers 0x04, 0x06 and 0xff, which are supported by other Maxim chips. Reading such registers returns the previously read value.

Signed-off-by Per Dalen <[email protected]>

Applied, with a couple of spelling errors fixed.

Quoted 1 line

---

On a side note, your comments from above should go here. As written, the comments end up in the commit log, but the description and your signature don't.

Thanks,
Guenter

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

#2 lore raw
In reply to Per Dalén
Quoted 19 lines · Per Dalén

Changed according to the suggestions from Guenter Roeck:

* Check the manufacturer ID directly. Fastens the test if it's not a Maxim chip.
* Read the other non-existing registers after the manufacturer ID.

---
Check for non-existing registers, registers 0x04, 0x06 and 0xff, which are supported by other Maxim chips. Reading such registers returns the previously read value.

Signed-off-by Per Dalen <[email protected]>

---

drivers/hwmon/max6642.c +0 -0
index 0f9fc40..565b507 100644

drivers/hwmon/max6642.c static const unsigned short normal_i2c[] = { · line 72

Quoted 9 lines
drivers/hwmon/max6642.c +6 -0
@@ -64,6 +64,14 @@ static const unsigned short normal_i2c[] = {
 #define MAX6642_REG_W_REMOTE_HIGH	0x0D
 
 /*
+ * Registers for detection tests, these registers isn't pressent and
+ * will only show 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

I don't even think you want defines for these. They don't exist! The comment would be better placed right before the relevant piece of code.

Quoted 21 lines
drivers/hwmon/max6642.c +6 -1
+
+/*
  * Conversions
  */
 

@@ -126,7 +134,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;
+	u8 reg_config, reg_status, man_id, dummy_reg;
 
 	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA))
 		return -ENODEV;

@@ -136,6 +144,19 @@ static int max6642_detect(struct i2c_client *client,
 	if (man_id != 0x4D)
 		return -ENODEV;
 
+	/* sanity check */
+	dummy_reg = i2c_smbus_read_byte_data(client, MAX6642_REG_R_DUMMY_1);
+	if (dummy_reg != 0x4D)

drivers/hwmon/max6642.c static int max6642_detect(struct i2c_client *client, · line 159

Quoted 10 lines
drivers/hwmon/max6642.c +10 -0
+		return -ENODEV;
+
+	dummy_reg = i2c_smbus_read_byte_data(client, MAX6642_REG_R_DUMMY_2);
+	if (dummy_reg != 0x4D)
+		return -ENODEV;
+
+	dummy_reg = i2c_smbus_read_byte_data(client, MAX6642_REG_R_DUMMY_3);
+	if (dummy_reg != 0x4D)
+		return -ENODEV;
+

Same as for sensors-detect, please do a second round of checks after reading the config or status register. You may want to move the checks to a small function to avoid repeating the code.

Quoted 4 lines
drivers/hwmon/max6642.c +0 -0
 	/*
 	 * 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
Signature · 6 lines
Jean Delvare

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors