All of lore.kernel.org
 help / color / mirror / Atom feed
* [lm-sensors] [PATCH v3] sensors-detect: Add code to detect TMP400 and TMP435
@ 2014-12-09  3:42 Guenter Roeck
  2014-12-09 10:45 ` Jean Delvare
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Guenter Roeck @ 2014-12-09  3:42 UTC (permalink / raw)
  To: lm-sensors

Also strengthen chip detection for other TMP4xx chips,
and update driver support status for TMP431 and TMP432.

Write new function for various TMP4xx chips and separate
from lm90 detection.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v3: Move chip detection for various TMP4xx chips to its own function
    Add TMP400 detection

The new detection function was tested with TMP401, TMP411A, TMP431, TMP432,
and TMP435.

Note: Register dumps for the tested chips are available from
https://github.com/groeck/module-tests.git in the register-dumps directory.

 CHANGES                    |   2 +
 prog/detect/sensors-detect | 123 ++++++++++++++++++++++++++++++---------------
 2 files changed, 84 insertions(+), 41 deletions(-)

diff --git a/CHANGES b/CHANGES
index 638a8bf..e462aea 100644
--- a/CHANGES
+++ b/CHANGES
@@ -24,6 +24,8 @@ SVN HEAD
                   Document support for EMC1402, EMC1404, and EMC1424
                   Detect new revisions of EMC14xx
                   Add detection of EMC1422
+                  Document driver support for TMP431 and TMP432
+                  Add detection of TMP400 and TMP435
 
 3.3.5 "Happy Birthday Beddy" (2014-01-22)
   libsensors: Improve documentation of two functions
diff --git a/prog/detect/sensors-detect b/prog/detect/sensors-detect
index 448cf22..be10643 100755
--- a/prog/detect/sensors-detect
+++ b/prog/detect/sensors-detect
@@ -1003,15 +1003,30 @@ use vars qw(@i2c_adapter_names);
 		i2c_addrs => [0x4c],
 		i2c_detect => sub { lm90_detect(@_, 11); },
 	}, {
+		name => "Texas Instruments TMP400",
+		driver => "to-be-written",	# tmp401
+		i2c_addrs => [0x18..0x1a, 0x29..0x2b, 0x4c..0x4e],
+		i2c_detect => sub { tmp4xx_detect(@_, 0); },
+	}, {
 		name => "Texas Instruments TMP401",
 		driver => "tmp401",
 		i2c_addrs => [0x4c],
-		i2c_detect => sub { lm90_detect(@_, 9); },
+		i2c_detect => sub { tmp4xx_detect(@_, 1); },
+	}, {
+		name => "Texas Instruments TMP411A",
+		driver => "tmp401",
+		i2c_addrs => [0x4c],
+		i2c_detect => sub { tmp4xx_detect(@_, 2); },
 	}, {
-		name => "Texas Instruments TMP411",
+		name => "Texas Instruments TMP411B",
 		driver => "tmp401",
-		i2c_addrs => [0x4c..0x4e],
-		i2c_detect => sub { lm90_detect(@_, 10); },
+		i2c_addrs => [0x4d],
+		i2c_detect => sub { tmp4xx_detect(@_, 3); },
+	}, {
+		name => "Texas Instruments TMP411C",
+		driver => "tmp401",
+		i2c_addrs => [0x4e],
+		i2c_detect => sub { tmp4xx_detect(@_, 4); },
 	}, {
 		name => "Texas Instruments TMP421",
 		driver => "tmp421",
@@ -1029,14 +1044,19 @@ use vars qw(@i2c_adapter_names);
 		i2c_detect => sub { tmp42x_detect(@_, 2); },
 	}, {
 		name => "Texas Instruments TMP431",
-		driver => "to-be-written",		# tmp401
+		driver => "tmp401",
 		i2c_addrs => [0x4c, 0x4d],
-		i2c_detect => sub { lm90_detect(@_, 16); },
+		i2c_detect => sub { tmp4xx_detect(@_, 5); },
 	}, {
 		name => "Texas Instruments TMP432",
-		driver => "to-be-written",		# tmp401
+		driver => "tmp401",
 		i2c_addrs => [0x4c, 0x4d],
-		i2c_detect => sub { lm90_detect(@_, 17); },
+		i2c_detect => sub { tmp4xx_detect(@_, 6); },
+	}, {
+		name => "Texas Instruments TMP435",
+		driver => "tmp401",
+		i2c_addrs => [0x37, 0x48..0x4f],
+		i2c_detect => sub { tmp4xx_detect(@_, 7); },
 	}, {
 		name => "Texas Instruments TMP441",
 		driver => "tmp421",
@@ -1051,7 +1071,7 @@ use vars qw(@i2c_adapter_names);
 		name => "Texas Instruments TMP451",
 		driver => "lm90",
 		i2c_addrs => [0x4c],
-		i2c_detect => sub { lm90_detect(@_, 18); },
+		i2c_detect => sub { tmp4xx_detect(@_, 8); },
 	}, {
 		name => "Texas Instruments AMC6821",
 		driver => "amc6821",
@@ -4671,10 +4691,10 @@ sub max6680_95_detect
 # Chip to detect: 0 = LM90, 1 = LM89/LM99, 2 = LM86, 3 = ADM1032,
 #		  4 = MAX6654, 5 = ADT7461,
 #		  6 = MAX6646/MAX6647/MAX6648/MAX6649/MAX6692,
-#		  8 = W83L771W/G, 9 = TMP401, 10 = TMP411,
+#		  8 = W83L771W/G,
 #		  11 = W83L771AWG/ASG, 12 = MAX6690,
 #		  13 = ADT7461A/NCT1008, 14 = SA56004,
-#		  15 = G781, 16 = TMP431, 17 = TMP432, 18 = TMP451
+#		  15 = G781
 # Registers used:
 #   0x03: Configuration
 #   0x04: Conversion rate
@@ -4746,20 +4766,6 @@ sub lm90_detect
 		return if $mid != 0x5c;		# Winbond
 		return 6 if ($cid & 0xfe) = 0x00; # W83L771W/G
 	}
-	if ($chip = 9) {
-		return if ($conf & 0x1B) != 0;
-		return if $rate > 0x0F;
-		return if $mid != 0x55;		# Texas Instruments
-		return 8 if $cid = 0x11;	# TMP401
-	}
-	if ($chip = 10) {
-		return if ($conf & 0x1B) != 0;
-		return if $rate > 0x0F;
-		return if $mid != 0x55;		# Texas Instruments
-		return 6 if ($addr = 0x4c && $cid = 0x12); # TMP411A
-		return 6 if ($addr = 0x4d && $cid = 0x13); # TMP411B
-		return 6 if ($addr = 0x4e && $cid = 0x10); # TMP411C
-	}
 	if ($chip = 11) {
 		return if ($conf & 0x2a) != 0;
 		return if ($conf2 & 0xf8) != 0;
@@ -4792,24 +4798,59 @@ sub lm90_detect
 		return if $mid != 0x47;		# GMT
 		return 8 if $cid = 0x01;	# G781
 	}
-	if ($chip = 16) {
-		return if ($conf & 0x1B) != 0;
-		return if $rate > 0x0F;
-		return if $mid != 0x55;		# Texas Instruments
-		return 6 if ($cid = 0x31);	# TMP431A/B/C/D
-	}
-	if ($chip = 17) {
-		return if ($conf & 0x1B) != 0;
-		return if $rate > 0x0F;
-		return if $mid != 0x55;		# Texas Instruments
-		return 6 if ($cid = 0x32);	# TMP432A/B
-	}
-	if ($chip = 18) {
-		return if ($conf & 0x1B) != 0;
+	return;
+}
+
+# Chip to detect: 0 = TMP400, 1 = TMP401, 2 = TMP411A, 3 = TMP411B, 4 = TMP411C,
+#		  5 = TMP431, 6 = TMP432, 7 = TMP435, 8 = TMP451
+# Registers used:
+#   0x03: Configuration (4 unused bits)
+#   0x04: Conversion rate (value 0x0f or lower, 0x09 or lower for TMP451)
+#   0x10: Remote temperature low byte (4 unused bits)
+#   0x13: Remote temperature low limit, low byte (4 unused bits)
+#   0x14: Remote temperature high limit, low byte (4 unused bits)
+#   0x24: Digital filter control, 6 unused bits (TMP451 only)
+#   0xfe: Manufacturer ID
+#   0xff: Device ID
+sub tmp4xx_detect()
+{
+	my ($file, $addr, $chip) = @_;
+
+	my $mid = i2c_smbus_read_byte_data($file, 0xfe);
+	return if ($mid != 0x55);
+
+	my $cid = i2c_smbus_read_byte_data($file, 0xff);
+	my $conf = i2c_smbus_read_byte_data($file, 0x03);
+	my $rate = i2c_smbus_read_byte_data($file, 0x04);
+	my $limit1 = i2c_smbus_read_byte_data($file, 0x10);
+	my $limit2 = i2c_smbus_read_byte_data($file, 0x13);
+	my $limit3 = i2c_smbus_read_byte_data($file, 0x14);
+
+	return if ($conf & 0x1b);
+	return if ($rate > 0x0f);
+	return if ($limit1 & 0x0f);
+	return if ($limit2 & 0x0f);
+	return if ($limit3 & 0x0f);
+
+	return 8 if ($chip = 0 && $cid = 0x01); # TMP400
+	return 8 if ($chip = 1 && $cid = 0x11); # TMP401
+	return 8 if ($chip = 2 && $cid = 0x12); # TMP411A
+	return 8 if ($chip = 3 && $cid = 0x13); # TMP411B
+	return 8 if ($chip = 4 && $cid = 0x10); # TMP411C
+	return 8 if ($chip = 5 && $cid = 0x31); # TMP431
+	return 8 if ($chip = 6 && $cid = 0x32); # TMP432
+	return 8 if ($chip = 7 && $cid = 0x35); # TMP435
+
+	if ($chip = 8) {			  # TMP451
+		my $filter = i2c_smbus_read_byte_data($file, 0x24);
+
+		return if $cid != 0x00;
 		return if $rate > 0x09;
-		return if $mid != 0x55;		# Texas Instruments
-		return 4 if ($cid = 0x00);	# TMP451
+		return if $filter & 0xfc;
+
+		return 4;
 	}
+
 	return;
 }
 
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [lm-sensors] [PATCH v3] sensors-detect: Add code to detect TMP400 and TMP435
  2014-12-09  3:42 [lm-sensors] [PATCH v3] sensors-detect: Add code to detect TMP400 and TMP435 Guenter Roeck
@ 2014-12-09 10:45 ` Jean Delvare
  2014-12-09 11:34 ` Jean Delvare
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Jean Delvare @ 2014-12-09 10:45 UTC (permalink / raw)
  To: lm-sensors

Hi Guenter,

On Mon,  8 Dec 2014 19:42:40 -0800, Guenter Roeck wrote:
> Also strengthen chip detection for other TMP4xx chips,
> and update driver support status for TMP431 and TMP432.
> 
> Write new function for various TMP4xx chips and separate
> from lm90 detection.

Looks good. Minor comments inline, but this version is already good
enough to commit.

> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> v3: Move chip detection for various TMP4xx chips to its own function
>     Add TMP400 detection
> 
> The new detection function was tested with TMP401, TMP411A, TMP431, TMP432,
> and TMP435.
> 
> Note: Register dumps for the tested chips are available from
> https://github.com/groeck/module-tests.git in the register-dumps directory.
> 
>  CHANGES                    |   2 +
>  prog/detect/sensors-detect | 123 ++++++++++++++++++++++++++++++---------------
>  2 files changed, 84 insertions(+), 41 deletions(-)
> 
> diff --git a/CHANGES b/CHANGES
> index 638a8bf..e462aea 100644
> --- a/CHANGES
> +++ b/CHANGES
> @@ -24,6 +24,8 @@ SVN HEAD
>                    Document support for EMC1402, EMC1404, and EMC1424
>                    Detect new revisions of EMC14xx
>                    Add detection of EMC1422
> +                  Document driver support for TMP431 and TMP432
> +                  Add detection of TMP400 and TMP435
>  
>  3.3.5 "Happy Birthday Beddy" (2014-01-22)
>    libsensors: Improve documentation of two functions
> diff --git a/prog/detect/sensors-detect b/prog/detect/sensors-detect
> index 448cf22..be10643 100755
> --- a/prog/detect/sensors-detect
> +++ b/prog/detect/sensors-detect
> @@ -1003,15 +1003,30 @@ use vars qw(@i2c_adapter_names);
>  		i2c_addrs => [0x4c],
>  		i2c_detect => sub { lm90_detect(@_, 11); },
>  	}, {
> +		name => "Texas Instruments TMP400",
> +		driver => "to-be-written",	# tmp401
> +		i2c_addrs => [0x18..0x1a, 0x29..0x2b, 0x4c..0x4e],
> +		i2c_detect => sub { tmp4xx_detect(@_, 0); },
> +	}, {
>  		name => "Texas Instruments TMP401",
>  		driver => "tmp401",
>  		i2c_addrs => [0x4c],
> -		i2c_detect => sub { lm90_detect(@_, 9); },
> +		i2c_detect => sub { tmp4xx_detect(@_, 1); },
> +	}, {
> +		name => "Texas Instruments TMP411A",
> +		driver => "tmp401",
> +		i2c_addrs => [0x4c],
> +		i2c_detect => sub { tmp4xx_detect(@_, 2); },
>  	}, {
> -		name => "Texas Instruments TMP411",
> +		name => "Texas Instruments TMP411B",
>  		driver => "tmp401",
> -		i2c_addrs => [0x4c..0x4e],
> -		i2c_detect => sub { lm90_detect(@_, 10); },
> +		i2c_addrs => [0x4d],
> +		i2c_detect => sub { tmp4xx_detect(@_, 3); },
> +	}, {
> +		name => "Texas Instruments TMP411C",
> +		driver => "tmp401",
> +		i2c_addrs => [0x4e],
> +		i2c_detect => sub { tmp4xx_detect(@_, 4); },
>  	}, {
>  		name => "Texas Instruments TMP421",
>  		driver => "tmp421",
> @@ -1029,14 +1044,19 @@ use vars qw(@i2c_adapter_names);
>  		i2c_detect => sub { tmp42x_detect(@_, 2); },
>  	}, {
>  		name => "Texas Instruments TMP431",
> -		driver => "to-be-written",		# tmp401
> +		driver => "tmp401",
>  		i2c_addrs => [0x4c, 0x4d],
> -		i2c_detect => sub { lm90_detect(@_, 16); },
> +		i2c_detect => sub { tmp4xx_detect(@_, 5); },
>  	}, {
>  		name => "Texas Instruments TMP432",
> -		driver => "to-be-written",		# tmp401
> +		driver => "tmp401",
>  		i2c_addrs => [0x4c, 0x4d],
> -		i2c_detect => sub { lm90_detect(@_, 17); },
> +		i2c_detect => sub { tmp4xx_detect(@_, 6); },
> +	}, {
> +		name => "Texas Instruments TMP435",
> +		driver => "tmp401",
> +		i2c_addrs => [0x37, 0x48..0x4f],
> +		i2c_detect => sub { tmp4xx_detect(@_, 7); },
>  	}, {
>  		name => "Texas Instruments TMP441",
>  		driver => "tmp421",
> @@ -1051,7 +1071,7 @@ use vars qw(@i2c_adapter_names);
>  		name => "Texas Instruments TMP451",
>  		driver => "lm90",
>  		i2c_addrs => [0x4c],
> -		i2c_detect => sub { lm90_detect(@_, 18); },
> +		i2c_detect => sub { tmp4xx_detect(@_, 8); },
>  	}, {
>  		name => "Texas Instruments AMC6821",
>  		driver => "amc6821",
> @@ -4671,10 +4691,10 @@ sub max6680_95_detect
>  # Chip to detect: 0 = LM90, 1 = LM89/LM99, 2 = LM86, 3 = ADM1032,
>  #		  4 = MAX6654, 5 = ADT7461,
>  #		  6 = MAX6646/MAX6647/MAX6648/MAX6649/MAX6692,
> -#		  8 = W83L771W/G, 9 = TMP401, 10 = TMP411,
> +#		  8 = W83L771W/G,
>  #		  11 = W83L771AWG/ASG, 12 = MAX6690,
>  #		  13 = ADT7461A/NCT1008, 14 = SA56004,
> -#		  15 = G781, 16 = TMP431, 17 = TMP432, 18 = TMP451
> +#		  15 = G781
>  # Registers used:
>  #   0x03: Configuration
>  #   0x04: Conversion rate
> @@ -4746,20 +4766,6 @@ sub lm90_detect
>  		return if $mid != 0x5c;		# Winbond
>  		return 6 if ($cid & 0xfe) = 0x00; # W83L771W/G
>  	}
> -	if ($chip = 9) {
> -		return if ($conf & 0x1B) != 0;
> -		return if $rate > 0x0F;
> -		return if $mid != 0x55;		# Texas Instruments
> -		return 8 if $cid = 0x11;	# TMP401
> -	}
> -	if ($chip = 10) {
> -		return if ($conf & 0x1B) != 0;
> -		return if $rate > 0x0F;
> -		return if $mid != 0x55;		# Texas Instruments
> -		return 6 if ($addr = 0x4c && $cid = 0x12); # TMP411A
> -		return 6 if ($addr = 0x4d && $cid = 0x13); # TMP411B
> -		return 6 if ($addr = 0x4e && $cid = 0x10); # TMP411C
> -	}
>  	if ($chip = 11) {
>  		return if ($conf & 0x2a) != 0;
>  		return if ($conf2 & 0xf8) != 0;
> @@ -4792,24 +4798,59 @@ sub lm90_detect
>  		return if $mid != 0x47;		# GMT
>  		return 8 if $cid = 0x01;	# G781
>  	}
> -	if ($chip = 16) {
> -		return if ($conf & 0x1B) != 0;
> -		return if $rate > 0x0F;
> -		return if $mid != 0x55;		# Texas Instruments
> -		return 6 if ($cid = 0x31);	# TMP431A/B/C/D
> -	}
> -	if ($chip = 17) {
> -		return if ($conf & 0x1B) != 0;
> -		return if $rate > 0x0F;
> -		return if $mid != 0x55;		# Texas Instruments
> -		return 6 if ($cid = 0x32);	# TMP432A/B
> -	}
> -	if ($chip = 18) {
> -		return if ($conf & 0x1B) != 0;
> +	return;
> +}
> +
> +# Chip to detect: 0 = TMP400, 1 = TMP401, 2 = TMP411A, 3 = TMP411B, 4 = TMP411C,
> +#		  5 = TMP431, 6 = TMP432, 7 = TMP435, 8 = TMP451
> +# Registers used:
> +#   0x03: Configuration (4 unused bits)
> +#   0x04: Conversion rate (value 0x0f or lower, 0x09 or lower for TMP451)
> +#   0x10: Remote temperature low byte (4 unused bits)
> +#   0x13: Remote temperature low limit, low byte (4 unused bits)
> +#   0x14: Remote temperature high limit, low byte (4 unused bits)
> +#   0x24: Digital filter control, 6 unused bits (TMP451 only)
> +#   0xfe: Manufacturer ID
> +#   0xff: Device ID
> +sub tmp4xx_detect()

I think I would name this function tmp401_detect instead, for the same
reason I don't like x's in driver names: tmp4xx matches tmp421, but the
function doesn't cover it.

> +{
> +	my ($file, $addr, $chip) = @_;
> +
> +	my $mid = i2c_smbus_read_byte_data($file, 0xfe);
> +	return if ($mid != 0x55);
> +
> +	my $cid = i2c_smbus_read_byte_data($file, 0xff);
> +	my $conf = i2c_smbus_read_byte_data($file, 0x03);
> +	my $rate = i2c_smbus_read_byte_data($file, 0x04);
> +	my $limit1 = i2c_smbus_read_byte_data($file, 0x10);

Name is confusing, as this is not a limit register.

> +	my $limit2 = i2c_smbus_read_byte_data($file, 0x13);
> +	my $limit3 = i2c_smbus_read_byte_data($file, 0x14);
> +
> +	return if ($conf & 0x1b);
> +	return if ($rate > 0x0f);
> +	return if ($limit1 & 0x0f);
> +	return if ($limit2 & 0x0f);
> +	return if ($limit3 & 0x0f);
> +
> +	return 8 if ($chip = 0 && $cid = 0x01); # TMP400
> +	return 8 if ($chip = 1 && $cid = 0x11); # TMP401
> +	return 8 if ($chip = 2 && $cid = 0x12); # TMP411A
> +	return 8 if ($chip = 3 && $cid = 0x13); # TMP411B
> +	return 8 if ($chip = 4 && $cid = 0x10); # TMP411C
> +	return 8 if ($chip = 5 && $cid = 0x31); # TMP431
> +	return 8 if ($chip = 6 && $cid = 0x32); # TMP432
> +	return 8 if ($chip = 7 && $cid = 0x35); # TMP435
> +
> +	if ($chip = 8) {			  # TMP451
> +		my $filter = i2c_smbus_read_byte_data($file, 0x24);
> +
> +		return if $cid != 0x00;
>  		return if $rate > 0x09;
> -		return if $mid != 0x55;		# Texas Instruments
> -		return 4 if ($cid = 0x00);	# TMP451
> +		return if $filter & 0xfc;

If you want to improve reliability further, you could check for 4
unused bits in limit registers 0x12 and 0x15.

> +
> +		return 4;
>  	}
> +
>  	return;
>  }
>  

Tested OK.

Thanks,
-- 
Jean Delvare
SUSE L3 Support

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [lm-sensors] [PATCH v3] sensors-detect: Add code to detect TMP400 and TMP435
  2014-12-09  3:42 [lm-sensors] [PATCH v3] sensors-detect: Add code to detect TMP400 and TMP435 Guenter Roeck
  2014-12-09 10:45 ` Jean Delvare
@ 2014-12-09 11:34 ` Jean Delvare
  2014-12-09 14:46 ` Guenter Roeck
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Jean Delvare @ 2014-12-09 11:34 UTC (permalink / raw)
  To: lm-sensors

On Tue, 9 Dec 2014 11:45:52 +0100, Jean Delvare wrote:
> Hi Guenter,
> 
> On Mon,  8 Dec 2014 19:42:40 -0800, Guenter Roeck wrote:
> > Also strengthen chip detection for other TMP4xx chips,
> > and update driver support status for TMP431 and TMP432.
> > 
> > Write new function for various TMP4xx chips and separate
> > from lm90 detection.
> 
> Looks good. Minor comments inline, but this version is already good
> enough to commit.

Oh, and please add an entry for the TMP400 to wiki/Devices.

Thanks,
-- 
Jean Delvare
SUSE L3 Support

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [lm-sensors] [PATCH v3] sensors-detect: Add code to detect TMP400 and TMP435
  2014-12-09  3:42 [lm-sensors] [PATCH v3] sensors-detect: Add code to detect TMP400 and TMP435 Guenter Roeck
  2014-12-09 10:45 ` Jean Delvare
  2014-12-09 11:34 ` Jean Delvare
@ 2014-12-09 14:46 ` Guenter Roeck
  2014-12-09 16:24 ` Guenter Roeck
  2014-12-11 12:54 ` Jean Delvare
  4 siblings, 0 replies; 6+ messages in thread
From: Guenter Roeck @ 2014-12-09 14:46 UTC (permalink / raw)
  To: lm-sensors

On 12/09/2014 02:45 AM, Jean Delvare wrote:
> Hi Guenter,
>
> On Mon,  8 Dec 2014 19:42:40 -0800, Guenter Roeck wrote:
>> Also strengthen chip detection for other TMP4xx chips,
>> and update driver support status for TMP431 and TMP432.
>>
>> Write new function for various TMP4xx chips and separate
>> from lm90 detection.
>
> Looks good. Minor comments inline, but this version is already good
> enough to commit.
>
Hi Jean,

thanks a lot for the review.

>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> ---
>> v3: Move chip detection for various TMP4xx chips to its own function
>>      Add TMP400 detection
>>
>> The new detection function was tested with TMP401, TMP411A, TMP431, TMP432,
>> and TMP435.
>>
>> Note: Register dumps for the tested chips are available from
>> https://github.com/groeck/module-tests.git in the register-dumps directory.
>>
>>   CHANGES                    |   2 +
>>   prog/detect/sensors-detect | 123 ++++++++++++++++++++++++++++++---------------
>>   2 files changed, 84 insertions(+), 41 deletions(-)
>>
>> diff --git a/CHANGES b/CHANGES
>> index 638a8bf..e462aea 100644
>> --- a/CHANGES
>> +++ b/CHANGES
>> @@ -24,6 +24,8 @@ SVN HEAD
>>                     Document support for EMC1402, EMC1404, and EMC1424
>>                     Detect new revisions of EMC14xx
>>                     Add detection of EMC1422
>> +                  Document driver support for TMP431 and TMP432
>> +                  Add detection of TMP400 and TMP435
>>
>>   3.3.5 "Happy Birthday Beddy" (2014-01-22)
>>     libsensors: Improve documentation of two functions
>> diff --git a/prog/detect/sensors-detect b/prog/detect/sensors-detect
>> index 448cf22..be10643 100755
>> --- a/prog/detect/sensors-detect
>> +++ b/prog/detect/sensors-detect
>> @@ -1003,15 +1003,30 @@ use vars qw(@i2c_adapter_names);
>>   		i2c_addrs => [0x4c],
>>   		i2c_detect => sub { lm90_detect(@_, 11); },
>>   	}, {
>> +		name => "Texas Instruments TMP400",
>> +		driver => "to-be-written",	# tmp401
>> +		i2c_addrs => [0x18..0x1a, 0x29..0x2b, 0x4c..0x4e],
>> +		i2c_detect => sub { tmp4xx_detect(@_, 0); },
>> +	}, {
>>   		name => "Texas Instruments TMP401",
>>   		driver => "tmp401",
>>   		i2c_addrs => [0x4c],
>> -		i2c_detect => sub { lm90_detect(@_, 9); },
>> +		i2c_detect => sub { tmp4xx_detect(@_, 1); },
>> +	}, {
>> +		name => "Texas Instruments TMP411A",
>> +		driver => "tmp401",
>> +		i2c_addrs => [0x4c],
>> +		i2c_detect => sub { tmp4xx_detect(@_, 2); },
>>   	}, {
>> -		name => "Texas Instruments TMP411",
>> +		name => "Texas Instruments TMP411B",
>>   		driver => "tmp401",
>> -		i2c_addrs => [0x4c..0x4e],
>> -		i2c_detect => sub { lm90_detect(@_, 10); },
>> +		i2c_addrs => [0x4d],
>> +		i2c_detect => sub { tmp4xx_detect(@_, 3); },
>> +	}, {
>> +		name => "Texas Instruments TMP411C",
>> +		driver => "tmp401",
>> +		i2c_addrs => [0x4e],
>> +		i2c_detect => sub { tmp4xx_detect(@_, 4); },
>>   	}, {
>>   		name => "Texas Instruments TMP421",
>>   		driver => "tmp421",
>> @@ -1029,14 +1044,19 @@ use vars qw(@i2c_adapter_names);
>>   		i2c_detect => sub { tmp42x_detect(@_, 2); },
>>   	}, {
>>   		name => "Texas Instruments TMP431",
>> -		driver => "to-be-written",		# tmp401
>> +		driver => "tmp401",
>>   		i2c_addrs => [0x4c, 0x4d],
>> -		i2c_detect => sub { lm90_detect(@_, 16); },
>> +		i2c_detect => sub { tmp4xx_detect(@_, 5); },
>>   	}, {
>>   		name => "Texas Instruments TMP432",
>> -		driver => "to-be-written",		# tmp401
>> +		driver => "tmp401",
>>   		i2c_addrs => [0x4c, 0x4d],
>> -		i2c_detect => sub { lm90_detect(@_, 17); },
>> +		i2c_detect => sub { tmp4xx_detect(@_, 6); },
>> +	}, {
>> +		name => "Texas Instruments TMP435",
>> +		driver => "tmp401",
>> +		i2c_addrs => [0x37, 0x48..0x4f],
>> +		i2c_detect => sub { tmp4xx_detect(@_, 7); },
>>   	}, {
>>   		name => "Texas Instruments TMP441",
>>   		driver => "tmp421",
>> @@ -1051,7 +1071,7 @@ use vars qw(@i2c_adapter_names);
>>   		name => "Texas Instruments TMP451",
>>   		driver => "lm90",
>>   		i2c_addrs => [0x4c],
>> -		i2c_detect => sub { lm90_detect(@_, 18); },
>> +		i2c_detect => sub { tmp4xx_detect(@_, 8); },
>>   	}, {
>>   		name => "Texas Instruments AMC6821",
>>   		driver => "amc6821",
>> @@ -4671,10 +4691,10 @@ sub max6680_95_detect
>>   # Chip to detect: 0 = LM90, 1 = LM89/LM99, 2 = LM86, 3 = ADM1032,
>>   #		  4 = MAX6654, 5 = ADT7461,
>>   #		  6 = MAX6646/MAX6647/MAX6648/MAX6649/MAX6692,
>> -#		  8 = W83L771W/G, 9 = TMP401, 10 = TMP411,
>> +#		  8 = W83L771W/G,
>>   #		  11 = W83L771AWG/ASG, 12 = MAX6690,
>>   #		  13 = ADT7461A/NCT1008, 14 = SA56004,
>> -#		  15 = G781, 16 = TMP431, 17 = TMP432, 18 = TMP451
>> +#		  15 = G781
>>   # Registers used:
>>   #   0x03: Configuration
>>   #   0x04: Conversion rate
>> @@ -4746,20 +4766,6 @@ sub lm90_detect
>>   		return if $mid != 0x5c;		# Winbond
>>   		return 6 if ($cid & 0xfe) = 0x00; # W83L771W/G
>>   	}
>> -	if ($chip = 9) {
>> -		return if ($conf & 0x1B) != 0;
>> -		return if $rate > 0x0F;
>> -		return if $mid != 0x55;		# Texas Instruments
>> -		return 8 if $cid = 0x11;	# TMP401
>> -	}
>> -	if ($chip = 10) {
>> -		return if ($conf & 0x1B) != 0;
>> -		return if $rate > 0x0F;
>> -		return if $mid != 0x55;		# Texas Instruments
>> -		return 6 if ($addr = 0x4c && $cid = 0x12); # TMP411A
>> -		return 6 if ($addr = 0x4d && $cid = 0x13); # TMP411B
>> -		return 6 if ($addr = 0x4e && $cid = 0x10); # TMP411C
>> -	}
>>   	if ($chip = 11) {
>>   		return if ($conf & 0x2a) != 0;
>>   		return if ($conf2 & 0xf8) != 0;
>> @@ -4792,24 +4798,59 @@ sub lm90_detect
>>   		return if $mid != 0x47;		# GMT
>>   		return 8 if $cid = 0x01;	# G781
>>   	}
>> -	if ($chip = 16) {
>> -		return if ($conf & 0x1B) != 0;
>> -		return if $rate > 0x0F;
>> -		return if $mid != 0x55;		# Texas Instruments
>> -		return 6 if ($cid = 0x31);	# TMP431A/B/C/D
>> -	}
>> -	if ($chip = 17) {
>> -		return if ($conf & 0x1B) != 0;
>> -		return if $rate > 0x0F;
>> -		return if $mid != 0x55;		# Texas Instruments
>> -		return 6 if ($cid = 0x32);	# TMP432A/B
>> -	}
>> -	if ($chip = 18) {
>> -		return if ($conf & 0x1B) != 0;
>> +	return;
>> +}
>> +
>> +# Chip to detect: 0 = TMP400, 1 = TMP401, 2 = TMP411A, 3 = TMP411B, 4 = TMP411C,
>> +#		  5 = TMP431, 6 = TMP432, 7 = TMP435, 8 = TMP451
>> +# Registers used:
>> +#   0x03: Configuration (4 unused bits)
>> +#   0x04: Conversion rate (value 0x0f or lower, 0x09 or lower for TMP451)
>> +#   0x10: Remote temperature low byte (4 unused bits)
>> +#   0x13: Remote temperature low limit, low byte (4 unused bits)
>> +#   0x14: Remote temperature high limit, low byte (4 unused bits)
>> +#   0x24: Digital filter control, 6 unused bits (TMP451 only)
>> +#   0xfe: Manufacturer ID
>> +#   0xff: Device ID
>> +sub tmp4xx_detect()
>
> I think I would name this function tmp401_detect instead, for the same
> reason I don't like x's in driver names: tmp4xx matches tmp421, but the
> function doesn't cover it.
>
Renamed.

>> +{
>> +	my ($file, $addr, $chip) = @_;
>> +
>> +	my $mid = i2c_smbus_read_byte_data($file, 0xfe);
>> +	return if ($mid != 0x55);
>> +
>> +	my $cid = i2c_smbus_read_byte_data($file, 0xff);
>> +	my $conf = i2c_smbus_read_byte_data($file, 0x03);
>> +	my $rate = i2c_smbus_read_byte_data($file, 0x04);
>> +	my $limit1 = i2c_smbus_read_byte_data($file, 0x10);
>
> Name is confusing, as this is not a limit register.
>
Renamed to rtemp, rhigh, rlow.

>> +	my $limit2 = i2c_smbus_read_byte_data($file, 0x13);
>> +	my $limit3 = i2c_smbus_read_byte_data($file, 0x14);
>> +
>> +	return if ($conf & 0x1b);
>> +	return if ($rate > 0x0f);
>> +	return if ($limit1 & 0x0f);
>> +	return if ($limit2 & 0x0f);
>> +	return if ($limit3 & 0x0f);
>> +
>> +	return 8 if ($chip = 0 && $cid = 0x01); # TMP400
>> +	return 8 if ($chip = 1 && $cid = 0x11); # TMP401
>> +	return 8 if ($chip = 2 && $cid = 0x12); # TMP411A
>> +	return 8 if ($chip = 3 && $cid = 0x13); # TMP411B
>> +	return 8 if ($chip = 4 && $cid = 0x10); # TMP411C
>> +	return 8 if ($chip = 5 && $cid = 0x31); # TMP431
>> +	return 8 if ($chip = 6 && $cid = 0x32); # TMP432
>> +	return 8 if ($chip = 7 && $cid = 0x35); # TMP435
>> +
>> +	if ($chip = 8) {			  # TMP451
>> +		my $filter = i2c_smbus_read_byte_data($file, 0x24);
>> +
>> +		return if $cid != 0x00;
>>   		return if $rate > 0x09;
>> -		return if $mid != 0x55;		# Texas Instruments
>> -		return 4 if ($cid = 0x00);	# TMP451
>> +		return if $filter & 0xfc;
>
> If you want to improve reliability further, you could check for 4
> unused bits in limit registers 0x12 and 0x15.
>
Not sure if that would improve much. I would rather test for some
non-zeros. I am going to get one of those chips and check if
the consecutive alert register (0x22) has bit 0 set or not
(the datasheet contradicts itself, and the bit is set in the
register dump I have available).

We could also check if (some of) the limit registers are non-zero.
Would that make sense ?

Thanks,
Guenter


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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [lm-sensors] [PATCH v3] sensors-detect: Add code to detect TMP400 and TMP435
  2014-12-09  3:42 [lm-sensors] [PATCH v3] sensors-detect: Add code to detect TMP400 and TMP435 Guenter Roeck
                   ` (2 preceding siblings ...)
  2014-12-09 14:46 ` Guenter Roeck
@ 2014-12-09 16:24 ` Guenter Roeck
  2014-12-11 12:54 ` Jean Delvare
  4 siblings, 0 replies; 6+ messages in thread
From: Guenter Roeck @ 2014-12-09 16:24 UTC (permalink / raw)
  To: lm-sensors

On Tue, Dec 09, 2014 at 12:34:53PM +0100, Jean Delvare wrote:
> On Tue, 9 Dec 2014 11:45:52 +0100, Jean Delvare wrote:
> > Hi Guenter,
> > 
> > On Mon,  8 Dec 2014 19:42:40 -0800, Guenter Roeck wrote:
> > > Also strengthen chip detection for other TMP4xx chips,
> > > and update driver support status for TMP431 and TMP432.
> > > 
> > > Write new function for various TMP4xx chips and separate
> > > from lm90 detection.
> > 
> > Looks good. Minor comments inline, but this version is already good
> > enough to commit.
> 
> Oh, and please add an entry for the TMP400 to wiki/Devices.
> 

Done.

Thanks,
Guenter

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [lm-sensors] [PATCH v3] sensors-detect: Add code to detect TMP400 and TMP435
  2014-12-09  3:42 [lm-sensors] [PATCH v3] sensors-detect: Add code to detect TMP400 and TMP435 Guenter Roeck
                   ` (3 preceding siblings ...)
  2014-12-09 16:24 ` Guenter Roeck
@ 2014-12-11 12:54 ` Jean Delvare
  4 siblings, 0 replies; 6+ messages in thread
From: Jean Delvare @ 2014-12-11 12:54 UTC (permalink / raw)
  To: lm-sensors

Hi Guenter,

On Tue, 09 Dec 2014 06:46:46 -0800, Guenter Roeck wrote:
> On 12/09/2014 02:45 AM, Jean Delvare wrote:
> > On Mon,  8 Dec 2014 19:42:40 -0800, Guenter Roeck wrote:
> >> +	if ($chip = 8) {			  # TMP451
> >> +		my $filter = i2c_smbus_read_byte_data($file, 0x24);
> >> +
> >> +		return if $cid != 0x00;
> >>   		return if $rate > 0x09;
> >> -		return if $mid != 0x55;		# Texas Instruments
> >> -		return 4 if ($cid = 0x00);	# TMP451
> >> +		return if $filter & 0xfc;
> >
> > If you want to improve reliability further, you could check for 4
> > unused bits in limit registers 0x12 and 0x15.
>
> Not sure if that would improve much. I would rather test for some
> non-zeros. I am going to get one of those chips and check if
> the consecutive alert register (0x22) has bit 0 set or not
> (the datasheet contradicts itself, and the bit is set in the
> register dump I have available).

Of course, testing for a non-zero bit is more valuable than testing for
a zero bit. But non-zero bits are rare.

> We could also check if (some of) the limit registers are non-zero.
> Would that make sense ?

Yes it would. For all chips without a reasonable chip ID register
value, every trick is welcome. You can also check that the low limit is
strictly lower than the high temperature limit, for example.

-- 
Jean Delvare
SUSE L3 Support

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-12-11 12:54 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-09  3:42 [lm-sensors] [PATCH v3] sensors-detect: Add code to detect TMP400 and TMP435 Guenter Roeck
2014-12-09 10:45 ` Jean Delvare
2014-12-09 11:34 ` Jean Delvare
2014-12-09 14:46 ` Guenter Roeck
2014-12-09 16:24 ` Guenter Roeck
2014-12-11 12:54 ` Jean Delvare

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.