: Add automatic PWM mode to it87.c
PATCH 1/1 : Add automatic PWM mode to it87.c
Adds options to enable the SmartGuardian automatic fan control mode in the IT8712 chip.
This patch adds following new /sys-files:
pwmX_auto_enable: Enables automatic mode (manual mode is also working)pwmX_limit_off: If this temperature is reached the fan is disabledpwmX_limit_start: Temperatures above this will enable the fanpwmX_limit_full: Temperatures above this will set the max. fan speedpwmX_temp_input: Selects which temperature is usedpwmX_start: PWM value to start the fan with if it was offpwmX_step: PWM step size / temperature C
The fan is controlled linear between limit_start and limit_full by increasing the PWM value by pwmX_step every temperature increase.
Bye,
Sebastian
-------------- next part --------------
it87.c renamed from it87.c-orig +307 -47
@@ -29,6 +29,10 @@Modified to fix bug with not all alarms enabled. Added ability to read battery voltage and select temperature sensor type at module load time. ++ [email protected] Sebastian Witt 5/31/05 +Added support for SmartGuardian automatic mode and added +module option to force PWM initializing */ #include <linux/config.h>
@@ -104,6 +108,9 @@/* Update battery voltage after every reading if true */ static int update_vbat; +/* Force enabling PWM */ +static int force_pwm; +/* Chip Type */ static u16 chip_type;
@@ -151,6 +158,12 @@#define IT87_REG_CHIPID 0x58 +#define IT87_REG_SG_TEMP_OFF (nr) (0x60 + (nr) * 8) +#define IT87_REG_SG_TEMP_START (nr) (0x61 + (nr) * 8) +#define IT87_REG_SG_TEMP_FULL (nr) (0x62 + (nr) * 8) +#define IT87_REG_SG_START_PWM (nr) (0x63 + (nr) * 8) +#define IT87_REG_SG_AM_CTRL (nr) (0x64 + (nr) * 8) +#define IN_TO_REG (val) (SENSORS_LIMIT((((val) + 8)/16),0,255))#define IN_FROM_REG (val) ((val) * 16)
@@ -197,21 +210,27 @@char valid;/* !=0 if following fields are valid */ unsigned long last_updated;/* In jiffies */ -u8 in[9 ];/* Register value */ +u8 in[9 ];/* Register value */ u8 in_max[9 ];/* Register value */ u8 in_min[9 ];/* Register value */ -u8 fan[3 ];/* Register value */ +u8 fan[3 ];/* Register value */ u8 fan_min[3 ];/* Register value */ -u8 temp[3 ];/* Register value */ +u8 temp[3 ];/* Register value */ u8 temp_high[3 ];/* Register value */ u8 temp_low[3 ];/* Register value */ -u8 sensor;/* Register value */ +u8 sensor;/* Register value */ u8 fan_div[3 ];/* Register encoding, shifted right */ -u8 vid;/* Register encoding, combined */ +u8 vid;/* Register encoding, combined */ int vrm; -u32 alarms;/* Register encoding, combined */ +u32 alarms;/* Register encoding, combined */ u8 fan_main_ctrl;/* Register value */ -u8 manual_pwm_ctl[3 ];/* manual PWM value set by user */ +u8 fan_ctrl;/* Register value */ +u8 manual_pwm_ctl[3 ];/* manual PWM value set by user */ +u8 fan_limit_off[3 ];/* Register value */ +u8 fan_limit_start[3 ];/* Register value */ +u8 fan_limit_full[3 ];/* Register value */ +u8 fan_limit_stpwm[3 ];/* Register value */ +u8 fan_am_ctrl[3 ];/* Register value */ };
@@ -468,10 +487,45 @@struct it87_data *data =it87_update_device (dev);return sprintf (buf,"%d\n" , (data->fan_main_ctrl & (1 << nr)) ?1 :0 ); } +static ssize_t show_pwm_auto_enable (struct device *dev,char *buf,int nr) +{ +struct it87_data *data =it87_update_device (dev); +return sprintf (buf,"%d\n" , (data->manual_pwm_ctl[nr] &0x80 ) ?1 :0 ); +}static ssize_t show_pwm (struct device *dev,char *buf,int nr) {struct it87_data *data =it87_update_device (dev); -return sprintf (buf,"%d\n" , data->manual_pwm_ctl[nr]); +return sprintf (buf,"%d\n" , (data->manual_pwm_ctl[nr] &0x80 ) ?0 :PWM_FROM_REG (data->manual_pwm_ctl[nr])); +} +static ssize_t show_pwm_limit_off (struct device *dev,char *buf,int nr) +{ +struct it87_data *data =it87_update_device (dev); +return sprintf (buf,"%d\n" , data->fan_limit_off[nr]); +} +static ssize_t show_pwm_limit_start (struct device *dev,char *buf,int nr) +{ +struct it87_data *data =it87_update_device (dev); +return sprintf (buf,"%d\n" , data->fan_limit_start[nr]); +} +static ssize_t show_pwm_limit_full (struct device *dev,char *buf,int nr) +{ +struct it87_data *data =it87_update_device (dev); +return sprintf (buf,"%d\n" , data->fan_limit_full[nr]); +} +static ssize_t show_pwm_start (struct device *dev,char *buf,int nr) +{ +struct it87_data *data =it87_update_device (dev); +return sprintf (buf,"%d\n" ,PWM_FROM_REG (data->fan_limit_stpwm[nr])); +} +static ssize_t show_pwm_temp_input (struct device *dev,char *buf,int nr) +{ +struct it87_data *data =it87_update_device (dev); +return sprintf (buf,"%d\n" , (data->manual_pwm_ctl[nr] &0x80 ) ? (data->manual_pwm_ctl[nr] &0x03 ) +1 :0 ); +} +static ssize_t show_pwm_step (struct device *dev,char *buf,int nr) +{ +struct it87_data *data =it87_update_device (dev); +return sprintf (buf,"%d\n" , (data->fan_am_ctrl[nr] &0x07 ) ? (1 << ((data->fan_am_ctrl[nr] &0x07 )-1 )) :0 ); }static ssize_t set_fan_min (struct device *dev,const char *buf,size_t count,int nr)
@@ -528,9 +582,9 @@if (val =0 ) {int tmp; -/* make sure the fan is on when in on/off mode */ +/* make sure the fan is on when in on/off mode and set polarity mode to active high */ tmp =it87_read_value (client,IT87_REG_FAN_CTL ); -it87_write_value (client,IT87_REG_FAN_CTL , tmp | (1 << nr)); +it87_write_value (client,IT87_REG_FAN_CTL , tmp | (1 << nr) |0x80 );/* set on/off mode */ data->fan_main_ctrl &= ~(1 << nr);it87_write_value (client,IT87_REG_FAN_MAIN_CTRL , data->fan_main_ctrl);
@@ -538,8 +592,35 @@/* set SmartGuardian mode */ data->fan_main_ctrl |= (1 << nr);it87_write_value (client,IT87_REG_FAN_MAIN_CTRL , data->fan_main_ctrl); -/* set saved pwm value, clear FAN_CTLX PWM mode bit */ -it87_write_value (client,IT87_REG_PWM (nr),PWM_TO_REG (data->manual_pwm_ctl[nr])); +/* set saved pwm value */ +it87_write_value (client,IT87_REG_PWM (nr), data->manual_pwm_ctl[nr]); + }else +return -EINVAL ; + +return count; +} +static ssize_t set_pwm_auto_enable (struct device *dev,const char *buf, +size_t count,int nr) +{ +struct i2c_client *client =to_i2c_client (dev); +struct it87_data *data =i2c_get_clientdata (client); +int val =simple_strtol (buf,NULL ,10 ); + +if (val =0 ) { +/* Disable automatic mode, set fans to full speed */ + data->manual_pwm_ctl[nr] =0x7f ; +it87_write_value (client,IT87_REG_PWM (nr), data->manual_pwm_ctl[nr]); + }else if (val =1 ) { +/* Start PWM control if required */ +if ((data->fan_main_ctrl & (1 << nr)) =0 ) { +set_pwm_enable (dev,"1" ,1 , nr); + } +/* Enable automatic mode */ + data->manual_pwm_ctl[nr] |=0x80 ; +it87_write_value (client,IT87_REG_PWM (nr), data->manual_pwm_ctl[nr]); +/* Enable temp. smoothing, fan spin up time and 16 PWM / C */ + data->fan_am_ctrl[nr] =0x80 |0x18 |0x05 ; +it87_write_value (client,IT87_REG_SG_AM_CTRL (nr), data->fan_am_ctrl[nr]); }else return -EINVAL ;
@@ -555,9 +636,97 @@if (val <0 || val >255 )return -EINVAL ; - data->manual_pwm_ctl[nr] = val; +/* Automatic mode gets disabled if writing a manual PWM value */ + data->manual_pwm_ctl[nr] =PWM_TO_REG (val);if (data->fan_main_ctrl & (1 << nr)) -it87_write_value (client,IT87_REG_PWM (nr),PWM_TO_REG (data->manual_pwm_ctl[nr])); +it87_write_value (client,IT87_REG_PWM (nr), data->manual_pwm_ctl[nr]); + +return count; +} +static ssize_t set_pwm_limit_off (struct device *dev,const char *buf, +size_t count,int nr) +{ +struct i2c_client *client =to_i2c_client (dev); +struct it87_data *data =i2c_get_clientdata (client); +int val =simple_strtol (buf,NULL ,10 ); + + data->fan_limit_off[nr] = val; +it87_write_value (client,IT87_REG_SG_TEMP_OFF (nr), data->fan_limit_off[nr]); + +return count; +} +static ssize_t set_pwm_limit_start (struct device *dev,const char *buf, +size_t count,int nr) +{ +struct i2c_client *client =to_i2c_client (dev); +struct it87_data *data =i2c_get_clientdata (client); +int val =simple_strtol (buf,NULL ,10 ); + + data->fan_limit_start[nr] = val; +it87_write_value (client,IT87_REG_SG_TEMP_START (nr), data->fan_limit_start[nr]); + +return count; +} +static ssize_t set_pwm_limit_full (struct device *dev,const char *buf, +size_t count,int nr) +{ +struct i2c_client *client =to_i2c_client (dev); +struct it87_data *data =i2c_get_clientdata (client); +int val =simple_strtol (buf,NULL ,10 ); + + data->fan_limit_full[nr] = val; +it87_write_value (client,IT87_REG_SG_TEMP_FULL (nr), data->fan_limit_full[nr]); + +return count; +} +static ssize_t set_pwm_start (struct device *dev,const char *buf, +size_t count,int nr) +{ +struct i2c_client *client =to_i2c_client (dev); +struct it87_data *data =i2c_get_clientdata (client); +int val =simple_strtol (buf,NULL ,10 ); + +if (val <0 || val >255 ) +return -EINVAL ; + + data->fan_limit_stpwm[nr] =PWM_TO_REG (val); +it87_write_value (client,IT87_REG_SG_START_PWM (nr), data->fan_limit_stpwm[nr]); + +return count; +} +static ssize_t set_pwm_temp_input (struct device *dev,const char *buf, +size_t count,int nr) +{ +struct i2c_client *client =to_i2c_client (dev); +struct it87_data *data =i2c_get_clientdata (client); +int val =simple_strtol (buf,NULL ,10 ); + +if (val <1 || val >3 ) +return -EINVAL ; + +/* Check if automatic control is enabled */ +if (data->manual_pwm_ctl[nr] &0x80 ) { + data->manual_pwm_ctl[nr] =0x80 | (val -1 ); +it87_write_value (client,IT87_REG_PWM (nr), data->manual_pwm_ctl[nr]); + } + +return count; +} +static ssize_t set_pwm_step (struct device *dev,const char *buf, +size_t count,int nr) +{ +struct i2c_client *client =to_i2c_client (dev); +struct it87_data *data =i2c_get_clientdata (client); +int val =simple_strtol (buf,NULL ,10 ); + +if (val <0 || val >7 ) +return -EINVAL ; + + data->fan_am_ctrl[nr] &= ~0x07 ; + data->fan_am_ctrl[nr] |= val; + +it87_write_value (client,IT87_REG_SG_AM_CTRL (nr), data->fan_am_ctrl[nr]); +return count; }
@@ -601,25 +770,104 @@ { \return show_pwm_enable (dev, buf, offset -1 ); \ } \ +static ssize_t show_pwm##offset##_auto_enable (struct device *dev, \ + char *buf) \ +{ \ + return show_pwm_auto_enable(dev, buf, offset - 1); \ +} \ static ssize_t show_pwm##offset (struct device *dev, char *buf) \ { \ return show_pwm(dev, buf, offset - 1); \ } \ +static ssize_t show_pwm##offset##_limit_off (struct device *dev, char *buf) \ +{ \ + return show_pwm_limit_off(dev, buf, offset - 1); \ +} \ +static ssize_t show_pwm##offset##_limit_start (struct device *dev, char *buf) \ +{ \ + return show_pwm_limit_start(dev, buf, offset - 1); \ +} \ +static ssize_t show_pwm##offset##_limit_full (struct device *dev, char *buf) \ +{ \ + return show_pwm_limit_full(dev, buf, offset - 1); \ +} \ +static ssize_t show_pwm##offset##_start (struct device *dev, char *buf) \ +{ \ + return show_pwm_start(dev, buf, offset - 1); \ +} \ +static ssize_t show_pwm##offset##_temp_input (struct device *dev, char *buf) \ +{ \ + return show_pwm_temp_input(dev, buf, offset - 1); \ +} \ +static ssize_t show_pwm##offset##_step (struct device *dev, char *buf) \ +{ \ + return show_pwm_step(dev, buf, offset - 1); \ +} \ static ssize_t set_pwm##offset##_enable (struct device *dev, \ const char *buf, size_t count) \ { \ return set_pwm_enable(dev, buf, count, offset - 1); \ } \ +static ssize_t set_pwm##offset##_auto_enable (struct device *dev, \ + const char *buf, size_t count) \ +{ \ + return set_pwm_auto_enable(dev, buf, count, offset - 1); \ +} \ static ssize_t set_pwm##offset (struct device *dev, \ const char *buf, size_t count) \ { \ return set_pwm(dev, buf, count, offset - 1); \ } \ +static ssize_t set_pwm##offset##_limit_off (struct device *dev, \ + const char *buf, size_t count) \ +{ \ + return set_pwm_limit_off(dev, buf, count, offset - 1); \ +} \ +static ssize_t set_pwm##offset##_limit_start (struct device *dev, \ + const char *buf, size_t count) \ +{ \ + return set_pwm_limit_start(dev, buf, count, offset - 1); \ +} \ +static ssize_t set_pwm##offset##_limit_full (struct device *dev, \ + const char *buf, size_t count) \ +{ \ + return set_pwm_limit_full(dev, buf, count, offset - 1); \ +} \ +static ssize_t set_pwm##offset##_start (struct device *dev, \ + const char *buf, size_t count) \ +{ \ + return set_pwm_start(dev, buf, count, offset - 1); \ +} \ +static ssize_t set_pwm##offset##_temp_input (struct device *dev, \ + const char *buf, size_t count) \ +{ \ + return set_pwm_temp_input(dev, buf, count, offset - 1); \ +} \ +static ssize_t set_pwm##offset##_step (struct device *dev, \ + const char *buf, size_t count) \ +{ \ + return set_pwm_step(dev, buf, count, offset - 1); \ +} \ static DEVICE_ATTR(pwm##offset##_enable, S_IRUGO | S_IWUSR, \ show_pwm##offset##_enable, \ set_pwm##offset##_enable); \ +static DEVICE_ATTR(pwm##offset##_auto_enable, S_IRUGO | S_IWUSR, \ + show_pwm##offset##_auto_enable, \ + set_pwm##offset##_auto_enable); \ static DEVICE_ATTR(pwm##offset, S_IRUGO | S_IWUSR, \ - show_pwm##offset , set_pwm##offset ); + show_pwm##offset , set_pwm##offset ); \ +static DEVICE_ATTR(pwm##offset##_limit_off, S_IRUGO | S_IWUSR, \ + show_pwm##offset##_limit_off , set_pwm##offset##_limit_off ); \ +static DEVICE_ATTR(pwm##offset##_limit_start, S_IRUGO | S_IWUSR, \ + show_pwm##offset##_limit_start , set_pwm##offset##_limit_start ); \ +static DEVICE_ATTR(pwm##offset##_limit_full, S_IRUGO | S_IWUSR, \ + show_pwm##offset##_limit_full , set_pwm##offset##_limit_full ); \ +static DEVICE_ATTR(pwm##offset##_start, S_IRUGO | S_IWUSR, \ + show_pwm##offset##_start , set_pwm##offset##_start ); \ +static DEVICE_ATTR(pwm##offset##_temp_input, S_IRUGO | S_IWUSR, \ + show_pwm##offset##_temp_input , set_pwm##offset##_temp_input ); \ +static DEVICE_ATTR(pwm##offset##_step, S_IRUGO | S_IWUSR, \ + show_pwm##offset##_step , set_pwm##offset##_step );show_pwm_offset (1 );show_pwm_offset (2 );
@@ -829,11 +1077,13 @@* and polarity set to active low is sign that this is the case so we * disable pwm control to protect the user. */ enable_pwm_interface =1 ; - tmp =it87_read_value (new_client,IT87_REG_FAN_CTL ); -if ((tmp &0x87 ) =0 ) { - enable_pwm_interface =0 ; -dev_info (&new_client->dev, -"detected broken BIOS defaults, disabling pwm interface" ); +if (!force_pwm) { + tmp =it87_read_value (new_client,IT87_REG_FAN_CTL ); +if ((tmp &0x87 ) =0 ) { + enable_pwm_interface =0 ; +dev_info (&new_client->dev, +"detected broken BIOS defaults, disabling pwm interface" ); + } }/* Register sysfs hooks */
@@ -888,9 +1138,30 @@device_create_file (&new_client->dev, &dev_attr_pwm1_enable);device_create_file (&new_client->dev, &dev_attr_pwm2_enable);device_create_file (&new_client->dev, &dev_attr_pwm3_enable); +device_create_file (&new_client->dev, &dev_attr_pwm1_auto_enable); +device_create_file (&new_client->dev, &dev_attr_pwm2_auto_enable); +device_create_file (&new_client->dev, &dev_attr_pwm3_auto_enable);device_create_file (&new_client->dev, &dev_attr_pwm1);device_create_file (&new_client->dev, &dev_attr_pwm2);device_create_file (&new_client->dev, &dev_attr_pwm3); +device_create_file (&new_client->dev, &dev_attr_pwm1_limit_off); +device_create_file (&new_client->dev, &dev_attr_pwm2_limit_off); +device_create_file (&new_client->dev, &dev_attr_pwm3_limit_off); +device_create_file (&new_client->dev, &dev_attr_pwm1_limit_start); +device_create_file (&new_client->dev, &dev_attr_pwm2_limit_start); +device_create_file (&new_client->dev, &dev_attr_pwm3_limit_start); +device_create_file (&new_client->dev, &dev_attr_pwm1_limit_full); +device_create_file (&new_client->dev, &dev_attr_pwm2_limit_full); +device_create_file (&new_client->dev, &dev_attr_pwm3_limit_full); +device_create_file (&new_client->dev, &dev_attr_pwm1_start); +device_create_file (&new_client->dev, &dev_attr_pwm2_start); +device_create_file (&new_client->dev, &dev_attr_pwm3_start); +device_create_file (&new_client->dev, &dev_attr_pwm1_temp_input); +device_create_file (&new_client->dev, &dev_attr_pwm2_temp_input); +device_create_file (&new_client->dev, &dev_attr_pwm3_temp_input); +device_create_file (&new_client->dev, &dev_attr_pwm1_step); +device_create_file (&new_client->dev, &dev_attr_pwm2_step); +device_create_file (&new_client->dev, &dev_attr_pwm3_step); }if (data->type = it8712) {
@@ -971,17 +1242,6 @@ {int tmp, i; -/* initialize to sane defaults: -* - if the chip is in manual pwm mode, this will be overwritten with -* the actual settings on the chip (so in this case, initialization -* is not needed) -* - if in automatic or on/off mode, we could switch to manual mode, -* read the registers and set manual_pwm_ctl accordingly, but currently -* this is not implemented, so we initialize to something sane */ -for (i =0 ; i <3 ; i++) { - data->manual_pwm_ctl[i] =0xff ; - } -/* Check if temperature channnels are reset manually or by some reason */ tmp =it87_read_value (client,IT87_REG_TEMP_ENABLE );if ((tmp &0x3f ) =0 ) {
@@ -1006,24 +1266,21 @@it87_write_value (client,IT87_REG_FAN_MAIN_CTRL , data->fan_main_ctrl); } -/* Set current fan mode registers and the default settings for the -* other mode registers */ +/* Set current fan mode registers */ for (i =0 ; i <3 ; i++) { -if (data->fan_main_ctrl & (1 << i)) { -/* pwm mode */ - tmp =it87_read_value (client,IT87_REG_PWM (i)); -if (tmp &0x80 ) { -/* automatic pwm - not yet implemented, but -* leave the settings made by the BIOS alone -* until a change is requested via the sysfs -* interface */ - }else { -/* manual pwm */ - data->manual_pwm_ctl[i] =PWM_FROM_REG (tmp); - } - } + data->manual_pwm_ctl[i] =it87_read_value (client,IT87_REG_PWM (i)); } - + +/* Get fan control registers for SmartGuardian automatic mode */ + data->fan_ctrl =it87_read_value (client,IT87_REG_FAN_CTL ); +for (i =0 ; i <3 ; i++) { + data->fan_limit_off[i] =it87_read_value (client,IT87_REG_SG_TEMP_OFF (i)); + data->fan_limit_start[i] =it87_read_value (client,IT87_REG_SG_TEMP_START (i)); + data->fan_limit_full[i] =it87_read_value (client,IT87_REG_SG_TEMP_FULL (i)); + data->fan_limit_stpwm[i] =it87_read_value (client,IT87_REG_SG_START_PWM (i)); + data->fan_am_ctrl[i] =it87_read_value (client,IT87_REG_SG_AM_CTRL (i)); + } +/* Start monitoring */ it87_write_value (client,IT87_REG_CONFIG , (it87_read_value(client, IT87_REG_CONFIG) & 0x36)
@@ -1123,9 +1380,12 @@MODULE_AUTHOR ("Chris Gauthron <[email protected]>" ); +MODULE_AUTHOR ("Sebastian Witt <[email protected]>" );MODULE_DESCRIPTION ("IT8705F, IT8712F, Sis950 driver" );module_param (update_vbat, bool,0 ); +module_param (force_pwm, bool,0 );MODULE_PARM_DESC (update_vbat,"Update vbat if set else return powerup value" ); +MODULE_PARM_DESC (force_pwm,"Force initializing PWM" );MODULE_LICENSE ("GPL" );module_init (sm_it87_init);
PATCH 1/1 : Add automatic PWM mode to it87.c
Hi Sebastian
I will put some comments into the code. I think those I made are enough for next iteration.
regards
Rudolf
Quoted 7 lines
Adds options to enable the SmartGuardian automatic fan control mode in the IT8712 chip.
This patch adds following new /sys-files:
pwmX_auto_enable: Enables automatic mode (manual mode is also working)
Please could you use pwmX_enable for this?
If user will write 0 -> disable PWM
1 -> manual PWM
2 -> Automatic Mode - SmartGuardian
3 -> some other...Quoted 10 lines
pwmX_limit_off: If this temperature is reached the fan is disabled pwmX_limit_start: Temperatures above this will enable the fan pwmX_limit_full: Temperatures above this will set the max. fan speed pwmX_temp_input: Selects which temperature is used pwmX_start: PWM value to start the fan with if it was off pwmX_step: PWM step size / temperature CThe fan is controlled linear between
limit_startandlimit_fullby increasing
the PWM value bypwmX_stepevery temperature increase.
This must go into the i2c/chips/it87 documentation file. Currently you can find the file here:
Of course you can inspire from 2.4 file but be careful to add only stuff that is valid.
http://ftp.sh.cvut.cz/MIRRORS/kernel/linux/kernel/people/gregkh/gregkh-2.6/gregkh-02-i2c/i2c-docs-update-2.patch
Please generate also patch for this file.
it87.c
Quoted 9 lines
Bye,
Sebastian------------------------------------------------------------------------
it87.c renamed from it87.c-orig +0
-0
we need classic:
diff -Naur vanilla-kernel-mm-series/ modified/ > your_patch
Please note we accept patches right now only against -mm- tree because it has some sysfs changes.
-static ssize_t show_xxx(struct device * dev, char * buf)
+static ssize_t show_xxx(struct device * dev, struct device_attribute *attr, char * buf)
Related info: http://lists.lm-sensors.org/pipermail/lm-sensors/2005-May/012251.html
it87.c line 36
Quoted 9 lines
it87.c renamed from it87.c-orig +4
-0@@ -29,6 +29,10 @@Modified to fix bug with not all alarms enabled. Added ability to read battery voltage and select temperature sensor type at module load time. ++ [email protected] Sebastian Witt 5/31/05 +Added support for SmartGuardian automatic mode and added +module option to force PWM initializing */
Please do not add change log, Greg does not like it. It is not maitainable.
it87.c line 113
Quoted 8 lines
it87.c renamed from it87.c-orig +3
-0#include <linux/config.h> @@ -104,6 +108,9 @@/* Update battery voltage after every reading if true */ static int update_vbat; +/* Force enabling PWM */ +static int force_pwm; +
I'm a bit confused that I cannot see static int fix_pwm_polarity;
linux-2.6.12-rc5 has something called:
/* Not all BIOSes properly configure the PWM registers */ static int fix_pwm_polarity;
http://lists.lm-sensors.org/pipermail/lm-sensors/2005-March/010860.html
Maybe it is same thing but I will check later.
it87.c line 165
Quoted 12 lines
it87.c renamed from it87.c-orig +5
-0/* Chip Type */ static u16 chip_type;@@ -151,6 +158,12 @@#define IT87_REG_CHIPID 0x58 +#define IT87_REG_SG_TEMP_OFF (nr) (0x60 + (nr) * 8) +#define IT87_REG_SG_TEMP_START (nr) (0x61 + (nr) * 8) +#define IT87_REG_SG_TEMP_FULL (nr) (0x62 + (nr) * 8) +#define IT87_REG_SG_START_PWM (nr) (0x63 + (nr) * 8) +#define IT87_REG_SG_AM_CTRL (nr) (0x64 + (nr) * 8)
I guess better would be to align via tabs instead of spaces.
Quoted 25 lines
it87.c renamed from it87.c-orig +5
-6#define IN_TO_REG (val) (SENSORS_LIMIT((((val) + 8)/16),0,255))#define IN_FROM_REG (val) ((val) * 16)@@ -197,21 +210,27 @@char valid;/* !=0 if following fields are valid */ unsigned long last_updated;/* In jiffies */ -u8 in[9 ];/* Register value */ +u8 in[9 ];/* Register value */ u8 in_max[9 ];/* Register value */ u8 in_min[9 ];/* Register value */ -u8 fan[3 ];/* Register value */ +u8 fan[3 ];/* Register value */ u8 fan_min[3 ];/* Register value */ -u8 temp[3 ];/* Register value */ +u8 temp[3 ];/* Register value */ u8 temp_high[3 ];/* Register value */ u8 temp_low[3 ];/* Register value */ -u8 sensor;/* Register value */ +u8 sensor;/* Register value */ u8 fan_div[3 ];/* Register encoding, shifted right */ -u8 vid;/* Register encoding, combined */ +u8 vid;/* Register encoding, combined */ int vrm; -u32 alarms;/* Register encoding, combined */
it87.c line 233
Quoted 10 lines
it87.c renamed from it87.c-orig +8
-1+u32 alarms;/* Register encoding, combined */ u8 fan_main_ctrl;/* Register value */ -u8 manual_pwm_ctl[3 ];/* manual PWM value set by user */ +u8 fan_ctrl;/* Register value */ +u8 manual_pwm_ctl[3 ];/* manual PWM value set by user */ +u8 fan_limit_off[3 ];/* Register value */ +u8 fan_limit_start[3 ];/* Register value */ +u8 fan_limit_full[3 ];/* Register value */ +u8 fan_limit_stpwm[3 ];/* Register value */ +u8 fan_am_ctrl[3 ];/* Register value */
I dont know if others will like the 4 space tabs.
Quoted 7 lines
it87.c renamed from it87.c-orig +0
-0};@@ -468,10 +487,45 @@struct it87_data *data =it87_update_device (dev);return sprintf (buf,"%d\n" , (data->fan_main_ctrl & (1 << nr)) ?1 :0 ); }
it87.c line 498
Quoted 10 lines
it87.c renamed from it87.c-orig +6
-1+static ssize_t show_pwm_auto_enable (struct device *dev,char *buf,int nr) +{ +struct it87_data *data =it87_update_device (dev); +return sprintf (buf,"%d\n" , (data->manual_pwm_ctl[nr] &0x80 ) ?1 :0 ); +}static ssize_t show_pwm (struct device *dev,char *buf,int nr) {struct it87_data *data =it87_update_device (dev); -return sprintf (buf,"%d\n" , data->manual_pwm_ctl[nr]); +return sprintf (buf,"%d\n" , (data->manual_pwm_ctl[nr] &0x80 ) ?0 :PWM_FROM_REG (data->manual_pwm_ctl[nr]));
See above the pwmX_enable...
Quoted 34 lines
it87.c renamed from it87.c-orig +30
-0+} +static ssize_t show_pwm_limit_off (struct device *dev,char *buf,int nr) +{ +struct it87_data *data =it87_update_device (dev); +return sprintf (buf,"%d\n" , data->fan_limit_off[nr]); +} +static ssize_t show_pwm_limit_start (struct device *dev,char *buf,int nr) +{ +struct it87_data *data =it87_update_device (dev); +return sprintf (buf,"%d\n" , data->fan_limit_start[nr]); +} +static ssize_t show_pwm_limit_full (struct device *dev,char *buf,int nr) +{ +struct it87_data *data =it87_update_device (dev); +return sprintf (buf,"%d\n" , data->fan_limit_full[nr]); +} +static ssize_t show_pwm_start (struct device *dev,char *buf,int nr) +{ +struct it87_data *data =it87_update_device (dev); +return sprintf (buf,"%d\n" ,PWM_FROM_REG (data->fan_limit_stpwm[nr])); +} +static ssize_t show_pwm_temp_input (struct device *dev,char *buf,int nr) +{ +struct it87_data *data =it87_update_device (dev); +return sprintf (buf,"%d\n" , (data->manual_pwm_ctl[nr] &0x80 ) ? (data->manual_pwm_ctl[nr] &0x03 ) +1 :0 ); +} +static ssize_t show_pwm_step (struct device *dev,char *buf,int nr) +{ +struct it87_data *data =it87_update_device (dev); +return sprintf (buf,"%d\n" , (data->fan_am_ctrl[nr] &0x07 ) ? (1 << ((data->fan_am_ctrl[nr] &0x07 )-1 )) :0 ); }static ssize_t set_fan_min (struct device *dev,const char *buf,size_t count,int nr)
it87.c line 585
Quoted 5 lines
it87.c renamed from it87.c-orig +1
-1@@ -528,9 +582,9 @@if (val =0 ) {int tmp; -/* make sure the fan is on when in on/off mode */ +/* make sure the fan is on when in on/off mode and set polarity mode to active high */
Aha see the polarity fixing stuff
Quoted 29 lines
it87.c renamed from it87.c-orig +18
-3tmp =it87_read_value (client,IT87_REG_FAN_CTL ); -it87_write_value (client,IT87_REG_FAN_CTL , tmp | (1 << nr)); +it87_write_value (client,IT87_REG_FAN_CTL , tmp | (1 << nr) |0x80 );/* set on/off mode */ data->fan_main_ctrl &= ~(1 << nr);it87_write_value (client,IT87_REG_FAN_MAIN_CTRL , data->fan_main_ctrl);@@ -538,8 +592,35 @@/* set SmartGuardian mode */ data->fan_main_ctrl |= (1 << nr);it87_write_value (client,IT87_REG_FAN_MAIN_CTRL , data->fan_main_ctrl); -/* set saved pwm value, clear FAN_CTLX PWM mode bit */ -it87_write_value (client,IT87_REG_PWM (nr),PWM_TO_REG (data->manual_pwm_ctl[nr])); +/* set saved pwm value */ +it87_write_value (client,IT87_REG_PWM (nr), data->manual_pwm_ctl[nr]); + }else +return -EINVAL ; + +return count; +} +static ssize_t set_pwm_auto_enable (struct device *dev,const char *buf, +size_t count,int nr) +{ +struct i2c_client *client =to_i2c_client (dev); +struct it87_data *data =i2c_get_clientdata (client); +int val =simple_strtol (buf,NULL ,10 ); + +if (val =0 ) { +/* Disable automatic mode, set fans to full speed */ + data->manual_pwm_ctl[nr] =0x7f ;
it87.c line 621
Quoted 10 lines
it87.c renamed from it87.c-orig +10
-0+it87_write_value (client,IT87_REG_PWM (nr), data->manual_pwm_ctl[nr]); + }else if (val =1 ) { +/* Start PWM control if required */ +if ((data->fan_main_ctrl & (1 << nr)) =0 ) { +set_pwm_enable (dev,"1" ,1 , nr); + } +/* Enable automatic mode */ + data->manual_pwm_ctl[nr] |=0x80 ; +it87_write_value (client,IT87_REG_PWM (nr), data->manual_pwm_ctl[nr]); +/* Enable temp. smoothing, fan spin up time and 16 PWM / C */
more spaces up ... time
Quoted 318 lines
it87.c renamed from it87.c-orig +213
-35+ data->fan_am_ctrl[nr] =0x80 |0x18 |0x05 ; +it87_write_value (client,IT87_REG_SG_AM_CTRL (nr), data->fan_am_ctrl[nr]); }else return -EINVAL ;@@ -555,9 +636,97 @@if (val <0 || val >255 )return -EINVAL ; - data->manual_pwm_ctl[nr] = val; +/* Automatic mode gets disabled if writing a manual PWM value */ + data->manual_pwm_ctl[nr] =PWM_TO_REG (val);if (data->fan_main_ctrl & (1 << nr)) -it87_write_value (client,IT87_REG_PWM (nr),PWM_TO_REG (data->manual_pwm_ctl[nr])); +it87_write_value (client,IT87_REG_PWM (nr), data->manual_pwm_ctl[nr]); + +return count; +} +static ssize_t set_pwm_limit_off (struct device *dev,const char *buf, +size_t count,int nr) +{ +struct i2c_client *client =to_i2c_client (dev); +struct it87_data *data =i2c_get_clientdata (client); +int val =simple_strtol (buf,NULL ,10 ); + + data->fan_limit_off[nr] = val; +it87_write_value (client,IT87_REG_SG_TEMP_OFF (nr), data->fan_limit_off[nr]); + +return count; +} +static ssize_t set_pwm_limit_start (struct device *dev,const char *buf, +size_t count,int nr) +{ +struct i2c_client *client =to_i2c_client (dev); +struct it87_data *data =i2c_get_clientdata (client); +int val =simple_strtol (buf,NULL ,10 ); + + data->fan_limit_start[nr] = val; +it87_write_value (client,IT87_REG_SG_TEMP_START (nr), data->fan_limit_start[nr]); + +return count; +} +static ssize_t set_pwm_limit_full (struct device *dev,const char *buf, +size_t count,int nr) +{ +struct i2c_client *client =to_i2c_client (dev); +struct it87_data *data =i2c_get_clientdata (client); +int val =simple_strtol (buf,NULL ,10 ); + + data->fan_limit_full[nr] = val; +it87_write_value (client,IT87_REG_SG_TEMP_FULL (nr), data->fan_limit_full[nr]); + +return count; +} +static ssize_t set_pwm_start (struct device *dev,const char *buf, +size_t count,int nr) +{ +struct i2c_client *client =to_i2c_client (dev); +struct it87_data *data =i2c_get_clientdata (client); +int val =simple_strtol (buf,NULL ,10 ); + +if (val <0 || val >255 ) +return -EINVAL ; + + data->fan_limit_stpwm[nr] =PWM_TO_REG (val); +it87_write_value (client,IT87_REG_SG_START_PWM (nr), data->fan_limit_stpwm[nr]); + +return count; +} +static ssize_t set_pwm_temp_input (struct device *dev,const char *buf, +size_t count,int nr) +{ +struct i2c_client *client =to_i2c_client (dev); +struct it87_data *data =i2c_get_clientdata (client); +int val =simple_strtol (buf,NULL ,10 ); + +if (val <1 || val >3 ) +return -EINVAL ; + +/* Check if automatic control is enabled */ +if (data->manual_pwm_ctl[nr] &0x80 ) { + data->manual_pwm_ctl[nr] =0x80 | (val -1 ); +it87_write_value (client,IT87_REG_PWM (nr), data->manual_pwm_ctl[nr]); + } + +return count; +} +static ssize_t set_pwm_step (struct device *dev,const char *buf, +size_t count,int nr) +{ +struct i2c_client *client =to_i2c_client (dev); +struct it87_data *data =i2c_get_clientdata (client); +int val =simple_strtol (buf,NULL ,10 ); + +if (val <0 || val >7 ) +return -EINVAL ; + + data->fan_am_ctrl[nr] &= ~0x07 ; + data->fan_am_ctrl[nr] |= val; + +it87_write_value (client,IT87_REG_SG_AM_CTRL (nr), data->fan_am_ctrl[nr]); +return count; }@@ -601,25 +770,104 @@ { \return show_pwm_enable (dev, buf, offset -1 ); \ } \ +static ssize_t show_pwm##offset##_auto_enable (struct device *dev, \ + char *buf) \ +{ \ + return show_pwm_auto_enable(dev, buf, offset - 1); \ +} \ static ssize_t show_pwm##offset (struct device *dev, char *buf) \ { \ return show_pwm(dev, buf, offset - 1); \ } \ +static ssize_t show_pwm##offset##_limit_off (struct device *dev, char *buf) \ +{ \ + return show_pwm_limit_off(dev, buf, offset - 1); \ +} \ +static ssize_t show_pwm##offset##_limit_start (struct device *dev, char *buf) \ +{ \ + return show_pwm_limit_start(dev, buf, offset - 1); \ +} \ +static ssize_t show_pwm##offset##_limit_full (struct device *dev, char *buf) \ +{ \ + return show_pwm_limit_full(dev, buf, offset - 1); \ +} \ +static ssize_t show_pwm##offset##_start (struct device *dev, char *buf) \ +{ \ + return show_pwm_start(dev, buf, offset - 1); \ +} \ +static ssize_t show_pwm##offset##_temp_input (struct device *dev, char *buf) \ +{ \ + return show_pwm_temp_input(dev, buf, offset - 1); \ +} \ +static ssize_t show_pwm##offset##_step (struct device *dev, char *buf) \ +{ \ + return show_pwm_step(dev, buf, offset - 1); \ +} \ static ssize_t set_pwm##offset##_enable (struct device *dev, \ const char *buf, size_t count) \ { \ return set_pwm_enable(dev, buf, count, offset - 1); \ } \ +static ssize_t set_pwm##offset##_auto_enable (struct device *dev, \ + const char *buf, size_t count) \ +{ \ + return set_pwm_auto_enable(dev, buf, count, offset - 1); \ +} \ static ssize_t set_pwm##offset (struct device *dev, \ const char *buf, size_t count) \ { \ return set_pwm(dev, buf, count, offset - 1); \ } \ +static ssize_t set_pwm##offset##_limit_off (struct device *dev, \ + const char *buf, size_t count) \ +{ \ + return set_pwm_limit_off(dev, buf, count, offset - 1); \ +} \ +static ssize_t set_pwm##offset##_limit_start (struct device *dev, \ + const char *buf, size_t count) \ +{ \ + return set_pwm_limit_start(dev, buf, count, offset - 1); \ +} \ +static ssize_t set_pwm##offset##_limit_full (struct device *dev, \ + const char *buf, size_t count) \ +{ \ + return set_pwm_limit_full(dev, buf, count, offset - 1); \ +} \ +static ssize_t set_pwm##offset##_start (struct device *dev, \ + const char *buf, size_t count) \ +{ \ + return set_pwm_start(dev, buf, count, offset - 1); \ +} \ +static ssize_t set_pwm##offset##_temp_input (struct device *dev, \ + const char *buf, size_t count) \ +{ \ + return set_pwm_temp_input(dev, buf, count, offset - 1); \ +} \ +static ssize_t set_pwm##offset##_step (struct device *dev, \ + const char *buf, size_t count) \ +{ \ + return set_pwm_step(dev, buf, count, offset - 1); \ +} \ static DEVICE_ATTR(pwm##offset##_enable, S_IRUGO | S_IWUSR, \ show_pwm##offset##_enable, \ set_pwm##offset##_enable); \ +static DEVICE_ATTR(pwm##offset##_auto_enable, S_IRUGO | S_IWUSR, \ + show_pwm##offset##_auto_enable, \ + set_pwm##offset##_auto_enable); \ static DEVICE_ATTR(pwm##offset, S_IRUGO | S_IWUSR, \ - show_pwm##offset , set_pwm##offset ); + show_pwm##offset , set_pwm##offset ); \ +static DEVICE_ATTR(pwm##offset##_limit_off, S_IRUGO | S_IWUSR, \ + show_pwm##offset##_limit_off , set_pwm##offset##_limit_off ); \ +static DEVICE_ATTR(pwm##offset##_limit_start, S_IRUGO | S_IWUSR, \ + show_pwm##offset##_limit_start , set_pwm##offset##_limit_start ); \ +static DEVICE_ATTR(pwm##offset##_limit_full, S_IRUGO | S_IWUSR, \ + show_pwm##offset##_limit_full , set_pwm##offset##_limit_full ); \ +static DEVICE_ATTR(pwm##offset##_start, S_IRUGO | S_IWUSR, \ + show_pwm##offset##_start , set_pwm##offset##_start ); \ +static DEVICE_ATTR(pwm##offset##_temp_input, S_IRUGO | S_IWUSR, \ + show_pwm##offset##_temp_input , set_pwm##offset##_temp_input ); \ +static DEVICE_ATTR(pwm##offset##_step, S_IRUGO | S_IWUSR, \ + show_pwm##offset##_step , set_pwm##offset##_step );show_pwm_offset (1 );show_pwm_offset (2 );@@ -829,11 +1077,13 @@* and polarity set to active low is sign that this is the case so we * disable pwm control to protect the user. */ enable_pwm_interface =1 ; - tmp =it87_read_value (new_client,IT87_REG_FAN_CTL ); -if ((tmp &0x87 ) =0 ) { - enable_pwm_interface =0 ; -dev_info (&new_client->dev, -"detected broken BIOS defaults, disabling pwm interface" ); +if (!force_pwm) { + tmp =it87_read_value (new_client,IT87_REG_FAN_CTL ); +if ((tmp &0x87 ) =0 ) { + enable_pwm_interface =0 ; +dev_info (&new_client->dev, +"detected broken BIOS defaults, disabling pwm interface" ); + } }/* Register sysfs hooks */ @@ -888,9 +1138,30 @@device_create_file (&new_client->dev, &dev_attr_pwm1_enable);device_create_file (&new_client->dev, &dev_attr_pwm2_enable);device_create_file (&new_client->dev, &dev_attr_pwm3_enable); +device_create_file (&new_client->dev, &dev_attr_pwm1_auto_enable); +device_create_file (&new_client->dev, &dev_attr_pwm2_auto_enable); +device_create_file (&new_client->dev, &dev_attr_pwm3_auto_enable);device_create_file (&new_client->dev, &dev_attr_pwm1);device_create_file (&new_client->dev, &dev_attr_pwm2);device_create_file (&new_client->dev, &dev_attr_pwm3); +device_create_file (&new_client->dev, &dev_attr_pwm1_limit_off); +device_create_file (&new_client->dev, &dev_attr_pwm2_limit_off); +device_create_file (&new_client->dev, &dev_attr_pwm3_limit_off); +device_create_file (&new_client->dev, &dev_attr_pwm1_limit_start); +device_create_file (&new_client->dev, &dev_attr_pwm2_limit_start); +device_create_file (&new_client->dev, &dev_attr_pwm3_limit_start); +device_create_file (&new_client->dev, &dev_attr_pwm1_limit_full); +device_create_file (&new_client->dev, &dev_attr_pwm2_limit_full); +device_create_file (&new_client->dev, &dev_attr_pwm3_limit_full); +device_create_file (&new_client->dev, &dev_attr_pwm1_start); +device_create_file (&new_client->dev, &dev_attr_pwm2_start); +device_create_file (&new_client->dev, &dev_attr_pwm3_start); +device_create_file (&new_client->dev, &dev_attr_pwm1_temp_input); +device_create_file (&new_client->dev, &dev_attr_pwm2_temp_input); +device_create_file (&new_client->dev, &dev_attr_pwm3_temp_input); +device_create_file (&new_client->dev, &dev_attr_pwm1_step); +device_create_file (&new_client->dev, &dev_attr_pwm2_step); +device_create_file (&new_client->dev, &dev_attr_pwm3_step); }if (data->type = it8712) {@@ -971,17 +1242,6 @@ {int tmp, i; -/* initialize to sane defaults: -* - if the chip is in manual pwm mode, this will be overwritten with -* the actual settings on the chip (so in this case, initialization -* is not needed) -* - if in automatic or on/off mode, we could switch to manual mode, -* read the registers and set manual_pwm_ctl accordingly, but currently -* this is not implemented, so we initialize to something sane */ -for (i =0 ; i <3 ; i++) { - data->manual_pwm_ctl[i] =0xff ; - } -/* Check if temperature channnels are reset manually or by some reason */ tmp =it87_read_value (client,IT87_REG_TEMP_ENABLE );if ((tmp &0x3f ) =0 ) {@@ -1006,24 +1266,21 @@it87_write_value (client,IT87_REG_FAN_MAIN_CTRL , data->fan_main_ctrl); } -/* Set current fan mode registers and the default settings for the -* other mode registers */ +/* Set current fan mode registers */ for (i =0 ; i <3 ; i++) { -if (data->fan_main_ctrl & (1 << i)) { -/* pwm mode */ - tmp =it87_read_value (client,IT87_REG_PWM (i)); -if (tmp &0x80 ) { -/* automatic pwm - not yet implemented, but -* leave the settings made by the BIOS alone -* until a change is requested via the sysfs -* interface */ - }else { -/* manual pwm */ - data->manual_pwm_ctl[i] =PWM_FROM_REG (tmp); - } - } + data->manual_pwm_ctl[i] =it87_read_value (client,IT87_REG_PWM (i)); } - + +/* Get fan control registers for SmartGuardian automatic mode */ + data->fan_ctrl =it87_read_value (client,IT87_REG_FAN_CTL ); +for (i =0 ; i <3 ; i++) { + data->fan_limit_off[i] =it87_read_value (client,IT87_REG_SG_TEMP_OFF (i)); + data->fan_limit_start[i] =it87_read_value (client,IT87_REG_SG_TEMP_START (i)); + data->fan_limit_full[i] =it87_read_value (client,IT87_REG_SG_TEMP_FULL (i)); + data->fan_limit_stpwm[i] =it87_read_value (client,IT87_REG_SG_START_PWM (i)); + data->fan_am_ctrl[i] =it87_read_value (client,IT87_REG_SG_AM_CTRL (i)); + } +/* Start monitoring */ it87_write_value (client,IT87_REG_CONFIG , (it87_read_value(client, IT87_REG_CONFIG) & 0x36)
it87.c line 1383
Quoted 4 lines
it87.c renamed from it87.c-orig +1
-0@@ -1123,9 +1380,12 @@MODULE_AUTHOR ("Chris Gauthron <[email protected]>" ); +MODULE_AUTHOR ("Sebastian Witt <[email protected]>" );
Are two allowed?
it87.c line 1386
Quoted 3 lines
it87.c renamed from it87.c-orig +1
-0MODULE_DESCRIPTION ("IT8705F, IT8712F, Sis950 driver" );module_param (update_vbat, bool,0 ); +module_param (force_pwm, bool,0 );
This does not help if it is undocumented and without any comment.
it87.c line 1388
Quoted 2 lines
it87.c renamed from it87.c-orig +1
-0MODULE_PARM_DESC (update_vbat,"Update vbat if set else return powerup value" ); +MODULE_PARM_DESC (force_pwm,"Force initializing PWM" );
This does not help if it is undocumented and without any comment.
Quoted 11 lines
it87.c renamed from it87.c-orig +0
-1MODULE_LICENSE ("GPL" );module_init (sm_it87_init); ------------------------------------------------------------------------_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
PATCH 1/1 : Add automatic PWM mode to it87.c
In reply to Sebastian WittQuoted 5 lines · Rudolf Marek
Hi Sebastian
I will put some comments into the code. I think those I made are enough for next iteration.
Hi Rudolf,
thanks for the comments, here's the next one. Patch against 2.6.12-rc5-mm2.
Regards,
Sebastian
-------------- next part --------------
drivers/i2c/chips/it87.c +257 -33
@@ -154,6 +154,12 @@#define IT87_REG_CHIPID 0x58 +#define IT87_REG_SG_TEMP_OFF (nr) (0x60 + (nr) * 8) +#define IT87_REG_SG_TEMP_START (nr) (0x61 + (nr) * 8) +#define IT87_REG_SG_TEMP_FULL (nr) (0x62 + (nr) * 8) +#define IT87_REG_SG_START_PWM (nr) (0x63 + (nr) * 8) +#define IT87_REG_SG_AM_CTRL (nr) (0x64 + (nr) * 8) +#define IN_TO_REG (val) (SENSORS_LIMIT((((val) + 8)/16),0,255))#define IN_FROM_REG (val) ((val) * 16)
@@ -213,6 +219,11 @@u32 alarms;/* Register encoding, combined */ u8 fan_main_ctrl;/* Register value */ u8 manual_pwm_ctl[3 ];/* manual PWM value set by user */ +u8 fan_limit_off[3 ];/* Register value */ +u8 fan_limit_start[3 ];/* Register value */ +u8 fan_limit_full[3 ];/* Register value */ +u8 fan_limit_stpwm[3 ];/* Register value */ +u8 fan_am_ctrl[3 ];/* Register value */ };
@@ -486,13 +497,52 @@ }static ssize_t show_pwm_enable (struct device *dev,char *buf,int nr) { +int val; +struct it87_data *data =it87_update_device (dev); -return sprintf (buf,"%d\n" , (data->fan_main_ctrl & (1 << nr)) ?1 :0 ); + +/* PWM enabled or disabled */ + val = (data->fan_main_ctrl & (1 << nr)) ?1 :0 ; +/* Automatic mode - SmartGuardian */ + val += (val && (data->manual_pwm_ctl[nr] &0x80 )) ?1 :0 ; + +return sprintf (buf,"%d\n" , val); }static ssize_t show_pwm (struct device *dev,char *buf,int nr) {struct it87_data *data =it87_update_device (dev); -return sprintf (buf,"%d\n" , data->manual_pwm_ctl[nr]); +/* Return 0 if automatic mode is enabled */ +return sprintf (buf,"%d\n" , (data->manual_pwm_ctl[nr] &0x80 ) ?0 :PWM_FROM_REG (data->manual_pwm_ctl[nr])); +} +static ssize_t show_pwm_limit_off (struct device *dev,char *buf,int nr) +{ +struct it87_data *data =it87_update_device (dev); +return sprintf (buf,"%d\n" , data->fan_limit_off[nr]); +} +static ssize_t show_pwm_limit_start (struct device *dev,char *buf,int nr) +{ +struct it87_data *data =it87_update_device (dev); +return sprintf (buf,"%d\n" , data->fan_limit_start[nr]); +} +static ssize_t show_pwm_limit_full (struct device *dev,char *buf,int nr) +{ +struct it87_data *data =it87_update_device (dev); +return sprintf (buf,"%d\n" , data->fan_limit_full[nr]); +} +static ssize_t show_pwm_start (struct device *dev,char *buf,int nr) +{ +struct it87_data *data =it87_update_device (dev); +return sprintf (buf,"%d\n" ,PWM_FROM_REG (data->fan_limit_stpwm[nr])); +} +static ssize_t show_pwm_temp_input (struct device *dev,char *buf,int nr) +{ +struct it87_data *data =it87_update_device (dev); +return sprintf (buf,"%d\n" , (data->manual_pwm_ctl[nr] &0x80 ) ? (data->manual_pwm_ctl[nr] &0x03 ) +1 :0 ); +} +static ssize_t show_pwm_step (struct device *dev,char *buf,int nr) +{ +struct it87_data *data =it87_update_device (dev); +return sprintf (buf,"%d\n" , data->fan_am_ctrl[nr] &0x07 ); }static ssize_t set_fan_min (struct device *dev,const char *buf,size_t count,int nr)
@@ -564,12 +614,28 @@/* set on/off mode */ data->fan_main_ctrl &= ~(1 << nr);it87_write_value (client,IT87_REG_FAN_MAIN_CTRL , data->fan_main_ctrl); - }else if (val =1 ) { + }else if ((val =1 ) || (val =2 )) {/* set SmartGuardian mode */ data->fan_main_ctrl |= (1 << nr);it87_write_value (client,IT87_REG_FAN_MAIN_CTRL , data->fan_main_ctrl); -/* set saved pwm value, clear FAN_CTLX PWM mode bit */ -it87_write_value (client,IT87_REG_PWM (nr),PWM_TO_REG (data->manual_pwm_ctl[nr])); +if (val =2 ) { +/* Set automatic SmartGuardian mode */ + data->manual_pwm_ctl[nr] |=0x80 ; +it87_write_value (client,IT87_REG_PWM (nr), data->manual_pwm_ctl[nr]); +/* Enable temperature smoothing and fan spin up time */ + data->fan_am_ctrl[nr] =0x80 |0x18 ; +it87_write_value (client,IT87_REG_SG_AM_CTRL (nr), data->fan_am_ctrl[nr]); +/* If start pwm and pwm step size not set, apply safe values */ +if (data->fan_limit_stpwm[nr] =0 ) { + data->fan_limit_stpwm[nr] =PWM_TO_REG (128 ); +it87_write_value (client,IT87_REG_SG_START_PWM (nr), data->fan_limit_stpwm[nr]); + } +if ((data->fan_am_ctrl[nr] &0x07 ) =0 ) { +/* 16 PWM values / K */ + data->fan_am_ctrl[nr] |=0x05 ; +it87_write_value (client,IT87_REG_SG_AM_CTRL (nr), data->fan_am_ctrl[nr]); + } + } }else {up (&data->update_lock);return -EINVAL ;
@@ -595,6 +661,93 @@up (&data->update_lock);return count; } +static ssize_t set_pwm_limit_off (struct device *dev,const char *buf, +size_t count,int nr) +{ +struct i2c_client *client =to_i2c_client (dev); +struct it87_data *data =i2c_get_clientdata (client); +int val =simple_strtol (buf,NULL ,10 ); + + data->fan_limit_off[nr] = val; +it87_write_value (client,IT87_REG_SG_TEMP_OFF (nr), data->fan_limit_off[nr]); + +return count; +} +static ssize_t set_pwm_limit_start (struct device *dev,const char *buf, +size_t count,int nr) +{ +struct i2c_client *client =to_i2c_client (dev); +struct it87_data *data =i2c_get_clientdata (client); +int val =simple_strtol (buf,NULL ,10 ); + + data->fan_limit_start[nr] = val; +it87_write_value (client,IT87_REG_SG_TEMP_START (nr), data->fan_limit_start[nr]); + +return count; +} +static ssize_t set_pwm_limit_full (struct device *dev,const char *buf, +size_t count,int nr) +{ +struct i2c_client *client =to_i2c_client (dev); +struct it87_data *data =i2c_get_clientdata (client); +int val =simple_strtol (buf,NULL ,10 ); + + data->fan_limit_full[nr] = val; +it87_write_value (client,IT87_REG_SG_TEMP_FULL (nr), data->fan_limit_full[nr]); + +return count; +} +static ssize_t set_pwm_start (struct device *dev,const char *buf, +size_t count,int nr) +{ +struct i2c_client *client =to_i2c_client (dev); +struct it87_data *data =i2c_get_clientdata (client); +int val =simple_strtol (buf,NULL ,10 ); + +if (val <0 || val >255 ) +return -EINVAL ; + + data->fan_limit_stpwm[nr] =PWM_TO_REG (val); +it87_write_value (client,IT87_REG_SG_START_PWM (nr), data->fan_limit_stpwm[nr]); + +return count; +} +static ssize_t set_pwm_temp_input (struct device *dev,const char *buf, +size_t count,int nr) +{ +struct i2c_client *client =to_i2c_client (dev); +struct it87_data *data =i2c_get_clientdata (client); +int val =simple_strtol (buf,NULL ,10 ); + +if (val <1 || val >3 ) +return -EINVAL ; + +/* Check if automatic control is enabled */ +if (data->manual_pwm_ctl[nr] &0x80 ) { + data->manual_pwm_ctl[nr] =0x80 | (val -1 ); +it87_write_value (client,IT87_REG_PWM (nr), data->manual_pwm_ctl[nr]); + } + +return count; +} +static ssize_t set_pwm_step (struct device *dev,const char *buf, +size_t count,int nr) +{ +struct i2c_client *client =to_i2c_client (dev); +struct it87_data *data =i2c_get_clientdata (client); +int val =simple_strtol (buf,NULL ,10 ); + +if (val <0 || val >7 ) +return -EINVAL ; + + data->fan_am_ctrl[nr] &= ~0x07 ; + data->fan_am_ctrl[nr] |= val; + +it87_write_value (client,IT87_REG_SG_AM_CTRL (nr), data->fan_am_ctrl[nr]); + + +return count; +}#define show_fan_offset (offset) \ static ssize_t show_fan_##offset (struct device *dev, struct device_attribute *attr, char *buf) \
@@ -639,6 +792,30 @@ { \return show_pwm (dev, buf, offset -1 ); \ } \ +static ssize_t show_pwm##offset##_limit_off (struct device *dev, struct device_attribute *attr, char *buf) \ +{ \ + return show_pwm_limit_off(dev, buf, offset - 1); \ +} \ +static ssize_t show_pwm##offset##_limit_start (struct device *dev, struct device_attribute *attr, char *buf) \ +{ \ + return show_pwm_limit_start(dev, buf, offset - 1); \ +} \ +static ssize_t show_pwm##offset##_limit_full (struct device *dev, struct device_attribute *attr, char *buf) \ +{ \ + return show_pwm_limit_full(dev, buf, offset - 1); \ +} \ +static ssize_t show_pwm##offset##_start (struct device *dev, struct device_attribute *attr, char *buf) \ +{ \ + return show_pwm_start(dev, buf, offset - 1); \ +} \ +static ssize_t show_pwm##offset##_temp_input (struct device *dev, struct device_attribute *attr, char *buf) \ +{ \ + return show_pwm_temp_input(dev, buf, offset - 1); \ +} \ +static ssize_t show_pwm##offset##_step (struct device *dev, struct device_attribute *attr, char *buf) \ +{ \ + return show_pwm_step(dev, buf, offset - 1); \ +} \ static ssize_t set_pwm##offset##_enable (struct device *dev, struct device_attribute *attr, \ const char *buf, size_t count) \ { \
@@ -649,11 +826,55 @@ { \return set_pwm (dev, buf, count, offset -1 ); \ } \ +static ssize_t set_pwm##offset##_limit_off (struct device *dev, struct device_attribute *attr, \ + const char *buf, size_t count) \ +{ \ + return set_pwm_limit_off(dev, buf, count, offset - 1); \ +} \ +static ssize_t set_pwm##offset##_limit_start (struct device *dev, struct device_attribute *attr, \ + const char *buf, size_t count) \ +{ \ + return set_pwm_limit_start(dev, buf, count, offset - 1); \ +} \ +static ssize_t set_pwm##offset##_limit_full (struct device *dev, struct device_attribute *attr, \ + const char *buf, size_t count) \ +{ \ + return set_pwm_limit_full(dev, buf, count, offset - 1); \ +} \ +static ssize_t set_pwm##offset##_start (struct device *dev, struct device_attribute *attr, \ + const char *buf, size_t count) \ +{ \ + return set_pwm_start(dev, buf, count, offset - 1); \ +} \ +static ssize_t set_pwm##offset##_temp_input (struct device *dev, struct device_attribute *attr, \ + const char *buf, size_t count) \ +{ \ + return set_pwm_temp_input(dev, buf, count, offset - 1); \ +} \ +static ssize_t set_pwm##offset##_step (struct device *dev, struct device_attribute *attr, \ + const char *buf, size_t count) \ +{ \ + return set_pwm_step(dev, buf, count, offset - 1); \ +} \ static DEVICE_ATTR(pwm##offset##_enable, S_IRUGO | S_IWUSR, \ show_pwm##offset##_enable, \ set_pwm##offset##_enable); \ static DEVICE_ATTR(pwm##offset, S_IRUGO | S_IWUSR, \ - show_pwm##offset , set_pwm##offset ); + show_pwm##offset , set_pwm##offset ); \ +static DEVICE_ATTR(pwm##offset##_limit_off, S_IRUGO | S_IWUSR, \ + show_pwm##offset##_limit_off , set_pwm##offset##_limit_off ); \ +static DEVICE_ATTR(pwm##offset##_limit_start, S_IRUGO | S_IWUSR, \ + show_pwm##offset##_limit_start , set_pwm##offset##_limit_start ); \ +static DEVICE_ATTR(pwm##offset##_limit_full, S_IRUGO | S_IWUSR, \ + show_pwm##offset##_limit_full , set_pwm##offset##_limit_full ); \ +static DEVICE_ATTR(pwm##offset##_start, S_IRUGO | S_IWUSR, \ + show_pwm##offset##_start , set_pwm##offset##_start ); \ +static DEVICE_ATTR(pwm##offset##_temp_input, S_IRUGO | S_IWUSR, \ + show_pwm##offset##_temp_input , set_pwm##offset##_temp_input ); \ +static DEVICE_ATTR(pwm##offset##_step, S_IRUGO | S_IWUSR, \ + show_pwm##offset##_step , set_pwm##offset##_step ); + +show_pwm_offset (1 );show_pwm_offset (2 );
@@ -915,6 +1136,24 @@device_create_file (&new_client->dev, &dev_attr_pwm1);device_create_file (&new_client->dev, &dev_attr_pwm2);device_create_file (&new_client->dev, &dev_attr_pwm3); +device_create_file (&new_client->dev, &dev_attr_pwm1_limit_off); +device_create_file (&new_client->dev, &dev_attr_pwm2_limit_off); +device_create_file (&new_client->dev, &dev_attr_pwm3_limit_off); +device_create_file (&new_client->dev, &dev_attr_pwm1_limit_start); +device_create_file (&new_client->dev, &dev_attr_pwm2_limit_start); +device_create_file (&new_client->dev, &dev_attr_pwm3_limit_start); +device_create_file (&new_client->dev, &dev_attr_pwm1_limit_full); +device_create_file (&new_client->dev, &dev_attr_pwm2_limit_full); +device_create_file (&new_client->dev, &dev_attr_pwm3_limit_full); +device_create_file (&new_client->dev, &dev_attr_pwm1_start); +device_create_file (&new_client->dev, &dev_attr_pwm2_start); +device_create_file (&new_client->dev, &dev_attr_pwm3_start); +device_create_file (&new_client->dev, &dev_attr_pwm1_temp_input); +device_create_file (&new_client->dev, &dev_attr_pwm2_temp_input); +device_create_file (&new_client->dev, &dev_attr_pwm3_temp_input); +device_create_file (&new_client->dev, &dev_attr_pwm1_step); +device_create_file (&new_client->dev, &dev_attr_pwm2_step); +device_create_file (&new_client->dev, &dev_attr_pwm3_step); }if (data->type = it8712) {
@@ -1045,17 +1284,6 @@ {int tmp, i; -/* initialize to sane defaults: -* - if the chip is in manual pwm mode, this will be overwritten with -* the actual settings on the chip (so in this case, initialization -* is not needed) -* - if in automatic or on/off mode, we could switch to manual mode, -* read the registers and set manual_pwm_ctl accordingly, but currently -* this is not implemented, so we initialize to something sane */ -for (i =0 ; i <3 ; i++) { - data->manual_pwm_ctl[i] =0xff ; - } -/* Check if temperature channnels are reset manually or by some reason */ tmp =it87_read_value (client,IT87_REG_TEMP_ENABLE );if ((tmp &0x3f ) =0 ) {
@@ -1080,24 +1308,20 @@it87_write_value (client,IT87_REG_FAN_MAIN_CTRL , data->fan_main_ctrl); } -/* Set current fan mode registers and the default settings for the -* other mode registers */ +/* Set current fan mode registers */ for (i =0 ; i <3 ; i++) { -if (data->fan_main_ctrl & (1 << i)) { -/* pwm mode */ - tmp =it87_read_value (client,IT87_REG_PWM (i)); -if (tmp &0x80 ) { -/* automatic pwm - not yet implemented, but -* leave the settings made by the BIOS alone -* until a change is requested via the sysfs -* interface */ - }else { -/* manual pwm */ - data->manual_pwm_ctl[i] =PWM_FROM_REG (tmp); - } - } + data->manual_pwm_ctl[i] =it87_read_value (client,IT87_REG_PWM (i)); } - + +/* Get fan control registers for SmartGuardian automatic mode */ +for (i =0 ; i <3 ; i++) { + data->fan_limit_off[i] =it87_read_value (client,IT87_REG_SG_TEMP_OFF (i)); + data->fan_limit_start[i] =it87_read_value (client,IT87_REG_SG_TEMP_START (i)); + data->fan_limit_full[i] =it87_read_value (client,IT87_REG_SG_TEMP_FULL (i)); + data->fan_limit_stpwm[i] =it87_read_value (client,IT87_REG_SG_START_PWM (i)); + data->fan_am_ctrl[i] =it87_read_value (client,IT87_REG_SG_AM_CTRL (i)); + } +/* Start monitoring */ it87_write_value (client,IT87_REG_CONFIG , (it87_read_value(client, IT87_REG_CONFIG) & 0x36)
PATCH 1/1 : Add automatic PWM mode to it87.c
In reply to Sebastian WittHi Sebastian,
Quoted 2 lines
thanks for the comments, here's the next one. Patch against 2.6.12-rc5-mm2.
Thanks for taking Rudolf's comment into account, and for your work on the it87 driver in general. Several users have been asking for the Smart Guardian feature already.
However, there are a few problems with your patch.
First, there is a big pending patch for it87 which updates the driver to make use of the new sysfs callbacks. I wouldn't want your patch to collide with this one, so I'd like you to build your patch on top of this one.
http://lists.lm-sensors.org/pipermail/lm-sensors/2005-June/012597.html
This will also have the advantage that you'll see how the new sysfs callbacks can be written, and this should hopefully make your code much more simple.
Second, your interface doesn't quite comply with the standard automatic PWM interface as defined in Documentation/i2c/sysfs-interface. Please take a look. The interface basically defines trip points (PWM, temperature "coordinates"). You should come to something similar to what you have already by just renaming a few files, except for the slope/step concept which won't fit directly in, so I think you'll have to add some arithmetics (compute the slope from the requested trip point "coordinates", and vice-versa).
Third, the old datasheets for the IT8712F and IT8705F suggest that the first revisions of both chips had a different SmartGuardian implementation, trip-point based. If this is confirmed, then you will have to check the revision of the chip before enabling your interface. Which revision to you have yourself? I'll try to experiment on my IT8705F rev.2.
Can you provide a dump of your chip at startup (before loading the it87 driver)?
Thanks,
Signature · 1 line
Jean Delvare
PATCH 1/1 : Add automatic PWM mode to it87.c
In reply to Sebastian WittHi Jean,
Quoted 11 lines · Jean Delvare
First, there is a big pending patch for it87 which updates the driver to make use of the new sysfs callbacks. I wouldn't want your patch to collide with this one, so I'd like you to build your patch on top of this one.
http://lists.lm-sensors.org/pipermail/lm-sensors/2005-June/012597.html
This will also have the advantage that you'll see how the new sysfs callbacks can be written, and this should hopefully make your code much more simple.
Ok, is this patch against 2.6.12-rc5? Because it fails to apply.
Quoted 8 lines
Second, your interface doesn't quite comply with the standard automatic PWM interface as defined in
Documentation/i2c/sysfs-interface. Please take a look. The interface basically defines trip points (PWM, temperature "coordinates"). You should come to something similar to what you have already by just renaming a few files, except for the slope/step concept which won't fit directly in, so I think you'll have to add some arithmetics (compute the slope from the requested trip point "coordinates", and vice-versa).
Yes, no problem.
Quoted 6 lines
Third, the old datasheets for the IT8712F and IT8705F suggest that the first revisions of both chips had a different SmartGuardian implementation, trip-point based. If this is confirmed, then you will have to check the revision of the chip before enabling your interface. Which revision to you have yourself? I'll try to experiment on my IT8705F rev.2.
it87: Found IT8705F chip at 0x290, revision 3
Quoted 2 lines
Can you provide a dump of your chip at startup (before loading the it87 driver)?
0 1 2 3 4 5 6 7 8 9 a b c d e f
00: 13 00 ff 00 ff ff fd 00 00 00 3f 5b 00 48 ff ff 10: ff ff ff 30 00 00 00 00 00 ff ff ff ff ff ff ff 20: 44 a1 cb b6 b6 ba ca b3 ff 23 16 03 51 c1 c1 c1 30: ff ff ff ff ff ff ff ff ff fd ff ff ff ff ff ff 40: 3f 7f 3f 7f 3f 7f ff ff 2d ff ff ff ff ff ff ff 50: ff 2a 7f 7f 7f ff 5d 89 90 5d f9 12 80 00 00 00 60: 7f 7f 7f 00 00 ff ff ff 7f 7f 7f 00 00 ff ff ff 70: 7f 7f 7f 00 00 ff ff ff ff ff ff ff ff ff ff ff 80: 13 00 00 00 ff ff fd 00 00 00 3f 5b 00 48 ff ff 90: ff ff ff 30 00 00 00 00 00 ff ff ff ff ff ff ff
a0: 44 a1 cb b6 b6 ba ca b3 ff 23 16 03 51 c1 c1 c1
b0: ff ff ff ff ff ff ff ff ff fd ff ff ff ff ff ff
c0: 3f 7f 3f 7f 3f 7f ff ff 2d ff ff ff ff ff ff ff
d0: ff 2a 7f 7f 7f ff 5d 89 90 5d f9 12 80 00 00 00
e0: 7f 7f 7f 00 00 ff ff ff 7f 7f 7f 00 00 ff ff ff
f0: 7f 7f 7f 00 00 ff ff ff ff ff ff ff ff ff ff ff
Regards,
Sebastian
PATCH 1/1 : Add automatic PWM mode to it87.c
In reply to Sebastian WittHi Sebastian,
Quoted 7 lines
Quoted 5 lines
http://lists.lm-sensors.org/pipermail/lm-sensors/2005-June/012597.html
This will also have the advantage that you'll see how the new sysfs callbacks can be written, and this should hopefully make your code much more simple.
Ok, is this patch against 2.6.12-rc5? Because it fails to apply.
It was against Greg's current i2c tree as I wrote it. The easier for you would be to start from 2.6.12-rc6 + this patch:
http://khali.linux-fr.org/devel/i2c/linux-2.6/linux-2.6.12-rc6-i2c.patch.gz
Or the latest -mm, at your option (but be aware that linux-2.6.12-rc6-mm1 has a few known issues WRT hardware monitoring.)
Both include the all the latest it87 patches.
Quoted 1 line
it87: Found IT8705F chip at
0x290, revision 3
Hey, lucky you. I'd like to have this one, but unfortunately mine is revision 2 only. So, yours implements the newer auto-pwm model, while mine implements the old one. You might even get VID readings if your chip was wired for this.
Signature · 1 line
Jean Delvare
PATCH 1/1 : Add automatic PWM mode to it87.c
In reply to Sebastian WittHi,
Quoted 10 lines · Jean Delvare
It was against Greg's current i2c tree as I wrote it. The easier for you would be to start from 2.6.12-rc6 + this patch:
http://khali.linux-fr.org/devel/i2c/linux-2.6/linux-2.6.12-rc6-i2c.patch.gz
Or the latest -mm, at your option (but be aware that linux-2.6.12-rc6-mm1 has a few known issues WRT hardware monitoring.)
Both include the all the latest it87 patches.
Ok, here's a patch against 2.6.12-rc6.
Quoted 6 lines
Quoted 1 line
it87: Found IT8705F chip at
0x290, revision 3Hey, lucky you. I'd like to have this one, but unfortunately mine is revision 2 only. So, yours implements the newer auto-pwm model, while mine implements the old one. You might even get VID readings if your chip was wired for this.
I've now added only support for the auto-pwm with slope setting and a check for revision 3.
Bye,
Sebastian
-------------- next part --------------
drivers/i2c/chips/it87.c +292 -18
@@ -111,6 +111,7 @@/* Chip Type */ static u16 chip_type; +static u8 chip_revision;/* Many IT87 constants specified below */
@@ -155,6 +156,12 @@#define IT87_REG_CHIPID 0x58 +#define IT87_REG_SG_TEMP_OFF (nr) (0x60 + (nr) * 8) +#define IT87_REG_SG_TEMP_START (nr) (0x61 + (nr) * 8) +#define IT87_REG_SG_TEMP_FULL (nr) (0x62 + (nr) * 8) +#define IT87_REG_SG_START_PWM (nr) (0x63 + (nr) * 8) +#define IT87_REG_SG_AM_CTRL (nr) (0x64 + (nr) * 8) +#define IN_TO_REG (val) (SENSORS_LIMIT((((val) + 8)/16),0,255))#define IN_FROM_REG (val) ((val) * 16)
@@ -185,6 +192,7 @@ }#define DIV_FROM_REG (val) (1 << (val)) +#define SLOPE_FROM_REG (val) ((val) ? (1 << (val-1)) : 0)/* For each registered IT87, we need to keep some data in memory. That data is pointed to by it87_list[NR]->data. The structure itself is
@@ -214,6 +222,12 @@u32 alarms;/* Register encoding, combined */ u8 fan_main_ctrl;/* Register value */ u8 manual_pwm_ctl[3 ];/* manual PWM value set by user */ +u8 fan_limit_off[3 ];/* Register value */ +u8 fan_limit_start[3 ];/* Register value */ +u8 fan_limit_full[3 ];/* Register value */ +u8 fan_limit_stpwm[3 ];/* Register value */ +u8 fan_am_ctrl[3 ];/* Register value */ +u8 fan_point2_pwm[3 ];/* Used for slope calculation */ };
@@ -454,6 +468,31 @@show_sensor_offset (2 );show_sensor_offset (3 ); +static void calc_slope (struct it87_data *data,int nr) +{ +u8 val = data->fan_point2_pwm[nr]; + +if (data->fan_limit_full[nr] <= data->fan_limit_start[nr]) + val =14 ; +else { + val -= data->fan_limit_stpwm[nr]; + val /= (data->fan_limit_full[nr] - data->fan_limit_start[nr]); + } + + data->fan_am_ctrl[nr] &= ~0x03 ; +if (val >48 ) + data->fan_am_ctrl[nr] |=0x07 ;/* 64 PWM / K */ +else if (val >24 ) + data->fan_am_ctrl[nr] |=0x06 ;/* 32 PWM / K */ +else if (val >12 ) + data->fan_am_ctrl[nr] |=0x05 ;/* 16 PWM / K */ +else if (val >6 ) + data->fan_am_ctrl[nr] |=0x04 ;/* 8 PWM / K */ +else if (val >3 ) + data->fan_am_ctrl[nr] |=0x03 ;/* 4 PWM / K */ +else + data->fan_am_ctrl[nr] |=0x02 ;/* 2 PWM / K */ +}/* 3 Fans */ static ssize_t show_fan (struct device *dev,struct device_attribute *attr,char *buf)
@@ -489,9 +528,15 @@ {struct sensor_device_attribute *sensor_attr =to_sensor_dev_attr (attr);int nr = sensor_attr->index; +int val;struct it87_data *data =it87_update_device (dev); -return sprintf (buf,"%d\n" , (data->fan_main_ctrl & (1 << nr)) ?1 :0 ); +/* PWM enabled or disabled */ + val = (data->fan_main_ctrl & (1 << nr)) ?1 :0 ; +/* Automatic mode - SmartGuardian */ + val += (val && (data->manual_pwm_ctl[nr] &0x80 )) ?1 :0 ; + +return sprintf (buf,"%d\n" , val); }static ssize_t show_pwm (struct device *dev,struct device_attribute *attr,char *buf)
@@ -500,7 +545,64 @@int nr = sensor_attr->index;struct it87_data *data =it87_update_device (dev); -return sprintf (buf,"%d\n" , data->manual_pwm_ctl[nr]); +/* Return 0 if automatic mode is enabled */ +return sprintf (buf,"%d\n" , (data->manual_pwm_ctl[nr] &0x80 ) ?0 :PWM_FROM_REG (data->manual_pwm_ctl[nr])); +} +static ssize_t show_pwm_auto_channels_temp (struct device *dev,struct device_attribute *attr, +char *buf) +{ +struct sensor_device_attribute *sensor_attr =to_sensor_dev_attr (attr); +int nr = sensor_attr->index; + +struct it87_data *data =it87_update_device (dev); + +return sprintf (buf,"%d\n" , (data->manual_pwm_ctl[nr] &0x80 ) ? + (1 << (data->manual_pwm_ctl[nr] &0x03 )):0 ); +} +static ssize_t show_pwm_auto_point1_pwm (struct device *dev,struct device_attribute *attr, +char *buf) +{ +struct sensor_device_attribute *sensor_attr =to_sensor_dev_attr (attr); +int nr = sensor_attr->index; + +struct it87_data *data =it87_update_device (dev); +return sprintf (buf,"%d\n" ,PWM_FROM_REG (data->fan_limit_stpwm[nr])); +} +static ssize_t show_pwm_auto_point1_temp (struct device *dev,struct device_attribute *attr, +char *buf) +{ +struct sensor_device_attribute *sensor_attr =to_sensor_dev_attr (attr); +int nr = sensor_attr->index; + +struct it87_data *data =it87_update_device (dev); +return sprintf (buf,"%d\n" , data->fan_limit_start[nr]); +} +static ssize_t show_pwm_auto_point1_temp_hyst (struct device *dev,struct device_attribute *attr, +char *buf) +{ +struct sensor_device_attribute *sensor_attr =to_sensor_dev_attr (attr); +int nr = sensor_attr->index; + +struct it87_data *data =it87_update_device (dev); +return sprintf (buf,"%d\n" , data->fan_limit_off[nr]); +} +static ssize_t show_pwm_auto_point2_pwm (struct device *dev,struct device_attribute *attr, +char *buf) +{ +struct sensor_device_attribute *sensor_attr =to_sensor_dev_attr (attr); +int nr = sensor_attr->index; + +struct it87_data *data =it87_update_device (dev); +return sprintf (buf,"%d\n" , data->fan_point2_pwm[nr]); +} +static ssize_t show_pwm_auto_point2_temp (struct device *dev,struct device_attribute *attr, +char *buf) +{ +struct sensor_device_attribute *sensor_attr =to_sensor_dev_attr (attr); +int nr = sensor_attr->index; + +struct it87_data *data =it87_update_device (dev); +return sprintf (buf,"%d\n" , data->fan_limit_full[nr]); }static ssize_t set_fan_min (struct device *dev,struct device_attribute *attr,const char *buf,size_t count)
@@ -581,12 +683,33 @@/* set on/off mode */ data->fan_main_ctrl &= ~(1 << nr);it87_write_value (client,IT87_REG_FAN_MAIN_CTRL , data->fan_main_ctrl); - }else if (val =1 ) { + }else if ((val =1 ) || (val =2 )) {/* set SmartGuardian mode */ data->fan_main_ctrl |= (1 << nr);it87_write_value (client,IT87_REG_FAN_MAIN_CTRL , data->fan_main_ctrl); -/* set saved pwm value, clear FAN_CTLX PWM mode bit */ -it87_write_value (client,IT87_REG_PWM (nr),PWM_TO_REG (data->manual_pwm_ctl[nr])); +if ((val =2 ) && (chip_revision =0x03 )) { +/* Set automatic SmartGuardian mode */ + data->manual_pwm_ctl[nr] |=0x80 ; +it87_write_value (client,IT87_REG_PWM (nr), data->manual_pwm_ctl[nr]); +/* Enable temperature smoothing and fan spin up time */ + data->fan_am_ctrl[nr] =0x80 |0x18 ; +it87_write_value (client,IT87_REG_SG_AM_CTRL (nr), data->fan_am_ctrl[nr]); +/* If start pwm and pwm step size not set, apply safe values */ +if (data->fan_limit_stpwm[nr] =0 ) { + data->fan_limit_stpwm[nr] =PWM_TO_REG (128 ); +it87_write_value (client,IT87_REG_SG_START_PWM (nr), data->fan_limit_stpwm[nr]); + } +if ((data->fan_am_ctrl[nr] &0x07 ) =0 ) { +/* 16 PWM values / K */ + data->fan_am_ctrl[nr] |=0x05 ; +it87_write_value (client,IT87_REG_SG_AM_CTRL (nr), data->fan_am_ctrl[nr]); + } + } +else { +/* Disable automatic SmartGuardian mode */ + data->manual_pwm_ctl[nr] &= ~0x80 ; +it87_write_value (client,IT87_REG_PWM (nr), data->manual_pwm_ctl[nr]); + } }else {up (&data->update_lock);return -EINVAL ;
@@ -615,7 +738,123 @@up (&data->update_lock);return count; } +static ssize_t set_pwm_auto_point1_pwm (struct device *dev,struct device_attribute *attr, +const char *buf,size_t count) +{ +struct sensor_device_attribute *sensor_attr =to_sensor_dev_attr (attr); +int nr = sensor_attr->index; + +struct i2c_client *client =to_i2c_client (dev); +struct it87_data *data =i2c_get_clientdata (client); +int val =simple_strtol (buf,NULL ,10 ); + +if (val <0 || val >255 ) +return -EINVAL ; + +down (&data->update_lock); + data->fan_limit_stpwm[nr] =PWM_TO_REG (val); +calc_slope (data, nr); +it87_write_value (client,IT87_REG_SG_AM_CTRL (nr), data->fan_am_ctrl[nr]); +it87_write_value (client,IT87_REG_SG_START_PWM (nr), data->fan_limit_stpwm[nr]); +up (&data->update_lock); +return count; +} +static ssize_t set_pwm_auto_point1_temp (struct device *dev,struct device_attribute *attr, +const char *buf,size_t count) +{ +struct sensor_device_attribute *sensor_attr =to_sensor_dev_attr (attr); +int nr = sensor_attr->index; +struct i2c_client *client =to_i2c_client (dev); +struct it87_data *data =i2c_get_clientdata (client); +int val =simple_strtol (buf,NULL ,10 ); + +down (&data->update_lock); + data->fan_limit_start[nr] = val; +calc_slope (data, nr); +it87_write_value (client,IT87_REG_SG_AM_CTRL (nr), data->fan_am_ctrl[nr]); +it87_write_value (client,IT87_REG_SG_TEMP_START (nr), data->fan_limit_start[nr]); +up (&data->update_lock); +return count; +} +static ssize_t set_pwm_auto_point1_temp_hyst (struct device *dev,struct device_attribute *attr, +const char *buf,size_t count) +{ +struct sensor_device_attribute *sensor_attr =to_sensor_dev_attr (attr); +int nr = sensor_attr->index; + +struct i2c_client *client =to_i2c_client (dev); +struct it87_data *data =i2c_get_clientdata (client); +int val =simple_strtol (buf,NULL ,10 ); + +down (&data->update_lock); + data->fan_limit_off[nr] = val; +it87_write_value (client,IT87_REG_SG_TEMP_OFF (nr), data->fan_limit_off[nr]); +up (&data->update_lock); +return count; +} +static ssize_t set_pwm_auto_point2_pwm (struct device *dev,struct device_attribute *attr, +const char *buf,size_t count) +{ +struct sensor_device_attribute *sensor_attr =to_sensor_dev_attr (attr); +int nr = sensor_attr->index; + +struct i2c_client *client =to_i2c_client (dev); +struct it87_data *data =i2c_get_clientdata (client); +int val =simple_strtol (buf,NULL ,10 ); + +if (val <0 || val >255 ) +return -EINVAL ; + +down (&data->update_lock); + data->fan_point2_pwm[nr] = val; +calc_slope (data, nr); +it87_write_value (client,IT87_REG_SG_AM_CTRL (nr), data->fan_am_ctrl[nr]); +up (&data->update_lock); +return count; +} +static ssize_t set_pwm_auto_point2_temp (struct device *dev,struct device_attribute *attr, +const char *buf,size_t count) +{ +struct sensor_device_attribute *sensor_attr =to_sensor_dev_attr (attr); +int nr = sensor_attr->index; + +struct i2c_client *client =to_i2c_client (dev); +struct it87_data *data =i2c_get_clientdata (client); +int val =simple_strtol (buf,NULL ,10 ); + +down (&data->update_lock); + data->fan_limit_full[nr] = val; +calc_slope (data, nr); +it87_write_value (client,IT87_REG_SG_AM_CTRL (nr), data->fan_am_ctrl[nr]); +it87_write_value (client,IT87_REG_SG_TEMP_FULL (nr), data->fan_limit_full[nr]); +up (&data->update_lock); +return count; +} +static ssize_t set_pwm_auto_channels_temp (struct device *dev,struct device_attribute *attr, +const char *buf,size_t count) +{ +struct sensor_device_attribute *sensor_attr =to_sensor_dev_attr (attr); +int nr = sensor_attr->index; + +struct i2c_client *client =to_i2c_client (dev); +struct it87_data *data =i2c_get_clientdata (client); +int val =simple_strtol (buf,NULL ,10 ); + +if (val <0 || val >4 ) +return -EINVAL ; + +/* Check if automatic control is enabled */ +if (data->manual_pwm_ctl[nr] &0x80 ) { +down (&data->update_lock); + data->manual_pwm_ctl[nr] =0x80 | (val >>1 ); +it87_write_value (client,IT87_REG_PWM (nr), data->manual_pwm_ctl[nr]); +up (&data->update_lock); + } + +return count; +} +#define show_fan_offset (offset) \ static SENSOR_DEVICE_ATTR(fan##offset##_input, S_IRUGO, \ show_fan, NULL, offset - 1); \
@@ -632,7 +871,19 @@ static SENSOR_DEVICE_ATTR(pwm##offset##_enable, S_IRUGO | S_IWUSR, \ show_pwm_enable, set_pwm_enable, offset - 1); \ static SENSOR_DEVICE_ATTR(pwm##offset, S_IRUGO | S_IWUSR, \ - show_pwm, set_pwm, offset - 1); + show_pwm, set_pwm, offset - 1); \ +static SENSOR_DEVICE_ATTR(pwm##offset##_auto_channels_temp, S_IRUGO | S_IWUSR, \ + show_pwm_auto_channels_temp, set_pwm_auto_channels_temp, offset - 1); \ +static SENSOR_DEVICE_ATTR(pwm##offset##_auto_point1_pwm, S_IRUGO | S_IWUSR, \ + show_pwm_auto_point1_pwm, set_pwm_auto_point1_pwm, offset - 1); \ +static SENSOR_DEVICE_ATTR(pwm##offset##_auto_point1_temp, S_IRUGO | S_IWUSR, \ + show_pwm_auto_point1_temp, set_pwm_auto_point1_temp, offset - 1); \ +static SENSOR_DEVICE_ATTR(pwm##offset##_auto_point1_temp_hyst, S_IRUGO | S_IWUSR, \ + show_pwm_auto_point1_temp_hyst, set_pwm_auto_point1_temp_hyst, offset - 1);\ +static SENSOR_DEVICE_ATTR(pwm##offset##_auto_point2_pwm, S_IRUGO | S_IWUSR, \ + show_pwm_auto_point2_pwm, set_pwm_auto_point2_pwm, offset - 1); \ +static SENSOR_DEVICE_ATTR(pwm##offset##_auto_point2_temp, S_IRUGO | S_IWUSR, \ + show_pwm_auto_point2_temp, set_pwm_auto_point2_temp, offset - 1);show_pwm_offset (1 );show_pwm_offset (2 );
@@ -713,8 +964,9 @@ } err =0 ; + chip_revision =superio_inb (DEVREV ) &0x0f ;pr_info ("it87: Found IT%04xF chip at 0x%x, revision %d\n" , - chip_type, *address,superio_inb (DEVREV ) &0x0f ); + chip_type, *address, chip_revision); exit:superio_exit ();
@@ -894,6 +1146,27 @@device_create_file (&new_client->dev, &sensor_dev_attr_pwm1.dev_attr);device_create_file (&new_client->dev, &sensor_dev_attr_pwm2.dev_attr);device_create_file (&new_client->dev, &sensor_dev_attr_pwm3.dev_attr); +/* Enable automatic SmartGuardian mode */ +if (chip_revision =0x03 ) { +device_create_file (&new_client->dev, &sensor_dev_attr_pwm1_auto_channels_temp.dev_attr); +device_create_file (&new_client->dev, &sensor_dev_attr_pwm2_auto_channels_temp.dev_attr); +device_create_file (&new_client->dev, &sensor_dev_attr_pwm3_auto_channels_temp.dev_attr); +device_create_file (&new_client->dev, &sensor_dev_attr_pwm1_auto_point1_pwm.dev_attr); +device_create_file (&new_client->dev, &sensor_dev_attr_pwm2_auto_point1_pwm.dev_attr); +device_create_file (&new_client->dev, &sensor_dev_attr_pwm3_auto_point1_pwm.dev_attr); +device_create_file (&new_client->dev, &sensor_dev_attr_pwm1_auto_point1_temp.dev_attr); +device_create_file (&new_client->dev, &sensor_dev_attr_pwm2_auto_point1_temp.dev_attr); +device_create_file (&new_client->dev, &sensor_dev_attr_pwm3_auto_point1_temp.dev_attr); +device_create_file (&new_client->dev, &sensor_dev_attr_pwm1_auto_point1_temp_hyst.dev_attr); +device_create_file (&new_client->dev, &sensor_dev_attr_pwm2_auto_point1_temp_hyst.dev_attr); +device_create_file (&new_client->dev, &sensor_dev_attr_pwm3_auto_point1_temp_hyst.dev_attr); +device_create_file (&new_client->dev, &sensor_dev_attr_pwm1_auto_point2_pwm.dev_attr); +device_create_file (&new_client->dev, &sensor_dev_attr_pwm2_auto_point2_pwm.dev_attr); +device_create_file (&new_client->dev, &sensor_dev_attr_pwm3_auto_point2_pwm.dev_attr); +device_create_file (&new_client->dev, &sensor_dev_attr_pwm1_auto_point2_temp.dev_attr); +device_create_file (&new_client->dev, &sensor_dev_attr_pwm2_auto_point2_temp.dev_attr); +device_create_file (&new_client->dev, &sensor_dev_attr_pwm3_auto_point2_temp.dev_attr); + } }if (data->type = it8712) {
@@ -1024,17 +1297,6 @@ {int tmp, i; -/* initialize to sane defaults: -* - if the chip is in manual pwm mode, this will be overwritten with -* the actual settings on the chip (so in this case, initialization -* is not needed) -* - if in automatic or on/off mode, we could switch to manual mode, -* read the registers and set manual_pwm_ctl accordingly, but currently -* this is not implemented, so we initialize to something sane */ -for (i =0 ; i <3 ; i++) { - data->manual_pwm_ctl[i] =0xff ; - } -/* Check if temperature channnels are reset manually or by some reason */ tmp =it87_read_value (client,IT87_REG_TEMP_ENABLE );if ((tmp &0x3f ) =0 ) {
@@ -1059,6 +1321,18 @@it87_write_value (client,IT87_REG_FAN_MAIN_CTRL , data->fan_main_ctrl); } +/* Set current fan mode registers */ +for (i =0 ; i <3 ; i++) { + data->manual_pwm_ctl[i] =it87_read_value (client,IT87_REG_PWM (i)); + +/* Get fan control registers for SmartGuardian automatic mode */ + data->fan_limit_off[i] =it87_read_value (client,IT87_REG_SG_TEMP_OFF (i)); + data->fan_limit_start[i] =it87_read_value (client,IT87_REG_SG_TEMP_START (i)); + data->fan_limit_full[i] =it87_read_value (client,IT87_REG_SG_TEMP_FULL (i)); + data->fan_limit_stpwm[i] =it87_read_value (client,IT87_REG_SG_START_PWM (i)); + data->fan_am_ctrl[i] =it87_read_value (client,IT87_REG_SG_AM_CTRL (i)); + } +/* Set current fan mode registers and the default settings for the * other mode registers */ for (i =0 ; i <3 ; i++) {
PATCH 1/1 : Add automatic PWM mode to it87.c
In reply to Sebastian WittHi,
I've found out that I made a mistake: I used the IT8712_V07 datasheet and not the datasheet for the IT8705F. That's because my last board has a IT8712F and my current board has now a IT8705F...seen it while trying to get the VID reading.
However the automatic fan control registers/values are on both chipsets the same, I haven't seen a difference in the datasheets. So automatic fan control should work on both chipsets. The check for revision/version should be fixed.
Regards,
Sebastian
PATCH 1/1 : Add automatic PWM mode to it87.c
In reply to Sebastian WittHi Sebastian,
Quoted 9 lines · Sebastian Witt
I've found out that I made a mistake: I used the
IT8712_V07datasheet and not the datasheet for the IT8705F. That's because my last board has a IT8712F and my current board has now a IT8705F...seen it while trying to get the VID reading.However the automatic fan control registers/values are on both chipsets the same, I haven't seen a difference in the datasheets. So automatic fan control should work on both chipsets. The check for revision/version should be fixed.
Agreed, your code should work (and be enabled) on both the latest IT8505F and the latest IT8712F chips. I'm sorry I didn't have the time to review your patch yet, I hopefully will do so during the coming week-end. I'll probably have some comments and suggestions on it, so you'll have a chance to update the revision checks when submitting the final revision of your patch.
I would also like to document the revision letter vs. revision number for these ITE chips. This should help people (and us) understand which chip has what kind of auto-PWM implementation. Can you please visually check the revision letter of both chips you have (it's mentioned on the top marking) and give the value of the ID register? I'll do the same on my two chips, if we can gather a few more we should have an almost complete list.
Thanks,
--
Jean Delvare
PATCH 1/1 : Add automatic PWM mode to it87.c
In reply to Sebastian WittHi Sebastian,
Quoted 4 lines
Ok, here's a patch against 2.6.12-rc6.
(...)
I've now added only support for the auto-pwm with slope setting and a check for revision 3.
Here is my review of your patch. Sorry for the delay.
Quoted 2 lines
static u16
chip_type;
+static u8chip_revision;
No space alignment in the kernel code. Either align with tabs, or don't align.
it87.c line 164
Quoted 4 lines
it87.c renamed from it87.c-orig +4
-0+#define IT87_REG_SG_TEMP_OFF (nr) (0x60 + (nr) * 8) +#define IT87_REG_SG_TEMP_START (nr) (0x61 + (nr) * 8) +#define IT87_REG_SG_TEMP_FULL (nr) (0x62 + (nr) * 8) +#define IT87_REG_SG_START_PWM (nr) (0x63 + (nr) * 8)
This last name isn't logical, should be IT87_REG_SG_PWM_START (or swap the other ones, at your option).
Quoted 1 line
+#define
SLOPE_FROM_REG(val)((val) ? (1 << (val-1)) : 0)
Parentheses are not properly placed for macro safety. It should be:
#define SLOPE_FROM_REG(val) ((val) ? 1 << ((val)-1) : 0)
it87.c
Quoted 6 lines
it87.c renamed from it87.c-orig +6
-0+u8 fan_limit_off[3 ];/* Register value */ +u8 fan_limit_start[3 ];/* Register value */ +u8 fan_limit_full[3 ];/* Register value */ +u8 fan_limit_stpwm[3 ];/* Register value */ +u8 fan_am_ctrl[3 ];/* Register value */ +u8 fan_point2_pwm[3 ];/* Used for slope calculation */
I don't much like the names you chose. For one thing, it's not obvious that they are for SmartGuardian/auto-PWM. You should prefix them with sg_ or auto_pwm_ rather than fan_. For another, temperature values aren't clearly labelled as such. What about sg_temp_off, sg_temp_start, sg_temp_full, sg_pwm_start, sg_am_ctrl and sg_pwm_point2?
Review
Quoted 10 lines
(quoted diff) +10
-0+static void calc_slope(struct it87_data *data, int nr) +{ + u8 val = data->fan_point2_pwm[nr]; + + if (data->fan_limit_full[nr] <= data->fan_limit_start[nr]) + val = 14; + else { + val -= data->fan_limit_stpwm[nr]; + val /= (data->fan_limit_full[nr] - data->fan_limit_start[nr]); + }
Where does this "14" value come from? If full speed temperature happens to be below the start temperature, we're in trouble anyway, so why bother?
+ data->fan_am_ctrl[nr] &= ~0x03;
Shouldn't this be ~0x07?
+ if (val > 48)
+ data->fan_am_ctrl[nr] |= 0x07; /* 64 PWM / K */
What does the "K" stands for? Kelvin? I think "PWM/degree" would be clearer.
Review
Quoted 2 lines
(quoted diff) +2
-0+ /* Automatic mode - SmartGuardian */ + val += (val && (data->manual_pwm_ctl[nr] & 0x80)) ? 1 : 0;
An "if" statement would be clearer IMHO.
Review
Quoted 1 line
(quoted diff) +1
-0+ return sprintf(buf,"%d\n", val);
Add a space after the first comma (there are many of these in the rest of the patch).
Review
Quoted 2 lines
(quoted diff) +2
-0+ /* Return 0 if automatic mode is enabled */ + return sprintf(buf,"%d\n", (data->manual_pwm_ctl[nr] & 0x80) ? 0 : PWM_FROM_REG(data->manual_pwm_ctl[nr]));
This is unsafe. You should hold the lock (&data->update_lock), else data->manual_pwm_ctl[nr] may have a different on second read, and you may display a completely bogus PWM value. Same is true for all other functions in which you access more than one field of the data structure (or one, but more than once), you have to hold the lock.
Review
Quoted 1 line
(quoted diff) +1
-0+ /* Enable temperature smoothing and fan spin up time */
Extra space between "up" and "time". What is this spin up time BTW? The datasheet is unclear, to say the least.
Review
Quoted 12 lines
(quoted diff) +12
-0+ data->fan_am_ctrl[nr] = 0x80 | 0x18; + it87_write_value(client, IT87_REG_SG_AM_CTRL(nr), data->fan_am_ctrl[nr]); + /* If start pwm and pwm step size not set, apply safe values */ + if (data->fan_limit_stpwm[nr] = 0) { + data->fan_limit_stpwm[nr] = PWM_TO_REG(128); + it87_write_value(client, IT87_REG_SG_START_PWM(nr), data->fan_limit_stpwm[nr]); + } + if ((data->fan_am_ctrl[nr] & 0x07) = 0) { + /* 16 PWM values / K */ + data->fan_am_ctrl[nr] |= 0x05; + it87_write_value(client, IT87_REG_SG_AM_CTRL(nr), data->fan_am_ctrl[nr]); + }
This isn't correct. Tracking data->fan_am_ctrl[nr], I can see you are setting it to 0x98, then writing it to the chip, then testing it with & 0x07 = 0 (will always be true), set it to 0x9D and write it to the chip again. This isn't correct. You must not change the slope value if it was set to a sane value before, and you must not write twice to the same register (waste of time).
it87.c line 665
Quoted 1 line
it87.c renamed from it87.c-orig +1
-0+ data->fan_limit_start[nr] = val;
This lacks a sanity check. Same for the two other ones.
As a side note, I think I would have forced point2's PWM value to 255, rather than letting the user set it. Having this PWM value too low will have bad effects due to the lack of hysteresis (the fan will go from low speed to full speed and back again and again), forcing it to be as near from full speed as possible would solve this problem.
Review
Quoted 5 lines
(quoted diff) +4
-0+static ssize_t set_pwm_auto_channels_temp(struct device *dev, struct device_attribute *attr, + const char *buf, size_t count) + if (val < 0 || val > 4) + return -EINVAL;
3 isn't a valid value, but will pass through.
Review
Quoted 1 line
(quoted diff) +1
-0+ data->manual_pwm_ctl[nr] = 0x80 | (val >> 1);
This bit shifting is a trick which happens to work in this specific case, but wouldn't work anymore if there were a 4th temperature channel, for example. So please don't use it.
it87.c line 1282
Quoted 11 lines
it87.c renamed from it87.c-orig +11
-0+/* Set current fan mode registers */ +for (i =0 ; i <3 ; i++) { + data->manual_pwm_ctl[i] =it87_read_value (client,IT87_REG_PWM (i)); + +/* Get fan control registers for SmartGuardian automatic mode */ + data->fan_limit_off[i] =it87_read_value (client,IT87_REG_SG_TEMP_OFF (i)); + data->fan_limit_start[i] =it87_read_value (client,IT87_REG_SG_TEMP_START (i)); + data->fan_limit_full[i] =it87_read_value (client,IT87_REG_SG_TEMP_FULL (i)); + data->fan_limit_stpwm[i] =it87_read_value (client,IT87_REG_SG_START_PWM (i)); + data->fan_am_ctrl[i] =it87_read_value (client,IT87_REG_SG_AM_CTRL (i)); + }
This doesn't belong to the init function but to the device_update function. Additionally, you should do this if and only if the device actually supports the auto-pwm code (so you have to remember this in some structure field).
Please provide an updated patch. Don't forget to also update the revision detection code. You have to check for different revision for the IT8705F and IT8712F chips, so you really have to test the combination of chip and revision.
Thanks,
Signature · 1 line
Jean Delvare