HWMON: TMP102 - Added support for 12 bit data

PATCH In review Ankur Patel 2 participants 5 messages Jun 23, 2011 2 messages not in the archive Hardware monitoring

lore

Share & tools

lore

Ankur Patel

author

2011-06-23 07:53 UTC #0 lore raw

PATCH 1/1 HWMON: TMP102 - Added support for 12 bit data

From: Ankur Patel <[email protected]>

In the tmp102.ko driver support for 12 bit data is added. Also, the sysfs names are replaced with the actual chip supported fuctionality names.

Signed-off-by Ankur Patel <[email protected]>

---
 drivers/hwmon/tmp102.c |   40 ++++++++++++++++++++++++++--------------
 1 files changed, 26 insertions(+), 14 deletions(-)

drivers/hwmon/tmp102.c +26 ~~-14~~

index 5bd1949..a13b572 100644
@@ -47,6 +47,9 @@
 #define	TMP102_TLOW_REG			0x02
 #define	TMP102_THIGH_REG		0x03
 
+#define TMP102_CONFIG  (TMP102_CONF_TM | TMP102_CONF_EM | TMP102_CONF_CR1)
+#define TMP102_CONFIG_RD_ONLY (TMP102_CONF_R0 | TMP102_CONF_R1 | TMP102_CONF_AL)
+
 struct tmp102 {
 	struct device *hwmon_dev;
 	struct mutex lock;

@@ -68,16 +71,26 @@ static inline int tmp102_write_reg(struct i2c_client *client, u8 reg, u16 val)
 	return i2c_smbus_write_word_data(client, reg, swab16(val));
 }
 
-/* convert left adjusted 13-bit TMP102 register value to milliCelsius */
+/* convert left adjusted TMP102 register value to milliCelsius for 13 bit &
+ * 12 bit by dividing with 128 & 256 respectively*/
 static inline int tmp102_reg_to_mC(s16 val)
 {
-	return ((val & ~0x01) * 1000) / 128;
+	#if TMP102_CONFIG & TMP102_CONF_EM
+		return ((val & ~0x01) * 1000) / 128;
+	#else
+		return ((val & ~0x01) * 1000) / 256;
+	#endif
 }
 
-/* convert milliCelsius to left adjusted 13-bit TMP102 register value */
+/* convert milliCelsius to left adjusted TMP102 register value for 13 bit &
+ * 12 bit by multiplying with 128 & 256 respectivley*/
 static inline u16 tmp102_mC_to_reg(int val)
 {
-	return (val * 128) / 1000;
+	#if TMP102_CONFIG & TMP102_CONF_EM
+		return (val * 128) / 1000;
+	#else
+		return (val * 256) / 1000;
+	#endif
 }
 
 static const u8 tmp102_reg[] = {

@@ -126,7 +139,9 @@ static ssize_t tmp102_set_temp(struct device *dev,
 
 	if (strict_strtol(buf, 10, &val) < 0)
 		return -EINVAL;
-	val = SENSORS_LIMIT(val, -256000, 255000);
+	/* It will limit the Input temperature range to -55 Deg. Cel to
+	 * 150 Deg. Cel */
+	val = val = SENSORS_LIMIT(val, -55000, 150000);
 
 	mutex_lock(&tmp102->lock);
 	tmp102->temp[sda->index] = val;

@@ -138,16 +153,16 @@ static ssize_t tmp102_set_temp(struct device *dev,
 
 static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, tmp102_show_temp, NULL , 0);
 
-static SENSOR_DEVICE_ATTR(temp1_max_hyst, S_IWUSR | S_IRUGO, tmp102_show_temp,
-			  tmp102_set_temp, 1);
+static SENSOR_DEVICE_ATTR(temp1_low_thresold, S_IWUSR | S_IRUGO,
+					tmp102_show_temp, tmp102_set_temp, 1);
 
-static SENSOR_DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO, tmp102_show_temp,
-			  tmp102_set_temp, 2);
+static SENSOR_DEVICE_ATTR(temp1_max_thresold, S_IWUSR | S_IRUGO,
+\					tmp102_show_temp, tmp102_set_temp, 2);
 
 static struct attribute *tmp102_attributes[] = {
 	&sensor_dev_attr_temp1_input.dev_attr.attr,
-	&sensor_dev_attr_temp1_max_hyst.dev_attr.attr,
-	&sensor_dev_attr_temp1_max.dev_attr.attr,
+	&sensor_dev_attr_temp1_low_thresold.dev_attr.attr,
+	&sensor_dev_attr_temp1_max_thresold.dev_attr.attr,
 	NULL
 };

@@ -155,9 +170,6 @@ static const struct attribute_group tmp102_attr_group = {
 	.attrs = tmp102_attributes,
 };
 
-#define TMP102_CONFIG  (TMP102_CONF_TM | TMP102_CONF_EM | TMP102_CONF_CR1)
-#define TMP102_CONFIG_RD_ONLY (TMP102_CONF_R0 | TMP102_CONF_R1 | TMP102_CONF_AL)
-
 static int __devinit tmp102_probe(struct i2c_client *client,
 				  const struct i2c_device_id *id)
 {

Signature · 1 line

1.7.0.4

Jean Delvare

2011-06-23 08:06 UTC #1 lore raw

Quoted 5 lines · Ankur Patel

From: Ankur Patel <[email protected]>
In the tmp102.ko driver support for 12 bit data is added. Also, the sysfs names are replaced with the actual chip supported fuctionality names.

Nack. These attribute names are standard, see Documentation/hwmon/sysfs-interface. You don't get to change them.

What problem were you trying to solve? libsensors expects the attribute names which are in use today.

Quoted 95 lines

Signed-off-by Ankur Patel <[email protected]>

---
 drivers/hwmon/tmp102.c |   40 ++++++++++++++++++++++++++--------------
 1 files changed, 26 insertions(+), 14 deletions(-)

drivers/hwmon/tmp102.c +26 ~~-14~~

index 5bd1949..a13b572 100644
@@ -47,6 +47,9 @@
 #define	TMP102_TLOW_REG			0x02
 #define	TMP102_THIGH_REG		0x03
 
+#define TMP102_CONFIG  (TMP102_CONF_TM | TMP102_CONF_EM | TMP102_CONF_CR1)
+#define TMP102_CONFIG_RD_ONLY (TMP102_CONF_R0 | TMP102_CONF_R1 | TMP102_CONF_AL)
+
 struct tmp102 {
 	struct device *hwmon_dev;
 	struct mutex lock;

@@ -68,16 +71,26 @@ static inline int tmp102_write_reg(struct i2c_client *client, u8 reg, u16 val)
 	return i2c_smbus_write_word_data(client, reg, swab16(val));
 }
 
-/* convert left adjusted 13-bit TMP102 register value to milliCelsius */
+/* convert left adjusted TMP102 register value to milliCelsius for 13 bit &
+ * 12 bit by dividing with 128 & 256 respectively*/
 static inline int tmp102_reg_to_mC(s16 val)
 {
-	return ((val & ~0x01) * 1000) / 128;
+	#if TMP102_CONFIG & TMP102_CONF_EM
+		return ((val & ~0x01) * 1000) / 128;
+	#else
+		return ((val & ~0x01) * 1000) / 256;
+	#endif
 }
 
-/* convert milliCelsius to left adjusted 13-bit TMP102 register value */
+/* convert milliCelsius to left adjusted TMP102 register value for 13 bit &
+ * 12 bit by multiplying with 128 & 256 respectivley*/
 static inline u16 tmp102_mC_to_reg(int val)
 {
-	return (val * 128) / 1000;
+	#if TMP102_CONFIG & TMP102_CONF_EM
+		return (val * 128) / 1000;
+	#else
+		return (val * 256) / 1000;
+	#endif
 }
 
 static const u8 tmp102_reg[] = {

@@ -126,7 +139,9 @@ static ssize_t tmp102_set_temp(struct device *dev,
 
 	if (strict_strtol(buf, 10, &val) < 0)
 		return -EINVAL;
-	val = SENSORS_LIMIT(val, -256000, 255000);
+	/* It will limit the Input temperature range to -55 Deg. Cel to
+	 * 150 Deg. Cel */
+	val = val = SENSORS_LIMIT(val, -55000, 150000);
 
 	mutex_lock(&tmp102->lock);
 	tmp102->temp[sda->index] = val;

@@ -138,16 +153,16 @@ static ssize_t tmp102_set_temp(struct device *dev,
 
 static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, tmp102_show_temp, NULL , 0);
 
-static SENSOR_DEVICE_ATTR(temp1_max_hyst, S_IWUSR | S_IRUGO, tmp102_show_temp,
-			  tmp102_set_temp, 1);
+static SENSOR_DEVICE_ATTR(temp1_low_thresold, S_IWUSR | S_IRUGO,
+					tmp102_show_temp, tmp102_set_temp, 1);
 
-static SENSOR_DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO, tmp102_show_temp,
-			  tmp102_set_temp, 2);
+static SENSOR_DEVICE_ATTR(temp1_max_thresold, S_IWUSR | S_IRUGO,
+\					tmp102_show_temp, tmp102_set_temp, 2);
 
 static struct attribute *tmp102_attributes[] = {
 	&sensor_dev_attr_temp1_input.dev_attr.attr,
-	&sensor_dev_attr_temp1_max_hyst.dev_attr.attr,
-	&sensor_dev_attr_temp1_max.dev_attr.attr,
+	&sensor_dev_attr_temp1_low_thresold.dev_attr.attr,
+	&sensor_dev_attr_temp1_max_thresold.dev_attr.attr,
 	NULL
 };

@@ -155,9 +170,6 @@ static const struct attribute_group tmp102_attr_group = {
 	.attrs = tmp102_attributes,
 };
 
-#define TMP102_CONFIG  (TMP102_CONF_TM | TMP102_CONF_EM | TMP102_CONF_CR1)
-#define TMP102_CONFIG_RD_ONLY (TMP102_CONF_R0 | TMP102_CONF_R1 | TMP102_CONF_AL)
-
 static int __devinit tmp102_probe(struct i2c_client *client,
 				  const struct i2c_device_id *id)
 {

Signature · 1 line

Jean Delvare

A message that is not in the archive (sent off-list or not yet received): [email protected]

Jean Delvare

2011-06-23 08:45 UTC #3 lore raw

Quoted 13 lines · Ankur Patel

Quoted 11 lines · Jean Delvare
Quoted 5 lines · Ankur Patel
From: Ankur Patel <[email protected]>
In the tmp102.ko driver support for 12 bit data is added. Also, the sysfs names are replaced with the actual chip supported fuctionality names.
Nack. These attribute names are standard, see Documentation/hwmon/sysfs-interface. You don't get to change them.

Quoted 5 lines

Quoted 4 lines
What problem were you trying to solve? libsensors expects the attribute names which are in use today.
Agree, But using temp1_max_hyst as an attribute for presenting low level
threshold, will it not more feasible to use temp1_low instead?.

No. This isn't a min temperature limit. Please read the TMP102 datasheet and in particular Figure 10 "Output Transfer Function Diagrams" and the description of the AL configuration bit.

Signature · 1 line

Jean Delvare

A message that is not in the archive (sent off-list or not yet received): [email protected]

Jean Delvare

2011-06-23 09:08 UTC #5 lore raw

Quoted 24 lines · Ankur Patel

Quoted 22 lines · Jean Delvare
Quoted 1 line · Ankur Patel
On Thu, Jun 23, 2011 at 1:36 PM, Jean Delvare <[email protected]>
Quoted 8 lines · wrote:
Quoted 8 lines · Ankur Patel
Quoted 5 lines
From: Ankur Patel <[email protected]>
In the tmp102.ko driver support for 12 bit data is added. Also, the sysfs names are replaced with the actual chip supported fuctionality names.
Quoted 8 lines
Quoted 8 lines
Nack. These attribute names are standard, see Documentation/hwmon/sysfs-interface. You don't get to change them.
What problem were you trying to solve? libsensors expects the attribute names which are in use today.
Agree, But using temp1_max_hyst as an attribute for presenting low
level
Quoted 1 line
threshold, will it not more feasible to use temp1_low instead?.

Quoted 7 lines

Quoted 5 lines
No. This isn't a min temperature limit. Please read the TMP102 datasheet and in particular Figure 10 "Output Transfer Function Diagrams" and the description of the AL configuration bit.
Understood. Thank you. I am reverting back the changes for sysfs attribute
names in the submitted patch to original. Do you have other Issue apart from that?

I didn't read the patch, only the header. So I don't know yet. Just resubmit and wait for review.

Signature · 1 line

Jean Delvare

Ankur Patel

author

2011-06-23 09:26 UTC #6 lore raw

Quoted 31 lines · Jean Delvare

Quoted 29 lines · Ankur Patel
Quoted 27 lines · Jean Delvare
Quoted 1 line · Ankur Patel
On Thu, Jun 23, 2011 at 1:36 PM, Jean Delvare <[email protected]>
Quoted 8 lines · wrote:
Quoted 8 lines · Ankur Patel
Quoted 5 lines
From: Ankur Patel <[email protected]>
In the tmp102.ko driver support for 12 bit data is added. Also, the sysfs names are replaced with the actual chip supported fuctionality names.
Quoted 8 lines
Quoted 8 lines
Nack. These attribute names are standard, see Documentation/hwmon/sysfs-interface. You don't get to change them.
What problem were you trying to solve? libsensors expects the attribute names which are in use today.
Agree, But using temp1_max_hyst as an attribute for presenting low
level
Quoted 1 line
threshold, will it not more feasible to use temp1_low instead?.
No. This isn't a min temperature limit. Please read the TMP102 datasheet and in particular Figure 10 "Output Transfer Function Diagrams" and the description of the AL configuration bit.
Understood. Thank you. I am reverting back the changes for sysfs attribute

Quoted 8 lines

Quoted 2 lines
names in the submitted patch to original. Do you have other Issue apart from that?
I didn't read the patch, only the header. So I don't know yet. Just resubmit and wait for review.
--
Jean Delvare

I forgot to switch from HTML to plain text and it get bounced from mailing list. I will resubmit patch soon.

Thank you,
Ankur.