sensors-detect: Add capability to detect

This patch adds detection of EMC1002, EMC1033, EMC1046, EMC1047, EMC1072, EMC1073, EMC1074, EMC1402, and EMC1424 to sensors-detect.

--
v2:
- Updated CHANGES
- More detailed description

Index: prog/detect/sensors-detect
=================================> --- prog/detect/sensors-detect (revision 5913)
+++ prog/detect/sensors-detect (working copy)

(quoted diff) +40 -0

@@ -1195,16 +1195,36 @@
 		i2c_addrs => [0x2f],
 		i2c_detect => sub { fintek_detect(@_, 7); },
 	}, {
+		name => "SMSC EMC1002",
+		driver => "to-be-written",
+		i2c_addrs => [0x3c, 0x3d, 0x4c, 0x4d],
+		i2c_detect => sub { emc1403_detect(@_, 4); },
+	}, {
 		name => "SMSC EMC1023",
 		driver => "to-be-written",	# emc1023
 		i2c_addrs => [0x48, 0x49, 0x4c, 0x4d],
 		i2c_detect => sub { emc1023_detect(@_, 0); },
 	}, {
+		name => "SMSC EMC1033",
+		driver => "to-be-written",
+		i2c_addrs => [0x3c, 0x3d, 0x4c, 0x4d],
+		i2c_detect => sub { emc1403_detect(@_, 5); },
+	}, {
 		name => "SMSC EMC1043",
 		driver => "to-be-written",	# emc1023
 		i2c_addrs => [0x48, 0x49, 0x4c, 0x4d],
 		i2c_detect => sub { emc1023_detect(@_, 1); },
 	}, {
+		name => "SMSC EMC1046",
+		driver => "to-be-written",
+		i2c_addrs => [0x4c, 0x4d],
+		i2c_detect => sub { emc1403_detect(@_, 6); },
+	}, {
+		name => "SMSC EMC1047",
+		driver => "to-be-written",
+		i2c_addrs => [0x10, 0x18],
+		i2c_detect => sub { emc1403_detect(@_, 7); },
+	}, {
 		name => "SMSC EMC1053",
 		driver => "to-be-written",	# emc1023
 		i2c_addrs => [0x48, 0x49, 0x4c, 0x4d],

@@ -1215,6 +1235,26 @@
 		i2c_addrs => [0x48, 0x49, 0x4c, 0x4d],
 		i2c_detect => sub { emc1023_detect(@_, 3); },
 	}, {
+		name => "SMSC EMC1072",
+		driver => "to-be-written",
+		i2c_addrs => [0x1c, 0x3c, 0x4c, 0x5c, 0x6c, 0x7c],
+		i2c_detect => sub { emc1403_detect(@_, 8); },
+	}, {
+		name => "SMSC EMC1073",
+		driver => "to-be-written",
+		i2c_addrs => [0x1c, 0x3c, 0x4c, 0x5c, 0x6c, 0x7c],
+		i2c_detect => sub { emc1403_detect(@_, 9); },
+	}, {
+		name => "SMSC EMC1074",
+		driver => "to-be-written",
+		i2c_addrs => [0x1c, 0x3c, 0x4c, 0x5c, 0x6c, 0x7c],
+		i2c_detect => sub { emc1403_detect(@_, 10); },
+	}, {
+		name => "SMSC EMC1402",
+		driver => "to-be-written",
+		i2c_addrs => [0x18, 0x19, 0x4c, 0x4d],
+		i2c_detect => sub { emc1403_detect(@_, 11); },
+	}, {
 		name => "SMSC EMC1403",
 		driver => "emc1403",
 		i2c_addrs => [0x18, 0x2a, 0x4c, 0x4d],

@@ -1230,6 +1270,11 @@
 		i2c_addrs => [0x18, 0x2a, 0x4c, 0x4d],

(quoted diff) +26 -4

+#	4 = EMC1002, 5 = EMC1033, 6 = EMC1046, 7 = EMC1047, 8 = EMC1072,
+#	9 = EMC1073, 10 = EMC1074, 11 = EMC1402, 12 = EMC1424
 # Registers used:
 #   0xfd: Device ID register
 #   0xfe: Vendor ID register

@@ -5460,18 +5509,45 @@
 
 	return unless $man_id = 0x5d;	# SMSC
 
-	if ($chip = 0) {
+	if ($chip = 0) {		# EMC1403
 		return unless $dev_id = 0x21;
 		return unless $rev = 0x01;
-	} elsif ($chip = 1) {
+	} elsif ($chip = 1) {		# EMC1404
 		return unless $dev_id = 0x25;
 		return unless $rev = 0x01;
-	} elsif ($chip = 2) {
+	} elsif ($chip = 2) {		# EMC2103
 		return unless ($dev_id = 0x24) || ($dev_id = 0x26);
 		return unless $rev = 0x01;
-	} elsif ($chip = 3) {
+	} elsif ($chip = 3) {		# EMC1423
 		return unless $dev_id = 0x23;
 		return unless $rev = 0x01;
+	} elsif ($chip = 4) {		# EMC1002
+		return unless ($dev_id = 0x02) || ($dev_id = 0x03);
+		return unless $rev = 0x01;
+	} elsif ($chip = 5) {		# EMC1033
+		return unless ($dev_id = 0x0a) || ($dev_id = 0x0b);
+		return unless $rev = 0x01;
+	} elsif ($chip = 6) {		# EMC1046
+		return unless $dev_id = 0x1a;
+		return unless $rev = 0x01;
+	} elsif ($chip = 7) {		# EMC1047
+		return unless $dev_id = 0x1c;
+		return unless $rev = 0x01;
+	} elsif ($chip = 8) {		# EMC1072
+		return unless $dev_id = 0x20;
+		return unless $rev = 0x03;
+	} elsif ($chip = 9) {		# EMC1073
+		return unless $dev_id = 0x21;
+		return unless $rev = 0x03;
+	} elsif ($chip = 10) {		# EMC1074
+		return unless $dev_id = 0x25;

(quoted diff) +0 -0

 3.2.0 (2010-10-10)
   libsensors: Increase MAX_SENSORS_PER_TYPE to 24
 

Hi Guenter,

Quoted 86 lines · Guenter Roeck

This patch adds detection of EMC1002, EMC1033, EMC1046, EMC1047, EMC1072, EMC1073, EMC1074, EMC1402, and EMC1424 to sensors-detect.

--
v2:
- Updated CHANGES
- More detailed description

Index: prog/detect/sensors-detect
=================================> > --- prog/detect/sensors-detect (revision 5913)
+++ prog/detect/sensors-detect (working copy)

(quoted diff) +45 -0

@@ -1195,16 +1195,36 @@
 		i2c_addrs => [0x2f],
 		i2c_detect => sub { fintek_detect(@_, 7); },
 	}, {
+		name => "SMSC EMC1002",
+		driver => "to-be-written",
+		i2c_addrs => [0x3c, 0x3d, 0x4c, 0x4d],
+		i2c_detect => sub { emc1403_detect(@_, 4); },
+	}, {
 		name => "SMSC EMC1023",
 		driver => "to-be-written",	# emc1023
 		i2c_addrs => [0x48, 0x49, 0x4c, 0x4d],
 		i2c_detect => sub { emc1023_detect(@_, 0); },
 	}, {
+		name => "SMSC EMC1033",
+		driver => "to-be-written",
+		i2c_addrs => [0x3c, 0x3d, 0x4c, 0x4d],
+		i2c_detect => sub { emc1403_detect(@_, 5); },
+	}, {
 		name => "SMSC EMC1043",
 		driver => "to-be-written",	# emc1023
 		i2c_addrs => [0x48, 0x49, 0x4c, 0x4d],
 		i2c_detect => sub { emc1023_detect(@_, 1); },
 	}, {
+		name => "SMSC EMC1046",
+		driver => "to-be-written",
+		i2c_addrs => [0x4c, 0x4d],
+		i2c_detect => sub { emc1403_detect(@_, 6); },
+	}, {
+		name => "SMSC EMC1047",
+		driver => "to-be-written",
+		i2c_addrs => [0x10, 0x18],
+		i2c_detect => sub { emc1403_detect(@_, 7); },
+	}, {
 		name => "SMSC EMC1053",
 		driver => "to-be-written",	# emc1023
 		i2c_addrs => [0x48, 0x49, 0x4c, 0x4d],

@@ -1215,6 +1235,26 @@
 		i2c_addrs => [0x48, 0x49, 0x4c, 0x4d],
 		i2c_detect => sub { emc1023_detect(@_, 3); },
 	}, {
+		name => "SMSC EMC1072",
+		driver => "to-be-written",
+		i2c_addrs => [0x1c, 0x3c, 0x4c, 0x5c, 0x6c, 0x7c],
+		i2c_detect => sub { emc1403_detect(@_, 8); },
+	}, {
+		name => "SMSC EMC1073",
+		driver => "to-be-written",
+		i2c_addrs => [0x1c, 0x3c, 0x4c, 0x5c, 0x6c, 0x7c],
+		i2c_detect => sub { emc1403_detect(@_, 9); },
+	}, {
+		name => "SMSC EMC1074",
+		driver => "to-be-written",
+		i2c_addrs => [0x1c, 0x3c, 0x4c, 0x5c, 0x6c, 0x7c],
+		i2c_detect => sub { emc1403_detect(@_, 10); },
+	}, {
+		name => "SMSC EMC1402",
+		driver => "to-be-written",
+		i2c_addrs => [0x18, 0x19, 0x4c, 0x4d],
+		i2c_detect => sub { emc1403_detect(@_, 11); },
+	}, {
 		name => "SMSC EMC1403",
 		driver => "emc1403",
 		i2c_addrs => [0x18, 0x2a, 0x4c, 0x4d],

@@ -1230,6 +1270,11 @@
 		i2c_addrs => [0x18, 0x2a, 0x4c, 0x4d],
 		i2c_detect => sub { emc1403_detect(@_, 3); },
 	}, {
+		name => "SMSC EMC1424",
+		driver => "to-be-written",
+		i2c_addrs => [0x4c],
+		i2c_detect => sub { emc1403_detect(@_, 12); },
+	}, {
 		name => "ST STTS424",
 		driver => "jc42",

Review line 1280

Quoted 1 line

(quoted diff) +0 -0

 		i2c_addrs => [0x18..0x1f],

These changes add I2C addresses 0x10, 0x3c, 0x3d, 0x6c and 0x7c to the list of addresses being probed (check with sensors-detect --stat). Adding new probed addresses is potentially dangerous, as we have had (regular and sometimes dramatic) reports of issues related to probing specific I2C addresses in the past. In general, I strongly suggest to _not_ add new addresses without a good reason.

So we have to think about the usage of the EMC devices in question. Do you know if any of these are being, or will be, used on PC boards? Remember that the purpose of sensors-detect is not to be a universal hardware monitoring device detector. It's primarily aimed at PC users. Users of embedded devices or exotic hardware should know what hardware monitoring devices they have, and these device should be instantiated by the kernel directly, rather than being detected from user-space.

If you look at sensors-detect, you'll see that for some other chips, we already skip unusual addresses on purpose (for the Maxim MAX6633/MAX6634/MAX6635 we skip 0x40-0x47 for example.)

BTW, probing address 0x7c is out of the question: it's nor even a valid 7-bit I2C address (marked as "Reserved for future purposes" in the I2C specification version 2.1.) And 0x10 is used by IPMI, so probing it would certainly cause trouble on may systems.

Quoted 4 lines

(quoted diff) +0 -0

@@ -5446,7 +5491,11 @@
 	return 7;
 }
 

Review line 5496

Quoted 4 lines

(quoted diff) +3 -1

-# Chip to detect: 0 = EMC1403, 1 = EMC1404, 2 = EMC2103, 3 = EMC1423
+# Chip to detect: 0 = EMC1403, 1 = EMC1404, 2 = EMC2103, 3 = EMC1423,
+#	4 = EMC1033, 5 = EMC1073, 6 = EMC1074, 7 = EMC1046, 8 = EMC1047,
+#	9 = EMC1402, 10 = EMC1072, 11 = EMC1424, 12 = EMC1002

The two lines above should be deleted, right?

Quoted 45 lines

(quoted diff) +26 -4

+#	4 = EMC1002, 5 = EMC1033, 6 = EMC1046, 7 = EMC1047, 8 = EMC1072,
+#	9 = EMC1073, 10 = EMC1074, 11 = EMC1402, 12 = EMC1424
 # Registers used:
 #   0xfd: Device ID register
 #   0xfe: Vendor ID register

@@ -5460,18 +5509,45 @@
 
 	return unless $man_id = 0x5d;	# SMSC
 
-	if ($chip = 0) {
+	if ($chip = 0) {		# EMC1403
 		return unless $dev_id = 0x21;
 		return unless $rev = 0x01;
-	} elsif ($chip = 1) {
+	} elsif ($chip = 1) {		# EMC1404
 		return unless $dev_id = 0x25;
 		return unless $rev = 0x01;
-	} elsif ($chip = 2) {
+	} elsif ($chip = 2) {		# EMC2103
 		return unless ($dev_id = 0x24) || ($dev_id = 0x26);
 		return unless $rev = 0x01;
-	} elsif ($chip = 3) {
+	} elsif ($chip = 3) {		# EMC1423
 		return unless $dev_id = 0x23;
 		return unless $rev = 0x01;
+	} elsif ($chip = 4) {		# EMC1002
+		return unless ($dev_id = 0x02) || ($dev_id = 0x03);
+		return unless $rev = 0x01;
+	} elsif ($chip = 5) {		# EMC1033
+		return unless ($dev_id = 0x0a) || ($dev_id = 0x0b);
+		return unless $rev = 0x01;
+	} elsif ($chip = 6) {		# EMC1046
+		return unless $dev_id = 0x1a;
+		return unless $rev = 0x01;
+	} elsif ($chip = 7) {		# EMC1047
+		return unless $dev_id = 0x1c;
+		return unless $rev = 0x01;
+	} elsif ($chip = 8) {		# EMC1072
+		return unless $dev_id = 0x20;
+		return unless $rev = 0x03;
+	} elsif ($chip = 9) {		# EMC1073
+		return unless $dev_id = 0x21;
+		return unless $rev = 0x03;
+	} elsif ($chip = 10) {		# EMC1074
+		return unless $dev_id = 0x25;

Review line 5553

Quoted 10 lines

(quoted diff) +7 -0

+		return unless $rev = 0x03;
+	} elsif ($chip = 11) {		# EMC1402
+		return unless $dev_id = 0x20;
+		return unless $rev = 0x01;
+	} elsif ($chip = 12) {		# EMC1424
+		return unless $dev_id = 0x27;
+		return unless $rev = 0x01;
 	}
 
 	return 6;

Looks OK.

Review line 26

Quoted 9 lines

Index: CHANGES
=================================> > --- CHANGES (revision 5913)
+++ CHANGES (working copy)

(quoted diff) +2 -0

@@ -22,6 +22,8 @@
                   Add detection of Maxim MAX6639
                   Add detection of EMC1023, EMC1043, EMC1053, and EMC1063
                   Add detection of Nuvoton NCT5571D, NCT5577D and NCT6776F
+                  Add detection of EMC1002, EMC1033, EMC1046, EMC1047,
+                        EMC1072, EMC1073, EMC1074, EMC1402, and EMC1424

Please specify that these are SMSC chips.

Quoted 7 lines · Jean Delvare

So we have to think about the usage of the EMC devices in question. Do you know if any of these are being, or will be, used on PC boards? Remember that the purpose of sensors-detect is not to be a universal hardware monitoring device detector. It's primarily aimed at PC users. Users of embedded devices or exotic hardware should know what hardware monitoring devices they have, and these device should be instantiated by the kernel directly, rather than being detected from user-space.

No idea. It wasn't specifically that I need it or that we are going to use any of the chips. I just got into it after the recent emc driver submission, and figured that since I tracked down the addresses and chip IDs, I might as well put my knowledge to some use.

Also, while you are right about platform data on embedded devices for the running code, there are additional aspects to consider.

First, I have often seen that HW specs for embedded devices can more or less only be seeen as guidance. Not only do chips show up on unexpected addresses, I have seen instances where chips were not there at all, or undocumented chips show up. This gets difficult if one has to port linux to some older device, where the HW designers may no longer be there.

Second, there is the case of people porting Linux to a box which wasn't supposed to run Linux.

In both cases, sensors-detect can be a very useful tool to help figuring out what is actually there.

How about removing the new addresses from the list ? Would you be ok with the patch if I do that ?