Re: 2.6.23-rc1 regression: hwmon/w83627ehf: wrong fan speed

Jean Delvare
2007-08-12T16:06:01Z

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