[PATCH] hwmon: Consider LM64 temperature offset

[PATCH] hwmon: Consider LM64 temperature offset

[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] [PATCH] hwmon: Consider LM64 temperature offset

[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

Re: [PATCH] hwmon: Consider LM64 temperature offset

[Guenter Roeck]

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

Re: [lm-sensors] [PATCH] hwmon: Consider LM64 temperature offset

[Guenter Roeck]

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

RE: [PATCH] hwmon: Consider LM64 temperature offset

[Eibach, Dirk]

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

Re: [lm-sensors] [PATCH] hwmon: Consider LM64 temperature offset

[Eibach, Dirk]

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

Re: [PATCH] hwmon: Consider LM64 temperature offset

[Guenter Roeck]

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 

Re: [lm-sensors] [PATCH] hwmon: Consider LM64 temperature offset

[Guenter Roeck]

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

Re: [PATCH] hwmon: Consider LM64 temperature offset

[Jean Delvare]

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

Re: [lm-sensors] [PATCH] hwmon: Consider LM64 temperature offset

[Jean Delvare]

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

[PATCH v2] hwmon: Consider LM64 temperature offset

[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] [PATCH v2] hwmon: Consider LM64 temperature offset

[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

Re: [PATCH v2] hwmon: Consider LM64 temperature offset

[Guenter Roeck]

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

Re: [lm-sensors] [PATCH v2] hwmon: Consider LM64 temperature offset

[Guenter Roeck]

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