All of lore.kernel.org
 help / color / mirror / Atom feed
* [lm-sensors]  W83667HG driver test for in6 and temp3
@ 2009-02-25  9:13 JGong
  2009-02-25 15:22 ` Jean Delvare
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: JGong @ 2009-02-25  9:13 UTC (permalink / raw)
  To: lm-sensors

[-- Attachment #1: Type: text/plain, Size: 14025 bytes --]

Dear all,

>I suggest that you make a patch implementing what was said above. Then
>Michael will test it and we'll see which of in6 or temp3 will be
>created on his board. If in6 the problem is solved (as far as Michael's
>system is concerned, at least.) If the incorrect temp3 is displayed
>then we will have to think of a workaround. Either we check the
>temperature value and discard it if it looks unreasonable, or we let
>the user override the configuration manually as I initially proposed. I
>think I prefer the second option. Choosing the configuration based on
>the monitored values somehow voids the point of monitoring.
>Please make sure you include my fan fixes into your patch:
>http://lists.lm-sensors.org/pipermail/lm-sensors/2009-January/025232.html

I've made the patch out with what we discussed above (including the fan fix from Jean). It has been tested on my ASUS P5QL PRO.

w83667hg-isa-0290
Adapter: ISA adapter
in0:         +1.01 V  (min =  +0.00 V, max =  +1.74 V)
in1:         +1.69 V  (min =  +1.64 V, max =  +2.00 V)
in2:         +3.33 V  (min =  +3.14 V, max =  +3.47 V)
in3:         +3.30 V  (min =  +3.14 V, max =  +3.47 V)
in4:         +1.68 V  (min =  +0.02 V, max =  +0.35 V)   ALARM
in5:         +2.04 V  (min =  +0.26 V, max =  +1.03 V)   ALARM
in7:         +3.39 V  (min =  +3.14 V, max =  +3.47 V)
in8:         +3.28 V  (min =  +3.14 V, max =  +3.47 V)
fan1:          0 RPM  (min = 1318 RPM, div = 128)  ALARM
fan2:       1875 RPM  (min = 1704 RPM, div = 8)
fan3:          0 RPM  (min =    0 RPM, div = 128)  ALARM
fan4:          0 RPM  (min = 5273 RPM, div = 128)  ALARM
fan5:          0 RPM  (min = 10546 RPM, div = 128)  ALARM
temp1:       +40.0°C  (high = +45.0°C, hyst = +40.0°C)  sensor = thermistor
temp2:       +36.5°C  (high = +45.0°C, hyst = +40.0°C)  sensor = diode
temp3:       +21.0°C  (high = +80.0°C, hyst = +75.0°C)  sensor = thermistor
cpu0_vid:   +1.038 V


To Michael:
	Please try it on your platform. I am looking forward to your response.

Best regards,
Gong Jun


---
 w83627ehf.c |  150 +++++++++++++++++++++++++++++++++-----------------
 1 file changed, 99 insertions(+), 51 deletions(-)
---
--- w83627ehf.c.orig	2009-02-25 00:07:24.000000000 +0800
+++ w83627ehf.c	2009-02-26 00:41:58.000000000 +0800
@@ -36,6 +36,7 @@
     w83627ehf   10      5       4       3      0x8850 0x88    0x5ca3
                                                0x8860 0xa1
     w83627dhg    9      5       4       3      0xa020 0xc1    0x5ca3
+    w83667hg     9      5       3       3      0xa510 0xc1    0x5ca3
 */
 
 #include <linux/module.h>
@@ -51,12 +52,13 @@
 #include <asm/io.h>
 #include "lm75.h"
 
-enum kinds { w83627ehf, w83627dhg };
+enum kinds { w83627ehf, w83627dhg, w83667hg};
 
 /* used to set data->name = w83627ehf_device_names[data->sio_kind] */
 static const char * w83627ehf_device_names[] = {
 	"w83627ehf",
 	"w83627dhg",
+	"w83667hg",
 };
 
 static unsigned short force_id;
@@ -70,6 +72,7 @@ MODULE_PARM_DESC(force_id, "Override the
  */
 
 #define W83627EHF_LD_HWM	0x0b
+#define W83667HG_LD_VID 	0x0d
 
 #define SIO_REG_LDSEL		0x07	/* Logical device select */
 #define SIO_REG_DEVID		0x20	/* Device ID (2 bytes) */
@@ -82,6 +85,7 @@ MODULE_PARM_DESC(force_id, "Override the
 #define SIO_W83627EHF_ID	0x8850
 #define SIO_W83627EHG_ID	0x8860
 #define SIO_W83627DHG_ID	0xa020
+#define SIO_W83667HG_ID 	0xa510
 #define SIO_ID_MASK		0xFFF0
 
 static inline void
@@ -288,6 +292,7 @@ struct w83627ehf_data {
 	u8 pwm_mode[4]; /* 0->DC variable voltage, 1->PWM variable duty cycle */
 	u8 pwm_enable[4]; /* 1->manual
 			     2->thermal cruise (also called SmartFan I) */
+	u8 pwm_num;		/* number of pwm */
 	u8 pwm[4];
 	u8 target_temp[4];
 	u8 tolerance[4];
@@ -297,6 +302,8 @@ struct w83627ehf_data {
 
 	u8 vid;
 	u8 vrm;
+
+	u8 en_temp3;
 };
 
 struct w83627ehf_sio_data {
@@ -1180,6 +1187,8 @@ static void w83627ehf_device_remove_file
 	for (i = 0; i < ARRAY_SIZE(sda_sf3_arrays_fan4); i++)
 		device_remove_file(dev, &sda_sf3_arrays_fan4[i].dev_attr);
 	for (i = 0; i < data->in_num; i++) {
+		if ((i == 6) && (!strcmp(data->name, "w83667hg")) && !data->en_temp3)
+			continue;
 		device_remove_file(dev, &sda_in_input[i].dev_attr);
 		device_remove_file(dev, &sda_in_alarm[i].dev_attr);
 		device_remove_file(dev, &sda_in_min[i].dev_attr);
@@ -1191,15 +1200,19 @@ static void w83627ehf_device_remove_file
 		device_remove_file(dev, &sda_fan_div[i].dev_attr);
 		device_remove_file(dev, &sda_fan_min[i].dev_attr);
 	}
-	for (i = 0; i < 4; i++) {
+	for (i = 0; i < data->pwm_num; i++) {
 		device_remove_file(dev, &sda_pwm[i].dev_attr);
 		device_remove_file(dev, &sda_pwm_mode[i].dev_attr);
 		device_remove_file(dev, &sda_pwm_enable[i].dev_attr);
 		device_remove_file(dev, &sda_target_temp[i].dev_attr);
 		device_remove_file(dev, &sda_tolerance[i].dev_attr);
 	}
-	for (i = 0; i < ARRAY_SIZE(sda_temp); i++)
+	for (i = 0; i < ARRAY_SIZE(sda_temp); i++) {
+		if (strstr(sda_temp[i].dev_attr.attr.name, "temp3")
+                	&& (!strcmp(data->name, "w83667hg")) && data->en_temp3)
+			continue;
 		device_remove_file(dev, &sda_temp[i].dev_attr);
+	}
 
 	device_remove_file(dev, &dev_attr_name);
 	device_remove_file(dev, &dev_attr_cpu0_vid);
@@ -1219,6 +1232,8 @@ static inline void __devinit w83627ehf_i
 
 	/* Enable temp2 and temp3 if needed */
 	for (i = 0; i < 2; i++) {
+		if (!strcmp(data->name, "w83667hg") && (i == 1))
+			continue;
 		tmp = w83627ehf_read_value(data,
 					   W83627EHF_REG_TEMP_CONFIG[i]);
 		if (tmp & 0x01)
@@ -1271,8 +1286,10 @@ static int __devinit w83627ehf_probe(str
 	data->name = w83627ehf_device_names[sio_data->kind];
 	platform_set_drvdata(pdev, data);
 
-	/* 627EHG and 627EHF have 10 voltage inputs; DHG has 9 */
-	data->in_num = (sio_data->kind == w83627dhg) ? 9 : 10;
+	/* 627EHG and 627EHF have 10 voltage inputs; DHG and 667HG have 9 */
+	data->in_num = (sio_data->kind == w83627ehf) ? 10 : 9;
+	/* 667HG has 3 pwms */
+	data->pwm_num = (sio_data->kind == w83667hg) ? 3 : 4;
 
 	/* Initialize the chip */
 	w83627ehf_init_device(data);
@@ -1280,44 +1297,60 @@ static int __devinit w83627ehf_probe(str
 	data->vrm = vid_which_vrm();
 	superio_enter(sio_data->sioreg);
 	/* Read VID value */
-	superio_select(sio_data->sioreg, W83627EHF_LD_HWM);
-	if (superio_inb(sio_data->sioreg, SIO_REG_VID_CTRL) & 0x80) {
-		/* Set VID input sensibility if needed. In theory the BIOS
-		   should have set it, but in practice it's not always the
-		   case. We only do it for the W83627EHF/EHG because the
-		   W83627DHG is more complex in this respect. */
-		if (sio_data->kind == w83627ehf) {
-			en_vrm10 = superio_inb(sio_data->sioreg,
-					       SIO_REG_EN_VRM10);
-			if ((en_vrm10 & 0x08) && data->vrm == 90) {
-				dev_warn(dev, "Setting VID input voltage to "
-					 "TTL\n");
-				superio_outb(sio_data->sioreg, SIO_REG_EN_VRM10,
-					     en_vrm10 & ~0x08);
-			} else if (!(en_vrm10 & 0x08) && data->vrm == 100) {
-				dev_warn(dev, "Setting VID input voltage to "
-					 "VRM10\n");
-				superio_outb(sio_data->sioreg, SIO_REG_EN_VRM10,
-					     en_vrm10 | 0x08);
-			}
-		}
-
-		data->vid = superio_inb(sio_data->sioreg, SIO_REG_VID_DATA);
-		if (sio_data->kind == w83627ehf) /* 6 VID pins only */
-			data->vid &= 0x3f;
-
+	if (sio_data->kind == w83667hg) {
+		/* W83667HG has different pins for VID input and output, so 
+ 		we can get the VID input values directly at logical device D
+		0xe3. */
+		superio_select(sio_data->sioreg, W83667HG_LD_VID);
+		data->vid = superio_inb(sio_data->sioreg, 0xe3);
 		err = device_create_file(dev, &dev_attr_cpu0_vid);
 		if (err)
 			goto exit_release;
 	} else {
-		dev_info(dev, "VID pins in output mode, CPU VID not "
-			 "available\n");
+		superio_select(sio_data->sioreg, W83627EHF_LD_HWM);
+		if (superio_inb(sio_data->sioreg, SIO_REG_VID_CTRL) & 0x80) {
+			/* Set VID input sensibility if needed. In theory the BIOS
+			   should have set it, but in practice it's not always the
+			   case. We only do it for the W83627EHF/EHG because the
+			   W83627DHG is more complex in this respect. */
+			if (sio_data->kind == w83627ehf) {
+				en_vrm10 = superio_inb(sio_data->sioreg,
+						       SIO_REG_EN_VRM10);
+				if ((en_vrm10 & 0x08) && data->vrm == 90) {
+					dev_warn(dev, "Setting VID input voltage to "
+						 "TTL\n");
+					superio_outb(sio_data->sioreg, SIO_REG_EN_VRM10,
+						     en_vrm10 & ~0x08);
+				} else if (!(en_vrm10 & 0x08) && data->vrm == 100) {
+					dev_warn(dev, "Setting VID input voltage to "
+						 "VRM10\n");
+					superio_outb(sio_data->sioreg, SIO_REG_EN_VRM10,
+						     en_vrm10 | 0x08);
+				}
+			}
+
+			data->vid = superio_inb(sio_data->sioreg, SIO_REG_VID_DATA);
+			if (sio_data->kind == w83627ehf) /* 6 VID pins only */
+				data->vid &= 0x3f;
+
+			err = device_create_file(dev, &dev_attr_cpu0_vid);
+			if (err)
+				goto exit_release;
+		} else {
+			dev_info(dev, "VID pins in output mode, CPU VID not "
+				 "available\n");
+		}
 	}
 
 	/* fan4 and fan5 share some pins with the GPIO and serial flash */
 
-	fan5pin = superio_inb(sio_data->sioreg, 0x24) & 0x2;
-	fan4pin = superio_inb(sio_data->sioreg, 0x29) & 0x6;
+	if (sio_data->kind == w83667hg) {
+		fan5pin = superio_inb(sio_data->sioreg, 0x27) & 0x20;
+		fan4pin = superio_inb(sio_data->sioreg, 0x27) & 0x40;
+	} else {			
+		fan5pin = !(superio_inb(sio_data->sioreg, 0x24) & 0x02);
+		fan4pin = !(superio_inb(sio_data->sioreg, 0x29) & 0x06);
+	}
 	superio_exit(sio_data->sioreg);
 
 	/* It looks like fan4 and fan5 pins can be alternatively used
@@ -1328,9 +1361,9 @@ static int __devinit w83627ehf_probe(str
 
 	data->has_fan = 0x07; /* fan1, fan2 and fan3 */
 	i = w83627ehf_read_value(data, W83627EHF_REG_FANDIV1);
-	if ((i & (1 << 2)) && (!fan4pin))
+	if ((i & (1 << 2)) && fan4pin)
 		data->has_fan |= (1 << 3);
-	if (!(i & (1 << 1)) && (!fan5pin))
+	if (!(i & (1 << 1)) && fan5pin)
 		data->has_fan |= (1 << 4);
 
 	/* Read fan clock dividers immediately */
@@ -1343,23 +1376,29 @@ static int __devinit w83627ehf_probe(str
 			goto exit_remove;
 
 	/* if fan4 is enabled create the sf3 files for it */
-	if (data->has_fan & (1 << 3))
+	if ((sio_data->kind != w83667hg) && 
+		(data->has_fan & (1 << 3)))
 		for (i = 0; i < ARRAY_SIZE(sda_sf3_arrays_fan4); i++) {
 			if ((err = device_create_file(dev,
 				&sda_sf3_arrays_fan4[i].dev_attr)))
 				goto exit_remove;
 		}
+	
+	data->en_temp3 = w83627ehf_read_value(data, W83627EHF_REG_TEMP_CONFIG[1]) & 0x01;
 
-	for (i = 0; i < data->in_num; i++)
-		if ((err = device_create_file(dev, &sda_in_input[i].dev_attr))
-			|| (err = device_create_file(dev,
-				&sda_in_alarm[i].dev_attr))
-			|| (err = device_create_file(dev,
-				&sda_in_min[i].dev_attr))
-			|| (err = device_create_file(dev,
-				&sda_in_max[i].dev_attr)))
-			goto exit_remove;
-
+	for (i = 0; i < data->in_num; i++) {
+		if ((i == 6) && (sio_data->kind == w83667hg) && !data->en_temp3) 
+			continue;
+		else
+			if ((err = device_create_file(dev, &sda_in_input[i].dev_attr))
+				|| (err = device_create_file(dev,
+					&sda_in_alarm[i].dev_attr))
+				|| (err = device_create_file(dev,
+					&sda_in_min[i].dev_attr))
+				|| (err = device_create_file(dev,
+					&sda_in_max[i].dev_attr)))
+				goto exit_remove;
+		}
 	for (i = 0; i < 5; i++) {
 		if (data->has_fan & (1 << i)) {
 			if ((err = device_create_file(dev,
@@ -1371,8 +1410,8 @@ static int __devinit w83627ehf_probe(str
 				|| (err = device_create_file(dev,
 					&sda_fan_min[i].dev_attr)))
 				goto exit_remove;
-			if (i < 4 && /* w83627ehf only has 4 pwm */
-				((err = device_create_file(dev,
+	if(i < data->pwm_num &&
+			((err = device_create_file(dev,
 					&sda_pwm[i].dev_attr))
 				|| (err = device_create_file(dev,
 					&sda_pwm_mode[i].dev_attr))
@@ -1386,9 +1425,13 @@ static int __devinit w83627ehf_probe(str
 		}
 	}
 
-	for (i = 0; i < ARRAY_SIZE(sda_temp); i++)
+	for (i = 0; i < ARRAY_SIZE(sda_temp); i++) {
+		if (strstr(sda_temp[i].dev_attr.attr.name, "temp3")
+			&& (sio_data->kind == w83667hg) && data->en_temp3) 
+			continue;
 		if ((err = device_create_file(dev, &sda_temp[i].dev_attr)))
 			goto exit_remove;
+	}
 
 	err = device_create_file(dev, &dev_attr_name);
 	if (err)
@@ -1441,6 +1484,7 @@ static int __init w83627ehf_find(int sio
 	static const char __initdata sio_name_W83627EHF[] = "W83627EHF";
 	static const char __initdata sio_name_W83627EHG[] = "W83627EHG";
 	static const char __initdata sio_name_W83627DHG[] = "W83627DHG";
+	static const char __initdata sio_name_W83667HG[] = "W83667HG";
 
 	u16 val;
 	const char *sio_name;
@@ -1465,6 +1509,10 @@ static int __init w83627ehf_find(int sio
 		sio_data->kind = w83627dhg;
 		sio_name = sio_name_W83627DHG;
 		break;
+	case SIO_W83667HG_ID:
+		sio_data->kind = w83667hg;
+		sio_name = sio_name_W83667HG;
+		break;
 	default:
 		if (val != 0xffff)
 			pr_debug(DRVNAME ": unsupported chip ID: 0x%04x\n",



===========================================================================================
The privileged confidential information contained in this email is intended for use only by the addressees as indicated by the original sender of this email. If you are not the addressee indicated in this email or are not responsible for delivery of the email to such  a person, please kindly reply to the sender indicating this fact and delete all copies of it from your computer and network server immediately. Your cooperation is highly appreciated. It is advised that any unauthorized use of confidential information of Nuvoton is strictly prohibited; and any information in this email irrelevant to the official business of Nuvoton shall be deemed as neither given nor endorsed by Nuvoton.

[-- Attachment #2: hwmon-add-support-for-w83667hg-in-w83627ehf-driver-test-in6-temp3.patch --]
[-- Type: application/octet-stream, Size: 10849 bytes --]

---
 w83627ehf.c |  150 +++++++++++++++++++++++++++++++++-----------------
 1 file changed, 99 insertions(+), 51 deletions(-)
---
--- w83627ehf.c.orig	2009-02-25 00:07:24.000000000 +0800
+++ w83627ehf.c	2009-02-26 00:41:58.000000000 +0800
@@ -36,6 +36,7 @@
     w83627ehf   10      5       4       3      0x8850 0x88    0x5ca3
                                                0x8860 0xa1
     w83627dhg    9      5       4       3      0xa020 0xc1    0x5ca3
+    w83667hg     9      5       3       3      0xa510 0xc1    0x5ca3
 */
 
 #include <linux/module.h>
@@ -51,12 +52,13 @@
 #include <asm/io.h>
 #include "lm75.h"
 
-enum kinds { w83627ehf, w83627dhg };
+enum kinds { w83627ehf, w83627dhg, w83667hg};
 
 /* used to set data->name = w83627ehf_device_names[data->sio_kind] */
 static const char * w83627ehf_device_names[] = {
 	"w83627ehf",
 	"w83627dhg",
+	"w83667hg",
 };
 
 static unsigned short force_id;
@@ -70,6 +72,7 @@ MODULE_PARM_DESC(force_id, "Override the
  */
 
 #define W83627EHF_LD_HWM	0x0b
+#define W83667HG_LD_VID 	0x0d
 
 #define SIO_REG_LDSEL		0x07	/* Logical device select */
 #define SIO_REG_DEVID		0x20	/* Device ID (2 bytes) */
@@ -82,6 +85,7 @@ MODULE_PARM_DESC(force_id, "Override the
 #define SIO_W83627EHF_ID	0x8850
 #define SIO_W83627EHG_ID	0x8860
 #define SIO_W83627DHG_ID	0xa020
+#define SIO_W83667HG_ID 	0xa510
 #define SIO_ID_MASK		0xFFF0
 
 static inline void
@@ -288,6 +292,7 @@ struct w83627ehf_data {
 	u8 pwm_mode[4]; /* 0->DC variable voltage, 1->PWM variable duty cycle */
 	u8 pwm_enable[4]; /* 1->manual
 			     2->thermal cruise (also called SmartFan I) */
+	u8 pwm_num;		/* number of pwm */
 	u8 pwm[4];
 	u8 target_temp[4];
 	u8 tolerance[4];
@@ -297,6 +302,8 @@ struct w83627ehf_data {
 
 	u8 vid;
 	u8 vrm;
+
+	u8 en_temp3;
 };
 
 struct w83627ehf_sio_data {
@@ -1180,6 +1187,8 @@ static void w83627ehf_device_remove_file
 	for (i = 0; i < ARRAY_SIZE(sda_sf3_arrays_fan4); i++)
 		device_remove_file(dev, &sda_sf3_arrays_fan4[i].dev_attr);
 	for (i = 0; i < data->in_num; i++) {
+		if ((i == 6) && (!strcmp(data->name, "w83667hg")) && !data->en_temp3)
+			continue;
 		device_remove_file(dev, &sda_in_input[i].dev_attr);
 		device_remove_file(dev, &sda_in_alarm[i].dev_attr);
 		device_remove_file(dev, &sda_in_min[i].dev_attr);
@@ -1191,15 +1200,19 @@ static void w83627ehf_device_remove_file
 		device_remove_file(dev, &sda_fan_div[i].dev_attr);
 		device_remove_file(dev, &sda_fan_min[i].dev_attr);
 	}
-	for (i = 0; i < 4; i++) {
+	for (i = 0; i < data->pwm_num; i++) {
 		device_remove_file(dev, &sda_pwm[i].dev_attr);
 		device_remove_file(dev, &sda_pwm_mode[i].dev_attr);
 		device_remove_file(dev, &sda_pwm_enable[i].dev_attr);
 		device_remove_file(dev, &sda_target_temp[i].dev_attr);
 		device_remove_file(dev, &sda_tolerance[i].dev_attr);
 	}
-	for (i = 0; i < ARRAY_SIZE(sda_temp); i++)
+	for (i = 0; i < ARRAY_SIZE(sda_temp); i++) {
+		if (strstr(sda_temp[i].dev_attr.attr.name, "temp3")
+                	&& (!strcmp(data->name, "w83667hg")) && data->en_temp3)
+			continue;
 		device_remove_file(dev, &sda_temp[i].dev_attr);
+	}
 
 	device_remove_file(dev, &dev_attr_name);
 	device_remove_file(dev, &dev_attr_cpu0_vid);
@@ -1219,6 +1232,8 @@ static inline void __devinit w83627ehf_i
 
 	/* Enable temp2 and temp3 if needed */
 	for (i = 0; i < 2; i++) {
+		if (!strcmp(data->name, "w83667hg") && (i == 1))
+			continue;
 		tmp = w83627ehf_read_value(data,
 					   W83627EHF_REG_TEMP_CONFIG[i]);
 		if (tmp & 0x01)
@@ -1271,8 +1286,10 @@ static int __devinit w83627ehf_probe(str
 	data->name = w83627ehf_device_names[sio_data->kind];
 	platform_set_drvdata(pdev, data);
 
-	/* 627EHG and 627EHF have 10 voltage inputs; DHG has 9 */
-	data->in_num = (sio_data->kind == w83627dhg) ? 9 : 10;
+	/* 627EHG and 627EHF have 10 voltage inputs; DHG and 667HG have 9 */
+	data->in_num = (sio_data->kind == w83627ehf) ? 10 : 9;
+	/* 667HG has 3 pwms */
+	data->pwm_num = (sio_data->kind == w83667hg) ? 3 : 4;
 
 	/* Initialize the chip */
 	w83627ehf_init_device(data);
@@ -1280,44 +1297,60 @@ static int __devinit w83627ehf_probe(str
 	data->vrm = vid_which_vrm();
 	superio_enter(sio_data->sioreg);
 	/* Read VID value */
-	superio_select(sio_data->sioreg, W83627EHF_LD_HWM);
-	if (superio_inb(sio_data->sioreg, SIO_REG_VID_CTRL) & 0x80) {
-		/* Set VID input sensibility if needed. In theory the BIOS
-		   should have set it, but in practice it's not always the
-		   case. We only do it for the W83627EHF/EHG because the
-		   W83627DHG is more complex in this respect. */
-		if (sio_data->kind == w83627ehf) {
-			en_vrm10 = superio_inb(sio_data->sioreg,
-					       SIO_REG_EN_VRM10);
-			if ((en_vrm10 & 0x08) && data->vrm == 90) {
-				dev_warn(dev, "Setting VID input voltage to "
-					 "TTL\n");
-				superio_outb(sio_data->sioreg, SIO_REG_EN_VRM10,
-					     en_vrm10 & ~0x08);
-			} else if (!(en_vrm10 & 0x08) && data->vrm == 100) {
-				dev_warn(dev, "Setting VID input voltage to "
-					 "VRM10\n");
-				superio_outb(sio_data->sioreg, SIO_REG_EN_VRM10,
-					     en_vrm10 | 0x08);
-			}
-		}
-
-		data->vid = superio_inb(sio_data->sioreg, SIO_REG_VID_DATA);
-		if (sio_data->kind == w83627ehf) /* 6 VID pins only */
-			data->vid &= 0x3f;
-
+	if (sio_data->kind == w83667hg) {
+		/* W83667HG has different pins for VID input and output, so 
+ 		we can get the VID input values directly at logical device D
+		0xe3. */
+		superio_select(sio_data->sioreg, W83667HG_LD_VID);
+		data->vid = superio_inb(sio_data->sioreg, 0xe3);
 		err = device_create_file(dev, &dev_attr_cpu0_vid);
 		if (err)
 			goto exit_release;
 	} else {
-		dev_info(dev, "VID pins in output mode, CPU VID not "
-			 "available\n");
+		superio_select(sio_data->sioreg, W83627EHF_LD_HWM);
+		if (superio_inb(sio_data->sioreg, SIO_REG_VID_CTRL) & 0x80) {
+			/* Set VID input sensibility if needed. In theory the BIOS
+			   should have set it, but in practice it's not always the
+			   case. We only do it for the W83627EHF/EHG because the
+			   W83627DHG is more complex in this respect. */
+			if (sio_data->kind == w83627ehf) {
+				en_vrm10 = superio_inb(sio_data->sioreg,
+						       SIO_REG_EN_VRM10);
+				if ((en_vrm10 & 0x08) && data->vrm == 90) {
+					dev_warn(dev, "Setting VID input voltage to "
+						 "TTL\n");
+					superio_outb(sio_data->sioreg, SIO_REG_EN_VRM10,
+						     en_vrm10 & ~0x08);
+				} else if (!(en_vrm10 & 0x08) && data->vrm == 100) {
+					dev_warn(dev, "Setting VID input voltage to "
+						 "VRM10\n");
+					superio_outb(sio_data->sioreg, SIO_REG_EN_VRM10,
+						     en_vrm10 | 0x08);
+				}
+			}
+
+			data->vid = superio_inb(sio_data->sioreg, SIO_REG_VID_DATA);
+			if (sio_data->kind == w83627ehf) /* 6 VID pins only */
+				data->vid &= 0x3f;
+
+			err = device_create_file(dev, &dev_attr_cpu0_vid);
+			if (err)
+				goto exit_release;
+		} else {
+			dev_info(dev, "VID pins in output mode, CPU VID not "
+				 "available\n");
+		}
 	}
 
 	/* fan4 and fan5 share some pins with the GPIO and serial flash */
 
-	fan5pin = superio_inb(sio_data->sioreg, 0x24) & 0x2;
-	fan4pin = superio_inb(sio_data->sioreg, 0x29) & 0x6;
+	if (sio_data->kind == w83667hg) {
+		fan5pin = superio_inb(sio_data->sioreg, 0x27) & 0x20;
+		fan4pin = superio_inb(sio_data->sioreg, 0x27) & 0x40;
+	} else {			
+		fan5pin = !(superio_inb(sio_data->sioreg, 0x24) & 0x02);
+		fan4pin = !(superio_inb(sio_data->sioreg, 0x29) & 0x06);
+	}
 	superio_exit(sio_data->sioreg);
 
 	/* It looks like fan4 and fan5 pins can be alternatively used
@@ -1328,9 +1361,9 @@ static int __devinit w83627ehf_probe(str
 
 	data->has_fan = 0x07; /* fan1, fan2 and fan3 */
 	i = w83627ehf_read_value(data, W83627EHF_REG_FANDIV1);
-	if ((i & (1 << 2)) && (!fan4pin))
+	if ((i & (1 << 2)) && fan4pin)
 		data->has_fan |= (1 << 3);
-	if (!(i & (1 << 1)) && (!fan5pin))
+	if (!(i & (1 << 1)) && fan5pin)
 		data->has_fan |= (1 << 4);
 
 	/* Read fan clock dividers immediately */
@@ -1343,23 +1376,29 @@ static int __devinit w83627ehf_probe(str
 			goto exit_remove;
 
 	/* if fan4 is enabled create the sf3 files for it */
-	if (data->has_fan & (1 << 3))
+	if ((sio_data->kind != w83667hg) && 
+		(data->has_fan & (1 << 3)))
 		for (i = 0; i < ARRAY_SIZE(sda_sf3_arrays_fan4); i++) {
 			if ((err = device_create_file(dev,
 				&sda_sf3_arrays_fan4[i].dev_attr)))
 				goto exit_remove;
 		}
+	
+	data->en_temp3 = w83627ehf_read_value(data, W83627EHF_REG_TEMP_CONFIG[1]) & 0x01;
 
-	for (i = 0; i < data->in_num; i++)
-		if ((err = device_create_file(dev, &sda_in_input[i].dev_attr))
-			|| (err = device_create_file(dev,
-				&sda_in_alarm[i].dev_attr))
-			|| (err = device_create_file(dev,
-				&sda_in_min[i].dev_attr))
-			|| (err = device_create_file(dev,
-				&sda_in_max[i].dev_attr)))
-			goto exit_remove;
-
+	for (i = 0; i < data->in_num; i++) {
+		if ((i == 6) && (sio_data->kind == w83667hg) && !data->en_temp3) 
+			continue;
+		else
+			if ((err = device_create_file(dev, &sda_in_input[i].dev_attr))
+				|| (err = device_create_file(dev,
+					&sda_in_alarm[i].dev_attr))
+				|| (err = device_create_file(dev,
+					&sda_in_min[i].dev_attr))
+				|| (err = device_create_file(dev,
+					&sda_in_max[i].dev_attr)))
+				goto exit_remove;
+		}
 	for (i = 0; i < 5; i++) {
 		if (data->has_fan & (1 << i)) {
 			if ((err = device_create_file(dev,
@@ -1371,8 +1410,8 @@ static int __devinit w83627ehf_probe(str
 				|| (err = device_create_file(dev,
 					&sda_fan_min[i].dev_attr)))
 				goto exit_remove;
-			if (i < 4 && /* w83627ehf only has 4 pwm */
-				((err = device_create_file(dev,
+	if(i < data->pwm_num &&
+			((err = device_create_file(dev,
 					&sda_pwm[i].dev_attr))
 				|| (err = device_create_file(dev,
 					&sda_pwm_mode[i].dev_attr))
@@ -1386,9 +1425,13 @@ static int __devinit w83627ehf_probe(str
 		}
 	}
 
-	for (i = 0; i < ARRAY_SIZE(sda_temp); i++)
+	for (i = 0; i < ARRAY_SIZE(sda_temp); i++) {
+		if (strstr(sda_temp[i].dev_attr.attr.name, "temp3")
+			&& (sio_data->kind == w83667hg) && data->en_temp3) 
+			continue;
 		if ((err = device_create_file(dev, &sda_temp[i].dev_attr)))
 			goto exit_remove;
+	}
 
 	err = device_create_file(dev, &dev_attr_name);
 	if (err)
@@ -1441,6 +1484,7 @@ static int __init w83627ehf_find(int sio
 	static const char __initdata sio_name_W83627EHF[] = "W83627EHF";
 	static const char __initdata sio_name_W83627EHG[] = "W83627EHG";
 	static const char __initdata sio_name_W83627DHG[] = "W83627DHG";
+	static const char __initdata sio_name_W83667HG[] = "W83667HG";
 
 	u16 val;
 	const char *sio_name;
@@ -1465,6 +1509,10 @@ static int __init w83627ehf_find(int sio
 		sio_data->kind = w83627dhg;
 		sio_name = sio_name_W83627DHG;
 		break;
+	case SIO_W83667HG_ID:
+		sio_data->kind = w83667hg;
+		sio_name = sio_name_W83667HG;
+		break;
 	default:
 		if (val != 0xffff)
 			pr_debug(DRVNAME ": unsupported chip ID: 0x%04x\n",

[-- Attachment #3: Type: text/plain, Size: 153 bytes --]

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

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

* Re: [lm-sensors] W83667HG driver test for in6 and temp3
  2009-02-25  9:13 [lm-sensors] W83667HG driver test for in6 and temp3 JGong
@ 2009-02-25 15:22 ` Jean Delvare
  2009-02-25 16:18 ` Michael Hampton
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Jean Delvare @ 2009-02-25 15:22 UTC (permalink / raw)
  To: lm-sensors

Hi Gong Jun,

On Wed, 25 Feb 2009 17:13:05 +0800, JGong@nuvoton.com wrote:
> Dear all,
> 
> >I suggest that you make a patch implementing what was said above. Then
> >Michael will test it and we'll see which of in6 or temp3 will be
> >created on his board. If in6 the problem is solved (as far as Michael's
> >system is concerned, at least.) If the incorrect temp3 is displayed
> >then we will have to think of a workaround. Either we check the
> >temperature value and discard it if it looks unreasonable, or we let
> >the user override the configuration manually as I initially proposed. I
> >think I prefer the second option. Choosing the configuration based on
> >the monitored values somehow voids the point of monitoring.
> >Please make sure you include my fan fixes into your patch:
> >http://lists.lm-sensors.org/pipermail/lm-sensors/2009-January/025232.html
> 
> I've made the patch out with what we discussed above (including the fan fix from Jean). It has been tested on my ASUS P5QL PRO.
> 
> w83667hg-isa-0290
> Adapter: ISA adapter
> in0:         +1.01 V  (min =  +0.00 V, max =  +1.74 V)
> in1:         +1.69 V  (min =  +1.64 V, max =  +2.00 V)
> in2:         +3.33 V  (min =  +3.14 V, max =  +3.47 V)
> in3:         +3.30 V  (min =  +3.14 V, max =  +3.47 V)
> in4:         +1.68 V  (min =  +0.02 V, max =  +0.35 V)   ALARM
> in5:         +2.04 V  (min =  +0.26 V, max =  +1.03 V)   ALARM
> in7:         +3.39 V  (min =  +3.14 V, max =  +3.47 V)
> in8:         +3.28 V  (min =  +3.14 V, max =  +3.47 V)
> fan1:          0 RPM  (min = 1318 RPM, div = 128)  ALARM
> fan2:       1875 RPM  (min = 1704 RPM, div = 8)
> fan3:          0 RPM  (min =    0 RPM, div = 128)  ALARM
> fan4:          0 RPM  (min = 5273 RPM, div = 128)  ALARM
> fan5:          0 RPM  (min = 10546 RPM, div = 128)  ALARM
> temp1:       +40.0°C  (high = +45.0°C, hyst = +40.0°C)  sensor = thermistor
> temp2:       +36.5°C  (high = +45.0°C, hyst = +40.0°C)  sensor = diode
> temp3:       +21.0°C  (high = +80.0°C, hyst = +75.0°C)  sensor = thermistor
> cpu0_vid:   +1.038 V
> 
> 
> To Michael:
> 	Please try it on your platform. I am looking forward to your response.
> 
> Best regards,
> Gong Jun

First of all, in order to make reviewing of future changes easier, I
think I will apply your initial patch now, except for the special
handling of in6. Then we can add a patch fixing the handling of in6, it
will be much smaller and thus much easier to review.

Some comments on your code:

> 
> 
> ---
>  w83627ehf.c |  150 +++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 99 insertions(+), 51 deletions(-)
> ---
> --- w83627ehf.c.orig	2009-02-25 00:07:24.000000000 +0800
> +++ w83627ehf.c	2009-02-26 00:41:58.000000000 +0800
> @@ -36,6 +36,7 @@
>      w83627ehf   10      5       4       3      0x8850 0x88    0x5ca3
>                                                 0x8860 0xa1
>      w83627dhg    9      5       4       3      0xa020 0xc1    0x5ca3
> +    w83667hg     9      5       3       3      0xa510 0xc1    0x5ca3
>  */
>  
>  #include <linux/module.h>
> @@ -51,12 +52,13 @@
>  #include <asm/io.h>
>  #include "lm75.h"
>  
> -enum kinds { w83627ehf, w83627dhg };
> +enum kinds { w83627ehf, w83627dhg, w83667hg};
>  
>  /* used to set data->name = w83627ehf_device_names[data->sio_kind] */
>  static const char * w83627ehf_device_names[] = {
>  	"w83627ehf",
>  	"w83627dhg",
> +	"w83667hg",
>  };
>  
>  static unsigned short force_id;
> @@ -70,6 +72,7 @@ MODULE_PARM_DESC(force_id, "Override the
>   */
>  
>  #define W83627EHF_LD_HWM	0x0b
> +#define W83667HG_LD_VID 	0x0d
>  
>  #define SIO_REG_LDSEL		0x07	/* Logical device select */
>  #define SIO_REG_DEVID		0x20	/* Device ID (2 bytes) */
> @@ -82,6 +85,7 @@ MODULE_PARM_DESC(force_id, "Override the
>  #define SIO_W83627EHF_ID	0x8850
>  #define SIO_W83627EHG_ID	0x8860
>  #define SIO_W83627DHG_ID	0xa020
> +#define SIO_W83667HG_ID 	0xa510
>  #define SIO_ID_MASK		0xFFF0
>  
>  static inline void
> @@ -288,6 +292,7 @@ struct w83627ehf_data {
>  	u8 pwm_mode[4]; /* 0->DC variable voltage, 1->PWM variable duty cycle */
>  	u8 pwm_enable[4]; /* 1->manual
>  			     2->thermal cruise (also called SmartFan I) */
> +	u8 pwm_num;		/* number of pwm */
>  	u8 pwm[4];
>  	u8 target_temp[4];
>  	u8 tolerance[4];
> @@ -297,6 +302,8 @@ struct w83627ehf_data {
>  
>  	u8 vid;
>  	u8 vrm;
> +
> +	u8 en_temp3;
>  };
>  
>  struct w83627ehf_sio_data {
> @@ -1180,6 +1187,8 @@ static void w83627ehf_device_remove_file
>  	for (i = 0; i < ARRAY_SIZE(sda_sf3_arrays_fan4); i++)
>  		device_remove_file(dev, &sda_sf3_arrays_fan4[i].dev_attr);
>  	for (i = 0; i < data->in_num; i++) {
> +		if ((i = 6) && (!strcmp(data->name, "w83667hg")) && !data->en_temp3)
> +			continue;

Using strcmp should be avoided, as it's relatively slow. You should
either keep the chip type in struct data, or use a dedicated field (for
example data->skip_in6).

>  		device_remove_file(dev, &sda_in_input[i].dev_attr);
>  		device_remove_file(dev, &sda_in_alarm[i].dev_attr);
>  		device_remove_file(dev, &sda_in_min[i].dev_attr);
> @@ -1191,15 +1200,19 @@ static void w83627ehf_device_remove_file
>  		device_remove_file(dev, &sda_fan_div[i].dev_attr);
>  		device_remove_file(dev, &sda_fan_min[i].dev_attr);
>  	}
> -	for (i = 0; i < 4; i++) {
> +	for (i = 0; i < data->pwm_num; i++) {
>  		device_remove_file(dev, &sda_pwm[i].dev_attr);
>  		device_remove_file(dev, &sda_pwm_mode[i].dev_attr);
>  		device_remove_file(dev, &sda_pwm_enable[i].dev_attr);
>  		device_remove_file(dev, &sda_target_temp[i].dev_attr);
>  		device_remove_file(dev, &sda_tolerance[i].dev_attr);
>  	}
> -	for (i = 0; i < ARRAY_SIZE(sda_temp); i++)
> +	for (i = 0; i < ARRAY_SIZE(sda_temp); i++) {
> +		if (strstr(sda_temp[i].dev_attr.attr.name, "temp3")
> +                	&& (!strcmp(data->name, "w83667hg")) && data->en_temp3)
> +			continue;
>  		device_remove_file(dev, &sda_temp[i].dev_attr);
> +	}
>  
>  	device_remove_file(dev, &dev_attr_name);
>  	device_remove_file(dev, &dev_attr_cpu0_vid);
> @@ -1219,6 +1232,8 @@ static inline void __devinit w83627ehf_i
>  
>  	/* Enable temp2 and temp3 if needed */
>  	for (i = 0; i < 2; i++) {
> +		if (!strcmp(data->name, "w83667hg") && (i = 1))
> +			continue;
>  		tmp = w83627ehf_read_value(data,
>  					   W83627EHF_REG_TEMP_CONFIG[i]);
>  		if (tmp & 0x01)
> @@ -1271,8 +1286,10 @@ static int __devinit w83627ehf_probe(str
>  	data->name = w83627ehf_device_names[sio_data->kind];
>  	platform_set_drvdata(pdev, data);
>  
> -	/* 627EHG and 627EHF have 10 voltage inputs; DHG has 9 */
> -	data->in_num = (sio_data->kind = w83627dhg) ? 9 : 10;
> +	/* 627EHG and 627EHF have 10 voltage inputs; DHG and 667HG have 9 */
> +	data->in_num = (sio_data->kind = w83627ehf) ? 10 : 9;
> +	/* 667HG has 3 pwms */
> +	data->pwm_num = (sio_data->kind = w83667hg) ? 3 : 4;
>  
>  	/* Initialize the chip */
>  	w83627ehf_init_device(data);
> @@ -1280,44 +1297,60 @@ static int __devinit w83627ehf_probe(str
>  	data->vrm = vid_which_vrm();
>  	superio_enter(sio_data->sioreg);
>  	/* Read VID value */
> -	superio_select(sio_data->sioreg, W83627EHF_LD_HWM);
> -	if (superio_inb(sio_data->sioreg, SIO_REG_VID_CTRL) & 0x80) {
> -		/* Set VID input sensibility if needed. In theory the BIOS
> -		   should have set it, but in practice it's not always the
> -		   case. We only do it for the W83627EHF/EHG because the
> -		   W83627DHG is more complex in this respect. */
> -		if (sio_data->kind = w83627ehf) {
> -			en_vrm10 = superio_inb(sio_data->sioreg,
> -					       SIO_REG_EN_VRM10);
> -			if ((en_vrm10 & 0x08) && data->vrm = 90) {
> -				dev_warn(dev, "Setting VID input voltage to "
> -					 "TTL\n");
> -				superio_outb(sio_data->sioreg, SIO_REG_EN_VRM10,
> -					     en_vrm10 & ~0x08);
> -			} else if (!(en_vrm10 & 0x08) && data->vrm = 100) {
> -				dev_warn(dev, "Setting VID input voltage to "
> -					 "VRM10\n");
> -				superio_outb(sio_data->sioreg, SIO_REG_EN_VRM10,
> -					     en_vrm10 | 0x08);
> -			}
> -		}
> -
> -		data->vid = superio_inb(sio_data->sioreg, SIO_REG_VID_DATA);
> -		if (sio_data->kind = w83627ehf) /* 6 VID pins only */
> -			data->vid &= 0x3f;
> -
> +	if (sio_data->kind = w83667hg) {
> +		/* W83667HG has different pins for VID input and output, so 
> + 		we can get the VID input values directly at logical device D
> +		0xe3. */
> +		superio_select(sio_data->sioreg, W83667HG_LD_VID);
> +		data->vid = superio_inb(sio_data->sioreg, 0xe3);
>  		err = device_create_file(dev, &dev_attr_cpu0_vid);
>  		if (err)
>  			goto exit_release;
>  	} else {
> -		dev_info(dev, "VID pins in output mode, CPU VID not "
> -			 "available\n");
> +		superio_select(sio_data->sioreg, W83627EHF_LD_HWM);
> +		if (superio_inb(sio_data->sioreg, SIO_REG_VID_CTRL) & 0x80) {
> +			/* Set VID input sensibility if needed. In theory the BIOS
> +			   should have set it, but in practice it's not always the
> +			   case. We only do it for the W83627EHF/EHG because the
> +			   W83627DHG is more complex in this respect. */
> +			if (sio_data->kind = w83627ehf) {
> +				en_vrm10 = superio_inb(sio_data->sioreg,
> +						       SIO_REG_EN_VRM10);
> +				if ((en_vrm10 & 0x08) && data->vrm = 90) {
> +					dev_warn(dev, "Setting VID input voltage to "
> +						 "TTL\n");
> +					superio_outb(sio_data->sioreg, SIO_REG_EN_VRM10,
> +						     en_vrm10 & ~0x08);
> +				} else if (!(en_vrm10 & 0x08) && data->vrm = 100) {
> +					dev_warn(dev, "Setting VID input voltage to "
> +						 "VRM10\n");
> +					superio_outb(sio_data->sioreg, SIO_REG_EN_VRM10,
> +						     en_vrm10 | 0x08);
> +				}
> +			}
> +
> +			data->vid = superio_inb(sio_data->sioreg, SIO_REG_VID_DATA);
> +			if (sio_data->kind = w83627ehf) /* 6 VID pins only */
> +				data->vid &= 0x3f;
> +
> +			err = device_create_file(dev, &dev_attr_cpu0_vid);
> +			if (err)
> +				goto exit_release;
> +		} else {
> +			dev_info(dev, "VID pins in output mode, CPU VID not "
> +				 "available\n");
> +		}
>  	}
>  
>  	/* fan4 and fan5 share some pins with the GPIO and serial flash */
>  
> -	fan5pin = superio_inb(sio_data->sioreg, 0x24) & 0x2;
> -	fan4pin = superio_inb(sio_data->sioreg, 0x29) & 0x6;
> +	if (sio_data->kind = w83667hg) {
> +		fan5pin = superio_inb(sio_data->sioreg, 0x27) & 0x20;
> +		fan4pin = superio_inb(sio_data->sioreg, 0x27) & 0x40;
> +	} else {			
> +		fan5pin = !(superio_inb(sio_data->sioreg, 0x24) & 0x02);
> +		fan4pin = !(superio_inb(sio_data->sioreg, 0x29) & 0x06);
> +	}
>  	superio_exit(sio_data->sioreg);
>  
>  	/* It looks like fan4 and fan5 pins can be alternatively used
> @@ -1328,9 +1361,9 @@ static int __devinit w83627ehf_probe(str
>  
>  	data->has_fan = 0x07; /* fan1, fan2 and fan3 */
>  	i = w83627ehf_read_value(data, W83627EHF_REG_FANDIV1);
> -	if ((i & (1 << 2)) && (!fan4pin))
> +	if ((i & (1 << 2)) && fan4pin)
>  		data->has_fan |= (1 << 3);
> -	if (!(i & (1 << 1)) && (!fan5pin))
> +	if (!(i & (1 << 1)) && fan5pin)
>  		data->has_fan |= (1 << 4);
>  
>  	/* Read fan clock dividers immediately */
> @@ -1343,23 +1376,29 @@ static int __devinit w83627ehf_probe(str
>  			goto exit_remove;
>  
>  	/* if fan4 is enabled create the sf3 files for it */
> -	if (data->has_fan & (1 << 3))
> +	if ((sio_data->kind != w83667hg) && 
> +		(data->has_fan & (1 << 3)))
>  		for (i = 0; i < ARRAY_SIZE(sda_sf3_arrays_fan4); i++) {
>  			if ((err = device_create_file(dev,
>  				&sda_sf3_arrays_fan4[i].dev_attr)))
>  				goto exit_remove;
>  		}
> +	
> +	data->en_temp3 = w83627ehf_read_value(data, W83627EHF_REG_TEMP_CONFIG[1]) & 0x01;

If bit 0 is set it means that temp3 is _disabled_, so the field name
(en_temp3) is confusing. You should either negate the expression or
rename the field to temp3_disabled.

>  
> -	for (i = 0; i < data->in_num; i++)
> -		if ((err = device_create_file(dev, &sda_in_input[i].dev_attr))
> -			|| (err = device_create_file(dev,
> -				&sda_in_alarm[i].dev_attr))
> -			|| (err = device_create_file(dev,
> -				&sda_in_min[i].dev_attr))
> -			|| (err = device_create_file(dev,
> -				&sda_in_max[i].dev_attr)))
> -			goto exit_remove;
> -
> +	for (i = 0; i < data->in_num; i++) {
> +		if ((i = 6) && (sio_data->kind = w83667hg) && !data->en_temp3) 
> +			continue;
> +		else
> +			if ((err = device_create_file(dev, &sda_in_input[i].dev_attr))
> +				|| (err = device_create_file(dev,
> +					&sda_in_alarm[i].dev_attr))
> +				|| (err = device_create_file(dev,
> +					&sda_in_min[i].dev_attr))
> +				|| (err = device_create_file(dev,
> +					&sda_in_max[i].dev_attr)))
> +				goto exit_remove;
> +		}
>  	for (i = 0; i < 5; i++) {
>  		if (data->has_fan & (1 << i)) {
>  			if ((err = device_create_file(dev,
> @@ -1371,8 +1410,8 @@ static int __devinit w83627ehf_probe(str
>  				|| (err = device_create_file(dev,
>  					&sda_fan_min[i].dev_attr)))
>  				goto exit_remove;
> -			if (i < 4 && /* w83627ehf only has 4 pwm */
> -				((err = device_create_file(dev,
> +	if(i < data->pwm_num &&
> +			((err = device_create_file(dev,
>  					&sda_pwm[i].dev_attr))
>  				|| (err = device_create_file(dev,
>  					&sda_pwm_mode[i].dev_attr))
> @@ -1386,9 +1425,13 @@ static int __devinit w83627ehf_probe(str
>  		}
>  	}
>  
> -	for (i = 0; i < ARRAY_SIZE(sda_temp); i++)
> +	for (i = 0; i < ARRAY_SIZE(sda_temp); i++) {
> +		if (strstr(sda_temp[i].dev_attr.attr.name, "temp3")

strstr() is relatively slow, so it should be avoided. It would be
better to split sda_temp into sub-arrays, as we already have for
voltages and fans, so that we can easily access the attributes by
index. Or you can test for "i % 3 = 2", which corresponds to all temp3
attributes in the current array. This approach is a little fragile
though.

> +			&& (sio_data->kind = w83667hg) && data->en_temp3) 
> +			continue;
>  		if ((err = device_create_file(dev, &sda_temp[i].dev_attr)))
>  			goto exit_remove;
> +	}
>  
>  	err = device_create_file(dev, &dev_attr_name);
>  	if (err)
> @@ -1441,6 +1484,7 @@ static int __init w83627ehf_find(int sio
>  	static const char __initdata sio_name_W83627EHF[] = "W83627EHF";
>  	static const char __initdata sio_name_W83627EHG[] = "W83627EHG";
>  	static const char __initdata sio_name_W83627DHG[] = "W83627DHG";
> +	static const char __initdata sio_name_W83667HG[] = "W83667HG";
>  
>  	u16 val;
>  	const char *sio_name;
> @@ -1465,6 +1509,10 @@ static int __init w83627ehf_find(int sio
>  		sio_data->kind = w83627dhg;
>  		sio_name = sio_name_W83627DHG;
>  		break;
> +	case SIO_W83667HG_ID:
> +		sio_data->kind = w83667hg;
> +		sio_name = sio_name_W83667HG;
> +		break;
>  	default:
>  		if (val != 0xffff)
>  			pr_debug(DRVNAME ": unsupported chip ID: 0x%04x\n",


-- 
Jean Delvare

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

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

* Re: [lm-sensors] W83667HG driver test for in6 and temp3
  2009-02-25  9:13 [lm-sensors] W83667HG driver test for in6 and temp3 JGong
  2009-02-25 15:22 ` Jean Delvare
@ 2009-02-25 16:18 ` Michael Hampton
  2009-02-26  9:37 ` JGong
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Michael Hampton @ 2009-02-25 16:18 UTC (permalink / raw)
  To: lm-sensors

JGong@nuvoton.com wrote:
> Dear all,
>
>   
>> I suggest that you make a patch implementing what was said above. Then
>> Michael will test it and we'll see which of in6 or temp3 will be
>> created on his board. If in6 the problem is solved (as far as Michael's
>> system is concerned, at least.) If the incorrect temp3 is displayed
>> then we will have to think of a workaround. Either we check the
>> temperature value and discard it if it looks unreasonable, or we let
>> the user override the configuration manually as I initially proposed. I
>> think I prefer the second option. Choosing the configuration based on
>> the monitored values somehow voids the point of monitoring.
>> Please make sure you include my fan fixes into your patch:
>> http://lists.lm-sensors.org/pipermail/lm-sensors/2009-January/025232.html
>>     
>
> I've made the patch out with what we discussed above (including the fan fix from Jean). It has been tested on my ASUS P5QL PRO.
>   

[snip]

> To Michael:
> 	Please try it on your platform. I am looking forward to your response.
>
> Best regards,
> Gong Jun
>   

I applied this patch to my current Fedora 10 kernel but the patched
module fails to load:
root@underground ~ # modprobe w83627ehf
FATAL: Error inserting w83627ehf
(/lib/modules/2.6.27.15-170.2.24.fc10.x86_64/kernel/drivers/hwmon/w83627ehf.ko):
No such device

I haven't tried with a vanilla kernel yet. I don't usually run them.

Interestingly, sensors-detect says the device has a different ID:

Some Super I/O chips may also contain sensors. We have to write to
standard I/O ports to probe them. This is usually safe.
Do you want to scan for Super I/O sensors? (YES/no):
Probing for Super-I/O at 0x2e/0x2f
Trying family `National Semiconductor'...                   No
Trying family `SMSC'...                                     No
Trying family `VIA/Winbond/Fintek'...                       Yes
Found unknown chip with ID 0xa513
    (logical device B has address 0x290, could be sensors)
Probing for Super-I/O at 0x4e/0x4f
Trying family `National Semiconductor'...                   No
Trying family `SMSC'...                                     No
Trying family `VIA/Winbond/Fintek'...                       No
Trying family `ITE'...                                      No


-- 
Homeland Stupidity <http://www.homelandstupidity.us/>

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

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

* Re: [lm-sensors] W83667HG driver test for in6 and temp3
  2009-02-25  9:13 [lm-sensors] W83667HG driver test for in6 and temp3 JGong
  2009-02-25 15:22 ` Jean Delvare
  2009-02-25 16:18 ` Michael Hampton
@ 2009-02-26  9:37 ` JGong
  2009-02-26 15:09 ` Jean Delvare
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: JGong @ 2009-02-26  9:37 UTC (permalink / raw)
  To: lm-sensors

Dear Jean,

Thanks very much for your quick comment. I've added the skip_in6 and temp3_disable into the data struct, cut the sda_temp into sub-arrays now.

Best regards,
Gong Jun

---
 w83627ehf.c |  184 +++++++++++++++++++++++++++++++++++---------------
 1 file changed, 130 insertions(+), 54 deletions(-)
---
--- w83627ehf.c.orig	2009-02-25 00:07:24.000000000 +0800
+++ w83627ehf.c	2009-02-27 01:12:09.000000000 +0800
@@ -36,6 +36,7 @@
     w83627ehf   10      5       4       3      0x8850 0x88    0x5ca3
                                                0x8860 0xa1
     w83627dhg    9      5       4       3      0xa020 0xc1    0x5ca3
+    w83667hg     9      5       3       3      0xa510 0xc1    0x5ca3
 */
 
 #include <linux/module.h>
@@ -51,12 +52,13 @@
 #include <asm/io.h>
 #include "lm75.h"
 
-enum kinds { w83627ehf, w83627dhg };
+enum kinds { w83627ehf, w83627dhg, w83667hg};
 
 /* used to set data->name = w83627ehf_device_names[data->sio_kind] */
 static const char * w83627ehf_device_names[] = {
 	"w83627ehf",
 	"w83627dhg",
+	"w83667hg",
 };
 
 static unsigned short force_id;
@@ -70,6 +72,7 @@ MODULE_PARM_DESC(force_id, "Override the
  */
 
 #define W83627EHF_LD_HWM	0x0b
+#define W83667HG_LD_VID 	0x0d
 
 #define SIO_REG_LDSEL		0x07	/* Logical device select */
 #define SIO_REG_DEVID		0x20	/* Device ID (2 bytes) */
@@ -82,6 +85,7 @@ MODULE_PARM_DESC(force_id, "Override the
 #define SIO_W83627EHF_ID	0x8850
 #define SIO_W83627EHG_ID	0x8860
 #define SIO_W83627DHG_ID	0xa020
+#define SIO_W83667HG_ID 	0xa510
 #define SIO_ID_MASK		0xFFF0
 
 static inline void
@@ -288,6 +292,7 @@ struct w83627ehf_data {
 	u8 pwm_mode[4]; /* 0->DC variable voltage, 1->PWM variable duty cycle */
 	u8 pwm_enable[4]; /* 1->manual
 			     2->thermal cruise (also called SmartFan I) */
+	u8 pwm_num;		/* number of pwm */
 	u8 pwm[4];
 	u8 target_temp[4];
 	u8 tolerance[4];
@@ -297,6 +302,9 @@ struct w83627ehf_data {
 
 	u8 vid;
 	u8 vrm;
+
+	u8 temp3_disable;
+	u8 in6_skip;
 };
 
 struct w83627ehf_sio_data {
@@ -865,25 +873,37 @@ show_temp_type(struct device *dev, struc
 	return sprintf(buf, "%d\n", (int)data->temp_type[nr]);
 }
 
-static struct sensor_device_attribute sda_temp[] = {
+static struct sensor_device_attribute sda_temp_input[] = {
 	SENSOR_ATTR(temp1_input, S_IRUGO, show_temp1, NULL, 0),
 	SENSOR_ATTR(temp2_input, S_IRUGO, show_temp, NULL, 0),
 	SENSOR_ATTR(temp3_input, S_IRUGO, show_temp, NULL, 1),
+};
+
+static struct sensor_device_attribute sda_temp_max[] = {
 	SENSOR_ATTR(temp1_max, S_IRUGO | S_IWUSR, show_temp1_max,
 		    store_temp1_max, 0),
 	SENSOR_ATTR(temp2_max, S_IRUGO | S_IWUSR, show_temp_max,
 		    store_temp_max, 0),
 	SENSOR_ATTR(temp3_max, S_IRUGO | S_IWUSR, show_temp_max,
 		    store_temp_max, 1),
+};
+
+static struct sensor_device_attribute sda_temp_max_hyst[] = {
 	SENSOR_ATTR(temp1_max_hyst, S_IRUGO | S_IWUSR, show_temp1_max_hyst,
 		    store_temp1_max_hyst, 0),
 	SENSOR_ATTR(temp2_max_hyst, S_IRUGO | S_IWUSR, show_temp_max_hyst,
 		    store_temp_max_hyst, 0),
 	SENSOR_ATTR(temp3_max_hyst, S_IRUGO | S_IWUSR, show_temp_max_hyst,
 		    store_temp_max_hyst, 1),
+};
+
+static struct sensor_device_attribute sda_temp_alarm[] = {
 	SENSOR_ATTR(temp1_alarm, S_IRUGO, show_alarm, NULL, 4),
 	SENSOR_ATTR(temp2_alarm, S_IRUGO, show_alarm, NULL, 5),
 	SENSOR_ATTR(temp3_alarm, S_IRUGO, show_alarm, NULL, 13),
+};
+
+static struct sensor_device_attribute sda_temp_type[] = {
 	SENSOR_ATTR(temp1_type, S_IRUGO, show_temp_type, NULL, 0),
 	SENSOR_ATTR(temp2_type, S_IRUGO, show_temp_type, NULL, 1),
 	SENSOR_ATTR(temp3_type, S_IRUGO, show_temp_type, NULL, 2),
@@ -1180,6 +1200,8 @@ static void w83627ehf_device_remove_file
 	for (i = 0; i < ARRAY_SIZE(sda_sf3_arrays_fan4); i++)
 		device_remove_file(dev, &sda_sf3_arrays_fan4[i].dev_attr);
 	for (i = 0; i < data->in_num; i++) {
+		if ((i = 6) && data->in6_skip)
+			continue;
 		device_remove_file(dev, &sda_in_input[i].dev_attr);
 		device_remove_file(dev, &sda_in_alarm[i].dev_attr);
 		device_remove_file(dev, &sda_in_min[i].dev_attr);
@@ -1191,15 +1213,22 @@ static void w83627ehf_device_remove_file
 		device_remove_file(dev, &sda_fan_div[i].dev_attr);
 		device_remove_file(dev, &sda_fan_min[i].dev_attr);
 	}
-	for (i = 0; i < 4; i++) {
+	for (i = 0; i < data->pwm_num; i++) {
 		device_remove_file(dev, &sda_pwm[i].dev_attr);
 		device_remove_file(dev, &sda_pwm_mode[i].dev_attr);
 		device_remove_file(dev, &sda_pwm_enable[i].dev_attr);
 		device_remove_file(dev, &sda_target_temp[i].dev_attr);
 		device_remove_file(dev, &sda_tolerance[i].dev_attr);
 	}
-	for (i = 0; i < ARRAY_SIZE(sda_temp); i++)
-		device_remove_file(dev, &sda_temp[i].dev_attr);
+	for (i = 0; i < 3; i++) {
+		if(( i = 2) && data->temp3_disable)
+			continue;
+		device_remove_file(dev, &sda_temp_input[i].dev_attr);
+		device_remove_file(dev, &sda_temp_max[i].dev_attr);
+		device_remove_file(dev, &sda_temp_max_hyst[i].dev_attr);
+		device_remove_file(dev, &sda_temp_alarm[i].dev_attr);
+		device_remove_file(dev, &sda_temp_type[i].dev_attr);
+	}
 
 	device_remove_file(dev, &dev_attr_name);
 	device_remove_file(dev, &dev_attr_cpu0_vid);
@@ -1221,6 +1250,8 @@ static inline void __devinit w83627ehf_i
 	for (i = 0; i < 2; i++) {
 		tmp = w83627ehf_read_value(data,
 					   W83627EHF_REG_TEMP_CONFIG[i]);
+		if ((i = 1) && (data->in6_skip || data->temp3_disable))
+			continue;
 		if (tmp & 0x01)
 			w83627ehf_write_value(data,
 					      W83627EHF_REG_TEMP_CONFIG[i],
@@ -1271,8 +1302,16 @@ static int __devinit w83627ehf_probe(str
 	data->name = w83627ehf_device_names[sio_data->kind];
 	platform_set_drvdata(pdev, data);
 
-	/* 627EHG and 627EHF have 10 voltage inputs; DHG has 9 */
-	data->in_num = (sio_data->kind = w83627dhg) ? 9 : 10;
+	/* 627EHG and 627EHF have 10 voltage inputs; DHG and 667HG have 9 */
+	data->in_num = (sio_data->kind = w83627ehf) ? 10 : 9;
+	/* 667HG has 3 pwms */
+	data->pwm_num = (sio_data->kind = w83667hg) ? 3 : 4;
+
+	/* Check temp3 configuration bit for 667HG */
+	if (sio_data->kind = w83667hg) {
+		data->temp3_disable = w83627ehf_read_value(data, W83627EHF_REG_TEMP_CONFIG[1]) & 0x01;
+		data->in6_skip = !(w83627ehf_read_value(data, W83627EHF_REG_TEMP_CONFIG[1]) & 0x01);
+	}
 
 	/* Initialize the chip */
 	w83627ehf_init_device(data);
@@ -1280,44 +1319,60 @@ static int __devinit w83627ehf_probe(str
 	data->vrm = vid_which_vrm();
 	superio_enter(sio_data->sioreg);
 	/* Read VID value */
-	superio_select(sio_data->sioreg, W83627EHF_LD_HWM);
-	if (superio_inb(sio_data->sioreg, SIO_REG_VID_CTRL) & 0x80) {
-		/* Set VID input sensibility if needed. In theory the BIOS
-		   should have set it, but in practice it's not always the
-		   case. We only do it for the W83627EHF/EHG because the
-		   W83627DHG is more complex in this respect. */
-		if (sio_data->kind = w83627ehf) {
-			en_vrm10 = superio_inb(sio_data->sioreg,
-					       SIO_REG_EN_VRM10);
-			if ((en_vrm10 & 0x08) && data->vrm = 90) {
-				dev_warn(dev, "Setting VID input voltage to "
-					 "TTL\n");
-				superio_outb(sio_data->sioreg, SIO_REG_EN_VRM10,
-					     en_vrm10 & ~0x08);
-			} else if (!(en_vrm10 & 0x08) && data->vrm = 100) {
-				dev_warn(dev, "Setting VID input voltage to "
-					 "VRM10\n");
-				superio_outb(sio_data->sioreg, SIO_REG_EN_VRM10,
-					     en_vrm10 | 0x08);
-			}
-		}
-
-		data->vid = superio_inb(sio_data->sioreg, SIO_REG_VID_DATA);
-		if (sio_data->kind = w83627ehf) /* 6 VID pins only */
-			data->vid &= 0x3f;
-
+	if (sio_data->kind = w83667hg) {
+		/* W83667HG has different pins for VID input and output, so 
+ 		we can get the VID input values directly at logical device D
+		0xe3. */
+		superio_select(sio_data->sioreg, W83667HG_LD_VID);
+		data->vid = superio_inb(sio_data->sioreg, 0xe3);
 		err = device_create_file(dev, &dev_attr_cpu0_vid);
 		if (err)
 			goto exit_release;
 	} else {
-		dev_info(dev, "VID pins in output mode, CPU VID not "
-			 "available\n");
+		superio_select(sio_data->sioreg, W83627EHF_LD_HWM);
+		if (superio_inb(sio_data->sioreg, SIO_REG_VID_CTRL) & 0x80) {
+			/* Set VID input sensibility if needed. In theory the BIOS
+			   should have set it, but in practice it's not always the
+			   case. We only do it for the W83627EHF/EHG because the
+			   W83627DHG is more complex in this respect. */
+			if (sio_data->kind = w83627ehf) {
+				en_vrm10 = superio_inb(sio_data->sioreg,
+						       SIO_REG_EN_VRM10);
+				if ((en_vrm10 & 0x08) && data->vrm = 90) {
+					dev_warn(dev, "Setting VID input voltage to "
+						 "TTL\n");
+					superio_outb(sio_data->sioreg, SIO_REG_EN_VRM10,
+						     en_vrm10 & ~0x08);
+				} else if (!(en_vrm10 & 0x08) && data->vrm = 100) {
+					dev_warn(dev, "Setting VID input voltage to "
+						 "VRM10\n");
+					superio_outb(sio_data->sioreg, SIO_REG_EN_VRM10,
+						     en_vrm10 | 0x08);
+				}
+			}
+
+			data->vid = superio_inb(sio_data->sioreg, SIO_REG_VID_DATA);
+			if (sio_data->kind = w83627ehf) /* 6 VID pins only */
+				data->vid &= 0x3f;
+
+			err = device_create_file(dev, &dev_attr_cpu0_vid);
+			if (err)
+				goto exit_release;
+		} else {
+			dev_info(dev, "VID pins in output mode, CPU VID not "
+				 "available\n");
+		}
 	}
 
 	/* fan4 and fan5 share some pins with the GPIO and serial flash */
 
-	fan5pin = superio_inb(sio_data->sioreg, 0x24) & 0x2;
-	fan4pin = superio_inb(sio_data->sioreg, 0x29) & 0x6;
+	if (sio_data->kind = w83667hg) {
+		fan5pin = superio_inb(sio_data->sioreg, 0x27) & 0x20;
+		fan4pin = superio_inb(sio_data->sioreg, 0x27) & 0x40;
+	} else {			
+		fan5pin = !(superio_inb(sio_data->sioreg, 0x24) & 0x02);
+		fan4pin = !(superio_inb(sio_data->sioreg, 0x29) & 0x06);
+	}
 	superio_exit(sio_data->sioreg);
 
 	/* It looks like fan4 and fan5 pins can be alternatively used
@@ -1328,9 +1383,9 @@ static int __devinit w83627ehf_probe(str
 
 	data->has_fan = 0x07; /* fan1, fan2 and fan3 */
 	i = w83627ehf_read_value(data, W83627EHF_REG_FANDIV1);
-	if ((i & (1 << 2)) && (!fan4pin))
+	if ((i & (1 << 2)) && fan4pin)
 		data->has_fan |= (1 << 3);
-	if (!(i & (1 << 1)) && (!fan5pin))
+	if (!(i & (1 << 1)) && fan5pin)
 		data->has_fan |= (1 << 4);
 
 	/* Read fan clock dividers immediately */
@@ -1343,23 +1398,27 @@ static int __devinit w83627ehf_probe(str
 			goto exit_remove;
 
 	/* if fan4 is enabled create the sf3 files for it */
-	if (data->has_fan & (1 << 3))
+	if ((sio_data->kind != w83667hg) && 
+		(data->has_fan & (1 << 3)))
 		for (i = 0; i < ARRAY_SIZE(sda_sf3_arrays_fan4); i++) {
 			if ((err = device_create_file(dev,
 				&sda_sf3_arrays_fan4[i].dev_attr)))
 				goto exit_remove;
 		}
 
-	for (i = 0; i < data->in_num; i++)
-		if ((err = device_create_file(dev, &sda_in_input[i].dev_attr))
-			|| (err = device_create_file(dev,
-				&sda_in_alarm[i].dev_attr))
-			|| (err = device_create_file(dev,
-				&sda_in_min[i].dev_attr))
-			|| (err = device_create_file(dev,
-				&sda_in_max[i].dev_attr)))
-			goto exit_remove;
-
+	for (i = 0; i < data->in_num; i++) {
+		if ((i = 6) && data->in6_skip) 
+			continue;
+		else
+			if ((err = device_create_file(dev, &sda_in_input[i].dev_attr))
+				|| (err = device_create_file(dev,
+					&sda_in_alarm[i].dev_attr))
+				|| (err = device_create_file(dev,
+					&sda_in_min[i].dev_attr))
+				|| (err = device_create_file(dev,
+					&sda_in_max[i].dev_attr)))
+				goto exit_remove;
+		}
 	for (i = 0; i < 5; i++) {
 		if (data->has_fan & (1 << i)) {
 			if ((err = device_create_file(dev,
@@ -1371,8 +1430,8 @@ static int __devinit w83627ehf_probe(str
 				|| (err = device_create_file(dev,
 					&sda_fan_min[i].dev_attr)))
 				goto exit_remove;
-			if (i < 4 && /* w83627ehf only has 4 pwm */
-				((err = device_create_file(dev,
+	if(i < data->pwm_num &&
+			((err = device_create_file(dev,
 					&sda_pwm[i].dev_attr))
 				|| (err = device_create_file(dev,
 					&sda_pwm_mode[i].dev_attr))
@@ -1386,9 +1445,21 @@ static int __devinit w83627ehf_probe(str
 		}
 	}
 
-	for (i = 0; i < ARRAY_SIZE(sda_temp); i++)
-		if ((err = device_create_file(dev, &sda_temp[i].dev_attr)))
+	for (i = 0; i < 3; i++) {
+		if ( ( i = 2) && data->temp3_disable) 
+			continue;
+		if ((err = device_create_file(dev, 
+				&sda_temp_input[i].dev_attr))
+			|| (err = device_create_file(dev, 
+				&sda_temp_max[i].dev_attr))
+			|| (err = device_create_file(dev,
+				&sda_temp_max_hyst[i].dev_attr))
+			|| (err = device_create_file(dev,
+				&sda_temp_alarm[i].dev_attr))
+			|| (err = device_create_file(dev,
+				&sda_temp_type[i].dev_attr)))
 			goto exit_remove;
+	}
 
 	err = device_create_file(dev, &dev_attr_name);
 	if (err)
@@ -1441,6 +1512,7 @@ static int __init w83627ehf_find(int sio
 	static const char __initdata sio_name_W83627EHF[] = "W83627EHF";
 	static const char __initdata sio_name_W83627EHG[] = "W83627EHG";
 	static const char __initdata sio_name_W83627DHG[] = "W83627DHG";
+	static const char __initdata sio_name_W83667HG[] = "W83667HG";
 
 	u16 val;
 	const char *sio_name;
@@ -1465,6 +1537,10 @@ static int __init w83627ehf_find(int sio
 		sio_data->kind = w83627dhg;
 		sio_name = sio_name_W83627DHG;
 		break;
+	case SIO_W83667HG_ID:
+		sio_data->kind = w83667hg;
+		sio_name = sio_name_W83667HG;
+		break;
 	default:
 		if (val != 0xffff)
 			pr_debug(DRVNAME ": unsupported chip ID: 0x%04x\n",



=============================================The privileged confidential information contained in this email is intended for use only by the addressees as indicated by the original sender of this email. If you are not the addressee indicated in this email or are not responsible for delivery of the email to such  a person, please kindly reply to the sender indicating this fact and delete all copies of it from your computer and network server immediately. Your cooperation is highly appreciated. It is advised that any unauthorized use of confidential information of Nuvoton is strictly prohibited; and any information in this email irrelevant to the official business of Nuvoton shall be deemed as neither given nor endorsed by Nuvoton.

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

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

* Re: [lm-sensors] W83667HG driver test for in6 and temp3
  2009-02-25  9:13 [lm-sensors] W83667HG driver test for in6 and temp3 JGong
                   ` (2 preceding siblings ...)
  2009-02-26  9:37 ` JGong
@ 2009-02-26 15:09 ` Jean Delvare
  2009-02-26 16:10 ` Jean Delvare
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Jean Delvare @ 2009-02-26 15:09 UTC (permalink / raw)
  To: lm-sensors

Hi Michael,

On Wed, 25 Feb 2009 11:18:23 -0500, Michael Hampton wrote:
> I applied this patch to my current Fedora 10 kernel but the patched
> module fails to load:
> root@underground ~ # modprobe w83627ehf
> FATAL: Error inserting w83627ehf
> (/lib/modules/2.6.27.15-170.2.24.fc10.x86_64/kernel/drivers/hwmon/w83627ehf.ko):
> No such device
> 
> I haven't tried with a vanilla kernel yet. I don't usually run them.
> 
> Interestingly, sensors-detect says the device has a different ID:

Different from what? 

> Some Super I/O chips may also contain sensors. We have to write to
> standard I/O ports to probe them. This is usually safe.
> Do you want to scan for Super I/O sensors? (YES/no):
> Probing for Super-I/O at 0x2e/0x2f
> Trying family `National Semiconductor'...                   No
> Trying family `SMSC'...                                     No
> Trying family `VIA/Winbond/Fintek'...                       Yes
> Found unknown chip with ID 0xa513
>     (logical device B has address 0x290, could be sensors)
> Probing for Super-I/O at 0x4e/0x4f
> Trying family `National Semiconductor'...                   No
> Trying family `SMSC'...                                     No
> Trying family `VIA/Winbond/Fintek'...                       No
> Trying family `ITE'...                                      No

The driver accepts IDs 0xa510 to 0xa51f so it should be OK.

-- 
Jean Delvare

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

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

* Re: [lm-sensors] W83667HG driver test for in6 and temp3
  2009-02-25  9:13 [lm-sensors] W83667HG driver test for in6 and temp3 JGong
                   ` (3 preceding siblings ...)
  2009-02-26 15:09 ` Jean Delvare
@ 2009-02-26 16:10 ` Jean Delvare
  2009-02-26 18:33 ` Michael Hampton
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Jean Delvare @ 2009-02-26 16:10 UTC (permalink / raw)
  To: lm-sensors

Hi all,

On Thu, 26 Feb 2009 17:37:38 +0800, JGong@nuvoton.com wrote:
> Thanks very much for your quick comment. I've added the skip_in6 and
> temp3_disable into the data struct, cut the sda_temp into sub-arrays now.

Looks good to me. Only your Signed-off-by line is missing. Can I add it?

To all potential testers: I've split support for the W83667HG into 3
patches:

ftp://ftp.kernel.org/pub/linux/kernel/people/jdelvare/linux-2.6/jdelvare-hwmon/hwmon-w83627ehf-01-invert-fan-pin-logic.patch
ftp://ftp.kernel.org/pub/linux/kernel/people/jdelvare/linux-2.6/jdelvare-hwmon/hwmon-w83627ehf-02-add-support-for-w83667hg.patch
ftp://ftp.kernel.org/pub/linux/kernel/people/jdelvare/linux-2.6/jdelvare-hwmon/hwmon-w83627ehf-03-fix-temp3-vs-in6.patch

These patches are now in linux-next and will be merged into kernel
2.6.30 if we receive positive feedback.

Any further change should be in a new patch to be applied on top of
these 3 patches we already have.

Thanks,
-- 
Jean Delvare

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

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

* Re: [lm-sensors] W83667HG driver test for in6 and temp3
  2009-02-25  9:13 [lm-sensors] W83667HG driver test for in6 and temp3 JGong
                   ` (4 preceding siblings ...)
  2009-02-26 16:10 ` Jean Delvare
@ 2009-02-26 18:33 ` Michael Hampton
  2009-03-03 14:49 ` Michael Hampton
  2009-03-03 15:06 ` Jean Delvare
  7 siblings, 0 replies; 9+ messages in thread
From: Michael Hampton @ 2009-02-26 18:33 UTC (permalink / raw)
  To: lm-sensors

Jean Delvare wrote:
> Hi Michael,
>
> On Wed, 25 Feb 2009 11:18:23 -0500, Michael Hampton wrote:
>   
>> I applied this patch to my current Fedora 10 kernel but the patched
>> module fails to load:
>> root@underground ~ # modprobe w83627ehf
>> FATAL: Error inserting w83627ehf
>> (/lib/modules/2.6.27.15-170.2.24.fc10.x86_64/kernel/drivers/hwmon/w83627ehf.ko):
>> No such device
>>
>> I haven't tried with a vanilla kernel yet. I don't usually run them.
>>
>> Interestingly, sensors-detect says the device has a different ID:
>>     
>
> Different from what? 
>
>   
>> Some Super I/O chips may also contain sensors. We have to write to
>> standard I/O ports to probe them. This is usually safe.
>> Do you want to scan for Super I/O sensors? (YES/no):
>> Probing for Super-I/O at 0x2e/0x2f
>> Trying family `National Semiconductor'...                   No
>> Trying family `SMSC'...                                     No
>> Trying family `VIA/Winbond/Fintek'...                       Yes
>> Found unknown chip with ID 0xa513
>>     (logical device B has address 0x290, could be sensors)
>> Probing for Super-I/O at 0x4e/0x4f
>> Trying family `National Semiconductor'...                   No
>> Trying family `SMSC'...                                     No
>> Trying family `VIA/Winbond/Fintek'...                       No
>> Trying family `ITE'...                                      No
>>     
>
> The driver accepts IDs 0xa510 to 0xa51f so it should be OK.
>   

OK, then this is a misunderstanding on my part. I probably didn't do
something right when I was patching the kernel. I'll try it again with
the 3 patches you posted in your other message and report back.

-- 
Homeland Stupidity <http://www.homelandstupidity.us/>

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

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

* Re: [lm-sensors] W83667HG driver test for in6 and temp3
  2009-02-25  9:13 [lm-sensors] W83667HG driver test for in6 and temp3 JGong
                   ` (5 preceding siblings ...)
  2009-02-26 18:33 ` Michael Hampton
@ 2009-03-03 14:49 ` Michael Hampton
  2009-03-03 15:06 ` Jean Delvare
  7 siblings, 0 replies; 9+ messages in thread
From: Michael Hampton @ 2009-03-03 14:49 UTC (permalink / raw)
  To: lm-sensors

Jean Delvare wrote:
> To all potential testers: I've split support for the W83667HG into 3
> patches:
>
> ftp://ftp.kernel.org/pub/linux/kernel/people/jdelvare/linux-2.6/jdelvare-hwmon/hwmon-w83627ehf-01-invert-fan-pin-logic.patch
> ftp://ftp.kernel.org/pub/linux/kernel/people/jdelvare/linux-2.6/jdelvare-hwmon/hwmon-w83627ehf-02-add-support-for-w83667hg.patch
> ftp://ftp.kernel.org/pub/linux/kernel/people/jdelvare/linux-2.6/jdelvare-hwmon/hwmon-w83627ehf-03-fix-temp3-vs-in6.patch
>
> These patches are now in linux-next and will be merged into kernel
> 2.6.30 if we receive positive feedback.
>
> Any further change should be in a new patch to be applied on top of
> these 3 patches we already have.
>
> Thanks,
>   

I've applied these 3 patches against the latest Fedora 10 kernel
2.6.27.19-170.2.35.fc10.x86_64 and it's working, only I get temp3 (with
an invalid value) instead of in6. The motherboard is an ASUS P6T6 WS
Revolution.

w83667hg-isa-0290
Adapter: ISA adapter
in0:         +1.34 V  (min =  +0.00 V, max =  +1.74 V)  
in1:         +1.70 V  (min =  +0.02 V, max =  +0.00 V)   ALARM
in2:         +3.22 V  (min =  +1.02 V, max =  +0.26 V)   ALARM
in3:         +3.18 V  (min =  +2.56 V, max =  +0.00 V)   ALARM
in4:         +1.69 V  (min =  +0.00 V, max =  +1.04 V)   ALARM
in5:         +2.04 V  (min =  +0.77 V, max =  +0.52 V)   ALARM
in7:         +3.34 V  (min =  +0.02 V, max =  +1.34 V)   ALARM
in8:         +3.31 V  (min =  +0.86 V, max =  +1.09 V)   ALARM
fan1:       1917 RPM  (min = 1288 RPM, div = 8)
fan2:       1875 RPM  (min = 14062 RPM, div = 16)  ALARM
fan3:       1795 RPM  (min = 5273 RPM, div = 4)  ALARM
fan4:       1875 RPM  (min = 1298 RPM, div = 8)
fan5:       2481 RPM  (min = 2636 RPM, div = 8)  ALARM
temp1:       +43.0°C  (high =  +0.0°C, hyst =  +4.0°C)  ALARM  sensor thermistor
temp2:       +60.5°C  (high = +80.0°C, hyst = +75.0°C)  sensor = diode
temp3:      +127.0°C  (high = +80.0°C, hyst = +75.0°C)  ALARM  sensor thermistor
cpu0_vid:   +1.950 V

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

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

* Re: [lm-sensors] W83667HG driver test for in6 and temp3
  2009-02-25  9:13 [lm-sensors] W83667HG driver test for in6 and temp3 JGong
                   ` (6 preceding siblings ...)
  2009-03-03 14:49 ` Michael Hampton
@ 2009-03-03 15:06 ` Jean Delvare
  7 siblings, 0 replies; 9+ messages in thread
From: Jean Delvare @ 2009-03-03 15:06 UTC (permalink / raw)
  To: lm-sensors

Hi Michael,

On Tue, 03 Mar 2009 09:49:39 -0500, Michael Hampton wrote:
> Jean Delvare wrote:
> > To all potential testers: I've split support for the W83667HG into 3
> > patches:
> >
> > ftp://ftp.kernel.org/pub/linux/kernel/people/jdelvare/linux-2.6/jdelvare-hwmon/hwmon-w83627ehf-01-invert-fan-pin-logic.patch
> > ftp://ftp.kernel.org/pub/linux/kernel/people/jdelvare/linux-2.6/jdelvare-hwmon/hwmon-w83627ehf-02-add-support-for-w83667hg.patch
> > ftp://ftp.kernel.org/pub/linux/kernel/people/jdelvare/linux-2.6/jdelvare-hwmon/hwmon-w83627ehf-03-fix-temp3-vs-in6.patch
> >
> > These patches are now in linux-next and will be merged into kernel
> > 2.6.30 if we receive positive feedback.
> >
> > Any further change should be in a new patch to be applied on top of
> > these 3 patches we already have.
> >
> > Thanks,
> >   
> 
> I've applied these 3 patches against the latest Fedora 10 kernel
> 2.6.27.19-170.2.35.fc10.x86_64 and it's working, only I get temp3 (with
> an invalid value) instead of in6. The motherboard is an ASUS P6T6 WS
> Revolution.

OK, thanks for testing and reporting. OTOH, temp3 is invalid, but you
don't know if in6 would be valid, so it doesn't mean much. Is there any
temperature or voltage value shown in the BIOS which is missing in the
output of "sensors"?

Do you happen to have Windows installed on that machine for comparison
purposes?

> 
> w83667hg-isa-0290
> Adapter: ISA adapter
> in0:         +1.34 V  (min =  +0.00 V, max =  +1.74 V)  
> in1:         +1.70 V  (min =  +0.02 V, max =  +0.00 V)   ALARM
> in2:         +3.22 V  (min =  +1.02 V, max =  +0.26 V)   ALARM
> in3:         +3.18 V  (min =  +2.56 V, max =  +0.00 V)   ALARM
> in4:         +1.69 V  (min =  +0.00 V, max =  +1.04 V)   ALARM
> in5:         +2.04 V  (min =  +0.77 V, max =  +0.52 V)   ALARM
> in7:         +3.34 V  (min =  +0.02 V, max =  +1.34 V)   ALARM
> in8:         +3.31 V  (min =  +0.86 V, max =  +1.09 V)   ALARM
> fan1:       1917 RPM  (min = 1288 RPM, div = 8)
> fan2:       1875 RPM  (min = 14062 RPM, div = 16)  ALARM
> fan3:       1795 RPM  (min = 5273 RPM, div = 4)  ALARM
> fan4:       1875 RPM  (min = 1298 RPM, div = 8)
> fan5:       2481 RPM  (min = 2636 RPM, div = 8)  ALARM
> temp1:       +43.0°C  (high =  +0.0°C, hyst =  +4.0°C)  ALARM  sensor > thermistor
> temp2:       +60.5°C  (high = +80.0°C, hyst = +75.0°C)  sensor = diode
> temp3:      +127.0°C  (high = +80.0°C, hyst = +75.0°C)  ALARM  sensor > thermistor
> cpu0_vid:   +1.950 V


-- 
Jean Delvare

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

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

end of thread, other threads:[~2009-03-03 15:06 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-02-25  9:13 [lm-sensors] W83667HG driver test for in6 and temp3 JGong
2009-02-25 15:22 ` Jean Delvare
2009-02-25 16:18 ` Michael Hampton
2009-02-26  9:37 ` JGong
2009-02-26 15:09 ` Jean Delvare
2009-02-26 16:10 ` Jean Delvare
2009-02-26 18:33 ` Michael Hampton
2009-03-03 14:49 ` Michael Hampton
2009-03-03 15:06 ` 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.