LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* 2.6.23-rc1 regression: hwmon/w83627ehf: wrong fan speed
@ 2007-08-05 11:20 Stefan Richter
  2007-08-05 11:28 ` Stefan Richter
                   ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Stefan Richter @ 2007-08-05 11:20 UTC (permalink / raw)
  To: lm-sensors; +Cc: linux-kernel, Mark M. Hoffman, Jean Delvare

Now that I booted from 2.6.22-rc5 to 2.6.23-rc2 I noticed that ksensors
displayed 1/16 of the actual speed of the CPU fan (correct is ca. 1400
RPM under light load) and 0 for the case fan (correct is ca. 480 RPM
under light load).

I reverted patch
	hwmon/w83627ehf: No need to initialize fan_min
and the behaviour changed as follows:  ksensors displays the correct CPU
fan speed when loaded for the first time after the w83627ehf was loaded,
then drops to 1/16th the next time it refreshes the display.  (I
configured 30s update interval in ksensors.)  But the case fan's speed
is now correctly displayed all the time.  Ksensors' fan speed
multipliers are configured as 1.

I then also reverted
	hwmon/w83627ehf: Be quiet when no chip is found
	hwmon/w83627ehf: Export the thermal sensor types
	hwmon/w83627ehf: Enable VBAT monitoring
	hwmon/w83627ehf: Add support for the VID inputs
	hwmon/w83627ehf: Fix timing issues
	hwmon/w83627ehf: Add error messages for two error cases
one after another but it didn't change anything.  More surprisingly, if
I put all patches including "No need to initialize fan_min" back in, the
behaviour remains like after referting that single patch, i.e. wrong CPU
fan speed after first display update, correct case fan speed.  I didn't
reboot between tests though, I only unloaded and reloaded w83627ehf.

I wasn't able to revert
	hwmon/w83627ehf: Convert to a platform driver
to something that compiles but I didn't try very hard.

Kernel messages when I load the drivers:
w83627ehf: unsupported chip ID: 0xffff
w83627ehf: Found W83627EHG chip at 0x290
The former message is normally suppressed by patch "Be quiet when no
chip is found".  Mainboard is an MSI 945GT Speedster-A4R, userland is
Gentoo's lm_sensors-2.10.1 and ksensors-0.7.3.

Booting back into 2.6.22-rc5 (which seems identical with 2.6.22 as far
as w83627ehf is concerned) brings back the correct fan speeds.
-- 
Stefan Richter
-=====-=-=== =--- --=-=
http://arcgraph.de/sr/

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

* Re: 2.6.23-rc1 regression: hwmon/w83627ehf: wrong fan speed
  2007-08-05 11:20 2.6.23-rc1 regression: hwmon/w83627ehf: wrong fan speed Stefan Richter
@ 2007-08-05 11:28 ` Stefan Richter
  2007-08-05 13:55 ` Mark M. Hoffman
  2007-08-05 16:21 ` Mark M. Hoffman
  2 siblings, 0 replies; 20+ messages in thread
From: Stefan Richter @ 2007-08-05 11:28 UTC (permalink / raw)
  To: lm-sensors; +Cc: linux-kernel, Mark M. Hoffman, Jean Delvare

I wrote:
> Now that I booted from 2.6.22-rc5 to 2.6.23-rc2 I noticed that ksensors
> displayed 1/16 of the actual speed of the CPU fan (correct is ca. 1400
> RPM under light load) and 0 for the case fan (correct is ca. 480 RPM
> under light load).
> 
> I reverted patch
> 	hwmon/w83627ehf: No need to initialize fan_min
> and the behaviour changed as follows:  ksensors displays the correct CPU
> fan speed when loaded for the first time after the w83627ehf was loaded,
> then drops to 1/16th the next time it refreshes the display.  (I
> configured 30s update interval in ksensors.)  But the case fan's speed
> is now correctly displayed all the time.  Ksensors' fan speed
> multipliers are configured as 1.
...
> Booting back into 2.6.22-rc5 (which seems identical with 2.6.22 as far
> as w83627ehf is concerned) brings back the correct fan speeds.

PS:  The actual behavior under 2.6.22-rc5 is that the CPU fan speed is
displayed correctly from the start, while the case fan speed is
initially shown as 0 but jumps to the correct speed after ksensors'
first display update.
-- 
Stefan Richter
-=====-=-=== =--- --=-=
http://arcgraph.de/sr/

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

* Re: 2.6.23-rc1 regression: hwmon/w83627ehf: wrong fan speed
  2007-08-05 11:20 2.6.23-rc1 regression: hwmon/w83627ehf: wrong fan speed Stefan Richter
  2007-08-05 11:28 ` Stefan Richter
@ 2007-08-05 13:55 ` Mark M. Hoffman
  2007-08-05 14:51   ` Stefan Richter
  2007-08-05 16:21 ` Mark M. Hoffman
  2 siblings, 1 reply; 20+ messages in thread
From: Mark M. Hoffman @ 2007-08-05 13:55 UTC (permalink / raw)
  To: Stefan Richter; +Cc: lm-sensors, linux-kernel, Jean Delvare

Hi Stefan:

* Stefan Richter <stefanr@s5r6.in-berlin.de> [2007-08-05 13:20:48 +0200]:
> Now that I booted from 2.6.22-rc5 to 2.6.23-rc2 I noticed that ksensors
> displayed 1/16 of the actual speed of the CPU fan (correct is ca. 1400
> RPM under light load) and 0 for the case fan (correct is ca. 480 RPM
> under light load).
> 
> I reverted patch
> 	hwmon/w83627ehf: No need to initialize fan_min
> and the behaviour changed as follows:  ksensors displays the correct CPU
> fan speed when loaded for the first time after the w83627ehf was loaded,
> then drops to 1/16th the next time it refreshes the display.  (I
> configured 30s update interval in ksensors.)  But the case fan's speed
> is now correctly displayed all the time.  Ksensors' fan speed
> multipliers are configured as 1.
> 
> I then also reverted
> 	hwmon/w83627ehf: Be quiet when no chip is found
> 	hwmon/w83627ehf: Export the thermal sensor types
> 	hwmon/w83627ehf: Enable VBAT monitoring
> 	hwmon/w83627ehf: Add support for the VID inputs
> 	hwmon/w83627ehf: Fix timing issues
> 	hwmon/w83627ehf: Add error messages for two error cases
> one after another but it didn't change anything.  More surprisingly, if
> I put all patches including "No need to initialize fan_min" back in, the
> behaviour remains like after referting that single patch, i.e. wrong CPU
> fan speed after first display update, correct case fan speed.  I didn't
> reboot between tests though, I only unloaded and reloaded w83627ehf.

It's not always sufficient to just reload the driver, especially when it's an
initialization problem as yours seems to be.  So what you're seeing there is
not too surprising.

> I wasn't able to revert
> 	hwmon/w83627ehf: Convert to a platform driver
> to something that compiles but I didn't try very hard.

That's because a later patch removes the i2c-isa support on which this driver
depends, prior to this patch.  If you can use git to back up, that would be
easier.

That said, I doubt the "Convert to a platform driver" is the problem.  It's
more likely the patch before that:

	hwmon/w83627ehf: Preserve speed reading when changing fan min

I'll reexamine the patch series here.  If you have time, it would help if you
could do a git bisect.  Use this command:

$ git bisect start v2.6.23-rc2 v2.6.22-rc5 drivers/hwmon/w83627ehf.c

Doing it this way will force you to reboot between tests; I think we would
need that anyway.

> Kernel messages when I load the drivers:
> w83627ehf: unsupported chip ID: 0xffff
> w83627ehf: Found W83627EHG chip at 0x290
> The former message is normally suppressed by patch "Be quiet when no
> chip is found".  Mainboard is an MSI 945GT Speedster-A4R, userland is
> Gentoo's lm_sensors-2.10.1 and ksensors-0.7.3.
> 
> Booting back into 2.6.22-rc5 (which seems identical with 2.6.22 as far
> as w83627ehf is concerned) brings back the correct fan speeds.

Regards,

-- 
Mark M. Hoffman
mhoffman@lightlink.com


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

* Re: 2.6.23-rc1 regression: hwmon/w83627ehf: wrong fan speed
  2007-08-05 13:55 ` Mark M. Hoffman
@ 2007-08-05 14:51   ` Stefan Richter
  2007-08-05 15:17     ` Mark M. Hoffman
  0 siblings, 1 reply; 20+ messages in thread
From: Stefan Richter @ 2007-08-05 14:51 UTC (permalink / raw)
  To: Mark M. Hoffman; +Cc: lm-sensors, linux-kernel, Jean Delvare

Mark M. Hoffman wrote:
> It's more likely the patch before that:
> 
> 	hwmon/w83627ehf: Preserve speed reading when changing fan min

I reverted this patch alone and rebooted, still the same problem.
After reboot:
   0 RPM for case fan,
   correct CPU fan speed, drops to 1/16 after kmsensors' first refresh
After unloading and reloading the driver:
   correct speed of case fan,
   correct CPU fan speed, drops to 1/16 after kmsensors' first refresh

> I'll reexamine the patch series here.  If you have time, it would help if you
> could do a git bisect.  Use this command:
> 
> $ git bisect start v2.6.23-rc2 v2.6.22-rc5 drivers/hwmon/w83627ehf.c
> 
> Doing it this way will force you to reboot between tests; I think we would
> need that anyway.

I'll try to reserve the time for that on some evening next week.
Thanks,
-- 
Stefan Richter
-=====-=-=== =--- --=-=
http://arcgraph.de/sr/

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

* Re: 2.6.23-rc1 regression: hwmon/w83627ehf: wrong fan speed
  2007-08-05 14:51   ` Stefan Richter
@ 2007-08-05 15:17     ` Mark M. Hoffman
  0 siblings, 0 replies; 20+ messages in thread
From: Mark M. Hoffman @ 2007-08-05 15:17 UTC (permalink / raw)
  To: Stefan Richter; +Cc: lm-sensors, linux-kernel, Jean Delvare

Hi Stefan:

* Stefan Richter <stefanr@s5r6.in-berlin.de> [2007-08-05 16:51:07 +0200]:
> Mark M. Hoffman wrote:
> > It's more likely the patch before that:
> > 
> > 	hwmon/w83627ehf: Preserve speed reading when changing fan min
> 
> I reverted this patch alone and rebooted, still the same problem.
> After reboot:
>    0 RPM for case fan,
>    correct CPU fan speed, drops to 1/16 after kmsensors' first refresh
> After unloading and reloading the driver:
>    correct speed of case fan,
>    correct CPU fan speed, drops to 1/16 after kmsensors' first refresh

OK, thanks for trying that.  I'll keep looking here.

> > I'll reexamine the patch series here.  If you have time, it would help if you
> > could do a git bisect.  Use this command:
> > 
> > $ git bisect start v2.6.23-rc2 v2.6.22-rc5 drivers/hwmon/w83627ehf.c
> > 
> > Doing it this way will force you to reboot between tests; I think we would
> > need that anyway.
> 
> I'll try to reserve the time for that on some evening next week.
> Thanks,

There are a total of 9 patches between v2.6.22-rc5 and v2.6.23-rc2, so the
bisection shouldn't take long.  Also, please enable HWMON_DEBUG when you do
this, and send the resulting syslog messages.  Thanks.

Regards,

-- 
Mark M. Hoffman
mhoffman@lightlink.com


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

* Re: 2.6.23-rc1 regression: hwmon/w83627ehf: wrong fan speed
  2007-08-05 11:20 2.6.23-rc1 regression: hwmon/w83627ehf: wrong fan speed Stefan Richter
  2007-08-05 11:28 ` Stefan Richter
  2007-08-05 13:55 ` Mark M. Hoffman
@ 2007-08-05 16:21 ` Mark M. Hoffman
  2007-08-05 17:13   ` Stefan Richter
  2007-08-12 16:06   ` Jean Delvare
  2 siblings, 2 replies; 20+ messages in thread
From: Mark M. Hoffman @ 2007-08-05 16:21 UTC (permalink / raw)
  To: Stefan Richter; +Cc: lm-sensors, linux-kernel, Jean Delvare

Hi Stefan:

* Stefan Richter <stefanr@s5r6.in-berlin.de> [2007-08-05 13:20:48 +0200]:
> Now that I booted from 2.6.22-rc5 to 2.6.23-rc2 I noticed that ksensors
> displayed 1/16 of the actual speed of the CPU fan (correct is ca. 1400
> RPM under light load) and 0 for the case fan (correct is ca. 480 RPM
> under light load).
> 
> I reverted patch
> 	hwmon/w83627ehf: No need to initialize fan_min
> and the behaviour changed as follows:  ksensors displays the correct CPU
> fan speed when loaded for the first time after the w83627ehf was loaded,
> then drops to 1/16th the next time it refreshes the display.  (I
> configured 30s update interval in ksensors.)  But the case fan's speed
> is now correctly displayed all the time.  Ksensors' fan speed
> multipliers are configured as 1.
> 
> I then also reverted
> 	hwmon/w83627ehf: Be quiet when no chip is found
> 	hwmon/w83627ehf: Export the thermal sensor types
> 	hwmon/w83627ehf: Enable VBAT monitoring
> 	hwmon/w83627ehf: Add support for the VID inputs
> 	hwmon/w83627ehf: Fix timing issues
> 	hwmon/w83627ehf: Add error messages for two error cases
> one after another but it didn't change anything.  More surprisingly, if
> I put all patches including "No need to initialize fan_min" back in, the
> behaviour remains like after referting that single patch, i.e. wrong CPU
> fan speed after first display update, correct case fan speed.  I didn't
> reboot between tests though, I only unloaded and reloaded w83627ehf.
> 
> I wasn't able to revert
> 	hwmon/w83627ehf: Convert to a platform driver
> to something that compiles but I didn't try very hard.
> 
> Kernel messages when I load the drivers:
> w83627ehf: unsupported chip ID: 0xffff
> w83627ehf: Found W83627EHG chip at 0x290
> The former message is normally suppressed by patch "Be quiet when no
> chip is found".  Mainboard is an MSI 945GT Speedster-A4R, userland is
> Gentoo's lm_sensors-2.10.1 and ksensors-0.7.3.
> 
> Booting back into 2.6.22-rc5 (which seems identical with 2.6.22 as far
> as w83627ehf is concerned) brings back the correct fan speeds.

Does this patch (against v2.6.23-rc2) fix it?

commit f15c50e703c14ff7d72c3cb34e8e55417476a290
Author: Mark M. Hoffman <mhoffman@lightlink.com>
Date:   Sun Aug 5 12:19:01 2007 -0400

    hwmon: read fan_div values during probe
    
    This patch forces the driver to read the fan divider values during early init.
    Otherwise, a call to store_fan_min() could access uninitialized variables.
    
    Signed-off-by: Mark M. Hoffman <mhoffman@lightlink.com>

diff --git a/drivers/hwmon/w83627ehf.c b/drivers/hwmon/w83627ehf.c
index c51ae2e..bca7fbc 100644
--- a/drivers/hwmon/w83627ehf.c
+++ b/drivers/hwmon/w83627ehf.c
@@ -421,6 +421,31 @@ static void w83627ehf_write_fan_div(struct w83627ehf_data *data, int nr)
 	}
 }
 
+static void w83627ehf_update_fan_div(struct w83627ehf_data *data)
+{
+	int i;
+
+	i = w83627ehf_read_value(data, W83627EHF_REG_FANDIV1);
+	data->fan_div[0] = (i >> 4) & 0x03;
+	data->fan_div[1] = (i >> 6) & 0x03;
+	i = w83627ehf_read_value(data, W83627EHF_REG_FANDIV2);
+	data->fan_div[2] = (i >> 6) & 0x03;
+	i = w83627ehf_read_value(data, W83627EHF_REG_VBAT);
+	data->fan_div[0] |= (i >> 3) & 0x04;
+	data->fan_div[1] |= (i >> 4) & 0x04;
+	data->fan_div[2] |= (i >> 5) & 0x04;
+	if (data->has_fan & ((1 << 3) | (1 << 4))) {
+		i = w83627ehf_read_value(data, W83627EHF_REG_DIODE);
+		data->fan_div[3] = i & 0x03;
+		data->fan_div[4] = ((i >> 2) & 0x03)
+				 | ((i >> 5) & 0x04);
+	}
+	if (data->has_fan & (1 << 3)) {
+		i = w83627ehf_read_value(data, W83627EHF_REG_SMI_OVT);
+		data->fan_div[3] |= (i >> 5) & 0x04;
+	}
+}
+
 static struct w83627ehf_data *w83627ehf_update_device(struct device *dev)
 {
 	struct w83627ehf_data *data = dev_get_drvdata(dev);
@@ -432,25 +457,7 @@ static struct w83627ehf_data *w83627ehf_update_device(struct device *dev)
 	if (time_after(jiffies, data->last_updated + HZ + HZ/2)
 	 || !data->valid) {
 		/* Fan clock dividers */
-		i = w83627ehf_read_value(data, W83627EHF_REG_FANDIV1);
-		data->fan_div[0] = (i >> 4) & 0x03;
-		data->fan_div[1] = (i >> 6) & 0x03;
-		i = w83627ehf_read_value(data, W83627EHF_REG_FANDIV2);
-		data->fan_div[2] = (i >> 6) & 0x03;
-		i = w83627ehf_read_value(data, W83627EHF_REG_VBAT);
-		data->fan_div[0] |= (i >> 3) & 0x04;
-		data->fan_div[1] |= (i >> 4) & 0x04;
-		data->fan_div[2] |= (i >> 5) & 0x04;
-		if (data->has_fan & ((1 << 3) | (1 << 4))) {
-			i = w83627ehf_read_value(data, W83627EHF_REG_DIODE);
-			data->fan_div[3] = i & 0x03;
-			data->fan_div[4] = ((i >> 2) & 0x03)
-					 | ((i >> 5) & 0x04);
-		}
-		if (data->has_fan & (1 << 3)) {
-			i = w83627ehf_read_value(data, W83627EHF_REG_SMI_OVT);
-			data->fan_div[3] |= (i >> 5) & 0x04;
-		}
+		w83627ehf_update_fan_div(data);
 
 		/* Measured voltages and limits */
 		for (i = 0; i < data->in_num; i++) {
@@ -1312,6 +1319,9 @@ static int __devinit w83627ehf_probe(struct platform_device *pdev)
 	if (!(i & (1 << 1)) && (!fan5pin))
 		data->has_fan |= (1 << 4);
 
+	/* Read fan clock dividers immediately */
+	w83627ehf_update_fan_div(data);
+
 	/* Register sysfs hooks */
   	for (i = 0; i < ARRAY_SIZE(sda_sf3_arrays); i++)
 		if ((err = device_create_file(dev,
-- 
Mark M. Hoffman
mhoffman@lightlink.com


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

* Re: 2.6.23-rc1 regression: hwmon/w83627ehf: wrong fan speed
  2007-08-05 16:21 ` Mark M. Hoffman
@ 2007-08-05 17:13   ` Stefan Richter
  2007-08-10 21:08     ` Jean Delvare
  2007-08-12 16:06   ` Jean Delvare
  1 sibling, 1 reply; 20+ messages in thread
From: Stefan Richter @ 2007-08-05 17:13 UTC (permalink / raw)
  To: Mark M. Hoffman; +Cc: lm-sensors, linux-kernel, Jean Delvare

Mark M. Hoffman wrote:
> Does this patch (against v2.6.23-rc2) fix it?
> 
> commit f15c50e703c14ff7d72c3cb34e8e55417476a290
> Author: Mark M. Hoffman <mhoffman@lightlink.com>
> Date:   Sun Aug 5 12:19:01 2007 -0400
> 
>     hwmon: read fan_div values during probe
>     
>     This patch forces the driver to read the fan divider values during early init.
>     Otherwise, a call to store_fan_min() could access uninitialized variables.

Alas not; there is no change.
(I applied on vanilla 2.6.23-rc2 and rebooted.)
-- 
Stefan Richter
-=====-=-=== =--- --=-=
http://arcgraph.de/sr/

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

* Re: 2.6.23-rc1 regression: hwmon/w83627ehf: wrong fan speed
  2007-08-05 17:13   ` Stefan Richter
@ 2007-08-10 21:08     ` Jean Delvare
  2007-08-10 22:29       ` Stefan Richter
  0 siblings, 1 reply; 20+ messages in thread
From: Jean Delvare @ 2007-08-10 21:08 UTC (permalink / raw)
  To: Stefan Richter; +Cc: Mark M. Hoffman, lm-sensors, linux-kernel

Hi Stefan,

On Sun, 05 Aug 2007 19:13:34 +0200, Stefan Richter wrote:
> Mark M. Hoffman wrote:
> > Does this patch (against v2.6.23-rc2) fix it?
> > 
> > commit f15c50e703c14ff7d72c3cb34e8e55417476a290
> > Author: Mark M. Hoffman <mhoffman@lightlink.com>
> > Date:   Sun Aug 5 12:19:01 2007 -0400
> > 
> >     hwmon: read fan_div values during probe
> >     
> >     This patch forces the driver to read the fan divider values during early init.
> >     Otherwise, a call to store_fan_min() could access uninitialized variables.
> 
> Alas not; there is no change.
> (I applied on vanilla 2.6.23-rc2 and rebooted.)

I just tried 2.6.23-rc2 on a system where I use the w83627ehf hardware
monitoring driver, and was not able to reproduce the problem you
described. Fan speeds are reported properly for me. Which I kind of
expected, as I tested all my w83627ehf patches on this system before
submitting them.

Please try using sensors instead of ksensors, and confirm that the
behavior is the same. I'd like to rule out a problem in ksensors
itself. sensors will also report the fan divs, this is a useful
information given the problem you have.

Your original post suggests that the fan speed is supposed to change
depending on the system load? Or temperature? Please describe the
mechanism used to achieve this. Could it be that this mechanism isn't
working properly, and the reported (low) speeds are actually true?

What fan inputs are used by your CPU and system fans? "sensors
-c /dev/null" will tell.

Other than that, I can only ask for the same things Mark already
suggested: compile with HWMON debugging and provide the logs (this will
show what fan div the driver is trying to select), and try bisecting
using git to find out which patch exactly caused the problem.

Thanks,
-- 
Jean Delvare

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

* Re: 2.6.23-rc1 regression: hwmon/w83627ehf: wrong fan speed
  2007-08-10 21:08     ` Jean Delvare
@ 2007-08-10 22:29       ` Stefan Richter
  2007-08-10 22:39         ` Stefan Richter
  2007-08-11 11:57         ` Jean Delvare
  0 siblings, 2 replies; 20+ messages in thread
From: Stefan Richter @ 2007-08-10 22:29 UTC (permalink / raw)
  To: Jean Delvare; +Cc: Mark M. Hoffman, lm-sensors, linux-kernel

Jean Delvare wrote:
> I just tried 2.6.23-rc2 on a system where I use the w83627ehf hardware
> monitoring driver, and was not able to reproduce the problem you
> described. Fan speeds are reported properly for me. Which I kind of
> expected, as I tested all my w83627ehf patches on this system before
> submitting them.

Thanks that you are still after it.  I was busy with other stuff the
whole week, hence no git bisect result from me yet.

> Please try using sensors instead of ksensors, and confirm that the
> behavior is the same. I'd like to rule out a problem in ksensors
> itself. sensors will also report the fan divs, this is a useful
> information given the problem you have.

# sensors
w83627ehf-isa-0290
Adapter: ISA adapter
VCore:     +0.95 V  (min =  +0.00 V, max =  +1.74 V)
in1:      +12.30 V  (min =  +1.64 V, max =  +3.22 V) ALARM
AVCC:      +3.28 V  (min =  +1.89 V, max =  +1.94 V) ALARM
3VCC:      +3.26 V  (min =  +0.18 V, max =  +0.72 V) ALARM
in4:       +1.58 V  (min =  +0.57 V, max =  +0.90 V) ALARM
in5:       +1.70 V  (min =  +0.41 V, max =  +1.19 V) ALARM
in6:       +3.43 V  (min =  +0.31 V, max =  +3.05 V) ALARM
VSB:       +3.25 V  (min =  +0.37 V, max =  +3.01 V) ALARM
VBAT:      +3.18 V  (min =  +3.94 V, max =  +0.74 V) ALARM
in9:       +1.88 V  (min =  +0.79 V, max =  +1.40 V) ALARM
Case Fan:    0 RPM  (min =  753 RPM, div = 128) ALARM
CPU Fan:    88 RPM  (min =  659 RPM, div = 64) ALARM
Aux Fan:     0 RPM  (min = 10546 RPM, div = 128) ALARM
fan5:        0 RPM  (min =  753 RPM, div = 128) ALARM
Sys Temp:    +44 C  (high =    -5 C, hyst =   -34 C)   ALARM
CPU Temp:  +38.0 C  (high = +80.0 C, hyst = +75.0 C)
AUX Temp:  +43.5 C  (high = +80.0 C, hyst = +75.0 C)

coretemp-isa-0000
Adapter: ISA adapter

coretemp-isa-0001
Adapter: ISA adapter

(The aux fan and fan5 are not connected.)

> Your original post suggests that the fan speed is supposed to change
> depending on the system load? Or temperature? Please describe the
> mechanism used to achieve this. Could it be that this mechanism isn't
> working properly, and the reported (low) speeds are actually true?

The motherboard controls the CPU fan and I believe also the case fan,
probably based on temperatures.  (The manual is buried somewhere and
MSI's download site is down right in this moment.)

The low speeds or the dividers incorrect.  I'll reboot in a minute into
2.6.22(-rc5)  and post the "sensors" output.

> What fan inputs are used by your CPU and system fans? "sensors
> -c /dev/null" will tell.

...
fan1:      484 RPM  (min = 12053 RPM, div = 16) ALARM
fan2:       89 RPM  (min =  659 RPM, div = 64) ALARM
fan3:        0 RPM  (min = 10546 RPM, div = 128) ALARM
fan5:        0 RPM  (min = 1506 RPM, div = 128) ALARM
...

Hmm, interesting.  When I now re-run sensors I get
...
Case Fan:  484 RPM  (min = 12053 RPM, div = 16) ALARM
CPU Fan:    89 RPM  (min =  659 RPM, div = 64) ALARM
Aux Fan:     0 RPM  (min = 10546 RPM, div = 128) ALARM
fan5:        0 RPM  (min = 1506 RPM, div = 128) ALARM
...

(I'm still in 2.6.23-rc2.  Ksensors picked the 484 RPM of the case fan
up too, and that's most certainly the correct speed.  Just the CPU fan's
speed is still wrong; or rather its divider should be 16 rather than 64.)

> Other than that, I can only ask for the same things Mark already
> suggested: compile with HWMON debugging and provide the logs (this will
> show what fan div the driver is trying to select), and try bisecting
> using git to find out which patch exactly caused the problem.

How comes the divider of one of the fans changed from one minute to the
other?

FWIW, the ``chip "w83627ehf-*"´´ section in Gentoo's /etc/sensors.conf
provides only labels for fan{1,2,3}. It is titled
# Winbond W83627EHF configuration originally contributed by Leon Moonen
# This is for an Asus P5P800, voltages for A8V-E SE.

Should I hardwire correct dividers or pulse per rev in sensors.conf or
is the driver supposed to work the correct dividers out --- like it did
before 2.6.23-rc?
-- 
Stefan Richter
-=====-=-=== =--- -=-==
http://arcgraph.de/sr/

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

* Re: 2.6.23-rc1 regression: hwmon/w83627ehf: wrong fan speed
  2007-08-10 22:29       ` Stefan Richter
@ 2007-08-10 22:39         ` Stefan Richter
  2007-08-11  2:30           ` [lm-sensors] " David Hubbard
  2007-08-11 11:57         ` Jean Delvare
  1 sibling, 1 reply; 20+ messages in thread
From: Stefan Richter @ 2007-08-10 22:39 UTC (permalink / raw)
  To: Jean Delvare; +Cc: Mark M. Hoffman, lm-sensors, linux-kernel

I wrote:
> # sensors
> w83627ehf-isa-0290
> Adapter: ISA adapter
> VCore:     +0.95 V  (min =  +0.00 V, max =  +1.74 V)
> in1:      +12.30 V  (min =  +1.64 V, max =  +3.22 V) ALARM
> AVCC:      +3.28 V  (min =  +1.89 V, max =  +1.94 V) ALARM
> 3VCC:      +3.26 V  (min =  +0.18 V, max =  +0.72 V) ALARM
> in4:       +1.58 V  (min =  +0.57 V, max =  +0.90 V) ALARM
> in5:       +1.70 V  (min =  +0.41 V, max =  +1.19 V) ALARM
> in6:       +3.43 V  (min =  +0.31 V, max =  +3.05 V) ALARM
> VSB:       +3.25 V  (min =  +0.37 V, max =  +3.01 V) ALARM
> VBAT:      +3.18 V  (min =  +3.94 V, max =  +0.74 V) ALARM
> in9:       +1.88 V  (min =  +0.79 V, max =  +1.40 V) ALARM
> Case Fan:    0 RPM  (min =  753 RPM, div = 128) ALARM
> CPU Fan:    88 RPM  (min =  659 RPM, div = 64) ALARM
> Aux Fan:     0 RPM  (min = 10546 RPM, div = 128) ALARM
> fan5:        0 RPM  (min =  753 RPM, div = 128) ALARM
> Sys Temp:    +44 C  (high =    -5 C, hyst =   -34 C)   ALARM
> CPU Temp:  +38.0 C  (high = +80.0 C, hyst = +75.0 C)
> AUX Temp:  +43.5 C  (high = +80.0 C, hyst = +75.0 C)
> 
> coretemp-isa-0000
> Adapter: ISA adapter
> 
> coretemp-isa-0001
> Adapter: ISA adapter
...
> I'll reboot in a minute into 2.6.22(-rc5) and post the "sensors" output.

# sensors
w83627ehf-i2c-9191-290
 ERROR: Can't get adapter or algorithm?!?
VCore:     +0.95 V  (min =  +0.00 V, max =  +1.74 V)
in1:      +12.20 V  (min =  +1.64 V, max =  +3.22 V) ALARM
AVCC:      +3.26 V  (min =  +1.89 V, max =  +1.94 V) ALARM
3VCC:      +3.26 V  (min =  +0.18 V, max =  +0.72 V) ALARM
in4:       +1.58 V  (min =  +0.57 V, max =  +0.90 V) ALARM
in5:       +1.71 V  (min =  +0.41 V, max =  +1.19 V) ALARM
in6:       +3.43 V  (min =  +0.31 V, max =  +3.05 V) ALARM
VSB:       +3.26 V  (min =  +0.37 V, max =  +3.01 V) ALARM
VBAT:      +3.18 V  (min =  +3.94 V, max =  +0.74 V) ALARM
in9:       +1.88 V  (min =  +0.79 V, max =  +1.40 V) ALARM
Case Fan:  484 RPM  (min = 84375 RPM, div = 16) ALARM
CPU Fan:  1424 RPM  (min = 21093 RPM, div = 4) ALARM
Aux Fan:     0 RPM  (min = 10546 RPM, div = 128) ALARM
fan5:        0 RPM  (min = 10546 RPM, div = 128) ALARM
Sys Temp:    +45 C  (high =    -5 C, hyst =   -34 C)   ALARM
CPU Temp:  +39.5 C  (high = +80.0 C, hyst = +75.0 C)
AUX Temp:  +44.5 C  (high = +80.0 C, hyst = +75.0 C)

coretemp-isa-0000
Adapter: ISA adapter

coretemp-isa-0001
Adapter: ISA adapter

-- 
Stefan Richter
-=====-=-=== =--- -=-==
http://arcgraph.de/sr/

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

* Re: [lm-sensors] 2.6.23-rc1 regression: hwmon/w83627ehf: wrong fan speed
  2007-08-10 22:39         ` Stefan Richter
@ 2007-08-11  2:30           ` David Hubbard
  0 siblings, 0 replies; 20+ messages in thread
From: David Hubbard @ 2007-08-11  2:30 UTC (permalink / raw)
  To: Stefan Richter, Jean Delvare, Mark M. Hoffman, linux-kernel, lm-sensors

Hi Stefan, (Replying to everyone on the list, sorry!)

On 8/10/07, Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
> Should I hardwire correct dividers or pulse per rev in sensors.conf or
> is the driver supposed to work the correct dividers out --- like it did
> before 2.6.23-rc?

The dividers are read-only in userspace. The driver manages the
dividers automatically. The dividers are needed because the w83627ehf
chip only has an 8 bit register to count pulses for each fan. So if
the fan is moving slowly, you want the divider to be 128 so that every
pulse gets counted. If the fan is moving fast, you want the divider to
be 1 so that the register doesn't overflow. Once the register is read
in by the driver, the effect of the divider is cancelled out in
software so that you get an RPM reading from the fan. One side effect
of this is that a fast moving fan reports the RPM more quickly than a
slow moving fan.

If you turn on HWMON debugging, the driver will report when it is
changing the divider in dmesg.

Hope that helps,
David

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

* Re: 2.6.23-rc1 regression: hwmon/w83627ehf: wrong fan speed
  2007-08-10 22:29       ` Stefan Richter
  2007-08-10 22:39         ` Stefan Richter
@ 2007-08-11 11:57         ` Jean Delvare
  2007-08-11 15:41           ` Stefan Richter
  2007-08-12 17:49           ` Mark M. Hoffman
  1 sibling, 2 replies; 20+ messages in thread
From: Jean Delvare @ 2007-08-11 11:57 UTC (permalink / raw)
  To: Stefan Richter; +Cc: Mark M. Hoffman, lm-sensors, linux-kernel

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

Hi Stefan,

On Sat, 11 Aug 2007 00:29:36 +0200, Stefan Richter wrote:
> Jean Delvare wrote:
> > I just tried 2.6.23-rc2 on a system where I use the w83627ehf hardware
> > monitoring driver, and was not able to reproduce the problem you
> > described. Fan speeds are reported properly for me. Which I kind of
> > expected, as I tested all my w83627ehf patches on this system before
> > submitting them.
> 
> Thanks that you are still after it.  I was busy with other stuff the
> whole week, hence no git bisect result from me yet.
> 
> > Please try using sensors instead of ksensors, and confirm that the
> > behavior is the same. I'd like to rule out a problem in ksensors
> > itself. sensors will also report the fan divs, this is a useful
> > information given the problem you have.
> 
> # sensors
> w83627ehf-isa-0290
> Adapter: ISA adapter
> VCore:     +0.95 V  (min =  +0.00 V, max =  +1.74 V)
> in1:      +12.30 V  (min =  +1.64 V, max =  +3.22 V) ALARM
> AVCC:      +3.28 V  (min =  +1.89 V, max =  +1.94 V) ALARM
> 3VCC:      +3.26 V  (min =  +0.18 V, max =  +0.72 V) ALARM
> in4:       +1.58 V  (min =  +0.57 V, max =  +0.90 V) ALARM
> in5:       +1.70 V  (min =  +0.41 V, max =  +1.19 V) ALARM
> in6:       +3.43 V  (min =  +0.31 V, max =  +3.05 V) ALARM
> VSB:       +3.25 V  (min =  +0.37 V, max =  +3.01 V) ALARM
> VBAT:      +3.18 V  (min =  +3.94 V, max =  +0.74 V) ALARM
> in9:       +1.88 V  (min =  +0.79 V, max =  +1.40 V) ALARM
> Case Fan:    0 RPM  (min =  753 RPM, div = 128) ALARM
> CPU Fan:    88 RPM  (min =  659 RPM, div = 64) ALARM
> Aux Fan:     0 RPM  (min = 10546 RPM, div = 128) ALARM
> fan5:        0 RPM  (min =  753 RPM, div = 128) ALARM
> Sys Temp:    +44 C  (high =    -5 C, hyst =   -34 C)   ALARM
> CPU Temp:  +38.0 C  (high = +80.0 C, hyst = +75.0 C)
> AUX Temp:  +43.5 C  (high = +80.0 C, hyst = +75.0 C)
> 
> coretemp-isa-0000
> Adapter: ISA adapter
> 
> coretemp-isa-0001
> Adapter: ISA adapter

Note: a more recent version of lm-sensors would give you additional
temperature values here.

> 
> (The aux fan and fan5 are not connected.)
> 
> > Your original post suggests that the fan speed is supposed to change
> > depending on the system load? Or temperature? Please describe the
> > mechanism used to achieve this. Could it be that this mechanism isn't
> > working properly, and the reported (low) speeds are actually true?
> 
> The motherboard controls the CPU fan and I believe also the case fan,
> probably based on temperatures.  (The manual is buried somewhere and
> MSI's download site is down right in this moment.)

I would like to know what it is doing exactly, and how. Can this
feature be disabled? If the BIOS accesses the W83627EHG chip in our
back, this can cause lots of trouble, such as what you are seeing.

> The low speeds or the dividers incorrect.  I'll reboot in a minute into
> 2.6.22(-rc5)  and post the "sensors" output.
> 
> > What fan inputs are used by your CPU and system fans? "sensors
> > -c /dev/null" will tell.
> 
> ...
> fan1:      484 RPM  (min = 12053 RPM, div = 16) ALARM
> fan2:       89 RPM  (min =  659 RPM, div = 64) ALARM
> fan3:        0 RPM  (min = 10546 RPM, div = 128) ALARM
> fan5:        0 RPM  (min = 1506 RPM, div = 128) ALARM
> ...

Note: 89 RPM with div=64 corresponds to the same register value as 1424
RPM with div=4 (as shown in your next post). As if the chip and the
driver didn't agree on what the actual divider is.

> Hmm, interesting.  When I now re-run sensors I get
> ...
> Case Fan:  484 RPM  (min = 12053 RPM, div = 16) ALARM
> CPU Fan:    89 RPM  (min =  659 RPM, div = 64) ALARM
> Aux Fan:     0 RPM  (min = 10546 RPM, div = 128) ALARM
> fan5:        0 RPM  (min = 1506 RPM, div = 128) ALARM
> ...
> 
> (I'm still in 2.6.23-rc2.  Ksensors picked the 484 RPM of the case fan
> up too, and that's most certainly the correct speed.  Just the CPU fan's
> speed is still wrong; or rather its divider should be 16 rather than 64.)

Divider should be 4,not 16, methinks.

You can dump the chip registers using the following command:
isadump 0x295 0x296
From now on, whenever you paste the output of "sensors", please dump
the contents of the chip too and include the output.

> > Other than that, I can only ask for the same things Mark already
> > suggested: compile with HWMON debugging and provide the logs (this will
> > show what fan div the driver is trying to select), and try bisecting
> > using git to find out which patch exactly caused the problem.
> 
> How comes the divider of one of the fans changed from one minute to the
> other?

No idea. The new driver can only increase, not decrease, the clock
divider when you poll for a speed value. So the change above (from 128
to 16) is not supposed to happen. However... I also can't explain why
the original reading is 0 (with div=128). A reading of 0 only happens
if the divider is too low (i.e. less than 16.) If the driver increased
the divider to 16, it means that it was previously 8, not 128.

Now, given how dividers are encoded:
128 -> 111b
 64 -> 110b
  8 -> 011b
  4 -> 010b

See the pattern? The case fan's clock divider reads 128 when it is 8,
the CPU fan's clock divider reads 64 when it is 4. In both cases, it is
the most significant bit that is wrong. And it happens that this bit is
in a separate register (VBAT, 0x5d), which happens to be in the banked
register range of the W83627EHG (0x50-0x5f).

So my theory is that something else (BIOS, ACPI?) is changing the bank,
probably to read temperature values which are in banks 1 and 2, causing
the w83627ehf to get a wrong value for the VBAT register. If I am
right, then the attached patch should help. Please give it a try and
report.

Mark: the previous version of the driver was initializing the fan mins.
This wasn't needed, however the bank was reset to 0 as a side effect
when initializing fan5_min. When removing the unneeded code, I caused
the initial bank value to become undefined. This explains in part the
odd behavior reported by Stefan. The fix is to either set the bank to 0
explicitly on driver load, or to stop assuming that bank is 0 by
default. My patch does the latter, as it might also help in case
something is later doing concurrent accesses to the chip.

> FWIW, the ``chip "w83627ehf-*"´´ section in Gentoo's /etc/sensors.conf
> provides only labels for fan{1,2,3}. It is titled
> # Winbond W83627EHF configuration originally contributed by Leon Moonen
> # This is for an Asus P5P800, voltages for A8V-E SE.

This is from the standard default sensors configuration file. It is
expected that you tweak the labels, limits etc. to match your own
motherboard.

> Should I hardwire correct dividers or pulse per rev in sensors.conf or
> is the driver supposed to work the correct dividers out --- like it did
> before 2.6.23-rc?

The driver is supposed to pick the best divider if you set the min fan
speed limits properly (which it seems you didn't.) If you don't set the
min limits, all the driver will do is increase the divider as long as
it gets a 0 reading, so the dividers will be good, but not necessarily
optimum.

Thanks,
-- 
Jean Delvare

[-- Attachment #2: hwmon-w83627ehf-dont-assume-bank-0.patch --]
[-- Type: text/x-patch, Size: 1462 bytes --]

Don't assume that the default bank is 0. For one thing, we don't even
set it to 0 when the driver is loaded, so the initial state might be
different. For another, something (say, the BIOS) might access the chip
and leave with the bank set to something different, so assuming that
the bank value is 0 is not safe.

Signed-off-by: Jean Delvare <khali@linux-fr.org>
---
 drivers/hwmon/w83627ehf.c |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

--- linux-2.6.23-rc2.orig/drivers/hwmon/w83627ehf.c	2007-07-23 16:44:35.000000000 +0200
+++ linux-2.6.23-rc2/drivers/hwmon/w83627ehf.c	2007-08-11 12:55:58.000000000 +0200
@@ -309,18 +309,16 @@ static inline int is_word_sized(u16 reg)
 	      || (reg & 0x00ff) == 0x55));
 }
 
-/* We assume that the default bank is 0, thus the following two functions do
-   nothing for registers which live in bank 0. For others, they respectively
-   set the bank register to the correct value (before the register is
-   accessed), and back to 0 (afterwards). */
+/* Registers 0x50-0x5f are banked */
 static inline void w83627ehf_set_bank(struct w83627ehf_data *data, u16 reg)
 {
-	if (reg & 0xff00) {
+	if ((reg & 0x00f0) == 0x50) {
 		outb_p(W83627EHF_REG_BANK, data->addr + ADDR_REG_OFFSET);
 		outb_p(reg >> 8, data->addr + DATA_REG_OFFSET);
 	}
 }
 
+/* Not strictly necessary, but play it safe for now */
 static inline void w83627ehf_reset_bank(struct w83627ehf_data *data, u16 reg)
 {
 	if (reg & 0xff00) {

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

* Re: 2.6.23-rc1 regression: hwmon/w83627ehf: wrong fan speed
  2007-08-11 11:57         ` Jean Delvare
@ 2007-08-11 15:41           ` Stefan Richter
  2007-08-11 15:48             ` Stefan Richter
  2007-08-12  9:17             ` Jean Delvare
  2007-08-12 17:49           ` Mark M. Hoffman
  1 sibling, 2 replies; 20+ messages in thread
From: Stefan Richter @ 2007-08-11 15:41 UTC (permalink / raw)
  To: Jean Delvare; +Cc: Mark M. Hoffman, lm-sensors, linux-kernel

Jean Delvare wrote:
> On Sat, 11 Aug 2007 00:29:36 +0200, Stefan Richter wrote:
...
>> The motherboard controls the CPU fan and I believe also the case fan,
>> probably based on temperatures.  (The manual is buried somewhere and
>> MSI's download site is down right in this moment.)
> 
> I would like to know what it is doing exactly, and how. Can this
> feature be disabled? If the BIOS accesses the W83627EHG chip in our
> back, this can cause lots of trouble, such as what you are seeing.

While I test-booted 2.6.22(-rc) yesterday I had a look into the BIOS
setup.  There is a fan speed control based on a temperature threshold,
separately for CPU fan and case fan.  The thresholds are currently 55°C
and 50°C respectively.

During the time I spent in the BIOS setup, the CPU fan speed was
displayed as something more than 1400, and the case fan speed was
displayed as 0.  The latter is AFAIK typical with slow fans.

...
>> When I now re-run sensors I get
>> ...
>> Case Fan:  484 RPM  (min = 12053 RPM, div = 16) ALARM
[instead of what was shown a minute before:
   Case Fan:    0 RPM  (min =  753 RPM, div = 128) ALARM]
>> CPU Fan:    89 RPM  (min =  659 RPM, div = 64) ALARM
>> Aux Fan:     0 RPM  (min = 10546 RPM, div = 128) ALARM
>> fan5:        0 RPM  (min = 1506 RPM, div = 128) ALARM
>> ...
>>
>> (I'm still in 2.6.23-rc2.  Ksensors picked the 484 RPM of the case fan
>> up too, and that's most certainly the correct speed.  Just the CPU fan's
>> speed is still wrong; or rather its divider should be 16 rather than 64.)
> 
> Divider should be 4,not 16, methinks.

Yes, years of reliance on pocket calculators did that to me.

> You can dump the chip registers using the following command:
> isadump 0x295 0x296
> From now on, whenever you paste the output of "sensors", please dump
> the contents of the chip too and include the output.
> 
>>> Other than that, I can only ask for the same things Mark already
>>> suggested: compile with HWMON debugging and provide the logs (this will
>>> show what fan div the driver is trying to select), and try bisecting
>>> using git to find out which patch exactly caused the problem.
>> How comes the divider of one of the fans changed from one minute to the
>> other?
> 
> No idea. The new driver can only increase, not decrease, the clock
> divider when you poll for a speed value. So the change above (from 128
> to 16) is not supposed to happen. However... I also can't explain why
> the original reading is 0 (with div=128). A reading of 0 only happens
> if the divider is too low (i.e. less than 16.) If the driver increased
> the divider to 16, it means that it was previously 8, not 128.
> 
> Now, given how dividers are encoded:
> 128 -> 111b
>  64 -> 110b
>   8 -> 011b
>   4 -> 010b
> 
> See the pattern? The case fan's clock divider reads 128 when it is 8,
> the CPU fan's clock divider reads 64 when it is 4. In both cases, it is
> the most significant bit that is wrong. And it happens that this bit is
> in a separate register (VBAT, 0x5d), which happens to be in the banked
> register range of the W83627EHG (0x50-0x5f).
> 
> So my theory is that something else (BIOS, ACPI?) is changing the bank,
> probably to read temperature values which are in banks 1 and 2, causing
> the w83627ehf to get a wrong value for the VBAT register. If I am
> right, then the attached patch should help. Please give it a try and
> report.

Will try it in a minute.

> Mark: the previous version of the driver was initializing the fan mins.
> This wasn't needed, however the bank was reset to 0 as a side effect
> when initializing fan5_min. When removing the unneeded code, I caused
> the initial bank value to become undefined. This explains in part the
> odd behavior reported by Stefan. The fix is to either set the bank to 0
> explicitly on driver load, or to stop assuming that bank is 0 by
> default. My patch does the latter, as it might also help in case
> something is later doing concurrent accesses to the chip.
> 
>> FWIW, the ``chip "w83627ehf-*"´´ section in Gentoo's /etc/sensors.conf
>> provides only labels for fan{1,2,3}. It is titled
>> # Winbond W83627EHF configuration originally contributed by Leon Moonen
>> # This is for an Asus P5P800, voltages for A8V-E SE.
> 
> This is from the standard default sensors configuration file. It is
> expected that you tweak the labels, limits etc. to match your own
> motherboard.
> 
>> Should I hardwire correct dividers or pulse per rev in sensors.conf or
>> is the driver supposed to work the correct dividers out --- like it did
>> before 2.6.23-rc?
> 
> The driver is supposed to pick the best divider if you set the min fan
> speed limits properly (which it seems you didn't.) If you don't set the
> min limits, all the driver will do is increase the divider as long as
> it gets a 0 reading, so the dividers will be good, but not necessarily
> optimum.

I now updated to Gentoo's ksensors-0.7.3-r1 (which is v0.7.3 plus
patches from Debian) and lm_sensors-2.10.4, added

    ignore fan5
    set fan1_min    200
    set fan2_min    1000
    set fan3_min    0

to sensors.conf, compiled the drivers with CONFIG_HWMON_DEBUG_CHIP=y,
and "sensors" alone seems to behave fine now.  Or maybe it did so
already before that.  But as soon as I start "ksensors", "sensors" shows
that the CPU fan divider suddenly changed from 8 to 128.  "sensors -s"
will then cause the kernel to log
    w83627ehf w83627ehf.656: fan2 clock divider changed from 128 to 8
    w83627ehf w83627ehf.656: fan3 low limit and alarm disabled
and sensors will show the correct CPU fan speed again --- but soon after
that the divider will go up to 128 again if ksensors is running in parallel.

If I quit ksensors and run "sensors -s", sensors will continue to show
correct speeds.  Actually with ksensors running, "while sensors | grep
'CPU Fan'; do sleep .2; done" shows that the CPU fan divider oscillates
between 8 and 128 in ca. 5 seconds long periods:  16 times in a row it
prints div = 8, and 8 times it prints div = 128, then div = 8 again, and
so forth.  There are no dmesg messages during all that.

ksensors has different update interval settings, and although I had the
w83627ehf readings configured to be updated every 30 seconds, some
seemingly unrelated setting was at 5 seconds.  I changed that to 30
seconds too and the period of above loop increased to ca. 30 seconds
(127 times div = 8, 8 times div = 128).

So the BIOS seems innocent.
-- 
Stefan Richter
-=====-=-=== =--- -=-==
http://arcgraph.de/sr/

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

* Re: 2.6.23-rc1 regression: hwmon/w83627ehf: wrong fan speed
  2007-08-11 15:41           ` Stefan Richter
@ 2007-08-11 15:48             ` Stefan Richter
  2007-08-12  9:21               ` Jean Delvare
  2007-08-12  9:17             ` Jean Delvare
  1 sibling, 1 reply; 20+ messages in thread
From: Stefan Richter @ 2007-08-11 15:48 UTC (permalink / raw)
  To: Jean Delvare; +Cc: Mark M. Hoffman, lm-sensors, linux-kernel

> Jean Delvare wrote:
>> So my theory is that something else (BIOS, ACPI?)

(ksensors?)

>> is changing the bank,
>> probably to read temperature values which are in banks 1 and 2, causing
>> the w83627ehf to get a wrong value for the VBAT register. If I am
>> right, then the attached patch should help. Please give it a try and
>> report.

This patch fixes the issue.  (I didn't reboot, only reloaded the drivers
after patching.)

Thanks for your help and advice.
-- 
Stefan Richter
-=====-=-=== =--- -=-==
http://arcgraph.de/sr/

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

* Re: 2.6.23-rc1 regression: hwmon/w83627ehf: wrong fan speed
  2007-08-11 15:41           ` Stefan Richter
  2007-08-11 15:48             ` Stefan Richter
@ 2007-08-12  9:17             ` Jean Delvare
  2007-08-12 10:34               ` Stefan Richter
  1 sibling, 1 reply; 20+ messages in thread
From: Jean Delvare @ 2007-08-12  9:17 UTC (permalink / raw)
  To: Stefan Richter; +Cc: Mark M. Hoffman, lm-sensors, linux-kernel

Hi Stefan,

On Sat, 11 Aug 2007 17:41:39 +0200, Stefan Richter wrote:
> While I test-booted 2.6.22(-rc) yesterday I had a look into the BIOS
> setup.  There is a fan speed control based on a temperature threshold,
> separately for CPU fan and case fan.  The thresholds are currently 55°C
> and 50°C respectively.

Hopefully it is programming the W83627EHG at boot time and no longer
touching it after that. You can check
in /sys/class/hwmon/hwmon*/device, some of the pwm*_enable files should
have value >= 2.

> During the time I spent in the BIOS setup, the CPU fan speed was
> displayed as something more than 1400, and the case fan speed was
> displayed as 0.  The latter is AFAIK typical with slow fans.

Correct.

> (...)
> I now updated to Gentoo's ksensors-0.7.3-r1 (which is v0.7.3 plus
> patches from Debian) and lm_sensors-2.10.4, added
> 
>     ignore fan5
>     set fan1_min    200
>     set fan2_min    1000
>     set fan3_min    0
> 
> to sensors.conf, compiled the drivers with CONFIG_HWMON_DEBUG_CHIP=y,
> and "sensors" alone seems to behave fine now.  Or maybe it did so
> already before that.  But as soon as I start "ksensors", "sensors" shows
> that the CPU fan divider suddenly changed from 8 to 128.  "sensors -s"
> will then cause the kernel to log
>     w83627ehf w83627ehf.656: fan2 clock divider changed from 128 to 8
>     w83627ehf w83627ehf.656: fan3 low limit and alarm disabled
> and sensors will show the correct CPU fan speed again --- but soon after
> that the divider will go up to 128 again if ksensors is running in parallel.
> 
> If I quit ksensors and run "sensors -s", sensors will continue to show
> correct speeds.  Actually with ksensors running, "while sensors | grep
> 'CPU Fan'; do sleep .2; done" shows that the CPU fan divider oscillates
> between 8 and 128 in ca. 5 seconds long periods:  16 times in a row it
> prints div = 8, and 8 times it prints div = 128, then div = 8 again, and
> so forth.  There are no dmesg messages during all that.
> 
> ksensors has different update interval settings, and although I had the
> w83627ehf readings configured to be updated every 30 seconds, some
> seemingly unrelated setting was at 5 seconds.  I changed that to 30
> seconds too and the period of above loop increased to ca. 30 seconds
> (127 times div = 8, 8 times div = 128).

Hmm. Is this "seemingly unrelated setting" the "Mainboard sensors"
panel? Maybe I get what's going on then. This panel gets the CPU
temperature from the ACPI "thermal" driver (/proc/acpi/thermal). Your report
suggests that the temperature value is taken from the W83627EHF's
temp2, which is in bank 1. If I am correct, then simply reading
from /proc/acpi/thermal/*/temperature would break the fan divider (when
my fixup patch isn't applied, that is.)

I recommend that you don't load the ACPI "thermal" driver together with
the w83627ehf driver, it won't give you any additional information and
the race condition that exists between both drivers can still result in
wrong values being reported from times to times (even with my patch.)

> So the BIOS seems innocent.

Good news.

One remaining mystery is why you did not observe the problem with the
older kernel. Maybe the ACPI thermal driver was not loaded (or not
working) back then?

-- 
Jean Delvare

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

* Re: 2.6.23-rc1 regression: hwmon/w83627ehf: wrong fan speed
  2007-08-11 15:48             ` Stefan Richter
@ 2007-08-12  9:21               ` Jean Delvare
  0 siblings, 0 replies; 20+ messages in thread
From: Jean Delvare @ 2007-08-12  9:21 UTC (permalink / raw)
  To: Stefan Richter; +Cc: Mark M. Hoffman, lm-sensors, linux-kernel

On Sat, 11 Aug 2007 17:48:41 +0200, Stefan Richter wrote:
> Jean Delvare wrote:
>> So my theory is that something else (BIOS, ACPI?)
> 
> (ksensors?)

ACPI if my guess is correct.

>> is changing the bank,
>> probably to read temperature values which are in banks 1 and 2, causing
>> the w83627ehf to get a wrong value for the VBAT register. If I am
>> right, then the attached patch should help. Please give it a try and
>> report.
>
> This patch fixes the issue.  (I didn't reboot, only reloaded the drivers
> after patching.)
> 
> Thanks for your help and advice.

Great, thanks for testing and reporting. Mark, please add my patch to
your hwmon-testing queue, and send it to Linus in the next batch of
fixes.

If no problem is reported, we can get rid of w83627ehf_reset_bank() in
a later kernel version.

-- 
Jean Delvare

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

* Re: 2.6.23-rc1 regression: hwmon/w83627ehf: wrong fan speed
  2007-08-12  9:17             ` Jean Delvare
@ 2007-08-12 10:34               ` Stefan Richter
  2007-08-12 10:43                 ` Stefan Richter
  0 siblings, 1 reply; 20+ messages in thread
From: Stefan Richter @ 2007-08-12 10:34 UTC (permalink / raw)
  To: Jean Delvare; +Cc: Mark M. Hoffman, lm-sensors, linux-kernel

Jean Delvare wrote:
> On Sat, 11 Aug 2007 17:41:39 +0200, Stefan Richter wrote:
>> While I test-booted 2.6.22(-rc) yesterday I had a look into the BIOS
>> setup.  There is a fan speed control based on a temperature threshold,
>> separately for CPU fan and case fan.  The thresholds are currently 55°C
>> and 50°C respectively.
> 
> Hopefully it is programming the W83627EHG at boot time and no longer
> touching it after that. You can check
> in /sys/class/hwmon/hwmon*/device, some of the pwm*_enable files should
> have value >= 2.

$ cat /sys/class/hwmon/hwmon*/device/{name,pwm*_enable}
w83627ehf
coretemp
coretemp
2
2
1

[...]
>> ksensors has different update interval settings, and although I had the
>> w83627ehf readings configured to be updated every 30 seconds, some
>> seemingly unrelated setting was at 5 seconds.  I changed that to 30
>> seconds too and the period of above loop increased to ca. 30 seconds
>> (127 times div = 8, 8 times div = 128).
> 
> Hmm. Is this "seemingly unrelated setting" the "Mainboard sensors"
> panel?

Yes.

> Maybe I get what's going on then. This panel gets the CPU
> temperature from the ACPI "thermal" driver (/proc/acpi/thermal). Your report
> suggests that the temperature value is taken from the W83627EHF's
> temp2, which is in bank 1. If I am correct, then simply reading
> from /proc/acpi/thermal/*/temperature would break the fan divider (when
> my fixup patch isn't applied, that is.)
> 
> I recommend that you don't load the ACPI "thermal" driver together with
> the w83627ehf driver, it won't give you any additional information and
> the race condition that exists between both drivers can still result in
> wrong values being reported from times to times (even with my patch.)

I reverted your patch, reloaded the modules, unloaded thermal, checked
readings with "sensors" (OK), started ksensors:  1.) Its "Mainboard
sensors" config panel is gone.  2.) The problem of the fan divider
oscillating between 8 and 128 is gone.  Fan speeds are now correct all
the time.

>> So the BIOS seems innocent.
> 
> Good news.
> 
> One remaining mystery is why you did not observe the problem with the
> older kernel. Maybe the ACPI thermal driver was not loaded (or not
> working) back then?

I found two differences:  Gentoo's udev init script autoloads thermal on
boot under 2.6.23-rc2 but it doesn't under 2.6.22(-rc5), for a reason
unknown to me.  If I manually load thermal under 2.6.22(-rc5), the
oscillation happens too --- also in the rythm of ksensors' update
interval but with a slightly different partition into div = 8 periods
and div = 128 periods.  Ksensors then happens to display the correct div
= 8 reading, while sensors shows either the wrong or the correct reading
depending on the moment it is issued.  By playing with the update
intervals in ksensors, ksensors can be tricked into getting the correct
value under 2.6.23-rc2 too.

So, it isn't really a regression from 2.6.22 to 2.6.23-rc1.  It merely
became more apparent after 2.6.23-rc1.

And yes, "cat /proc/acpi/thermal_zone/THRM/temperature" causes the CPU
fan divider to flip to 128 immediately.
-- 
Stefan Richter
-=====-=-=== =--- -==--
http://arcgraph.de/sr/

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

* Re: 2.6.23-rc1 regression: hwmon/w83627ehf: wrong fan speed
  2007-08-12 10:34               ` Stefan Richter
@ 2007-08-12 10:43                 ` Stefan Richter
  0 siblings, 0 replies; 20+ messages in thread
From: Stefan Richter @ 2007-08-12 10:43 UTC (permalink / raw)
  To: Jean Delvare; +Cc: Mark M. Hoffman, lm-sensors, linux-kernel

I wrote:
> Jean Delvare wrote:
>> One remaining mystery is why you did not observe the problem with the
>> older kernel. Maybe the ACPI thermal driver was not loaded (or not
>> working) back then?
> 
> I found two differences:  Gentoo's udev init script autoloads thermal on
> boot under 2.6.23-rc2 but it doesn't under 2.6.22(-rc5), for a reason
> unknown to me.  If I manually load thermal under 2.6.22(-rc5),
[...]

The reason are obviously some post 2.6.22/ pre 2.6.23-rc1 commits which
add module aliases to ACPI drivers.
-- 
Stefan Richter
-=====-=-=== =--- -==--
http://arcgraph.de/sr/

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

* Re: 2.6.23-rc1 regression: hwmon/w83627ehf: wrong fan speed
  2007-08-05 16:21 ` Mark M. Hoffman
  2007-08-05 17:13   ` Stefan Richter
@ 2007-08-12 16:06   ` Jean Delvare
  1 sibling, 0 replies; 20+ messages in thread
From: Jean Delvare @ 2007-08-12 16:06 UTC (permalink / raw)
  To: Mark M. Hoffman; +Cc: Stefan Richter, lm-sensors, linux-kernel

Hi Mark,

On Sun, 5 Aug 2007 12:21:41 -0400, Mark M. Hoffman wrote:
> Author: Mark M. Hoffman <mhoffman@lightlink.com>
> Date:   Sun Aug 5 12:19:01 2007 -0400
> 
>     hwmon: read fan_div values during probe
>     
>     This patch forces the driver to read the fan divider values during early init.
>     Otherwise, a call to store_fan_min() could access uninitialized variables.
>     
>     Signed-off-by: Mark M. Hoffman <mhoffman@lightlink.com>

Good catch.

Signed-off-by: Jean Delvare <khali@linux-fr.org>

> 
> diff --git a/drivers/hwmon/w83627ehf.c b/drivers/hwmon/w83627ehf.c
> index c51ae2e..bca7fbc 100644
> --- a/drivers/hwmon/w83627ehf.c
> +++ b/drivers/hwmon/w83627ehf.c
> @@ -421,6 +421,31 @@ static void w83627ehf_write_fan_div(struct w83627ehf_data *data, int nr)
>  	}
>  }
>  
> +static void w83627ehf_update_fan_div(struct w83627ehf_data *data)
> +{
> +	int i;
> +
> +	i = w83627ehf_read_value(data, W83627EHF_REG_FANDIV1);
> +	data->fan_div[0] = (i >> 4) & 0x03;
> +	data->fan_div[1] = (i >> 6) & 0x03;
> +	i = w83627ehf_read_value(data, W83627EHF_REG_FANDIV2);
> +	data->fan_div[2] = (i >> 6) & 0x03;
> +	i = w83627ehf_read_value(data, W83627EHF_REG_VBAT);
> +	data->fan_div[0] |= (i >> 3) & 0x04;
> +	data->fan_div[1] |= (i >> 4) & 0x04;
> +	data->fan_div[2] |= (i >> 5) & 0x04;
> +	if (data->has_fan & ((1 << 3) | (1 << 4))) {
> +		i = w83627ehf_read_value(data, W83627EHF_REG_DIODE);
> +		data->fan_div[3] = i & 0x03;
> +		data->fan_div[4] = ((i >> 2) & 0x03)
> +				 | ((i >> 5) & 0x04);
> +	}
> +	if (data->has_fan & (1 << 3)) {
> +		i = w83627ehf_read_value(data, W83627EHF_REG_SMI_OVT);
> +		data->fan_div[3] |= (i >> 5) & 0x04;
> +	}
> +}
> +
>  static struct w83627ehf_data *w83627ehf_update_device(struct device *dev)
>  {
>  	struct w83627ehf_data *data = dev_get_drvdata(dev);
> @@ -432,25 +457,7 @@ static struct w83627ehf_data *w83627ehf_update_device(struct device *dev)
>  	if (time_after(jiffies, data->last_updated + HZ + HZ/2)
>  	 || !data->valid) {
>  		/* Fan clock dividers */
> -		i = w83627ehf_read_value(data, W83627EHF_REG_FANDIV1);
> -		data->fan_div[0] = (i >> 4) & 0x03;
> -		data->fan_div[1] = (i >> 6) & 0x03;
> -		i = w83627ehf_read_value(data, W83627EHF_REG_FANDIV2);
> -		data->fan_div[2] = (i >> 6) & 0x03;
> -		i = w83627ehf_read_value(data, W83627EHF_REG_VBAT);
> -		data->fan_div[0] |= (i >> 3) & 0x04;
> -		data->fan_div[1] |= (i >> 4) & 0x04;
> -		data->fan_div[2] |= (i >> 5) & 0x04;
> -		if (data->has_fan & ((1 << 3) | (1 << 4))) {
> -			i = w83627ehf_read_value(data, W83627EHF_REG_DIODE);
> -			data->fan_div[3] = i & 0x03;
> -			data->fan_div[4] = ((i >> 2) & 0x03)
> -					 | ((i >> 5) & 0x04);
> -		}
> -		if (data->has_fan & (1 << 3)) {
> -			i = w83627ehf_read_value(data, W83627EHF_REG_SMI_OVT);
> -			data->fan_div[3] |= (i >> 5) & 0x04;
> -		}
> +		w83627ehf_update_fan_div(data);
>  
>  		/* Measured voltages and limits */
>  		for (i = 0; i < data->in_num; i++) {
> @@ -1312,6 +1319,9 @@ static int __devinit w83627ehf_probe(struct platform_device *pdev)
>  	if (!(i & (1 << 1)) && (!fan5pin))
>  		data->has_fan |= (1 << 4);
>  
> +	/* Read fan clock dividers immediately */
> +	w83627ehf_update_fan_div(data);
> +
>  	/* Register sysfs hooks */
>    	for (i = 0; i < ARRAY_SIZE(sda_sf3_arrays); i++)
>  		if ((err = device_create_file(dev,


-- 
Jean Delvare

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

* Re: 2.6.23-rc1 regression: hwmon/w83627ehf: wrong fan speed
  2007-08-11 11:57         ` Jean Delvare
  2007-08-11 15:41           ` Stefan Richter
@ 2007-08-12 17:49           ` Mark M. Hoffman
  1 sibling, 0 replies; 20+ messages in thread
From: Mark M. Hoffman @ 2007-08-12 17:49 UTC (permalink / raw)
  To: Jean Delvare; +Cc: Stefan Richter, lm-sensors, linux-kernel

Hi Jean:

* Jean Delvare <khali@linux-fr.org> [2007-08-11 13:57:05 +0200]:
> Don't assume that the default bank is 0. For one thing, we don't even
> set it to 0 when the driver is loaded, so the initial state might be
> different. For another, something (say, the BIOS) might access the chip
> and leave with the bank set to something different, so assuming that
> the bank value is 0 is not safe.
> 
> Signed-off-by: Jean Delvare <khali@linux-fr.org>
> ---
>  drivers/hwmon/w83627ehf.c |    8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> --- linux-2.6.23-rc2.orig/drivers/hwmon/w83627ehf.c	2007-07-23 16:44:35.000000000 +0200
> +++ linux-2.6.23-rc2/drivers/hwmon/w83627ehf.c	2007-08-11 12:55:58.000000000 +0200
> @@ -309,18 +309,16 @@ static inline int is_word_sized(u16 reg)
>  	      || (reg & 0x00ff) == 0x55));
>  }
>  
> -/* We assume that the default bank is 0, thus the following two functions do
> -   nothing for registers which live in bank 0. For others, they respectively
> -   set the bank register to the correct value (before the register is
> -   accessed), and back to 0 (afterwards). */
> +/* Registers 0x50-0x5f are banked */
>  static inline void w83627ehf_set_bank(struct w83627ehf_data *data, u16 reg)
>  {
> -	if (reg & 0xff00) {
> +	if ((reg & 0x00f0) == 0x50) {
>  		outb_p(W83627EHF_REG_BANK, data->addr + ADDR_REG_OFFSET);
>  		outb_p(reg >> 8, data->addr + DATA_REG_OFFSET);
>  	}
>  }
>  
> +/* Not strictly necessary, but play it safe for now */
>  static inline void w83627ehf_reset_bank(struct w83627ehf_data *data, u16 reg)
>  {
>  	if (reg & 0xff00) {

Applied to hwmon-2.6.git/testing, thanks.

-- 
Mark M. Hoffman
mhoffman@lightlink.com


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

end of thread, other threads:[~2007-08-12 17:53 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-08-05 11:20 2.6.23-rc1 regression: hwmon/w83627ehf: wrong fan speed Stefan Richter
2007-08-05 11:28 ` Stefan Richter
2007-08-05 13:55 ` Mark M. Hoffman
2007-08-05 14:51   ` Stefan Richter
2007-08-05 15:17     ` Mark M. Hoffman
2007-08-05 16:21 ` Mark M. Hoffman
2007-08-05 17:13   ` Stefan Richter
2007-08-10 21:08     ` Jean Delvare
2007-08-10 22:29       ` Stefan Richter
2007-08-10 22:39         ` Stefan Richter
2007-08-11  2:30           ` [lm-sensors] " David Hubbard
2007-08-11 11:57         ` Jean Delvare
2007-08-11 15:41           ` Stefan Richter
2007-08-11 15:48             ` Stefan Richter
2007-08-12  9:21               ` Jean Delvare
2007-08-12  9:17             ` Jean Delvare
2007-08-12 10:34               ` Stefan Richter
2007-08-12 10:43                 ` Stefan Richter
2007-08-12 17:49           ` Mark M. Hoffman
2007-08-12 16:06   ` Jean Delvare

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).