[lm-sensors] W83667HG on ASUS P6T6 WS Revolution

[lm-sensors] W83667HG on ASUS P6T6 WS Revolution

[Michael Hampton]

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

This is just a report to state that the two patches posted to this list
previously [1] [2] got my sensors up and running on my new ASUS P6T6 WS
Revolution board. They aren't perfect yet, as temp3 is reporting an
obviously incorrect value, and I haven't figured out which sensors map
to which physical devices on the motherboard yet, but it seems to be OK.
Perhaps temp3 just isn't hooked up to anything.

For the sake of convenience I've merged the two patches together and
included it here. Hopefully this gets into a future kernel release, and
soon.

w83667hg-isa-0290
Adapter: ISA adapter
in0:         +1.22 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.59 V, max =  +0.00 V)   ALARM
in4:         +1.70 V  (min =  +0.13 V, max =  +1.04 V)   ALARM
in5:         +2.04 V  (min =  +0.77 V, max =  +0.62 V)   ALARM
in6:         +1.02 V  (min =  +0.58 V, max =  +0.13 V)   ALARM
in7:         +3.36 V  (min =  +0.02 V, max =  +1.34 V)   ALARM
in8:         +3.31 V  (min =  +2.91 V, max =  +1.22 V)   ALARM
fan1:       1854 RPM  (min = 1288 RPM, div = 8)
fan2:       1917 RPM  (min = 14062 RPM, div = 16)  ALARM
fan3:       1795 RPM  (min = 5273 RPM, div = 4)  ALARM
fan4:       1896 RPM  (min = 1298 RPM, div = 8)
fan5:       2410 RPM  (min = 2636 RPM, div = 8)  ALARM
temp1:       +37.0°C  (high =  +0.0°C, hyst =  +4.0°C)  ALARM  sensor =
thermistor
temp2:       +50.5°C  (high = +80.0°C, hyst = +75.0°C)  sensor = diode
temp3:      +124.5°C  (high = +80.0°C, hyst = +75.0°C)  ALARM  sensor =
thermistor
cpu0_vid:   +1.950 V

[1]
http://lists.lm-sensors.org/pipermail/lm-sensors/2008-December/024903.html
[2]
http://lists.lm-sensors.org/pipermail/lm-sensors/2009-January/025232.html


[-- Attachment #2: hwmon-add-support-for-w83667hg.patch --]
[-- Type: text/plain, Size: 12407 bytes --]

diff -ur linux-2.6.28.orig/Documentation/hwmon/w83627ehf linux-2.6.28/Documentation/hwmon/w83627ehf
--- linux-2.6.28.orig/Documentation/hwmon/w83627ehf	2008-12-24 18:26:37.000000000 -0500
+++ linux-2.6.28/Documentation/hwmon/w83627ehf	2009-02-05 17:02:45.000000000 -0500
@@ -2,30 +2,40 @@
 =======================
 
 Supported chips:
-  * Winbond W83627EHF/EHG/DHG (ISA access ONLY)
+  * Winbond W83627EHF/EHG (ISA access ONLY)
     Prefix: 'w83627ehf'
     Addresses scanned: ISA address retrieved from Super I/O registers
     Datasheet:
-        http://www.winbond-usa.com/products/winbond_products/pdfs/PCIC/W83627EHF_%20W83627EHGb.pdf
-        DHG datasheet confidential.
+        http://www.nuvoton.com.tw/NR/rdonlyres/A6A258F0-F0C9-4F97-81C0-C4D29E7E943E/0/W83627EHF.pdf
+  * Winbond W83627DHG
+    Prefix: 'w83627dhg'
+    Addresses scanned: ISA address retrieved from Super I/O registers
+    Datasheet:
+        http://www.nuvoton.com.tw/NR/rdonlyres/7885623D-A487-4CF9-A47F-30C5F73D6FE6/0/W83627DHG.pdf
+ * Winbond W83667HG
+   Prefix: 'w83667hg'
+   Addresses scanned: ISA address retrieved from Super I/O registers
+   Datasheet: not available
 
 Authors:
         Jean Delvare <khali@linux-fr.org>
         Yuan Mu (Winbond)
         Rudolf Marek <r.marek@assembler.cz>
         David Hubbard <david.c.hubbard@gmail.com>
+        Gong Jun <JGong@nuvoton.com>
 
 Description
 -----------
 
-This driver implements support for the Winbond W83627EHF, W83627EHG, and
-W83627DHG super I/O chips. We will refer to them collectively as Winbond chips.
+This driver implements support for the Winbond W83627EHF, W83627EHG,
+W83627DHG and W83667HG super I/O chips. We will refer to them collectively
+as Winbond chips.
 
 The chips implement three temperature sensors, five fan rotation
 speed sensors, ten analog voltage sensors (only nine for the 627DHG), one
-VID (6 pins for the 627EHF/EHG, 8 pins for the 627DHG), alarms with beep
-warnings (control unimplemented), and some automatic fan regulation
-strategies (plus manual fan control mode).
+VID (6 pins for the 627EHF/EHG, 8 pins for the 627DHG and 667HG), alarms
+with beep warnings (control unimplemented), and some automatic fan
+regulation strategies (plus manual fan control mode).
 
 Temperatures are measured in degrees Celsius and measurement resolution is 1
 degC for temp1 and 0.5 degC for temp2 and temp3. An alarm is triggered when
@@ -54,7 +64,8 @@
 temp1 -> pwm1
 temp2 -> pwm2
 temp3 -> pwm3
-prog  -> pwm4 (the programmable setting is not supported by the driver)
+prog  -> pwm4 (not on 667HG, the programmable setting is not supported by
+	       the driver)
 
 /sys files
 ----------
diff -ur linux-2.6.28.orig/drivers/hwmon/Kconfig linux-2.6.28/drivers/hwmon/Kconfig
--- linux-2.6.28.orig/drivers/hwmon/Kconfig	2008-12-24 18:26:37.000000000 -0500
+++ linux-2.6.28/drivers/hwmon/Kconfig	2009-02-05 17:02:45.000000000 -0500
@@ -794,7 +794,7 @@
 	  will be called w83627hf.
 
 config SENSORS_W83627EHF
-	tristate "Winbond W83627EHF/DHG"
+	tristate "Winbond W83627EHF/EHG/DHG, W83667HG"
 	select HWMON_VID
 	help
 	  If you say yes here you get support for the hardware
@@ -805,6 +805,8 @@
 	  chip suited for specific Intel processors that use PECI such as
 	  the Core 2 Duo.
 
+	  This driver also supports the W83667HG chip.
+
 	  This driver can also be built as a module.  If so, the module
 	  will be called w83627ehf.
 
diff -ur linux-2.6.28.orig/drivers/hwmon/w83627ehf.c linux-2.6.28/drivers/hwmon/w83627ehf.c
--- linux-2.6.28.orig/drivers/hwmon/w83627ehf.c	2008-12-24 18:26:37.000000000 -0500
+++ linux-2.6.28/drivers/hwmon/w83627ehf.c	2009-02-05 17:02:45.000000000 -0500
@@ -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 @@
  */
 
 #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 @@
 #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
@@ -148,6 +152,7 @@
 					 (0x555 + (((nr) - 7) * 2)))
 #define W83627EHF_REG_IN(nr)		((nr < 7) ? (0x20 + (nr)) : \
 					 (0x550 + (nr) - 7))
+#define W83627EHF_REG_IN6_667HG		0x250
 
 #define W83627EHF_REG_TEMP1		0x27
 #define W83627EHF_REG_TEMP1_HYST	0x3a
@@ -288,6 +293,7 @@
 	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];
@@ -453,6 +459,8 @@
 	struct w83627ehf_data *data = dev_get_drvdata(dev);
 	int pwmcfg = 0, tolerance = 0; /* shut up the compiler */
 	int i;
+	struct w83627ehf_sio_data *sio_data = dev->platform_data;
+	u8 reg;
 
 	mutex_lock(&data->update_lock);
 
@@ -463,8 +471,21 @@
 
 		/* Measured voltages and limits */
 		for (i = 0; i < data->in_num; i++) {
-			data->in[i] = w83627ehf_read_value(data,
-				      W83627EHF_REG_IN(i));
+			if ((sio_data->kind == w83667hg) && (i == 6)) {
+				/* W83667HG, try fix in6 lost problem */
+				reg = w83627ehf_read_value(data,
+				      W83627EHF_REG_TEMP_CONFIG[1]);
+				w83627ehf_write_value(data,
+						W83627EHF_REG_TEMP_CONFIG[1],
+						reg | 0x01);
+				data->in[i] = w83627ehf_read_value(data,
+					      W83627EHF_REG_IN6_667HG);
+				w83627ehf_write_value(data,
+						W83627EHF_REG_TEMP_CONFIG[1],
+						reg);
+			} else
+				data->in[i] = w83627ehf_read_value(data,
+					      W83627EHF_REG_IN(i));
 			data->in_min[i] = w83627ehf_read_value(data,
 					  W83627EHF_REG_IN_MIN(i));
 			data->in_max[i] = w83627ehf_read_value(data,
@@ -1191,7 +1212,7 @@
 		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);
@@ -1271,8 +1292,10 @@
 	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; 627DHG 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 +1303,64 @@
 	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 +1371,9 @@
 
 	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,7 +1386,8 @@
 			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)))
@@ -1371,8 +1415,8 @@
 				|| (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))
@@ -1441,6 +1485,7 @@
 	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 +1510,10 @@
 		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

Re: [lm-sensors] W83667HG on ASUS P6T6 WS Revolution

[Jean Delvare]

Hi Michael,

On Thu, 05 Feb 2009 20:37:14 -0500, Michael Hampton wrote:
> This is just a report to state that the two patches posted to this list
> previously [1] [2] got my sensors up and running on my new ASUS P6T6 WS
> Revolution board. They aren't perfect yet, as temp3 is reporting an
> obviously incorrect value, and I haven't figured out which sensors map
> to which physical devices on the motherboard yet, but it seems to be OK.
> Perhaps temp3 just isn't hooked up to anything.

On this chip, in6 and temp3 share the same pin, so you can't have both.
Which one is present depends on the motherboard wiring. So it is
possible that on your chip the pin is used for in6. Ideally we'd have a
way to know which is there and only export that reading. Gong Jun is
supposedly working on this. Gong Jun, any progress? How is the chip
told which function is used? Pin strapping, configuration register...?

For the labels, you should be able to figure out the fan and temp
labels by comparing with the BIOS. For voltages that's a little more
difficult, but I would assume that the internal voltages are the same
as those of the W83627DHG:

    label in0 "Vcore"
    label in2 "AVCC"
    label in3 "VCC"
    label in7 "3VSB"
    label in8 "Vbat"

    set in2_min  3.3 * 0.90
    set in2_max  3.3 * 1.10
    set in3_min  3.3 * 0.90
    set in3_max  3.3 * 1.10
    set in7_min  3.3 * 0.90
    set in7_max  3.3 * 1.10
    set in8_min  3.0 * 0.90
    set in8_max  3.0 * 1.10

> For the sake of convenience I've merged the two patches together and
> included it here. Hopefully this gets into a future kernel release, and
> soon.
> 
> w83667hg-isa-0290
> Adapter: ISA adapter
> in0:         +1.22 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.59 V, max =  +0.00 V)   ALARM
> in4:         +1.70 V  (min =  +0.13 V, max =  +1.04 V)   ALARM
> in5:         +2.04 V  (min =  +0.77 V, max =  +0.62 V)   ALARM
> in6:         +1.02 V  (min =  +0.58 V, max =  +0.13 V)   ALARM
> in7:         +3.36 V  (min =  +0.02 V, max =  +1.34 V)   ALARM
> in8:         +3.31 V  (min =  +2.91 V, max =  +1.22 V)   ALARM
> fan1:       1854 RPM  (min = 1288 RPM, div = 8)
> fan2:       1917 RPM  (min = 14062 RPM, div = 16)  ALARM
> fan3:       1795 RPM  (min = 5273 RPM, div = 4)  ALARM
> fan4:       1896 RPM  (min = 1298 RPM, div = 8)
> fan5:       2410 RPM  (min = 2636 RPM, div = 8)  ALARM
> temp1:       +37.0°C  (high =  +0.0°C, hyst =  +4.0°C)  ALARM  sensor > thermistor
> temp2:       +50.5°C  (high = +80.0°C, hyst = +75.0°C)  sensor = diode
> temp3:      +124.5°C  (high = +80.0°C, hyst = +75.0°C)  ALARM  sensor > thermistor
> cpu0_vid:   +1.950 V
> 
> [1]
> http://lists.lm-sensors.org/pipermail/lm-sensors/2008-December/024903.html
> [2]
> http://lists.lm-sensors.org/pipermail/lm-sensors/2009-January/025232.html
> 


-- 
Jean Delvare

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

Re: [lm-sensors] W83667HG on ASUS P6T6 WS Revolution

[JGong]

Dear Jean,

So far, the w83667hg's pin 109(in6 and temp3's shared pin) selection is only decided by BIOS. So maybe the user has to config the value with the help of its own BIOS readings, and then disables the meaningless output in the sensors.conf file.


Best regards,
Gong Jun


=============================================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

Re: [lm-sensors] W83667HG on ASUS P6T6 WS Revolution

[Jean Delvare]

Hi Gong Jun,

On Tue, 24 Feb 2009 16:49:53 +0800, JGong@nuvoton.com wrote:
> Dear Jean,
> 
> So far, the w83667hg's pin 109(in6 and temp3's shared pin) selection is
> only decided by BIOS.

Can you please clarify what you mean by "decided by BIOS"? Do you mean
that the BIOS is writing something to the chip to configure pin 109? Or
do you mean that the BIOS author has the knowledge of the board and
displays either in6 or temp3 depending on the board's wiring?

> So maybe the user has to config the value with the help of its own
> BIOS readings, and then disables the meaningless output in the
> sensors.conf file.

This is really not ideal from a usability perspective.

Why don't we simply trust bit 0 of register 0x52 in bank 2? After all,
your original code changes this bit to switch between temp3 and in6, so
it would seem reasonable that the BIOS sets it to the right value as
well.

So I propose the following approach:
* Make the driver trust bit 0 of register 0x52 in bank 2. If the bit is
  set, create in6 attributes but not temp3 attributes. If it is clear,
  create temp3 attributes but not in6 attributes. Issue a debug message
  explaining the decision. For the W83667HG only, of course; for older
  chips we keep always creating all attributes.
* If it is ever found that some motherboard doesn't properly initialize
  this configuration bit, try to get the manufacturer to fix the BIOS.
  If we fail, either add a per-motherboard override in the driver, or
  add a module parameter or a sysfs attribute to let the user override
  the default configuration.

What do you think? It would be great to get this solved quickly so that
support for the W83667HG makes it into kernel 2.6.30.

Thanks,
-- 
Jean Delvare

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

Re: [lm-sensors] W83667HG on ASUS P6T6 WS Revolution

[JGong]

Dear Jean,

>Can you please clarify what you mean by "decided by BIOS"? Do you mean
>that the BIOS is writing something to the chip to configure pin 109? Or
>do you mean that the BIOS author has the knowledge of the board and
>displays either in6 or temp3 depending on the board's wiring?

I mean the second one. 

>This is really not ideal from a usability perspective.
>Why don't we simply trust bit 0 of register 0x52 in bank 2? After all,
>your original code changes this bit to switch between temp3 and in6, so
>it would seem reasonable that the BIOS sets it to the right value as
>well.

I made a mistake in the last patch. Recently my colleague explains that setting the bit 0 can only stop monitoring the temp3, not to switch the readings to in6, and for in6, we still need to read it at bank 0 index 26h.

>So I propose the following approach:
>* Make the driver trust bit 0 of register 0x52 in bank 2. If the bit is
>  set, create in6 attributes but not temp3 attributes. If it is clear,
>  create temp3 attributes but not in6 attributes. Issue a debug message
>  explaining the decision. For the W83667HG only, of course; for older
>  chips we keep always creating all attributes.
>* If it is ever found that some motherboard doesn't properly initialize
>  this configuration bit, try to get the manufacturer to fix the BIOS.
>  If we fail, either add a per-motherboard override in the driver, or
>  add a module parameter or a sysfs attribute to let the user override
>  the default configuration.

OK, if we trust the bit 0 and the bit is set, we create in6 not temp3, otherwise we create the temp3 not in6. (The bit's default value is 0.)

But what can we do if the temp3 is created and its value is unacceptable like 124.5°C? 

>It would be great to get this solved quickly so that support for the >W83667HG makes it into kernel 2.6.30.

As soon as we've found a proper solution of this problem, I'll make out the patch and recommit it. 

Best regards,
Gong Jun


=============================================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

Re: [lm-sensors] W83667HG on ASUS P6T6 WS Revolution

[Jean Delvare]

On Tue, 24 Feb 2009 18:09:54 +0800, JGong@nuvoton.com wrote:
> >So I propose the following approach:
> >* Make the driver trust bit 0 of register 0x52 in bank 2. If the bit is
> >  set, create in6 attributes but not temp3 attributes. If it is clear,
> >  create temp3 attributes but not in6 attributes. Issue a debug message
> >  explaining the decision. For the W83667HG only, of course; for older
> >  chips we keep always creating all attributes.
> >* If it is ever found that some motherboard doesn't properly initialize
> >  this configuration bit, try to get the manufacturer to fix the BIOS.
> >  If we fail, either add a per-motherboard override in the driver, or
> >  add a module parameter or a sysfs attribute to let the user override
> >  the default configuration.
> 
> OK, if we trust the bit 0 and the bit is set, we create in6 not temp3,
> otherwise we create the temp3 not in6. (The bit's default value is 0.)

Yes, exactly.

> But what can we do if the temp3 is created and its value is unacceptable
> like 124.5°C?

Well, it remains to be seen if this will ever happen. With your current
patch, both in6 and temp3 are created, so we don't know what was the
initial state of the configuration bit.

> >It would be great to get this solved quickly so that support for the
> >W83667HG makes it into kernel 2.6.30.
> 
> As soon as we've found a proper solution of this problem, I'll make out
> the patch and recommit it.

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

Thanks,
-- 
Jean Delvare

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