From 09e87e5c4f9af656af2a8a3afc03487c5d9287c3 Mon Sep 17 00:00:00 2001 From: Nikolaus Schulz Date: Wed, 8 Feb 2012 18:56:08 +0100 Subject: hwmon: (f75375s) Fix automatic pwm mode setting for F75373 & F75375 In order to enable temperature mode aka automatic mode for the F75373 and F75375 chips, the two FANx_MODE bits in the fan configuration register need be set to 01, not 10. Signed-off-by: Nikolaus Schulz Cc: stable@kernel.org # 2.6.32+ Signed-off-by: Guenter Roeck --- drivers/hwmon/f75375s.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/hwmon/f75375s.c') diff --git a/drivers/hwmon/f75375s.c b/drivers/hwmon/f75375s.c index eedf574ab539..c81ad45d9fa9 100644 --- a/drivers/hwmon/f75375s.c +++ b/drivers/hwmon/f75375s.c @@ -369,7 +369,7 @@ static int set_pwm_enable_direct(struct i2c_client *client, int nr, int val) fanmode |= (3 << FAN_CTRL_MODE(nr)); break; case 2: /* AUTOMATIC*/ - fanmode |= (2 << FAN_CTRL_MODE(nr)); + fanmode |= (1 << FAN_CTRL_MODE(nr)); break; case 3: /* fan speed */ break; -- cgit v1.2.3 From a367a1e08b21e995c7112ff32b5efc97d104d405 Mon Sep 17 00:00:00 2001 From: Nikolaus Schulz Date: Wed, 8 Feb 2012 18:56:09 +0100 Subject: hwmon: (f75375s) Fix reading of wrong register when initializing the F75387 Unlike the other chips supported by this driver, the F75387 stores the pwm_mode in register F75375_REG_FAN_TIMER, not F75375_REG_CONFIG1. Signed-off-by: Nikolaus Schulz Signed-off-by: Guenter Roeck --- drivers/hwmon/f75375s.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/hwmon/f75375s.c') diff --git a/drivers/hwmon/f75375s.c b/drivers/hwmon/f75375s.c index c81ad45d9fa9..b3eef4474a98 100644 --- a/drivers/hwmon/f75375s.c +++ b/drivers/hwmon/f75375s.c @@ -723,7 +723,7 @@ static void f75375_init(struct i2c_client *client, struct f75375_data *data, if (data->kind == f75387) { bool manu, duty; - if (!(conf & (1 << F75387_FAN_CTRL_LINEAR(nr)))) + if (!(mode & (1 << F75387_FAN_CTRL_LINEAR(nr)))) data->pwm_mode[nr] = 1; manu = ((mode >> F75387_FAN_MANU_MODE(nr)) & 1); -- cgit v1.2.3 From eb2f255b2d360df3f500042a2258dcf2fcbe89a2 Mon Sep 17 00:00:00 2001 From: Nikolaus Schulz Date: Wed, 8 Feb 2012 18:56:10 +0100 Subject: hwmon: (f75375s) Fix bit shifting in f75375_write16 In order to extract the high byte of the 16-bit word, shift the word to the right, not to the left. Signed-off-by: Nikolaus Schulz Cc: stable@kernel.org # 2.6.32+ Signed-off-by: Guenter Roeck --- drivers/hwmon/f75375s.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/hwmon/f75375s.c') diff --git a/drivers/hwmon/f75375s.c b/drivers/hwmon/f75375s.c index b3eef4474a98..0aabaaf359ac 100644 --- a/drivers/hwmon/f75375s.c +++ b/drivers/hwmon/f75375s.c @@ -172,7 +172,7 @@ static inline void f75375_write8(struct i2c_client *client, u8 reg, static inline void f75375_write16(struct i2c_client *client, u8 reg, u16 value) { - int err = i2c_smbus_write_byte_data(client, reg, (value << 8)); + int err = i2c_smbus_write_byte_data(client, reg, (value >> 8)); if (err) return; i2c_smbus_write_byte_data(client, reg + 1, (value & 0xFF)); -- cgit v1.2.3 From a1c1baf00e5c08dda9c0ee25658bf0ce301102ae Mon Sep 17 00:00:00 2001 From: Nikolaus Schulz Date: Wed, 8 Feb 2012 18:56:11 +0100 Subject: hwmon: (f75375s) Let f75375_update_device treat pwmX as a measured value Treat pwmX as a measured value, not as a (mostly static) limit value, so that it is updated more frequently from the device register. Signed-off-by: Nikolaus Schulz Signed-off-by: Guenter Roeck --- drivers/hwmon/f75375s.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'drivers/hwmon/f75375s.c') diff --git a/drivers/hwmon/f75375s.c b/drivers/hwmon/f75375s.c index 0aabaaf359ac..f609b5727ba9 100644 --- a/drivers/hwmon/f75375s.c +++ b/drivers/hwmon/f75375s.c @@ -200,9 +200,6 @@ static struct f75375_data *f75375_update_device(struct device *dev) f75375_read16(client, F75375_REG_FAN_MIN(nr)); data->fan_target[nr] = f75375_read16(client, F75375_REG_FAN_EXP(nr)); - data->pwm[nr] = f75375_read8(client, - F75375_REG_FAN_PWM_DUTY(nr)); - } for (nr = 0; nr < 4; nr++) { data->in_max[nr] = @@ -218,6 +215,8 @@ static struct f75375_data *f75375_update_device(struct device *dev) if (time_after(jiffies, data->last_updated + 2 * HZ) || !data->valid) { for (nr = 0; nr < 2; nr++) { + data->pwm[nr] = f75375_read8(client, + F75375_REG_FAN_PWM_DUTY(nr)); /* assign MSB, therefore shift it by 8 bits */ data->temp11[nr] = f75375_read8(client, F75375_REG_TEMP(nr)) << 8; -- cgit v1.2.3 From c1c1a3d012fe5e82a9a025fb4b5a4f8ee67a53f6 Mon Sep 17 00:00:00 2001 From: Nikolaus Schulz Date: Wed, 22 Feb 2012 23:18:44 +0100 Subject: hwmon: (f75375s) Fix register write order when setting fans to full speed By hwmon sysfs interface convention, setting pwm_enable to zero sets a fan to full speed. In the f75375s driver, this need be done by enabling manual fan control, plus duty mode for the F875387 chip, and then setting the maximum duty cycle. Fix a bug where the two necessary register writes were swapped, effectively discarding the setting to full-speed. Signed-off-by: Nikolaus Schulz Cc: Riku Voipio Signed-off-by: Guenter Roeck --- drivers/hwmon/f75375s.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'drivers/hwmon/f75375s.c') diff --git a/drivers/hwmon/f75375s.c b/drivers/hwmon/f75375s.c index f609b5727ba9..6bab2001ef3b 100644 --- a/drivers/hwmon/f75375s.c +++ b/drivers/hwmon/f75375s.c @@ -340,8 +340,6 @@ static int set_pwm_enable_direct(struct i2c_client *client, int nr, int val) fanmode |= (1 << F75387_FAN_MANU_MODE(nr)); fanmode |= (1 << F75387_FAN_DUTY_MODE(nr)); data->pwm[nr] = 255; - f75375_write8(client, F75375_REG_FAN_PWM_DUTY(nr), - data->pwm[nr]); break; case 1: /* PWM */ fanmode |= (1 << F75387_FAN_MANU_MODE(nr)); @@ -361,8 +359,6 @@ static int set_pwm_enable_direct(struct i2c_client *client, int nr, int val) case 0: /* full speed */ fanmode |= (3 << FAN_CTRL_MODE(nr)); data->pwm[nr] = 255; - f75375_write8(client, F75375_REG_FAN_PWM_DUTY(nr), - data->pwm[nr]); break; case 1: /* PWM */ fanmode |= (3 << FAN_CTRL_MODE(nr)); @@ -377,6 +373,9 @@ static int set_pwm_enable_direct(struct i2c_client *client, int nr, int val) f75375_write8(client, F75375_REG_FAN_TIMER, fanmode); data->pwm_enable[nr] = val; + if (val == 0) + f75375_write8(client, F75375_REG_FAN_PWM_DUTY(nr), + data->pwm[nr]); return 0; } -- cgit v1.2.3 From 331255d35d6f517020485aee38dbb8b8dfaa1642 Mon Sep 17 00:00:00 2001 From: Nikolaus Schulz Date: Tue, 28 Feb 2012 22:15:51 +0100 Subject: hwmon: (f75375s) Fix writes to the pwm* attribute for the F75387 For the F75387, the register holding the current PWM duty cycle value is r/o; changing it requires writing to the fan expect register instead. Signed-off-by: Nikolaus Schulz [guenter.roeck@ericsson.com: Simplified function parameters] Signed-off-by: Guenter Roeck --- drivers/hwmon/f75375s.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) (limited to 'drivers/hwmon/f75375s.c') diff --git a/drivers/hwmon/f75375s.c b/drivers/hwmon/f75375s.c index 6bab2001ef3b..61cc9c396a98 100644 --- a/drivers/hwmon/f75375s.c +++ b/drivers/hwmon/f75375s.c @@ -178,6 +178,16 @@ static inline void f75375_write16(struct i2c_client *client, u8 reg, i2c_smbus_write_byte_data(client, reg + 1, (value & 0xFF)); } +static void f75375_write_pwm(struct i2c_client *client, int nr) +{ + struct f75375_data *data = i2c_get_clientdata(client); + if (data->kind == f75387) + f75375_write16(client, F75375_REG_FAN_EXP(nr), data->pwm[nr]); + else + f75375_write8(client, F75375_REG_FAN_PWM_DUTY(nr), + data->pwm[nr]); +} + static struct f75375_data *f75375_update_device(struct device *dev) { struct i2c_client *client = to_i2c_client(dev); @@ -309,7 +319,7 @@ static ssize_t set_pwm(struct device *dev, struct device_attribute *attr, mutex_lock(&data->update_lock); data->pwm[nr] = SENSORS_LIMIT(val, 0, 255); - f75375_write8(client, F75375_REG_FAN_PWM_DUTY(nr), data->pwm[nr]); + f75375_write_pwm(client, nr); mutex_unlock(&data->update_lock); return count; } @@ -374,8 +384,7 @@ static int set_pwm_enable_direct(struct i2c_client *client, int nr, int val) f75375_write8(client, F75375_REG_FAN_TIMER, fanmode); data->pwm_enable[nr] = val; if (val == 0) - f75375_write8(client, F75375_REG_FAN_PWM_DUTY(nr), - data->pwm[nr]); + f75375_write_pwm(client, nr); return 0; } @@ -759,8 +768,7 @@ static void f75375_init(struct i2c_client *client, struct f75375_data *data, set_pwm_enable_direct(client, 1, f75375s_pdata->pwm_enable[1]); for (nr = 0; nr < 2; nr++) { data->pwm[nr] = SENSORS_LIMIT(f75375s_pdata->pwm[nr], 0, 255); - f75375_write8(client, F75375_REG_FAN_PWM_DUTY(nr), - data->pwm[nr]); + f75375_write_pwm(client, nr); } } -- cgit v1.2.3 From edeea102857e33b5e9b17a3a2640da390732a693 Mon Sep 17 00:00:00 2001 From: Nikolaus Schulz Date: Tue, 28 Feb 2012 22:15:52 +0100 Subject: hwmon: (f75375s) Make pwm*_mode writable for the F75387 Signed-off-by: Nikolaus Schulz Signed-off-by: Guenter Roeck --- drivers/hwmon/f75375s.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/hwmon/f75375s.c') diff --git a/drivers/hwmon/f75375s.c b/drivers/hwmon/f75375s.c index 61cc9c396a98..eb648d9c91d6 100644 --- a/drivers/hwmon/f75375s.c +++ b/drivers/hwmon/f75375s.c @@ -795,7 +795,7 @@ static int f75375_probe(struct i2c_client *client, if (err) goto exit_free; - if (data->kind == f75375) { + if (data->kind != f75373) { err = sysfs_chmod_file(&client->dev.kobj, &sensor_dev_attr_pwm1_mode.dev_attr.attr, S_IRUGO | S_IWUSR); -- cgit v1.2.3 From b17d6561acc16265b65b1e0d27b649829b61a7e3 Mon Sep 17 00:00:00 2001 From: Nikolaus Schulz Date: Tue, 28 Feb 2012 16:15:53 -0500 Subject: hwmon: (f75375s) Properly map the F75387 automatic modes to pwm_enable The F75387 supports automatic fan control using either PWM duty cycle or RPM speed values. Make the driver detect the latter mode, and expose the different modes in sysfs as per pwm_enable, so that the user can switch between them. The interpretation of the pwm_enable attribute for the F75387 is adjusted to be a superset of those values used for similar Fintek chips which do not support automatic duty mode, with 2 mapping to automatic speed mode, and moving automatic duty mode to the new value 4. Toggling the duty mode via pwm_enable is currently denied for the F75387, as the chip then simply reinterprets the fan configuration register values according to the new mode, switching between RPM and PWM units, which makes this a dangerous operation. This patch introduces a new pwm mode into the driver. This is necessary because the new mode (automatic pwm mode, 4) may already be enabled by the BIOS, and the driver should not break existing functionality. This was seen on at least one board. Signed-off-by: Nikolaus Schulz Signed-off-by: Guenter Roeck --- drivers/hwmon/f75375s.c | 42 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 34 insertions(+), 8 deletions(-) (limited to 'drivers/hwmon/f75375s.c') diff --git a/drivers/hwmon/f75375s.c b/drivers/hwmon/f75375s.c index eb648d9c91d6..9ab034a1b4c1 100644 --- a/drivers/hwmon/f75375s.c +++ b/drivers/hwmon/f75375s.c @@ -264,6 +264,21 @@ static inline u16 rpm_to_reg(int rpm) return 1500000 / rpm; } +static bool duty_mode_enabled(u8 pwm_enable) +{ + switch (pwm_enable) { + case 0: /* Manual, duty mode (full speed) */ + case 1: /* Manual, duty mode */ + case 4: /* Auto, duty mode */ + return true; + case 2: /* Auto, speed mode */ + case 3: /* Manual, speed mode */ + return false; + default: + BUG(); + } +} + static ssize_t set_fan_min(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) { @@ -337,11 +352,15 @@ static int set_pwm_enable_direct(struct i2c_client *client, int nr, int val) struct f75375_data *data = i2c_get_clientdata(client); u8 fanmode; - if (val < 0 || val > 3) + if (val < 0 || val > 4) return -EINVAL; fanmode = f75375_read8(client, F75375_REG_FAN_TIMER); if (data->kind == f75387) { + /* For now, deny dangerous toggling of duty mode */ + if (duty_mode_enabled(data->pwm_enable[nr]) != + duty_mode_enabled(val)) + return -EOPNOTSUPP; /* clear each fanX_mode bit before setting them properly */ fanmode &= ~(1 << F75387_FAN_DUTY_MODE(nr)); fanmode &= ~(1 << F75387_FAN_MANU_MODE(nr)); @@ -355,12 +374,14 @@ static int set_pwm_enable_direct(struct i2c_client *client, int nr, int val) fanmode |= (1 << F75387_FAN_MANU_MODE(nr)); fanmode |= (1 << F75387_FAN_DUTY_MODE(nr)); break; - case 2: /* AUTOMATIC*/ - fanmode |= (1 << F75387_FAN_DUTY_MODE(nr)); + case 2: /* Automatic, speed mode */ break; case 3: /* fan speed */ fanmode |= (1 << F75387_FAN_MANU_MODE(nr)); break; + case 4: /* Automatic, pwm */ + fanmode |= (1 << F75387_FAN_DUTY_MODE(nr)); + break; } } else { /* clear each fanX_mode bit before setting them properly */ @@ -378,6 +399,8 @@ static int set_pwm_enable_direct(struct i2c_client *client, int nr, int val) break; case 3: /* fan speed */ break; + case 4: /* Automatic pwm */ + return -EINVAL; } } @@ -735,14 +758,17 @@ static void f75375_init(struct i2c_client *client, struct f75375_data *data, manu = ((mode >> F75387_FAN_MANU_MODE(nr)) & 1); duty = ((mode >> F75387_FAN_DUTY_MODE(nr)) & 1); - if (manu && duty) - /* speed */ + if (!manu && duty) + /* auto, pwm */ + data->pwm_enable[nr] = 4; + else if (manu && !duty) + /* manual, speed */ data->pwm_enable[nr] = 3; - else if (!manu && duty) - /* automatic */ + else if (!manu && !duty) + /* automatic, speed */ data->pwm_enable[nr] = 2; else - /* manual */ + /* manual, pwm */ data->pwm_enable[nr] = 1; } else { if (!(conf & (1 << F75375_FAN_CTRL_LINEAR(nr)))) -- cgit v1.2.3 From 15d1ad0cc9d2d3f549afddbcdbc9c3637f0d1331 Mon Sep 17 00:00:00 2001 From: Nikolaus Schulz Date: Tue, 28 Feb 2012 16:15:54 -0500 Subject: hwmon: (f75375s) Catch some attempts to write to r/o registers It makes no sense to attempt to manually configure the fan in auto mode, or set the duty cycle directly in closed loop mode. The corresponding registers are then read-only. If the user tries it nonetheless, error out with EINVAL instead of silently doing nothing. Signed-off-by: Nikolaus Schulz [guenter.roeck@ericsson.com: Minor formatting cleanup] Signed-off-by: Guenter Roeck --- drivers/hwmon/f75375s.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) (limited to 'drivers/hwmon/f75375s.c') diff --git a/drivers/hwmon/f75375s.c b/drivers/hwmon/f75375s.c index 9ab034a1b4c1..6aa5a9fad879 100644 --- a/drivers/hwmon/f75375s.c +++ b/drivers/hwmon/f75375s.c @@ -279,6 +279,21 @@ static bool duty_mode_enabled(u8 pwm_enable) } } +static bool auto_mode_enabled(u8 pwm_enable) +{ + switch (pwm_enable) { + case 0: /* Manual, duty mode (full speed) */ + case 1: /* Manual, duty mode */ + case 3: /* Manual, speed mode */ + return false; + case 2: /* Auto, speed mode */ + case 4: /* Auto, duty mode */ + return true; + default: + BUG(); + } +} + static ssize_t set_fan_min(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) { @@ -312,6 +327,11 @@ static ssize_t set_fan_target(struct device *dev, struct device_attribute *attr, if (err < 0) return err; + if (auto_mode_enabled(data->pwm_enable[nr])) + return -EINVAL; + if (data->kind == f75387 && duty_mode_enabled(data->pwm_enable[nr])) + return -EINVAL; + mutex_lock(&data->update_lock); data->fan_target[nr] = rpm_to_reg(val); f75375_write16(client, F75375_REG_FAN_EXP(nr), data->fan_target[nr]); @@ -332,6 +352,10 @@ static ssize_t set_pwm(struct device *dev, struct device_attribute *attr, if (err < 0) return err; + if (auto_mode_enabled(data->pwm_enable[nr]) || + !duty_mode_enabled(data->pwm_enable[nr])) + return -EINVAL; + mutex_lock(&data->update_lock); data->pwm[nr] = SENSORS_LIMIT(val, 0, 255); f75375_write_pwm(client, nr); @@ -793,6 +817,9 @@ static void f75375_init(struct i2c_client *client, struct f75375_data *data, set_pwm_enable_direct(client, 0, f75375s_pdata->pwm_enable[0]); set_pwm_enable_direct(client, 1, f75375s_pdata->pwm_enable[1]); for (nr = 0; nr < 2; nr++) { + if (auto_mode_enabled(f75375s_pdata->pwm_enable[nr]) || + !duty_mode_enabled(f75375s_pdata->pwm_enable[nr])) + continue; data->pwm[nr] = SENSORS_LIMIT(f75375s_pdata->pwm[nr], 0, 255); f75375_write_pwm(client, nr); } -- cgit v1.2.3