Hi Jean,
On 02/23/2014 01:49 AM, Jean Delvare wrote:
Hi Guenter,
On Sat, 22 Feb 2014 09:30:09 -0800, Guenter Roeck wrote:
Hysteresis temperatures are defined as absolute temperatures,
not as delta value from the critical temperatures.
Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>
---
v2: Don't change indentation
Don't call lm95245_update_device from set function
In set function, actually write the calculated hysteresis
instead of the entered value.
drivers/hwmon/lm95245.c | 30 +++++++++++++++++++++---------
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/drivers/hwmon/lm95245.c b/drivers/hwmon/lm95245.c
index 3f0956e..d7bcea1 100644
--- a/drivers/hwmon/lm95245.c
+++ b/drivers/hwmon/lm95245.c
@@ -272,27 +272,39 @@ static ssize_t set_limit(struct device *dev, struct device_attribute *attr,
return count;
}
+static ssize_t show_crit_hyst(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct lm95245_data *data = lm95245_update_device(dev);
+ int index = to_sensor_dev_attr(attr)->index;
+ int hyst = data->regs[index] - data->regs[8];
+
+ return snprintf(buf, PAGE_SIZE - 1, "%d\n", hyst * 1000);
+}
+
static ssize_t set_crit_hyst(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
struct i2c_client *client = to_i2c_client(dev);
struct lm95245_data *data = i2c_get_clientdata(client);
+ int index = to_sensor_dev_attr(attr)->index;
unsigned long val;
+ long hyst;
+ int limit;
Nitpicking: it makes little sense to have hyst and limit as different
types.
Ok, I'll change that.
if (kstrtoul(buf, 10, &val) < 0)
return -EINVAL;
- val /= 1000;
-
- val = clamp_val(val, 0, 31);
-
mutex_lock(&data->update_lock);
+ limit = i2c_smbus_read_byte_data(client, lm95245_reg_address[index]);
+ hyst = limit - val / 1000;
+ hyst = clamp_val(hyst, 0, 31);
Note that you don't actually need to hold the lock for that. Access to
the SMBus itself is already serialized at the bus driver level, and
you're not touching any field of the data structure (that's what the
lock is protecting.)
I think it was because the valid flag is cleared, to make sure the data
is only re-read after the hysteresis register is written to.
data->valid = 0;
BTW do you have any idea why the whole cache is invalidated here? I'd
think that storing hyst as data->regs[8] would be enough?
It also ensures that the limit register value in the cache is current,
but since that is not volatile it should not really make a difference.
I'll change that and only update regs[8]. But then I do need the lock,
or at least I think so.
/* shared crit hysteresis */
i2c_smbus_write_byte_data(client, LM95245_REG_RW_COMMON_HYSTERESIS,
- val);
+ hyst);
mutex_unlock(&data->update_lock);
@@ -378,16 +390,16 @@ static ssize_t set_interval(struct device *dev, struct device_attribute *attr,
static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_input, NULL, 0);
static SENSOR_DEVICE_ATTR(temp1_crit, S_IWUSR | S_IRUGO, show_limit,
set_limit, 6);
-static SENSOR_DEVICE_ATTR(temp1_crit_hyst, S_IWUSR | S_IRUGO, show_limit,
- set_crit_hyst, 8);
+static SENSOR_DEVICE_ATTR(temp1_crit_hyst, S_IWUSR | S_IRUGO, show_crit_hyst,
+ set_crit_hyst, 6);
static SENSOR_DEVICE_ATTR(temp1_crit_alarm, S_IRUGO, show_alarm, NULL,
STATUS1_LOC);
static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_input, NULL, 2);
static SENSOR_DEVICE_ATTR(temp2_crit, S_IWUSR | S_IRUGO, show_limit,
set_limit, 7);
-static SENSOR_DEVICE_ATTR(temp2_crit_hyst, S_IWUSR | S_IRUGO, show_limit,
- set_crit_hyst, 8);
+static SENSOR_DEVICE_ATTR(temp2_crit_hyst, S_IWUSR | S_IRUGO, show_crit_hyst,
+ set_crit_hyst, 7);
static SENSOR_DEVICE_ATTR(temp2_crit_alarm, S_IRUGO, show_alarm, NULL,
STATUS1_RTCRIT);
static SENSOR_DEVICE_ATTR(temp2_type, S_IWUSR | S_IRUGO, show_type,
Reviewed-by: Jean Delvare <jdelvare@xxxxxxx>
Thanks!
Guenter
_______________________________________________
lm-sensors mailing list
lm-sensors@xxxxxxxxxxxxxx
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
