Re: [PATCH] hwmon:lm75 add ADT75 support - requires slightly different detection

Michael Hennerich <michael.hennerich@xxxxxxxxxx> · Thu, 13 Oct 2011 08:04:36 +0200

On 10/12/2011 05:25 PM, Jonathan Cameron wrote:
On 10/12/11 16:19, Guenter Roeck wrote:
On Wed, 2011-10-12 at 05:36 -0400, michael.hennerich@xxxxxxxxxx wrote:
From: Michael Hennerich<michael.hennerich@xxxxxxxxxx>

The Analog Devices ADT75 has an additional register at 0x04
for initiating oneshot captures. It is not actively used in
this driver but we must avoid assuming it will behave as
0x05-0x07 do and return the last read value. In fact register 0x04
reads the same as register 0x00. And all registers are repetitive
mirrored around register 0x04. This fact is used to detect the ADT75.

This patch is based on an reworked proposal from Jonathan Cameron<jic23@xxxxxxxxx>

Signed-off-by: Michael Hennerich<michael.hennerich@xxxxxxxxxx>
---
  Documentation/hwmon/lm75 |    5 +++++
  drivers/hwmon/Kconfig    |    1 +
  drivers/hwmon/lm75.c     |   30 ++++++++++++++++++++++++++----
  3 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/Documentation/hwmon/lm75 b/Documentation/hwmon/lm75
index a179040..8d40d0f 100644
--- a/Documentation/hwmon/lm75
+++ b/Documentation/hwmon/lm75
@@ -32,6 +32,11 @@ Supported chips:
      Addresses scanned: I2C 0x48 - 0x4f
      Datasheet: Publicly available at the Microchip website
                 http://www.microchip.com/
+  * Analog Devices ADT75
+    Prefix: 'adt75'
+    Addresses scanned: I2C 0x48 - 0x4f
+    Datasheet: Publicly available at the Analog Devices website
+               http://www.analog.com/adt75

  Author: Frodo Looijaard<frodol@xxxxxx>

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 0b62c3c..497fd14 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -531,6 +531,7 @@ config SENSORS_LM75
        If you say yes here you get support for one common type of
        temperature sensor chip, with models including:

+            - Analog Devices ADT75
              - Dallas Semiconductor DS75 and DS1775
              - Maxim MAX6625 and MAX6626
              - Microchip MCP980x
diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
index ef902d5..0f0291c 100644
--- a/drivers/hwmon/lm75.c
+++ b/drivers/hwmon/lm75.c
@@ -35,6 +35,7 @@
   */

  enum lm75_type {            /* keep sorted in alphabetical order */
+    adt75,
      ds1775,
      ds75,
      lm75,
@@ -213,6 +214,7 @@ static int lm75_remove(struct i2c_client *client)
  }

  static const struct i2c_device_id lm75_ids[] = {
+    { "adt75", adt75, },
      { "ds1775", ds1775, },
      { "ds75", ds75, },
      { "lm75", lm75, },
@@ -241,7 +243,7 @@ static int lm75_detect(struct i2c_client *new_client,
      struct i2c_adapter *adapter = new_client->adapter;
      int i;
      int conf, hyst, os;
-    bool is_lm75a = 0;
+    bool is_lm75a = 0, is_adt75 = 0;

      if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA |
                                   I2C_FUNC_SMBUS_WORD_DATA))
@@ -259,7 +261,15 @@ static int lm75_detect(struct i2c_client *new_client,
         LM75s.  It has an ID byte of 0xaX (where X is the chip
         revision, with 1 being the only revision in existence) in
         register 7, and unused registers return 0xff rather than the
-       last read value. */
+       last read value.
+
+       The Analog Devices ADT75 has an additional register at 0x04
+       for initiating oneshot captures. It is not actively used in
+       this driver but we must avoid assuming it will behave as
+       0x05-0x07 do and return the last read value. In fact register 0x04
+       reads the same as register 0x00. And all registers are repetitive
+       mirrored around register 0x04.
+    */

      /* Unused bits */
      conf = i2c_smbus_read_byte_data(new_client, 1);
@@ -277,7 +287,16 @@ static int lm75_detect(struct i2c_client *new_client,
              is_lm75a = 1;
              hyst = i2c_smbus_read_byte_data(new_client, 2);
              os = i2c_smbus_read_byte_data(new_client, 3);
-    } else { /* Traditional style LM75 detection */
+    } else { /* ADT75 detection: mirrored registers */
+            os = i2c_smbus_read_byte_data(new_client, 3);
+            hyst = i2c_smbus_read_byte_data(new_client, 2);
+            if (i2c_smbus_read_byte_data(new_client, 7) == os
+             || i2c_smbus_read_byte_data(new_client, 11) == os
+             || i2c_smbus_read_byte_data(new_client, 15) == os)
+                    is_adt75 = 1;
This seems to be quite weak. It assumes the chip is ADT75 if any of
registers {7, 11, 15} matches register 3. We already know that on a
regular LM75, register 11 will match register 3, because the address
cycling code checks for it. So it seems to me that the code here will
always conclude that the chip is an ADT75.
Dratt, I read that as a load of ands which I guess is the intent.
Oops! - Indeed this was my intent.

Good spot Guenter.
Thanks.
Am I missing something ? Did anyone check this code on a LM75 ?

Thanks,
Guenter

+    }
+
+    if (!is_lm75a&&  !is_adt75) { /* Traditional style LM75 detection */
              /* Unused addresses */
              hyst = i2c_smbus_read_byte_data(new_client, 2);
              if (i2c_smbus_read_byte_data(new_client, 4) != hyst
@@ -304,7 +323,10 @@ static int lm75_detect(struct i2c_client *new_client,
                      return -ENODEV;
      }

-    strlcpy(info->type, is_lm75a ? "lm75a" : "lm75", I2C_NAME_SIZE);
+    if (!is_adt75)
+            strlcpy(info->type, is_lm75a ? "lm75a" : "lm75", I2C_NAME_SIZE);
+    else
+            strlcpy(info->type, "adt75", I2C_NAME_SIZE);

      return 0;
  }




--
Greetings,
Michael

--
Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif



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





