[lm-sensors] [PATCH] hwmon:lm75 add ADT75 support - requires

[lm-sensors] [PATCH] hwmon:lm75 add ADT75 support - requires

[michael.hennerich]

From: Michael Hennerich <michael.hennerich@analog.com>

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@cam.ac.uk>

Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
---
 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@dds.nl>
 
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;
+	}
+
+	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;
 }
-- 
1.7.0.4



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

Re: [lm-sensors] [PATCH] hwmon:lm75 add ADT75 support - requires

[Jonathan Cameron]

On 10/12/11 10:36, michael.hennerich@analog.com wrote:
> From: Michael Hennerich <michael.hennerich@analog.com>
> 
> 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@cam.ac.uk>
Definitely looks like it will fail on all the other parts supported, so should be fine.
> 
> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Acked-by: Jonathan Cameron <jic23@cam.ac.uk>

> ---
>  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@dds.nl>
>  
> 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;
> +	}
> +
> +	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;
>  }


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

Re: [lm-sensors] [PATCH] hwmon:lm75 add ADT75 support - requires

[Guenter Roeck]

On Wed, 2011-10-12 at 05:36 -0400, michael.hennerich@analog.com wrote:
> From: Michael Hennerich <michael.hennerich@analog.com>
> 
> 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@cam.ac.uk>
> 
> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
> ---
>  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@dds.nl>
>  
> 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.

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;
>  }



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

Re: [lm-sensors] [PATCH] hwmon:lm75 add ADT75 support - requires

[Jonathan Cameron]

On 10/12/11 16:19, Guenter Roeck wrote:
> On Wed, 2011-10-12 at 05:36 -0400, michael.hennerich@analog.com wrote:
>> From: Michael Hennerich <michael.hennerich@analog.com>
>>
>> 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@cam.ac.uk>
>>
>> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
>> ---
>>  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@dds.nl>
>>  
>> 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.

Good spot Guenter.
> 
> 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;
>>  }
> 
> 


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

Re: [lm-sensors] [PATCH] hwmon:lm75 add ADT75 support - requires

[Guenter Roeck]

On Wed, 2011-10-12 at 11:25 -0400, Jonathan Cameron wrote:
> On 10/12/11 16:19, Guenter Roeck wrote:
> > On Wed, 2011-10-12 at 05:36 -0400, michael.hennerich@analog.com wrote:
> >> From: Michael Hennerich <michael.hennerich@analog.com>
> >>
> >> 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@cam.ac.uk>
> >>
> >> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
> >> ---
> >>  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@dds.nl>
> >>  
> >> 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.
> 
Yes, that would make more sense. To improve detection quality, it might
even make sense to additionally check {6, 10, 14} against hyst. Or maybe
check {7, 15} against os and {6, 14} against hyst, since the address
cycling code will already cover registers 10 and 11.

Having said that, turns out we use a LM75 in one of out systems which
returns 0xff in registers 4..7. That won't work with the regular LM75
detection either. Interesting. I'll try to find out the manufacturer.
Here is the register dump, in case Jean listens in and would like to
have a copy.

     0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f    0123456789abcdef
00: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
10: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
20: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
30: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
40: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
50: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
60: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
70: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
80: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
90: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
a0: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
b0: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
c0: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
d0: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
e0: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
f0: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....

Thanks,
Guenter



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

Re: [lm-sensors] [PATCH] hwmon:lm75 add ADT75 support - requires

[Jean Delvare]

On Wed, 12 Oct 2011 11:36:58 +0200, michael.hennerich@analog.com wrote:
> From: Michael Hennerich <michael.hennerich@analog.com>
> 
> 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.

I object. The detection of the original LM75 is there for historical
reasons (that chip was very popular on PC motherboards at the end of
the 90's. Other compatible chips were never so popular on PC
motherboards and this is the reason why they are _not_ detected by the
lm75 driver. If you have any of these chips, you have to instantiate
them explicitly.

Same goes for the ADT75. I don't expect it on PC boards, but rather on
embedded designs where it should simply be instantiated, rather than
detected.

It's still fine to document the ADT75 as being supported by the driver,
and adding an lm75_type enum value for it. But no detection, please.

> 
> This patch is based on an reworked proposal from Jonathan Cameron <jic23@cam.ac.uk>
> 
> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
> ---
>  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@dds.nl>
>  
> 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;
> +	}
> +
> +	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;
>  }


-- 
Jean Delvare

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

Re: [lm-sensors] [PATCH] hwmon:lm75 add ADT75 support - requires

[Guenter Roeck]

On Wed, 2011-10-12 at 11:47 -0400, Jean Delvare wrote:
> On Wed, 12 Oct 2011 11:36:58 +0200, michael.hennerich@analog.com wrote:
> > From: Michael Hennerich <michael.hennerich@analog.com>
> > 
> > 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.
> 
> I object. The detection of the original LM75 is there for historical
> reasons (that chip was very popular on PC motherboards at the end of
> the 90's. Other compatible chips were never so popular on PC
> motherboards and this is the reason why they are _not_ detected by the
> lm75 driver. If you have any of these chips, you have to instantiate
> them explicitly.
> 
> Same goes for the ADT75. I don't expect it on PC boards, but rather on
> embedded designs where it should simply be instantiated, rather than
> detected.
> 
> It's still fine to document the ADT75 as being supported by the driver,
> and adding an lm75_type enum value for it. But no detection, please.
> 
Good point. I am fine with that.

Turns out the chip I referred to in my other e-mail is a TI TMP75. It
won't be detected either, but works fine in our design because I
instantiate it with i2c_register_board_info() during board
initialization. Which is why I never noticed that auto-detection does
not work for it.

Thanks,
Guenter



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

Re: [lm-sensors] [PATCH] hwmon:lm75 add ADT75 support - requires

[Jean Delvare]

On Wed, 12 Oct 2011 08:41:23 -0700, Guenter Roeck wrote:
> Having said that, turns out we use a LM75 in one of out systems which
> returns 0xff in registers 4..7. That won't work with the regular LM75
> detection either. Interesting. I'll try to find out the manufacturer.
> Here is the register dump, in case Jean listens in and would like to
> have a copy.
> 
>      0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f    0123456789abcdef
> 00: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
> 10: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
> 20: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
> 30: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
> 40: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
> 50: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
> 60: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
> 70: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
> 80: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
> 90: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
> a0: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
> b0: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
> c0: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
> d0: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
> e0: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
> f0: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....

I already have that one in my collection (TMP75), thanks.

-- 
Jean Delvare

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

Re: [lm-sensors] [PATCH] hwmon:lm75 add ADT75 support - requires

[Guenter Roeck]

On Wed, 2011-10-12 at 16:34 -0400, Jean Delvare wrote:
> On Wed, 12 Oct 2011 08:41:23 -0700, Guenter Roeck wrote:
> > Having said that, turns out we use a LM75 in one of out systems which
> > returns 0xff in registers 4..7. That won't work with the regular LM75
> > detection either. Interesting. I'll try to find out the manufacturer.
> > Here is the register dump, in case Jean listens in and would like to
> > have a copy.
> > 
> >      0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f    0123456789abcdef
> > 00: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
> > 10: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
> > 20: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
> > 30: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
> > 40: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
> > 50: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
> > 60: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
> > 70: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
> > 80: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
> > 90: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
> > a0: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
> > b0: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
> > c0: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
> > d0: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
> > e0: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
> > f0: 15 00 4b 50 ff ff ff ff 15 00 4b 50 ff ff ff ff    ?.KP....?.KP....
> 
> I already have that one in my collection (TMP75), thanks.
> 
Yes, I found that it is TMP75 afterwards. 

Maybe it would make sense to add a note to lm75_detect, indicating that
it is not _supposed_ to work on LM75 compatible chips.

Guenter



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

Re: [lm-sensors] [PATCH] hwmon:lm75 add ADT75 support - requires

[Michael Hennerich]

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@analog.com wrote:
>>> From: Michael Hennerich<michael.hennerich@analog.com>
>>>
>>> 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@cam.ac.uk>
>>>
>>> Signed-off-by: Michael Hennerich<michael.hennerich@analog.com>
>>> ---
>>>   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@dds.nl>
>>>
>>> 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@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon:lm75 add ADT75 support - requires

[Michael Hennerich]

[-- Attachment #1.1: Type: text/plain, Size: 2590 bytes --]

On 10/12/2011 06:02 PM, Guenter Roeck wrote:
> On Wed, 2011-10-12 at 11:47 -0400, Jean Delvare wrote:
>> On Wed, 12 Oct 2011 11:36:58 +0200, michael.hennerich@analog.com wrote:
>>> From: Michael Hennerich<michael.hennerich@analog.com>
>>>
>>> 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.
>> I object. The detection of the original LM75 is there for historical
>> reasons (that chip was very popular on PC motherboards at the end of
>> the 90's. Other compatible chips were never so popular on PC
>> motherboards and this is the reason why they are _not_ detected by the
>> lm75 driver. If you have any of these chips, you have to instantiate
>> them explicitly.
>>
>> Same goes for the ADT75. I don't expect it on PC boards, but rather on
>> embedded designs where it should simply be instantiated, rather than
>> detected.
>>
>> It's still fine to document the ADT75 as being supported by the driver,
>> and adding an lm75_type enum value for it. But no detection, please.
>>
> Good point. I am fine with that.
I'm fine with that too. Auto-detect will always be problematic without
dedicated id registers. I'll submit a new patch shortly.
> Turns out the chip I referred to in my other e-mail is a TI TMP75. It
> won't be detected either, but works fine in our design because I
> instantiate it with i2c_register_board_info() during board
> initialization. Which is why I never noticed that auto-detection does
> not work for it.
Considering the number of other LM75 derivatives this driver 
supports.Text oder Webseite übersetzen
I had wondered if every derivativehad the same behavioron unspecified 
registers.

Meinten Sie: Ich hätte mich gewundert, */wenn/* sich gleich verhalten hätten
Geben Sie Text oder eine Website-Adresse ein oder lassen Sie ein 
Dokument übersetzen <http://translate.google.de/?tr=f&hl=de>.
Abbrechen <http://translate.google.de/?tr=t&hl=de>


      Übersetzung von Deutsch nach Englisch

-- 

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


[-- Attachment #1.2: Type: text/html, Size: 5982 bytes --]

[-- Attachment #2: 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] [PATCH] hwmon:lm75 add ADT75 support - requires

[Jean Delvare]

On Wed, 12 Oct 2011 13:45:48 -0700, Guenter Roeck wrote:
> Maybe it would make sense to add a note to lm75_detect, indicating that
> it is not _supposed_ to work on LM75 compatible chips.

Good point, as this isn't the first time users (or developers) report
with different expectations. Will you write the patch, or do you want
me to?

-- 
Jean Delvare

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

Re: [lm-sensors] [PATCH] hwmon:lm75 add ADT75 support - requires

[Jean Delvare]

On Thu, 13 Oct 2011 08:12:43 +0200, Michael Hennerich wrote:
> Considering the number of other LM75 derivatives this driver 
> supports,
> I had wondered if every derivativehad the same behavioron unspecified 
> registers.

No, they definitely don't, but the point was never to auto-detect them
all (some are plain undetectable anyway.) The point was get lm-sensors
to work out of the box on 1997-1999 PC hardware, and the detection code
we had was enough for this. It has been improved since then mainly to
prevent misdetection of newer (incompatible) chips.

-- 
Jean Delvare

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

Re: [lm-sensors] [PATCH] hwmon:lm75 add ADT75 support - requires

[Guenter Roeck]

On Thu, Oct 13, 2011 at 02:20:17AM -0400, Jean Delvare wrote:
> On Wed, 12 Oct 2011 13:45:48 -0700, Guenter Roeck wrote:
> > Maybe it would make sense to add a note to lm75_detect, indicating that
> > it is not _supposed_ to work on LM75 compatible chips.
> 
> Good point, as this isn't the first time users (or developers) report
> with different expectations. Will you write the patch, or do you want
> me to?
> 
How about you write it, and I apply it ?

Thanks,
Guenter

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