From 6229cdb23648d0c2875b3fb102cdaf4bf08fcfa4 Mon Sep 17 00:00:00 2001 From: Jean Delvare Date: Wed, 8 Dec 2010 16:27:22 +0100 Subject: [PATCH 1/3] hwmon: (it87) Fix manual fan speed control on IT8721F The manual fan speed control logic of the IT8721F is much different from what older devices had. Update the code to properly support that. Signed-off-by: Jean Delvare Acked-by: Guenter Roeck --- drivers/hwmon/it87.c | 61 ++++++++++++++++++++++++++++++++------------ 1 file changed, 45 insertions(+), 16 deletions(-) diff --git a/drivers/hwmon/it87.c b/drivers/hwmon/it87.c index 14a5d981be7d..a428a9264195 100644 --- a/drivers/hwmon/it87.c +++ b/drivers/hwmon/it87.c @@ -187,6 +187,7 @@ static const u8 IT87_REG_FANX_MIN[] = { 0x1b, 0x1c, 0x1d, 0x85, 0x87 }; #define IT87_REG_FAN_MAIN_CTRL 0x13 #define IT87_REG_FAN_CTL 0x14 #define IT87_REG_PWM(nr) (0x15 + (nr)) +#define IT87_REG_PWM_DUTY(nr) (0x63 + (nr) * 8) #define IT87_REG_VIN(nr) (0x20 + (nr)) #define IT87_REG_TEMP(nr) (0x29 + (nr)) @@ -251,12 +252,16 @@ struct it87_data { u8 fan_main_ctrl; /* Register value */ u8 fan_ctl; /* Register value */ - /* The following 3 arrays correspond to the same registers. The - * meaning of bits 6-0 depends on the value of bit 7, and we want - * to preserve settings on mode changes, so we have to track all - * values separately. */ + /* The following 3 arrays correspond to the same registers up to + * the IT8720F. The meaning of bits 6-0 depends on the value of bit + * 7, and we want to preserve settings on mode changes, so we have + * to track all values separately. + * Starting with the IT8721F, the manual PWM duty cycles are stored + * in separate registers (8-bit values), so the separate tracking + * is no longer needed, but it is still done to keep the driver + * simple. */ u8 pwm_ctrl[3]; /* Register value */ - u8 pwm_duty[3]; /* Manual PWM value set by user (bit 6-0) */ + u8 pwm_duty[3]; /* Manual PWM value set by user */ u8 pwm_temp_map[3]; /* PWM to temp. chan. mapping (bits 1-0) */ /* Automatic fan speed control registers */ @@ -832,7 +837,9 @@ static ssize_t set_pwm_enable(struct device *dev, data->fan_main_ctrl); } else { if (val == 1) /* Manual mode */ - data->pwm_ctrl[nr] = data->pwm_duty[nr]; + data->pwm_ctrl[nr] = data->type == it8721 ? + data->pwm_temp_map[nr] : + data->pwm_duty[nr]; else /* Automatic mode */ data->pwm_ctrl[nr] = 0x80 | data->pwm_temp_map[nr]; it87_write_value(data, IT87_REG_PWM(nr), data->pwm_ctrl[nr]); @@ -858,12 +865,25 @@ static ssize_t set_pwm(struct device *dev, struct device_attribute *attr, return -EINVAL; mutex_lock(&data->update_lock); - data->pwm_duty[nr] = pwm_to_reg(data, val); - /* If we are in manual mode, write the duty cycle immediately; - * otherwise, just store it for later use. */ - if (!(data->pwm_ctrl[nr] & 0x80)) { - data->pwm_ctrl[nr] = data->pwm_duty[nr]; - it87_write_value(data, IT87_REG_PWM(nr), data->pwm_ctrl[nr]); + if (data->type == it8721) { + /* If we are in automatic mode, the PWM duty cycle register + * is read-only so we can't write the value */ + if (data->pwm_ctrl[nr] & 0x80) { + mutex_unlock(&data->update_lock); + return -EBUSY; + } + data->pwm_duty[nr] = pwm_to_reg(data, val); + it87_write_value(data, IT87_REG_PWM_DUTY(nr), + data->pwm_duty[nr]); + } else { + data->pwm_duty[nr] = pwm_to_reg(data, val); + /* If we are in manual mode, write the duty cycle immediately; + * otherwise, just store it for later use. */ + if (!(data->pwm_ctrl[nr] & 0x80)) { + data->pwm_ctrl[nr] = data->pwm_duty[nr]; + it87_write_value(data, IT87_REG_PWM(nr), + data->pwm_ctrl[nr]); + } } mutex_unlock(&data->update_lock); return count; @@ -1958,7 +1978,10 @@ static void __devinit it87_init_device(struct platform_device *pdev) * channels to use when later setting to automatic mode later. * Use a 1:1 mapping by default (we are clueless.) * In both cases, the value can (and should) be changed by the user - * prior to switching to a different mode. */ + * prior to switching to a different mode. + * Note that this is no longer needed for the IT8721F and later, as + * these have separate registers for the temperature mapping and the + * manual duty cycle. */ for (i = 0; i < 3; i++) { data->pwm_temp_map[i] = i; data->pwm_duty[i] = 0x7f; /* Full speed */ @@ -2034,10 +2057,16 @@ static void __devinit it87_init_device(struct platform_device *pdev) static void it87_update_pwm_ctrl(struct it87_data *data, int nr) { data->pwm_ctrl[nr] = it87_read_value(data, IT87_REG_PWM(nr)); - if (data->pwm_ctrl[nr] & 0x80) /* Automatic mode */ + if (data->type == it8721) { data->pwm_temp_map[nr] = data->pwm_ctrl[nr] & 0x03; - else /* Manual mode */ - data->pwm_duty[nr] = data->pwm_ctrl[nr] & 0x7f; + data->pwm_duty[nr] = it87_read_value(data, + IT87_REG_PWM_DUTY(nr)); + } else { + if (data->pwm_ctrl[nr] & 0x80) /* Automatic mode */ + data->pwm_temp_map[nr] = data->pwm_ctrl[nr] & 0x03; + else /* Manual mode */ + data->pwm_duty[nr] = data->pwm_ctrl[nr] & 0x7f; + } if (has_old_autopwm(data)) { int i; From 52bc9802ce849d0d287cc5fe76d06b0daa3986ca Mon Sep 17 00:00:00 2001 From: Gabriele Gorla Date: Wed, 8 Dec 2010 16:27:22 +0100 Subject: [PATCH 2/3] hwmon: (adm1026) Fix setting fan_div Prevent setting fan_div from stomping on other fans that share the same I2C register. Signed-off-by: Gabriele Gorla Cc: stable@kernel.org Signed-off-by: Jean Delvare --- drivers/hwmon/adm1026.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/drivers/hwmon/adm1026.c b/drivers/hwmon/adm1026.c index 4bf969c0a32b..6f257ce1ae07 100644 --- a/drivers/hwmon/adm1026.c +++ b/drivers/hwmon/adm1026.c @@ -916,7 +916,7 @@ static ssize_t set_fan_div(struct device *dev, struct device_attribute *attr, int nr = sensor_attr->index; struct i2c_client *client = to_i2c_client(dev); struct adm1026_data *data = i2c_get_clientdata(client); - int val, orig_div, new_div, shift; + int val, orig_div, new_div; val = simple_strtol(buf, NULL, 10); new_div = DIV_TO_REG(val); @@ -928,15 +928,17 @@ static ssize_t set_fan_div(struct device *dev, struct device_attribute *attr, data->fan_div[nr] = DIV_FROM_REG(new_div); if (nr < 4) { /* 0 <= nr < 4 */ - shift = 2 * nr; adm1026_write_value(client, ADM1026_REG_FAN_DIV_0_3, - ((DIV_TO_REG(orig_div) & (~(0x03 << shift))) | - (new_div << shift))); + (DIV_TO_REG(data->fan_div[0]) << 0) | + (DIV_TO_REG(data->fan_div[1]) << 2) | + (DIV_TO_REG(data->fan_div[2]) << 4) | + (DIV_TO_REG(data->fan_div[3]) << 6)); } else { /* 3 < nr < 8 */ - shift = 2 * (nr - 4); adm1026_write_value(client, ADM1026_REG_FAN_DIV_4_7, - ((DIV_TO_REG(orig_div) & (~(0x03 << (2 * shift)))) | - (new_div << shift))); + (DIV_TO_REG(data->fan_div[4]) << 0) | + (DIV_TO_REG(data->fan_div[5]) << 2) | + (DIV_TO_REG(data->fan_div[6]) << 4) | + (DIV_TO_REG(data->fan_div[7]) << 6)); } if (data->fan_div[nr] != orig_div) { From 8b0f1840a46449e1946fc88860ef3ec8d6b1c2c7 Mon Sep 17 00:00:00 2001 From: Gabriele Gorla Date: Wed, 8 Dec 2010 16:27:22 +0100 Subject: [PATCH 3/3] hwmon: (adm1026) Allow 1 as a valid divider value Allow 1 as a valid div value as specified in the ADM1026 datasheet. Signed-off-by: Gabriele Gorla Cc: stable@kernel.org Signed-off-by: Jean Delvare --- drivers/hwmon/adm1026.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/hwmon/adm1026.c b/drivers/hwmon/adm1026.c index 6f257ce1ae07..be0fdd58aa29 100644 --- a/drivers/hwmon/adm1026.c +++ b/drivers/hwmon/adm1026.c @@ -920,9 +920,7 @@ static ssize_t set_fan_div(struct device *dev, struct device_attribute *attr, val = simple_strtol(buf, NULL, 10); new_div = DIV_TO_REG(val); - if (new_div == 0) { - return -EINVAL; - } + mutex_lock(&data->update_lock); orig_div = data->fan_div[nr]; data->fan_div[nr] = DIV_FROM_REG(new_div);