hwmon: (f75375s) Fix hysteresis register access for the F75387
PATCH 4/6 hwmon: (f75375s) Fix hysteresis register access for the F75387
The F75387 stores its temperature hysteresis in a different register than the other chips supported by that driver.
--- drivers/hwmon/f75375s.c | 36 ++++++++++++++++++++++++++++++------ 1 files changed, 30 insertions(+), 6 deletions(-)
drivers/hwmon/f75375s.c +12 -2
@@ -65,6 +65,7 @@ enum chips { f75373, f75375, f75387 };#define F75387_REG_TEMP11_LSB (nr) (0x1a + (nr))#define F75375_REG_TEMP_HIGH (nr) (0x28 + (nr) * 2)#define F75375_REG_TEMP_HYST (nr) (0x29 + (nr) * 2) +#define F75387_REG_TEMP_HYST 0x62#define F75375_REG_FAN (nr) (0x16 + (nr) * 2)#define F75375_REG_FAN_MIN (nr) (0x2C + (nr) * 2)
@@ -183,23 +184,34 @@ static struct f75375_data *f75375_update_device(struct device *dev)struct i2c_client *client =to_i2c_client (dev);struct f75375_data *data =i2c_get_clientdata (client);int nr; +u8 f75387_hyst_reg =0 ;mutex_lock (&data->update_lock);/* Limit registers cache is refreshed after 60 seconds */ if (time_after (jiffies, data->last_limits +60 *HZ ) || !data->valid) { +if (data->kind = f75387) + f75387_hyst_reg +f75375_read8 (client,F75387_REG_TEMP_HYST );for (nr =0 ; nr <2 ; nr++) { data->temp_high[nr] f75375_read8(client, F75375_REG_TEMP_HIGH(nr)); - data->temp_max_hyst[nr] -f75375_read8 (client,F75375_REG_TEMP_HYST (nr)); data->fan_max[nr] f75375_read16(client, F75375_REG_FAN_FULL(nr)); data->fan_min[nr] f75375_read16(client, F75375_REG_FAN_MIN(nr)); data->fan_target[nr] f75375_read16(client, F75375_REG_FAN_EXP(nr)); +if (data->kind = f75387) { + data->temp_max_hyst[nr] + data->temp_high[nr] - + (0xf & f75387_hyst_reg); + }else { + data->temp_max_hyst[nr] +f75375_read8 (client, +F75375_REG_TEMP_HYST (nr)); + } }for (nr =0 ; nr <4 ; nr++) { data->in_max[nr] @@ -599,11 +611,23 @@ static ssize_t set_temp_max_hyst(struct device *dev, if (err < 0) return err; - val =SENSORS_LIMIT (TEMP_TO_REG (val),0 ,127 ); + val =TEMP_TO_REG (val);mutex_lock (&data->update_lock);
- data->temp_max_hyst[nr] = val;
- f75375_write8(client, F75375_REG_TEMP_HYST(nr),
- data->temp_max_hyst[nr]);
+ if (data->kind = f75387) {
+ u8 regval;
+ val = SENSORS_LIMIT(val,
+ data->temp_high[nr] - 0xf,
+ data->temp_high[nr]);
+ data->temp_max_hyst[nr] = val;
+ regval = f75375_read8(client, F75387_REG_TEMP_HYST);
+ regval &= 0xf0;
+ regval |= data->temp_high[nr] - val;
+ f75375_write8(client, F75387_REG_TEMP_HYST, regval);
+ } else {
+ val = SENSORS_LIMIT(val, 0, 127);
+ data->temp_max_hyst[nr] = val;
+ f75375_write8(client, F75375_REG_TEMP_HYST(nr), val);
+ }
mutex_unlock(&data->update_lock);
return count;
}
Signature · 7 lines
1.7.9.1 _______________________________________________ lm-sensors mailing list [email protected] http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Quoted 32 lines · Nikolaus Schulz
The F75387 stores its temperature hysteresis in a different register than the other chips supported by that driver.
Signed-off-by Nikolaus Schulz <[email protected]>--- drivers/hwmon/f75375s.c | 36 ++++++++++++++++++++++++++++++------ 1 files changed, 30 insertions(+), 6 deletions(-)drivers/hwmon/f75375s.c +3
-0@@ -65,6 +65,7 @@ enum chips { f75373, f75375, f75387 };#define F75387_REG_TEMP11_LSB (nr) (0x1a + (nr))#define F75375_REG_TEMP_HIGH (nr) (0x28 + (nr) * 2)#define F75375_REG_TEMP_HYST (nr) (0x29 + (nr) * 2) +#define F75387_REG_TEMP_HYST 0x62#define F75375_REG_FAN (nr) (0x16 + (nr) * 2)#define F75375_REG_FAN_MIN (nr) (0x2C + (nr) * 2)@@ -183,23 +184,34 @@ static struct f75375_data *f75375_update_device(struct device *dev)struct i2c_client *client =to_i2c_client (dev);struct f75375_data *data =i2c_get_clientdata (client);int nr; +u8 f75387_hyst_reg =0 ;mutex_lock (&data->update_lock);/* Limit registers cache is refreshed after 60 seconds */ if (time_after (jiffies, data->last_limits +60 *HZ ) || !data->valid) { +if (data->kind = f75387)
drivers/hwmon/f75375s.c static struct f75375_data *f75375_update_device(struct device *dev) · line 203
Quoted 10 lines
drivers/hwmon/f75375s.c +4
-1+ f75387_hyst_reg > +f75375_read8 (client,F75387_REG_TEMP_HYST );for (nr =0 ; nr <2 ; nr++) { data->temp_high[nr] >f75375_read8 (client,F75375_REG_TEMP_HIGH (nr)); - data->temp_max_hyst[nr] > -f75375_read8 (client,F75375_REG_TEMP_HYST (nr)); data->fan_max[nr] >f75375_read16 (client,F75375_REG_FAN_FULL (nr)); data->fan_min[nr] >f75375_read16 (client,F75375_REG_FAN_MIN (nr)); data->fan_target[nr] >f75375_read16 (client,F75375_REG_FAN_EXP (nr)); +if (data->kind = f75387) { + data->temp_max_hyst[nr] > + data->temp_high[nr] - + (0xf & f75387_hyst_reg);
I really hate it when people put the constant first, and consider it confusing. Please change. Besides, you can do the masking above, when f75387_hyst_reg is read. This way it only needs to be done once.
Quoted 16 lines
drivers/hwmon/f75375s.c +5
-4+ }else { + data->temp_max_hyst[nr] > +f75375_read8 (client, +F75375_REG_TEMP_HYST (nr)); + } }for (nr =0 ; nr <4 ; nr++) { data->in_max[nr] > @@ -599,11 +611,23 @@ static ssize_t set_temp_max_hyst(struct device *dev, if (err < 0) return err; - val =SENSORS_LIMIT (TEMP_TO_REG (val),0 ,127 ); + val =TEMP_TO_REG (val);mutex_lock (&data->update_lock); - data->temp_max_hyst[nr] = val; -f75375_write8 (client,F75375_REG_TEMP_HYST (nr), - data->temp_max_hyst[nr]);
drivers/hwmon/f75375s.c static struct f75375_data *f75375_update_device(struct device *dev)
Quoted 10 lines
drivers/hwmon/f75375s.c +10
-0+if (data->kind = f75387) { +u8 regval; + val =SENSORS_LIMIT (val, + data->temp_high[nr] -0xf , + data->temp_high[nr]); + data->temp_max_hyst[nr] = val; + regval =f75375_read8 (client,F75387_REG_TEMP_HYST ); + regval &=0xf0 ; + regval |= data->temp_high[nr] - val; +f75375_write8 (client,F75387_REG_TEMP_HYST , regval);
Unless I am missing something, this affects the other hysteresis values as well, since there is really only a single hysteresis register. If so, you have to set data->valid to 0 to ensure that temp_max_hyst[] is updated with the next read cycle.
Quoted 11 lines
drivers/hwmon/f75375s.c +5
-0+ }else { + val =SENSORS_LIMIT (val,0 ,127 ); + data->temp_max_hyst[nr] = val; +f75375_write8 (client,F75375_REG_TEMP_HYST (nr), val); + }mutex_unlock (&data->update_lock);return count; }Signature · 1 line
1.7.9.1
_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Quoted 38 lines · Guenter Roeck
Quoted 37 lines · Nikolaus Schulz
The F75387 stores its temperature hysteresis in a different register than the other chips supported by that driver.
Signed-off-by Nikolaus Schulz <[email protected]>--- drivers/hwmon/f75375s.c | 36 ++++++++++++++++++++++++++++++------ 1 files changed, 30 insertions(+), 6 deletions(-)drivers/hwmon/f75375s.c +4
-1@@ -65,6 +65,7 @@ enum chips { f75373, f75375, f75387 };#define F75387_REG_TEMP11_LSB (nr) (0x1a + (nr))#define F75375_REG_TEMP_HIGH (nr) (0x28 + (nr) * 2)#define F75375_REG_TEMP_HYST (nr) (0x29 + (nr) * 2) +#define F75387_REG_TEMP_HYST 0x62#define F75375_REG_FAN (nr) (0x16 + (nr) * 2)#define F75375_REG_FAN_MIN (nr) (0x2C + (nr) * 2)@@ -183,23 +184,34 @@ static struct f75375_data *f75375_update_device(struct device *dev)struct i2c_client *client =to_i2c_client (dev);struct f75375_data *data =i2c_get_clientdata (client);int nr; +u8 f75387_hyst_reg =0 ;mutex_lock (&data->update_lock);/* Limit registers cache is refreshed after 60 seconds */ if (time_after (jiffies, data->last_limits +60 *HZ ) || !data->valid) { +if (data->kind = f75387) + f75387_hyst_reg > > + f75375_read8(client, F75387_REG_TEMP_HYST);for (nr =0 ; nr <2 ; nr++) { data->temp_high[nr] > > f75375_read8(client, F75375_REG_TEMP_HIGH(nr)); - data->temp_max_hyst[nr] > > - f75375_read8(client, F75375_REG_TEMP_HYST(nr)); data->fan_max[nr] > > f75375_read16(client, F75375_REG_FAN_FULL(nr));
Quoted 8 lines
drivers/hwmon/f75375s.c static struct f75375_data *f75375_update_device(struct device *dev) · line 203
Quoted 5 lines
drivers/hwmon/f75375s.c +3
-0data->fan_min[nr] > > f75375_read16(client, F75375_REG_FAN_MIN(nr)); data->fan_target[nr] > > f75375_read16(client, F75375_REG_FAN_EXP(nr)); +if (data->kind = f75387) { + data->temp_max_hyst[nr] > > + data->temp_high[nr] - + (0xf & f75387_hyst_reg);I really hate it when people put the constant first, and consider it confusing. Please change.
Ok.
Quoted 2 lines
Besides, you can do the masking above, when
f75387_hyst_regis read. This way it only needs to be done once.
Correct.
I didn't do that because there is also some fan-specific information encoded in that register, and I am working on code that uses it. But until that code is posted, the register should be masked beforehand. I'll update the patch accordingly.
Quoted 21 lines
Quoted 21 lines
drivers/hwmon/f75375s.c +10
-4+ }else { + data->temp_max_hyst[nr] > > + f75375_read8(client, +F75375_REG_TEMP_HYST (nr)); + } }for (nr =0 ; nr <4 ; nr++) { data->in_max[nr] > > @@ -599,11 +611,23 @@ static ssize_t set_temp_max_hyst(struct device *dev, if (err < 0) return err; - val =SENSORS_LIMIT (TEMP_TO_REG (val),0 ,127 ); + val =TEMP_TO_REG (val);mutex_lock (&data->update_lock); - data->temp_max_hyst[nr] = val; -f75375_write8 (client,F75375_REG_TEMP_HYST (nr), - data->temp_max_hyst[nr]); +if (data->kind = f75387) { +u8 regval; + val =SENSORS_LIMIT (val, + data->temp_high[nr] -0xf , + data->temp_high[nr]);
Quoted 8 lines
drivers/hwmon/f75375s.c
Quoted 5 lines
drivers/hwmon/f75375s.c +5
-0+ data->temp_max_hyst[nr] = val; + regval =f75375_read8 (client,F75387_REG_TEMP_HYST ); + regval &=0xf0 ; + regval |= data->temp_high[nr] - val; +f75375_write8 (client,F75387_REG_TEMP_HYST , regval);Unless I am missing something, this affects the other hysteresis values as well, since there is really only a single hysteresis register.
That's correct.
Quoted 2 lines
If so, you have to set
data->validto 0 to ensure that temp_max_hyst[] is updated with the next read cycle.
Good catch!
I will post an updated patch.
Quoted 10 lines
Quoted 10 lines
drivers/hwmon/f75375s.c +5
-0+ }else { + val =SENSORS_LIMIT (val,0 ,127 ); + data->temp_max_hyst[nr] = val; +f75375_write8 (client,F75375_REG_TEMP_HYST (nr), val); + }mutex_unlock (&data->update_lock);return count; }Signature · 1 line
1.7.9.1
_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Quoted 7 lines · Nikolaus Schulz
The F75387 stores its temperature hysteresis in a different register than the other chips supported by that driver.
Signed-off-by Nikolaus Schulz <[email protected]>--- drivers/hwmon/f75375s.c | 36 ++++++++++++++++++++++++++++++------ 1 files changed, 30 insertions(+), 6 deletions(-)
Oh, stupid me. I have to revoke this patch, because it is simply utter nonsense. Sorry for that.
The F75387 does indeed use register 0x62 to store a temperature hysteresis delta, but it applies only to the automatic, temperature-controlled fan control mode.
The high limit hysteresis temperatures, though, are still stored in registers 0x29 and 0x2B - here the F75387 is no different from the other chips supported by this driver.
Nikolaus (grabbing a brown paper bag)
Quoted 72 lines
drivers/hwmon/f75375s.c +27
-5@@ -65,6 +65,7 @@ enum chips { f75373, f75375, f75387 };#define F75387_REG_TEMP11_LSB (nr) (0x1a + (nr))#define F75375_REG_TEMP_HIGH (nr) (0x28 + (nr) * 2)#define F75375_REG_TEMP_HYST (nr) (0x29 + (nr) * 2) +#define F75387_REG_TEMP_HYST 0x62#define F75375_REG_FAN (nr) (0x16 + (nr) * 2)#define F75375_REG_FAN_MIN (nr) (0x2C + (nr) * 2)@@ -183,23 +184,34 @@ static struct f75375_data *f75375_update_device(struct device *dev)struct i2c_client *client =to_i2c_client (dev);struct f75375_data *data =i2c_get_clientdata (client);int nr; +u8 f75387_hyst_reg =0 ;mutex_lock (&data->update_lock);/* Limit registers cache is refreshed after 60 seconds */ if (time_after (jiffies, data->last_limits +60 *HZ ) || !data->valid) { +if (data->kind = f75387) + f75387_hyst_reg > +f75375_read8 (client,F75387_REG_TEMP_HYST );for (nr =0 ; nr <2 ; nr++) { data->temp_high[nr] >f75375_read8 (client,F75375_REG_TEMP_HIGH (nr)); - data->temp_max_hyst[nr] > -f75375_read8 (client,F75375_REG_TEMP_HYST (nr)); data->fan_max[nr] >f75375_read16 (client,F75375_REG_FAN_FULL (nr)); data->fan_min[nr] >f75375_read16 (client,F75375_REG_FAN_MIN (nr)); data->fan_target[nr] >f75375_read16 (client,F75375_REG_FAN_EXP (nr)); +if (data->kind = f75387) { + data->temp_max_hyst[nr] > + data->temp_high[nr] - + (0xf & f75387_hyst_reg); + }else { + data->temp_max_hyst[nr] > +f75375_read8 (client, +F75375_REG_TEMP_HYST (nr)); + } }for (nr =0 ; nr <4 ; nr++) { data->in_max[nr] > @@ -599,11 +611,23 @@ static ssize_t set_temp_max_hyst(struct device *dev, if (err < 0) return err; - val =SENSORS_LIMIT (TEMP_TO_REG (val),0 ,127 ); + val =TEMP_TO_REG (val);mutex_lock (&data->update_lock); - data->temp_max_hyst[nr] = val; -f75375_write8 (client,F75375_REG_TEMP_HYST (nr), - data->temp_max_hyst[nr]); +if (data->kind = f75387) { +u8 regval; + val =SENSORS_LIMIT (val, + data->temp_high[nr] -0xf , + data->temp_high[nr]); + data->temp_max_hyst[nr] = val; + regval =f75375_read8 (client,F75387_REG_TEMP_HYST ); + regval &=0xf0 ; + regval |= data->temp_high[nr] - val; +f75375_write8 (client,F75387_REG_TEMP_HYST , regval); + }else { + val =SENSORS_LIMIT (val,0 ,127 ); + data->temp_max_hyst[nr] = val; +f75375_write8 (client,F75375_REG_TEMP_HYST (nr), val); + }mutex_unlock (&data->update_lock);return count; }Signature · 1 line
1.7.9.1
_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors