Jean Delvare wrote:
Hi Brian,
On Fri, 29 Nov 2013 23:57:49 -0800, Brian Carnes wrote:
[snip]
Indeed, Richard (Cc'd) noticed some problems with fan speed control
back then, but he did not provide enough details for investigation. The
bug you found could well explain these problems, and Richard would
probably be interested in testing your fix.
Patch addressing just problem #1:
--- a/w83l786ng.c
+++ b/w83l786ng.c
@@ -523,7 +523,7 @@ store_pwm_enable(struct device *dev, struct
device_attribute *attr,
mutex_lock(&data->update_lock);
reg = w83l786ng_read_value(client, W83L786NG_REG_FAN_CFG);
data->pwm_enable[nr] = val;
- reg &= ~(0x02 << W83L786NG_PWM_ENABLE_SHIFT[nr]);
+ reg &= ~(0x03 << W83L786NG_PWM_ENABLE_SHIFT[nr]);
reg |= (val - 1) << W83L786NG_PWM_ENABLE_SHIFT[nr];
w83l786ng_write_value(client, W83L786NG_REG_FAN_CFG, reg);
mutex_unlock(&data->update_lock);
@@ -790,7 +790,7 @@ static struct w83l786ng_data
*w83l786ng_update_device(struct device *dev)
((pwmcfg >> W83L786NG_PWM_MODE_SHIFT[i]) & 1)
? 0 : 1;
data->pwm_enable[i] =
- ((pwmcfg >> W83L786NG_PWM_ENABLE_SHIFT[i]) & 2)
+ 1;
+ ((pwmcfg >> W83L786NG_PWM_ENABLE_SHIFT[i]) & 3)
+ 1;
data->pwm[i] = w83l786ng_read_value(client,
W83L786NG_REG_PWM[i]);
}
-----PATCH END-----
In short:
chunk 1 used to clear just bit 2 (so no modes 3 or 4), then always OR in
bit 1.
Once set, no going back! Now we clear both bits, and then OR in the mode.
chunk 2 used to always request bit 2 (which this driver always set to zero),
and add one. Thus you always appeared to be in "mode 1", unless the
hardware
was in mode 3 or 4 by means other than the driver, then you would
appear to be in mode 3...
Now the driver will correctly report the mode (1, 2, 3, or 4).
Yes, I came up to the same conclusion, your patch is correct. It should
be backported to the stable kernel series as well. However formatting
was destroyed by your e-mail client. Please resend with long line
wrapping disabled (or as an attachment if you can't manage to do that.)
You also must add your Signed-off-by line to it as explained in
Documentation/SubmittingPatches section 12.
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches#n307
First off, I'm sorry that I don't have any hardware with a w83l786ng chip.
Second, your patch looks correct to me after reading the datasheet and
the code. Thanks for fixing it.
Transcript after loading patched module:
$ sudo rmmod w83l786ng
$ sudo insmod ./w83l786ng.ko
$ cat pwm1_enable pwm2_enable
1
1
$ i2cget -f -y 0 0x2e 0x80
0x00
$ echo 1 > pwm1_enable
$ cat pwm1_enable pwm2_enable
1
1
$ i2cget -f -y 0 0x2e 0x80
0x00
$ echo 2 > pwm1_enable
$ cat pwm1_enable pwm2_enable
2
1
$ i2cget -f -y 0 0x2e 0x80
0x04
$ echo 1 > pwm1_enable
$ cat pwm1_enable pwm2_enable
1
1
$ i2cget -f -y 0 0x2e 0x80
0x00
Which is the expected behaviour.
Indeed. Thanks for finding and fixing this bug. Please resend your
patch as explained above and I'll be happy to apply it, forward it to
the stable kernel maintainers, and make the updated driver available in
a stand-alone flavor.
Kevin
_______________________________________________
lm-sensors mailing list
lm-sensors@xxxxxxxxxxxxxx
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
