* [PATCH] hwmon: Consider LM64 temperature offset @ 2011-02-08 13:16 ` Dirk Eibach 0 siblings, 0 replies; 14+ messages in thread From: Dirk Eibach @ 2011-02-08 13:16 UTC (permalink / raw) To: linux-kernel; +Cc: khali, guenter.roeck, lm-sensors, Dirk Eibach LM64 has 16 degrees Celsius temperature offset on all remote sensor registers. This was not considered When LM64 support was added to lm63.c. Signed-off-by: Dirk Eibach <eibach@gdsys.de> --- drivers/hwmon/lm63.c | 34 +++++++++++++++++++++++++++++----- 1 files changed, 29 insertions(+), 5 deletions(-) diff --git a/drivers/hwmon/lm63.c b/drivers/hwmon/lm63.c index 776aeb3..87bfefb 100644 --- a/drivers/hwmon/lm63.c +++ b/drivers/hwmon/lm63.c @@ -98,6 +98,9 @@ static const unsigned short normal_i2c[] = { 0x18, 0x4c, 0x4e, I2C_CLIENT_END }; * value, it uses signed 8-bit values with LSB = 1 degree Celsius. * For remote temperature, low and high limits, it uses signed 11-bit values * with LSB = 0.125 degree Celsius, left-justified in 16-bit registers. + * For LM64 the actual remote diode temperature is 16 degree Celsius higher + * than the register reading. Remote temperature setpoints have to be + * adapted accordingly. */ #define FAN_FROM_REG(reg) ((reg) == 0xFFFC || (reg) == 0 ? 0 : \ @@ -180,6 +183,7 @@ struct lm63_data { 2: remote high limit */ u8 temp2_crit_hyst; u8 alarms; + bool is_lm64; }; /* @@ -255,6 +259,17 @@ static ssize_t show_temp8(struct device *dev, struct device_attribute *devattr, return sprintf(buf, "%d\n", TEMP8_FROM_REG(data->temp8[attr->index])); } +static ssize_t show_remote_temp8(struct device *dev, + struct device_attribute *devattr, + char *buf) +{ + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); + struct lm63_data *data = lm63_update_device(dev); + int offset = data->is_lm64 ? 16000 : 0; + return sprintf(buf, "%d\n", + TEMP8_FROM_REG(data->temp8[attr->index]) + offset); +} + static ssize_t set_temp8(struct device *dev, struct device_attribute *dummy, const char *buf, size_t count) { @@ -274,7 +289,9 @@ static ssize_t show_temp11(struct device *dev, struct device_attribute *devattr, { struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); struct lm63_data *data = lm63_update_device(dev); - return sprintf(buf, "%d\n", TEMP11_FROM_REG(data->temp11[attr->index])); + int offset = data->is_lm64 ? 16000 : 0; + return sprintf(buf, "%d\n", + TEMP11_FROM_REG(data->temp11[attr->index]) + offset); } static ssize_t set_temp11(struct device *dev, struct device_attribute *devattr, @@ -292,9 +309,10 @@ static ssize_t set_temp11(struct device *dev, struct device_attribute *devattr, struct lm63_data *data = i2c_get_clientdata(client); long val = simple_strtol(buf, NULL, 10); int nr = attr->index; + int offset = data->is_lm64 ? 16000 : 0; mutex_lock(&data->update_lock); - data->temp11[nr] = TEMP11_TO_REG(val); + data->temp11[nr] = TEMP11_TO_REG(val - offset); i2c_smbus_write_byte_data(client, reg[(nr - 1) * 2], data->temp11[nr] >> 8); i2c_smbus_write_byte_data(client, reg[(nr - 1) * 2 + 1], @@ -309,7 +327,8 @@ static ssize_t show_temp2_crit_hyst(struct device *dev, struct device_attribute char *buf) { struct lm63_data *data = lm63_update_device(dev); - return sprintf(buf, "%d\n", TEMP8_FROM_REG(data->temp8[2]) + int offset = data->is_lm64 ? 16000 : 0; + return sprintf(buf, "%d\n", TEMP8_FROM_REG(data->temp8[2]) + offset - TEMP8_FROM_REG(data->temp2_crit_hyst)); } @@ -320,11 +339,12 @@ static ssize_t set_temp2_crit_hyst(struct device *dev, struct device_attribute * { struct i2c_client *client = to_i2c_client(dev); struct lm63_data *data = i2c_get_clientdata(client); + int offset = data->is_lm64 ? 16000 : 0; long val = simple_strtol(buf, NULL, 10); long hyst; mutex_lock(&data->update_lock); - hyst = TEMP8_FROM_REG(data->temp8[2]) - val; + hyst = TEMP8_FROM_REG(data->temp8[2]) + offset - val; i2c_smbus_write_byte_data(client, LM63_REG_REMOTE_TCRIT_HYST, HYST_TO_REG(hyst)); mutex_unlock(&data->update_lock); @@ -364,7 +384,7 @@ static SENSOR_DEVICE_ATTR(temp2_min, S_IWUSR | S_IRUGO, show_temp11, set_temp11, 1); static SENSOR_DEVICE_ATTR(temp2_max, S_IWUSR | S_IRUGO, show_temp11, set_temp11, 2); -static SENSOR_DEVICE_ATTR(temp2_crit, S_IRUGO, show_temp8, NULL, 2); +static SENSOR_DEVICE_ATTR(temp2_crit, S_IRUGO, show_remote_temp8, NULL, 2); static DEVICE_ATTR(temp2_crit_hyst, S_IWUSR | S_IRUGO, show_temp2_crit_hyst, set_temp2_crit_hyst); @@ -466,6 +486,7 @@ static int lm63_detect(struct i2c_client *new_client, static int lm63_probe(struct i2c_client *new_client, const struct i2c_device_id *id) { + u8 chip_id; struct lm63_data *data; int err; @@ -479,6 +500,9 @@ static int lm63_probe(struct i2c_client *new_client, data->valid = 0; mutex_init(&data->update_lock); + chip_id = i2c_smbus_read_byte_data(new_client, LM63_REG_CHIP_ID); + data->is_lm64 = (chip_id == 0x51); + /* Initialize the LM63 chip */ lm63_init_client(new_client); -- 1.5.6.5 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [lm-sensors] [PATCH] hwmon: Consider LM64 temperature offset @ 2011-02-08 13:16 ` Dirk Eibach 0 siblings, 0 replies; 14+ messages in thread From: Dirk Eibach @ 2011-02-08 13:16 UTC (permalink / raw) To: linux-kernel; +Cc: khali, guenter.roeck, lm-sensors, Dirk Eibach LM64 has 16 degrees Celsius temperature offset on all remote sensor registers. This was not considered When LM64 support was added to lm63.c. Signed-off-by: Dirk Eibach <eibach@gdsys.de> --- drivers/hwmon/lm63.c | 34 +++++++++++++++++++++++++++++----- 1 files changed, 29 insertions(+), 5 deletions(-) diff --git a/drivers/hwmon/lm63.c b/drivers/hwmon/lm63.c index 776aeb3..87bfefb 100644 --- a/drivers/hwmon/lm63.c +++ b/drivers/hwmon/lm63.c @@ -98,6 +98,9 @@ static const unsigned short normal_i2c[] = { 0x18, 0x4c, 0x4e, I2C_CLIENT_END }; * value, it uses signed 8-bit values with LSB = 1 degree Celsius. * For remote temperature, low and high limits, it uses signed 11-bit values * with LSB = 0.125 degree Celsius, left-justified in 16-bit registers. + * For LM64 the actual remote diode temperature is 16 degree Celsius higher + * than the register reading. Remote temperature setpoints have to be + * adapted accordingly. */ #define FAN_FROM_REG(reg) ((reg) = 0xFFFC || (reg) = 0 ? 0 : \ @@ -180,6 +183,7 @@ struct lm63_data { 2: remote high limit */ u8 temp2_crit_hyst; u8 alarms; + bool is_lm64; }; /* @@ -255,6 +259,17 @@ static ssize_t show_temp8(struct device *dev, struct device_attribute *devattr, return sprintf(buf, "%d\n", TEMP8_FROM_REG(data->temp8[attr->index])); } +static ssize_t show_remote_temp8(struct device *dev, + struct device_attribute *devattr, + char *buf) +{ + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); + struct lm63_data *data = lm63_update_device(dev); + int offset = data->is_lm64 ? 16000 : 0; + return sprintf(buf, "%d\n", + TEMP8_FROM_REG(data->temp8[attr->index]) + offset); +} + static ssize_t set_temp8(struct device *dev, struct device_attribute *dummy, const char *buf, size_t count) { @@ -274,7 +289,9 @@ static ssize_t show_temp11(struct device *dev, struct device_attribute *devattr, { struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); struct lm63_data *data = lm63_update_device(dev); - return sprintf(buf, "%d\n", TEMP11_FROM_REG(data->temp11[attr->index])); + int offset = data->is_lm64 ? 16000 : 0; + return sprintf(buf, "%d\n", + TEMP11_FROM_REG(data->temp11[attr->index]) + offset); } static ssize_t set_temp11(struct device *dev, struct device_attribute *devattr, @@ -292,9 +309,10 @@ static ssize_t set_temp11(struct device *dev, struct device_attribute *devattr, struct lm63_data *data = i2c_get_clientdata(client); long val = simple_strtol(buf, NULL, 10); int nr = attr->index; + int offset = data->is_lm64 ? 16000 : 0; mutex_lock(&data->update_lock); - data->temp11[nr] = TEMP11_TO_REG(val); + data->temp11[nr] = TEMP11_TO_REG(val - offset); i2c_smbus_write_byte_data(client, reg[(nr - 1) * 2], data->temp11[nr] >> 8); i2c_smbus_write_byte_data(client, reg[(nr - 1) * 2 + 1], @@ -309,7 +327,8 @@ static ssize_t show_temp2_crit_hyst(struct device *dev, struct device_attribute char *buf) { struct lm63_data *data = lm63_update_device(dev); - return sprintf(buf, "%d\n", TEMP8_FROM_REG(data->temp8[2]) + int offset = data->is_lm64 ? 16000 : 0; + return sprintf(buf, "%d\n", TEMP8_FROM_REG(data->temp8[2]) + offset - TEMP8_FROM_REG(data->temp2_crit_hyst)); } @@ -320,11 +339,12 @@ static ssize_t set_temp2_crit_hyst(struct device *dev, struct device_attribute * { struct i2c_client *client = to_i2c_client(dev); struct lm63_data *data = i2c_get_clientdata(client); + int offset = data->is_lm64 ? 16000 : 0; long val = simple_strtol(buf, NULL, 10); long hyst; mutex_lock(&data->update_lock); - hyst = TEMP8_FROM_REG(data->temp8[2]) - val; + hyst = TEMP8_FROM_REG(data->temp8[2]) + offset - val; i2c_smbus_write_byte_data(client, LM63_REG_REMOTE_TCRIT_HYST, HYST_TO_REG(hyst)); mutex_unlock(&data->update_lock); @@ -364,7 +384,7 @@ static SENSOR_DEVICE_ATTR(temp2_min, S_IWUSR | S_IRUGO, show_temp11, set_temp11, 1); static SENSOR_DEVICE_ATTR(temp2_max, S_IWUSR | S_IRUGO, show_temp11, set_temp11, 2); -static SENSOR_DEVICE_ATTR(temp2_crit, S_IRUGO, show_temp8, NULL, 2); +static SENSOR_DEVICE_ATTR(temp2_crit, S_IRUGO, show_remote_temp8, NULL, 2); static DEVICE_ATTR(temp2_crit_hyst, S_IWUSR | S_IRUGO, show_temp2_crit_hyst, set_temp2_crit_hyst); @@ -466,6 +486,7 @@ static int lm63_detect(struct i2c_client *new_client, static int lm63_probe(struct i2c_client *new_client, const struct i2c_device_id *id) { + u8 chip_id; struct lm63_data *data; int err; @@ -479,6 +500,9 @@ static int lm63_probe(struct i2c_client *new_client, data->valid = 0; mutex_init(&data->update_lock); + chip_id = i2c_smbus_read_byte_data(new_client, LM63_REG_CHIP_ID); + data->is_lm64 = (chip_id = 0x51); + /* Initialize the LM63 chip */ lm63_init_client(new_client); -- 1.5.6.5 _______________________________________________ lm-sensors mailing list lm-sensors@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/lm-sensors ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] hwmon: Consider LM64 temperature offset 2011-02-08 13:16 ` [lm-sensors] " Dirk Eibach @ 2011-02-08 15:28 ` Guenter Roeck -1 siblings, 0 replies; 14+ messages in thread From: Guenter Roeck @ 2011-02-08 15:28 UTC (permalink / raw) To: Dirk Eibach; +Cc: linux-kernel, khali, lm-sensors On Tue, Feb 08, 2011 at 08:16:06AM -0500, Dirk Eibach wrote: > LM64 has 16 degrees Celsius temperature offset on all > remote sensor registers. > This was not considered When LM64 support was added to lm63.c. > > Signed-off-by: Dirk Eibach <eibach@gdsys.de> Comments inline. > --- > drivers/hwmon/lm63.c | 34 +++++++++++++++++++++++++++++----- > 1 files changed, 29 insertions(+), 5 deletions(-) > > diff --git a/drivers/hwmon/lm63.c b/drivers/hwmon/lm63.c > index 776aeb3..87bfefb 100644 > --- a/drivers/hwmon/lm63.c > +++ b/drivers/hwmon/lm63.c > @@ -98,6 +98,9 @@ static const unsigned short normal_i2c[] = { 0x18, 0x4c, 0x4e, I2C_CLIENT_END }; > * value, it uses signed 8-bit values with LSB = 1 degree Celsius. > * For remote temperature, low and high limits, it uses signed 11-bit values > * with LSB = 0.125 degree Celsius, left-justified in 16-bit registers. > + * For LM64 the actual remote diode temperature is 16 degree Celsius higher > + * than the register reading. Remote temperature setpoints have to be > + * adapted accordingly. > */ > > #define FAN_FROM_REG(reg) ((reg) == 0xFFFC || (reg) == 0 ? 0 : \ > @@ -180,6 +183,7 @@ struct lm63_data { > 2: remote high limit */ > u8 temp2_crit_hyst; > u8 alarms; > + bool is_lm64; It would be easier to add an offset variable, since all you use it for is to calculate the offset. Then you can just use data->offset instead of calculating the offset repeatedly. > }; > > /* > @@ -255,6 +259,17 @@ static ssize_t show_temp8(struct device *dev, struct device_attribute *devattr, > return sprintf(buf, "%d\n", TEMP8_FROM_REG(data->temp8[attr->index])); > } > > +static ssize_t show_remote_temp8(struct device *dev, > + struct device_attribute *devattr, > + char *buf) > +{ > + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); > + struct lm63_data *data = lm63_update_device(dev); > + int offset = data->is_lm64 ? 16000 : 0; > + return sprintf(buf, "%d\n", > + TEMP8_FROM_REG(data->temp8[attr->index]) + offset); > +} For consistency, if you should name this function to match show_temp2_crit_hyst(), ie show_temp2_crit(). > + > static ssize_t set_temp8(struct device *dev, struct device_attribute *dummy, > const char *buf, size_t count) > { > @@ -274,7 +289,9 @@ static ssize_t show_temp11(struct device *dev, struct device_attribute *devattr, > { > struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); > struct lm63_data *data = lm63_update_device(dev); > - return sprintf(buf, "%d\n", TEMP11_FROM_REG(data->temp11[attr->index])); > + int offset = data->is_lm64 ? 16000 : 0; > + return sprintf(buf, "%d\n", > + TEMP11_FROM_REG(data->temp11[attr->index]) + offset); > } > > static ssize_t set_temp11(struct device *dev, struct device_attribute *devattr, > @@ -292,9 +309,10 @@ static ssize_t set_temp11(struct device *dev, struct device_attribute *devattr, > struct lm63_data *data = i2c_get_clientdata(client); > long val = simple_strtol(buf, NULL, 10); > int nr = attr->index; > + int offset = data->is_lm64 ? 16000 : 0; > > mutex_lock(&data->update_lock); > - data->temp11[nr] = TEMP11_TO_REG(val); > + data->temp11[nr] = TEMP11_TO_REG(val - offset); > i2c_smbus_write_byte_data(client, reg[(nr - 1) * 2], > data->temp11[nr] >> 8); > i2c_smbus_write_byte_data(client, reg[(nr - 1) * 2 + 1], > @@ -309,7 +327,8 @@ static ssize_t show_temp2_crit_hyst(struct device *dev, struct device_attribute > char *buf) > { > struct lm63_data *data = lm63_update_device(dev); > - return sprintf(buf, "%d\n", TEMP8_FROM_REG(data->temp8[2]) > + int offset = data->is_lm64 ? 16000 : 0; > + return sprintf(buf, "%d\n", TEMP8_FROM_REG(data->temp8[2]) + offset > - TEMP8_FROM_REG(data->temp2_crit_hyst)); > } > > @@ -320,11 +339,12 @@ static ssize_t set_temp2_crit_hyst(struct device *dev, struct device_attribute * > { > struct i2c_client *client = to_i2c_client(dev); > struct lm63_data *data = i2c_get_clientdata(client); > + int offset = data->is_lm64 ? 16000 : 0; > long val = simple_strtol(buf, NULL, 10); > long hyst; > > mutex_lock(&data->update_lock); > - hyst = TEMP8_FROM_REG(data->temp8[2]) - val; > + hyst = TEMP8_FROM_REG(data->temp8[2]) + offset - val; > i2c_smbus_write_byte_data(client, LM63_REG_REMOTE_TCRIT_HYST, > HYST_TO_REG(hyst)); > mutex_unlock(&data->update_lock); > @@ -364,7 +384,7 @@ static SENSOR_DEVICE_ATTR(temp2_min, S_IWUSR | S_IRUGO, show_temp11, > set_temp11, 1); > static SENSOR_DEVICE_ATTR(temp2_max, S_IWUSR | S_IRUGO, show_temp11, > set_temp11, 2); > -static SENSOR_DEVICE_ATTR(temp2_crit, S_IRUGO, show_temp8, NULL, 2); > +static SENSOR_DEVICE_ATTR(temp2_crit, S_IRUGO, show_remote_temp8, NULL, 2); Any idea why the remote critical limit can not be set ? > static DEVICE_ATTR(temp2_crit_hyst, S_IWUSR | S_IRUGO, show_temp2_crit_hyst, > set_temp2_crit_hyst); > > @@ -466,6 +486,7 @@ static int lm63_detect(struct i2c_client *new_client, > static int lm63_probe(struct i2c_client *new_client, > const struct i2c_device_id *id) > { > + u8 chip_id; > struct lm63_data *data; > int err; > > @@ -479,6 +500,9 @@ static int lm63_probe(struct i2c_client *new_client, > data->valid = 0; > mutex_init(&data->update_lock); > > + chip_id = i2c_smbus_read_byte_data(new_client, LM63_REG_CHIP_ID); > + data->is_lm64 = (chip_id == 0x51); > + Chip id is already detected in lm63_detect. You don't need to detect it again. The more common approach would be something along the line of data->kind = id->driver_data; You would then use if (data->kind == lm64) throughout the code. In addition to that, you could define data->kind = id->driver_data; if (data->kind == lm64) data->offset = 16000; which would save you the repeated recalculation of offset as mentioned before. Thanks, Guenter ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [lm-sensors] [PATCH] hwmon: Consider LM64 temperature offset @ 2011-02-08 15:28 ` Guenter Roeck 0 siblings, 0 replies; 14+ messages in thread From: Guenter Roeck @ 2011-02-08 15:28 UTC (permalink / raw) To: Dirk Eibach; +Cc: linux-kernel, khali, lm-sensors On Tue, Feb 08, 2011 at 08:16:06AM -0500, Dirk Eibach wrote: > LM64 has 16 degrees Celsius temperature offset on all > remote sensor registers. > This was not considered When LM64 support was added to lm63.c. > > Signed-off-by: Dirk Eibach <eibach@gdsys.de> Comments inline. > --- > drivers/hwmon/lm63.c | 34 +++++++++++++++++++++++++++++----- > 1 files changed, 29 insertions(+), 5 deletions(-) > > diff --git a/drivers/hwmon/lm63.c b/drivers/hwmon/lm63.c > index 776aeb3..87bfefb 100644 > --- a/drivers/hwmon/lm63.c > +++ b/drivers/hwmon/lm63.c > @@ -98,6 +98,9 @@ static const unsigned short normal_i2c[] = { 0x18, 0x4c, 0x4e, I2C_CLIENT_END }; > * value, it uses signed 8-bit values with LSB = 1 degree Celsius. > * For remote temperature, low and high limits, it uses signed 11-bit values > * with LSB = 0.125 degree Celsius, left-justified in 16-bit registers. > + * For LM64 the actual remote diode temperature is 16 degree Celsius higher > + * than the register reading. Remote temperature setpoints have to be > + * adapted accordingly. > */ > > #define FAN_FROM_REG(reg) ((reg) = 0xFFFC || (reg) = 0 ? 0 : \ > @@ -180,6 +183,7 @@ struct lm63_data { > 2: remote high limit */ > u8 temp2_crit_hyst; > u8 alarms; > + bool is_lm64; It would be easier to add an offset variable, since all you use it for is to calculate the offset. Then you can just use data->offset instead of calculating the offset repeatedly. > }; > > /* > @@ -255,6 +259,17 @@ static ssize_t show_temp8(struct device *dev, struct device_attribute *devattr, > return sprintf(buf, "%d\n", TEMP8_FROM_REG(data->temp8[attr->index])); > } > > +static ssize_t show_remote_temp8(struct device *dev, > + struct device_attribute *devattr, > + char *buf) > +{ > + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); > + struct lm63_data *data = lm63_update_device(dev); > + int offset = data->is_lm64 ? 16000 : 0; > + return sprintf(buf, "%d\n", > + TEMP8_FROM_REG(data->temp8[attr->index]) + offset); > +} For consistency, if you should name this function to match show_temp2_crit_hyst(), ie show_temp2_crit(). > + > static ssize_t set_temp8(struct device *dev, struct device_attribute *dummy, > const char *buf, size_t count) > { > @@ -274,7 +289,9 @@ static ssize_t show_temp11(struct device *dev, struct device_attribute *devattr, > { > struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); > struct lm63_data *data = lm63_update_device(dev); > - return sprintf(buf, "%d\n", TEMP11_FROM_REG(data->temp11[attr->index])); > + int offset = data->is_lm64 ? 16000 : 0; > + return sprintf(buf, "%d\n", > + TEMP11_FROM_REG(data->temp11[attr->index]) + offset); > } > > static ssize_t set_temp11(struct device *dev, struct device_attribute *devattr, > @@ -292,9 +309,10 @@ static ssize_t set_temp11(struct device *dev, struct device_attribute *devattr, > struct lm63_data *data = i2c_get_clientdata(client); > long val = simple_strtol(buf, NULL, 10); > int nr = attr->index; > + int offset = data->is_lm64 ? 16000 : 0; > > mutex_lock(&data->update_lock); > - data->temp11[nr] = TEMP11_TO_REG(val); > + data->temp11[nr] = TEMP11_TO_REG(val - offset); > i2c_smbus_write_byte_data(client, reg[(nr - 1) * 2], > data->temp11[nr] >> 8); > i2c_smbus_write_byte_data(client, reg[(nr - 1) * 2 + 1], > @@ -309,7 +327,8 @@ static ssize_t show_temp2_crit_hyst(struct device *dev, struct device_attribute > char *buf) > { > struct lm63_data *data = lm63_update_device(dev); > - return sprintf(buf, "%d\n", TEMP8_FROM_REG(data->temp8[2]) > + int offset = data->is_lm64 ? 16000 : 0; > + return sprintf(buf, "%d\n", TEMP8_FROM_REG(data->temp8[2]) + offset > - TEMP8_FROM_REG(data->temp2_crit_hyst)); > } > > @@ -320,11 +339,12 @@ static ssize_t set_temp2_crit_hyst(struct device *dev, struct device_attribute * > { > struct i2c_client *client = to_i2c_client(dev); > struct lm63_data *data = i2c_get_clientdata(client); > + int offset = data->is_lm64 ? 16000 : 0; > long val = simple_strtol(buf, NULL, 10); > long hyst; > > mutex_lock(&data->update_lock); > - hyst = TEMP8_FROM_REG(data->temp8[2]) - val; > + hyst = TEMP8_FROM_REG(data->temp8[2]) + offset - val; > i2c_smbus_write_byte_data(client, LM63_REG_REMOTE_TCRIT_HYST, > HYST_TO_REG(hyst)); > mutex_unlock(&data->update_lock); > @@ -364,7 +384,7 @@ static SENSOR_DEVICE_ATTR(temp2_min, S_IWUSR | S_IRUGO, show_temp11, > set_temp11, 1); > static SENSOR_DEVICE_ATTR(temp2_max, S_IWUSR | S_IRUGO, show_temp11, > set_temp11, 2); > -static SENSOR_DEVICE_ATTR(temp2_crit, S_IRUGO, show_temp8, NULL, 2); > +static SENSOR_DEVICE_ATTR(temp2_crit, S_IRUGO, show_remote_temp8, NULL, 2); Any idea why the remote critical limit can not be set ? > static DEVICE_ATTR(temp2_crit_hyst, S_IWUSR | S_IRUGO, show_temp2_crit_hyst, > set_temp2_crit_hyst); > > @@ -466,6 +486,7 @@ static int lm63_detect(struct i2c_client *new_client, > static int lm63_probe(struct i2c_client *new_client, > const struct i2c_device_id *id) > { > + u8 chip_id; > struct lm63_data *data; > int err; > > @@ -479,6 +500,9 @@ static int lm63_probe(struct i2c_client *new_client, > data->valid = 0; > mutex_init(&data->update_lock); > > + chip_id = i2c_smbus_read_byte_data(new_client, LM63_REG_CHIP_ID); > + data->is_lm64 = (chip_id = 0x51); > + Chip id is already detected in lm63_detect. You don't need to detect it again. The more common approach would be something along the line of data->kind = id->driver_data; You would then use if (data->kind = lm64) throughout the code. In addition to that, you could define data->kind = id->driver_data; if (data->kind = lm64) data->offset = 16000; which would save you the repeated recalculation of offset as mentioned before. Thanks, Guenter _______________________________________________ lm-sensors mailing list lm-sensors@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/lm-sensors ^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH] hwmon: Consider LM64 temperature offset 2011-02-08 15:28 ` [lm-sensors] " Guenter Roeck @ 2011-02-08 15:54 ` Eibach, Dirk -1 siblings, 0 replies; 14+ messages in thread From: Eibach, Dirk @ 2011-02-08 15:54 UTC (permalink / raw) To: Guenter Roeck; +Cc: linux-kernel, khali, lm-sensors Dear Guenter, > Chip id is already detected in lm63_detect. You don't need to > detect it again. > The more common approach would be something along the line of > data->kind = id->driver_data; > You would then use > if (data->kind == lm64) > throughout the code. In addition to that, you could define > data->kind = id->driver_data; > if (data->kind == lm64) > data->offset = 16000; > which would save you the repeated recalculation of offset > as mentioned before. I don't understand, what structures "data" and "id" you are referring to here and where the fields driver_data and kind come from. I remember to have seen such in older kernels, but wasn't that replaced sometime ago? Cheers Dirk ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [lm-sensors] [PATCH] hwmon: Consider LM64 temperature offset @ 2011-02-08 15:54 ` Eibach, Dirk 0 siblings, 0 replies; 14+ messages in thread From: Eibach, Dirk @ 2011-02-08 15:54 UTC (permalink / raw) To: Guenter Roeck; +Cc: linux-kernel, khali, lm-sensors Dear Guenter, > Chip id is already detected in lm63_detect. You don't need to > detect it again. > The more common approach would be something along the line of > data->kind = id->driver_data; > You would then use > if (data->kind = lm64) > throughout the code. In addition to that, you could define > data->kind = id->driver_data; > if (data->kind = lm64) > data->offset = 16000; > which would save you the repeated recalculation of offset > as mentioned before. I don't understand, what structures "data" and "id" you are referring to here and where the fields driver_data and kind come from. I remember to have seen such in older kernels, but wasn't that replaced sometime ago? Cheers Dirk _______________________________________________ lm-sensors mailing list lm-sensors@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/lm-sensors ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] hwmon: Consider LM64 temperature offset 2011-02-08 15:54 ` [lm-sensors] " Eibach, Dirk @ 2011-02-08 16:07 ` Guenter Roeck -1 siblings, 0 replies; 14+ messages in thread From: Guenter Roeck @ 2011-02-08 16:07 UTC (permalink / raw) To: Eibach, Dirk; +Cc: linux-kernel, khali, lm-sensors On Tue, Feb 08, 2011 at 10:54:52AM -0500, Eibach, Dirk wrote: > > Dear Guenter, > > > Chip id is already detected in lm63_detect. You don't need to > > detect it again. > > The more common approach would be something along the line of > > data->kind = id->driver_data; > > You would then use > > if (data->kind == lm64) > > throughout the code. In addition to that, you could define > > data->kind = id->driver_data; > > if (data->kind == lm64) > > data->offset = 16000; > > which would save you the repeated recalculation of offset > > as mentioned before. > > I don't understand, what structures "data" and "id" you are referring to > here and where the fields driver_data and kind come from. I remember to > have seen such in older kernels, but wasn't that replaced sometime ago? > static int lm63_probe(struct i2c_client *new_client, const struct i2c_device_id *id) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ { struct lm63_data *data; ^^^^^^^^^^^^^^^^^^^^^^ ... /* Set the device type */ data->kind = id->driver_data; with struct lm63_data { ... int kind; // instead of is_lm64 ... }; id is from static const struct i2c_device_id lm63_id[] = { { "lm63", lm63 }, { "lm64", lm64 }, { } }; id->driver_data is thus either lm63 or lm64, depending on the chip type detected in lm63_detect(). Thanks, Guenter ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [lm-sensors] [PATCH] hwmon: Consider LM64 temperature offset @ 2011-02-08 16:07 ` Guenter Roeck 0 siblings, 0 replies; 14+ messages in thread From: Guenter Roeck @ 2011-02-08 16:07 UTC (permalink / raw) To: Eibach, Dirk; +Cc: linux-kernel, khali, lm-sensors On Tue, Feb 08, 2011 at 10:54:52AM -0500, Eibach, Dirk wrote: > > Dear Guenter, > > > Chip id is already detected in lm63_detect. You don't need to > > detect it again. > > The more common approach would be something along the line of > > data->kind = id->driver_data; > > You would then use > > if (data->kind = lm64) > > throughout the code. In addition to that, you could define > > data->kind = id->driver_data; > > if (data->kind = lm64) > > data->offset = 16000; > > which would save you the repeated recalculation of offset > > as mentioned before. > > I don't understand, what structures "data" and "id" you are referring to > here and where the fields driver_data and kind come from. I remember to > have seen such in older kernels, but wasn't that replaced sometime ago? > static int lm63_probe(struct i2c_client *new_client, const struct i2c_device_id *id) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ { struct lm63_data *data; ^^^^^^^^^^^^^^^^^^^^^^ ... /* Set the device type */ data->kind = id->driver_data; with struct lm63_data { ... int kind; // instead of is_lm64 ... }; id is from static const struct i2c_device_id lm63_id[] = { { "lm63", lm63 }, { "lm64", lm64 }, { } }; id->driver_data is thus either lm63 or lm64, depending on the chip type detected in lm63_detect(). Thanks, Guenter _______________________________________________ lm-sensors mailing list lm-sensors@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/lm-sensors ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] hwmon: Consider LM64 temperature offset 2011-02-08 15:54 ` [lm-sensors] " Eibach, Dirk @ 2011-02-08 16:09 ` Jean Delvare -1 siblings, 0 replies; 14+ messages in thread From: Jean Delvare @ 2011-02-08 16:09 UTC (permalink / raw) To: Eibach, Dirk; +Cc: Guenter Roeck, linux-kernel, lm-sensors Hi Dirk, On Tue, 8 Feb 2011 16:54:52 +0100, Eibach, Dirk wrote: > > Dear Guenter, > > > Chip id is already detected in lm63_detect. You don't need to > > detect it again. > > The more common approach would be something along the line of > > data->kind = id->driver_data; > > You would then use > > if (data->kind == lm64) > > throughout the code. In addition to that, you could define > > data->kind = id->driver_data; > > if (data->kind == lm64) > > data->offset = 16000; > > which would save you the repeated recalculation of offset > > as mentioned before. > > I don't understand, what structures "data" and "id" you are referring to data is struct lm63_data and id is const struct i2c_device_id which is passed as a second parameter to the probe function. > here and where the fields driver_data and kind come from. I remember to > have seen such in older kernels, but wasn't that replaced sometime ago? There were indeed some changes in this area in the past few years, but what Guenter says still applies to current kernels. Look at lm90_probe() in drivers/hwmon/lm90.c for an example (amongst may others.) -- Jean Delvare ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [lm-sensors] [PATCH] hwmon: Consider LM64 temperature offset @ 2011-02-08 16:09 ` Jean Delvare 0 siblings, 0 replies; 14+ messages in thread From: Jean Delvare @ 2011-02-08 16:09 UTC (permalink / raw) To: Eibach, Dirk; +Cc: Guenter Roeck, linux-kernel, lm-sensors Hi Dirk, On Tue, 8 Feb 2011 16:54:52 +0100, Eibach, Dirk wrote: > > Dear Guenter, > > > Chip id is already detected in lm63_detect. You don't need to > > detect it again. > > The more common approach would be something along the line of > > data->kind = id->driver_data; > > You would then use > > if (data->kind = lm64) > > throughout the code. In addition to that, you could define > > data->kind = id->driver_data; > > if (data->kind = lm64) > > data->offset = 16000; > > which would save you the repeated recalculation of offset > > as mentioned before. > > I don't understand, what structures "data" and "id" you are referring to data is struct lm63_data and id is const struct i2c_device_id which is passed as a second parameter to the probe function. > here and where the fields driver_data and kind come from. I remember to > have seen such in older kernels, but wasn't that replaced sometime ago? There were indeed some changes in this area in the past few years, but what Guenter says still applies to current kernels. Look at lm90_probe() in drivers/hwmon/lm90.c for an example (amongst may others.) -- Jean Delvare _______________________________________________ lm-sensors mailing list lm-sensors@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/lm-sensors ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2] hwmon: Consider LM64 temperature offset 2011-02-08 15:28 ` [lm-sensors] " Guenter Roeck @ 2011-02-09 9:51 ` Dirk Eibach -1 siblings, 0 replies; 14+ messages in thread From: Dirk Eibach @ 2011-02-09 9:51 UTC (permalink / raw) To: linux-kernel; +Cc: khali, guenter.roeck, lm-sensors, Dirk Eibach LM64 has 16 degrees Celsius temperature offset on all remote sensor registers. This was not considered When LM64 support was added to lm63.c. Signed-off-by: Dirk Eibach <eibach@gdsys.de> --- Changes since v1: - avoid calculating offset repeatedly - gave reasonable names to 8bit temperature accessors - removed redundant chip id detection - added comment why temp2_crit cannot be set drivers/hwmon/lm63.c | 56 +++++++++++++++++++++++++++++++++++++++---------- 1 files changed, 44 insertions(+), 12 deletions(-) diff --git a/drivers/hwmon/lm63.c b/drivers/hwmon/lm63.c index 776aeb3..2d0cf6d 100644 --- a/drivers/hwmon/lm63.c +++ b/drivers/hwmon/lm63.c @@ -98,6 +98,9 @@ static const unsigned short normal_i2c[] = { 0x18, 0x4c, 0x4e, I2C_CLIENT_END }; * value, it uses signed 8-bit values with LSB = 1 degree Celsius. * For remote temperature, low and high limits, it uses signed 11-bit values * with LSB = 0.125 degree Celsius, left-justified in 16-bit registers. + * For LM64 the actual remote diode temperature is 16 degree Celsius higher + * than the register reading. Remote temperature setpoints have to be + * adapted accordingly. */ #define FAN_FROM_REG(reg) ((reg) == 0xFFFC || (reg) == 0 ? 0 : \ @@ -165,6 +168,8 @@ struct lm63_data { struct mutex update_lock; char valid; /* zero until following fields are valid */ unsigned long last_updated; /* in jiffies */ + int kind; + int temp2_offset; /* registers values */ u8 config, config_fan; @@ -247,16 +252,34 @@ static ssize_t show_pwm1_enable(struct device *dev, struct device_attribute *dum return sprintf(buf, "%d\n", data->config_fan & 0x20 ? 1 : 2); } -static ssize_t show_temp8(struct device *dev, struct device_attribute *devattr, - char *buf) +/* + * There are 8bit registers for both local(temp1) and remote(temp2) sensor. + * For remote sensor registers temp2_offset has to be considered, + * for local sensor it must not. + * So we need separate 8bit accessors for local and remote sensor. + */ +static ssize_t show_local_temp8(struct device *dev, + struct device_attribute *devattr, + char *buf) { struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); struct lm63_data *data = lm63_update_device(dev); return sprintf(buf, "%d\n", TEMP8_FROM_REG(data->temp8[attr->index])); } -static ssize_t set_temp8(struct device *dev, struct device_attribute *dummy, - const char *buf, size_t count) +static ssize_t show_remote_temp8(struct device *dev, + struct device_attribute *devattr, + char *buf) +{ + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); + struct lm63_data *data = lm63_update_device(dev); + return sprintf(buf, "%d\n", TEMP8_FROM_REG(data->temp8[attr->index]) + + data->temp2_offset); +} + +static ssize_t set_local_temp8(struct device *dev, + struct device_attribute *dummy, + const char *buf, size_t count) { struct i2c_client *client = to_i2c_client(dev); struct lm63_data *data = i2c_get_clientdata(client); @@ -274,7 +297,8 @@ static ssize_t show_temp11(struct device *dev, struct device_attribute *devattr, { struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); struct lm63_data *data = lm63_update_device(dev); - return sprintf(buf, "%d\n", TEMP11_FROM_REG(data->temp11[attr->index])); + return sprintf(buf, "%d\n", TEMP11_FROM_REG(data->temp11[attr->index]) + + data->temp2_offset); } static ssize_t set_temp11(struct device *dev, struct device_attribute *devattr, @@ -294,7 +318,7 @@ static ssize_t set_temp11(struct device *dev, struct device_attribute *devattr, int nr = attr->index; mutex_lock(&data->update_lock); - data->temp11[nr] = TEMP11_TO_REG(val); + data->temp11[nr] = TEMP11_TO_REG(val - data->temp2_offset); i2c_smbus_write_byte_data(client, reg[(nr - 1) * 2], data->temp11[nr] >> 8); i2c_smbus_write_byte_data(client, reg[(nr - 1) * 2 + 1], @@ -310,6 +334,7 @@ static ssize_t show_temp2_crit_hyst(struct device *dev, struct device_attribute { struct lm63_data *data = lm63_update_device(dev); return sprintf(buf, "%d\n", TEMP8_FROM_REG(data->temp8[2]) + + data->temp2_offset - TEMP8_FROM_REG(data->temp2_crit_hyst)); } @@ -324,7 +349,7 @@ static ssize_t set_temp2_crit_hyst(struct device *dev, struct device_attribute * long hyst; mutex_lock(&data->update_lock); - hyst = TEMP8_FROM_REG(data->temp8[2]) - val; + hyst = TEMP8_FROM_REG(data->temp8[2]) + data->temp2_offset - val; i2c_smbus_write_byte_data(client, LM63_REG_REMOTE_TCRIT_HYST, HYST_TO_REG(hyst)); mutex_unlock(&data->update_lock); @@ -355,16 +380,18 @@ static SENSOR_DEVICE_ATTR(fan1_min, S_IWUSR | S_IRUGO, show_fan, static DEVICE_ATTR(pwm1, S_IWUSR | S_IRUGO, show_pwm1, set_pwm1); static DEVICE_ATTR(pwm1_enable, S_IRUGO, show_pwm1_enable, NULL); -static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp8, NULL, 0); -static SENSOR_DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO, show_temp8, - set_temp8, 1); +static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_local_temp8, NULL, 0); +static SENSOR_DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO, show_local_temp8, + set_local_temp8, 1); static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp11, NULL, 0); static SENSOR_DEVICE_ATTR(temp2_min, S_IWUSR | S_IRUGO, show_temp11, set_temp11, 1); static SENSOR_DEVICE_ATTR(temp2_max, S_IWUSR | S_IRUGO, show_temp11, set_temp11, 2); -static SENSOR_DEVICE_ATTR(temp2_crit, S_IRUGO, show_temp8, NULL, 2); +/* temp2_crit can only be set once which should be job of the bootloader */ +static SENSOR_DEVICE_ATTR(temp2_crit, S_IRUGO, show_remote_temp8, + NULL, 2); static DEVICE_ATTR(temp2_crit_hyst, S_IWUSR | S_IRUGO, show_temp2_crit_hyst, set_temp2_crit_hyst); @@ -479,7 +506,12 @@ static int lm63_probe(struct i2c_client *new_client, data->valid = 0; mutex_init(&data->update_lock); - /* Initialize the LM63 chip */ + /* Set the device type */ + data->kind = id->driver_data; + if (data->kind == lm64) + data->temp2_offset = 16000; + + /* Initialize chip */ lm63_init_client(new_client); /* Register sysfs hooks */ -- 1.5.6.5 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [lm-sensors] [PATCH v2] hwmon: Consider LM64 temperature offset @ 2011-02-09 9:51 ` Dirk Eibach 0 siblings, 0 replies; 14+ messages in thread From: Dirk Eibach @ 2011-02-09 9:51 UTC (permalink / raw) To: linux-kernel; +Cc: khali, guenter.roeck, lm-sensors, Dirk Eibach LM64 has 16 degrees Celsius temperature offset on all remote sensor registers. This was not considered When LM64 support was added to lm63.c. Signed-off-by: Dirk Eibach <eibach@gdsys.de> --- Changes since v1: - avoid calculating offset repeatedly - gave reasonable names to 8bit temperature accessors - removed redundant chip id detection - added comment why temp2_crit cannot be set drivers/hwmon/lm63.c | 56 +++++++++++++++++++++++++++++++++++++++---------- 1 files changed, 44 insertions(+), 12 deletions(-) diff --git a/drivers/hwmon/lm63.c b/drivers/hwmon/lm63.c index 776aeb3..2d0cf6d 100644 --- a/drivers/hwmon/lm63.c +++ b/drivers/hwmon/lm63.c @@ -98,6 +98,9 @@ static const unsigned short normal_i2c[] = { 0x18, 0x4c, 0x4e, I2C_CLIENT_END }; * value, it uses signed 8-bit values with LSB = 1 degree Celsius. * For remote temperature, low and high limits, it uses signed 11-bit values * with LSB = 0.125 degree Celsius, left-justified in 16-bit registers. + * For LM64 the actual remote diode temperature is 16 degree Celsius higher + * than the register reading. Remote temperature setpoints have to be + * adapted accordingly. */ #define FAN_FROM_REG(reg) ((reg) = 0xFFFC || (reg) = 0 ? 0 : \ @@ -165,6 +168,8 @@ struct lm63_data { struct mutex update_lock; char valid; /* zero until following fields are valid */ unsigned long last_updated; /* in jiffies */ + int kind; + int temp2_offset; /* registers values */ u8 config, config_fan; @@ -247,16 +252,34 @@ static ssize_t show_pwm1_enable(struct device *dev, struct device_attribute *dum return sprintf(buf, "%d\n", data->config_fan & 0x20 ? 1 : 2); } -static ssize_t show_temp8(struct device *dev, struct device_attribute *devattr, - char *buf) +/* + * There are 8bit registers for both local(temp1) and remote(temp2) sensor. + * For remote sensor registers temp2_offset has to be considered, + * for local sensor it must not. + * So we need separate 8bit accessors for local and remote sensor. + */ +static ssize_t show_local_temp8(struct device *dev, + struct device_attribute *devattr, + char *buf) { struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); struct lm63_data *data = lm63_update_device(dev); return sprintf(buf, "%d\n", TEMP8_FROM_REG(data->temp8[attr->index])); } -static ssize_t set_temp8(struct device *dev, struct device_attribute *dummy, - const char *buf, size_t count) +static ssize_t show_remote_temp8(struct device *dev, + struct device_attribute *devattr, + char *buf) +{ + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); + struct lm63_data *data = lm63_update_device(dev); + return sprintf(buf, "%d\n", TEMP8_FROM_REG(data->temp8[attr->index]) + + data->temp2_offset); +} + +static ssize_t set_local_temp8(struct device *dev, + struct device_attribute *dummy, + const char *buf, size_t count) { struct i2c_client *client = to_i2c_client(dev); struct lm63_data *data = i2c_get_clientdata(client); @@ -274,7 +297,8 @@ static ssize_t show_temp11(struct device *dev, struct device_attribute *devattr, { struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); struct lm63_data *data = lm63_update_device(dev); - return sprintf(buf, "%d\n", TEMP11_FROM_REG(data->temp11[attr->index])); + return sprintf(buf, "%d\n", TEMP11_FROM_REG(data->temp11[attr->index]) + + data->temp2_offset); } static ssize_t set_temp11(struct device *dev, struct device_attribute *devattr, @@ -294,7 +318,7 @@ static ssize_t set_temp11(struct device *dev, struct device_attribute *devattr, int nr = attr->index; mutex_lock(&data->update_lock); - data->temp11[nr] = TEMP11_TO_REG(val); + data->temp11[nr] = TEMP11_TO_REG(val - data->temp2_offset); i2c_smbus_write_byte_data(client, reg[(nr - 1) * 2], data->temp11[nr] >> 8); i2c_smbus_write_byte_data(client, reg[(nr - 1) * 2 + 1], @@ -310,6 +334,7 @@ static ssize_t show_temp2_crit_hyst(struct device *dev, struct device_attribute { struct lm63_data *data = lm63_update_device(dev); return sprintf(buf, "%d\n", TEMP8_FROM_REG(data->temp8[2]) + + data->temp2_offset - TEMP8_FROM_REG(data->temp2_crit_hyst)); } @@ -324,7 +349,7 @@ static ssize_t set_temp2_crit_hyst(struct device *dev, struct device_attribute * long hyst; mutex_lock(&data->update_lock); - hyst = TEMP8_FROM_REG(data->temp8[2]) - val; + hyst = TEMP8_FROM_REG(data->temp8[2]) + data->temp2_offset - val; i2c_smbus_write_byte_data(client, LM63_REG_REMOTE_TCRIT_HYST, HYST_TO_REG(hyst)); mutex_unlock(&data->update_lock); @@ -355,16 +380,18 @@ static SENSOR_DEVICE_ATTR(fan1_min, S_IWUSR | S_IRUGO, show_fan, static DEVICE_ATTR(pwm1, S_IWUSR | S_IRUGO, show_pwm1, set_pwm1); static DEVICE_ATTR(pwm1_enable, S_IRUGO, show_pwm1_enable, NULL); -static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp8, NULL, 0); -static SENSOR_DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO, show_temp8, - set_temp8, 1); +static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_local_temp8, NULL, 0); +static SENSOR_DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO, show_local_temp8, + set_local_temp8, 1); static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp11, NULL, 0); static SENSOR_DEVICE_ATTR(temp2_min, S_IWUSR | S_IRUGO, show_temp11, set_temp11, 1); static SENSOR_DEVICE_ATTR(temp2_max, S_IWUSR | S_IRUGO, show_temp11, set_temp11, 2); -static SENSOR_DEVICE_ATTR(temp2_crit, S_IRUGO, show_temp8, NULL, 2); +/* temp2_crit can only be set once which should be job of the bootloader */ +static SENSOR_DEVICE_ATTR(temp2_crit, S_IRUGO, show_remote_temp8, + NULL, 2); static DEVICE_ATTR(temp2_crit_hyst, S_IWUSR | S_IRUGO, show_temp2_crit_hyst, set_temp2_crit_hyst); @@ -479,7 +506,12 @@ static int lm63_probe(struct i2c_client *new_client, data->valid = 0; mutex_init(&data->update_lock); - /* Initialize the LM63 chip */ + /* Set the device type */ + data->kind = id->driver_data; + if (data->kind = lm64) + data->temp2_offset = 16000; + + /* Initialize chip */ lm63_init_client(new_client); /* Register sysfs hooks */ -- 1.5.6.5 _______________________________________________ lm-sensors mailing list lm-sensors@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/lm-sensors ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2] hwmon: Consider LM64 temperature offset 2011-02-09 9:51 ` [lm-sensors] " Dirk Eibach @ 2011-02-09 18:17 ` Guenter Roeck -1 siblings, 0 replies; 14+ messages in thread From: Guenter Roeck @ 2011-02-09 18:17 UTC (permalink / raw) To: Dirk Eibach; +Cc: linux-kernel, khali, lm-sensors Hi Dirk, On Wed, Feb 09, 2011 at 04:51:34AM -0500, Dirk Eibach wrote: [ ... ] > +/* temp2_crit can only be set once which should be job of the bootloader */ > +static SENSOR_DEVICE_ATTR(temp2_crit, S_IRUGO, show_remote_temp8, > + NULL, 2); That only applies to lm63, though. It can be rewritten anytime on LM64. That would be a separate patch, though, to not mix improvements with bug fixes. Otherwise, looks good, so I'll apply it with an added remark that the restriction only applies to lm63. Thanks, Guenter ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [lm-sensors] [PATCH v2] hwmon: Consider LM64 temperature offset @ 2011-02-09 18:17 ` Guenter Roeck 0 siblings, 0 replies; 14+ messages in thread From: Guenter Roeck @ 2011-02-09 18:17 UTC (permalink / raw) To: Dirk Eibach; +Cc: linux-kernel, khali, lm-sensors Hi Dirk, On Wed, Feb 09, 2011 at 04:51:34AM -0500, Dirk Eibach wrote: [ ... ] > +/* temp2_crit can only be set once which should be job of the bootloader */ > +static SENSOR_DEVICE_ATTR(temp2_crit, S_IRUGO, show_remote_temp8, > + NULL, 2); That only applies to lm63, though. It can be rewritten anytime on LM64. That would be a separate patch, though, to not mix improvements with bug fixes. Otherwise, looks good, so I'll apply it with an added remark that the restriction only applies to lm63. Thanks, Guenter _______________________________________________ lm-sensors mailing list lm-sensors@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/lm-sensors ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2011-02-09 18:17 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-02-08 13:16 [PATCH] hwmon: Consider LM64 temperature offset Dirk Eibach 2011-02-08 13:16 ` [lm-sensors] " Dirk Eibach 2011-02-08 15:28 ` Guenter Roeck 2011-02-08 15:28 ` [lm-sensors] " Guenter Roeck 2011-02-08 15:54 ` Eibach, Dirk 2011-02-08 15:54 ` [lm-sensors] " Eibach, Dirk 2011-02-08 16:07 ` Guenter Roeck 2011-02-08 16:07 ` [lm-sensors] " Guenter Roeck 2011-02-08 16:09 ` Jean Delvare 2011-02-08 16:09 ` [lm-sensors] " Jean Delvare 2011-02-09 9:51 ` [PATCH v2] " Dirk Eibach 2011-02-09 9:51 ` [lm-sensors] " Dirk Eibach 2011-02-09 18:17 ` Guenter Roeck 2011-02-09 18:17 ` [lm-sensors] " Guenter Roeck
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.