coretemp: Add TEMP_TARGET support

PATCH In review Rudolf Marek 2 participants 2 messages Dec 1, 2007 – Dec 16, 2007

Subsystems (3)

Documentation Hardware monitoring Coretemp hardware monitoring driver

lore

Share & tools

lore

Rudolf Marek

author

2007-12-01 20:37 UTC #0 lore raw

PATCH 1/1 coretemp: Add TEMP_TARGET support

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hello,

Attached patch enables driver to export the target temperature. This temperature is calibrated into each CPU, and serves as some indicator to cooling platform. All fans should spin at full speed if the temperature is reached.

Signed-off-by Rudolf Marek <[email protected]>

I managed to get answer from Intel at:

http://softwarecommunity.intel.com/isn/Community/en-US/forums/permalink/30228436/30230975/ShowThread.aspx#30230975

Can someone test if it works please? The lines in the patch are longer than 80 chars, imho it is allowed now?

coretemp-isa-0000
Adapter: ISA adapter
Core 0: +36.0°C (high = +65.0°C, crit = +85.0°C)

I have in plan to add Penryn CPUs to driver once I know the details.

Thanks,
Rudolf
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFHUcYT3J9wPJqZRNURAmVIAJ9nAzEvV5ga4aJOoQvoQ4+Z0KfKdACgsPzn zm379OPOCKjYP5TupXZXeqU=
=PMIb
-----END PGP SIGNATURE-----

drivers/hwmon/coretemp.c +28 -4

Index: linux-2.6.23.9/drivers/hwmon/coretemp.c
===================================================================
@@ -38,7 +38,7 @@
 
 #define DRVNAME	"coretemp"
 
-typedef enum { SHOW_TEMP, SHOW_TJMAX, SHOW_LABEL, SHOW_NAME } SHOW;
+typedef enum { SHOW_TEMP, SHOW_TJMAX, SHOW_TTARGET, SHOW_LABEL, SHOW_NAME } SHOW;
 
 /*
  * Functions declaration

@@ -55,6 +55,7 @@
 	unsigned long last_updated;	/* in jiffies */
 	int temp;
 	int tjmax;
+	int ttarget;
 	u8 alarm;
 };

@@ -95,9 +96,10 @@
 
 	if (attr->index == SHOW_TEMP)
 		err = data->valid ? sprintf(buf, "%d\n", data->temp) : -EAGAIN;
-	else
+	else if (attr->index == SHOW_TJMAX)
 		err = sprintf(buf, "%d\n", data->tjmax);
-
+	else
+		err = sprintf(buf, "%d\n", data->ttarget);
 	return err;
 }

@@ -105,6 +107,8 @@
 			  SHOW_TEMP);
 static SENSOR_DEVICE_ATTR(temp1_crit, S_IRUGO, show_temp, NULL,
 			  SHOW_TJMAX);
+static SENSOR_DEVICE_ATTR(temp1_max, S_IRUGO, show_temp, NULL,
+			  SHOW_TTARGET);
 static DEVICE_ATTR(temp1_crit_alarm, S_IRUGO, show_alarm, NULL);
 static SENSOR_DEVICE_ATTR(temp1_label, S_IRUGO, show_name, NULL, SHOW_LABEL);
 static SENSOR_DEVICE_ATTR(name, S_IRUGO, show_name, NULL, SHOW_NAME);

@@ -223,10 +227,27 @@
 			 "temperature might be wrong!\n");
 	}
 
+	data->ttarget = data->tjmax;
+
+	/* read the still undocumented IA32_TEMPERATURE_TARGET it exists
+	   on older CPUs but not here */
+	if (c->x86_model > 0xe) {
+		err = rdmsr_safe_on_cpu(data->id, 0x1a2, &eax, &edx);
+		if (err) {
+			dev_warn(&pdev->dev, "Unable to read IA32_TEMPERATURE_TARGET MSR\n");
+		} else {
+			data->ttarget = data->tjmax - (((eax >> 8) & 0xff) * 1000);
+			err = device_create_file(&pdev->dev,
+							&sensor_dev_attr_temp1_max.dev_attr);
+			if (err)
+				goto exit_free;
+		}
+	}
+
 	platform_set_drvdata(pdev, data);
 
 	if ((err = sysfs_create_group(&pdev->dev.kobj, &coretemp_group)))
-		goto exit_free;
+		goto exit_dev;
 
 	data->class_dev = hwmon_device_register(&pdev->dev);
 	if (IS_ERR(data->class_dev)) {

@@ -240,6 +261,8 @@
 
 exit_class:
 	sysfs_remove_group(&pdev->dev.kobj, &coretemp_group);
+exit_dev:
+	device_remove_file(&pdev->dev, &sensor_dev_attr_temp1_max.dev_attr);
 exit_free:
 	kfree(data);
 exit:

@@ -252,6 +275,7 @@
 
 	hwmon_device_unregister(data->class_dev);
 	sysfs_remove_group(&pdev->dev.kobj, &coretemp_group);
+	device_remove_file(&pdev->dev, &sensor_dev_attr_temp1_max.dev_attr);
 	platform_set_drvdata(pdev, NULL);
 	kfree(data);
 	return 0;

Documentation/hwmon/coretemp +2 -1

Index: linux-2.6.23.9/Documentation/hwmon/coretemp
===================================================================
@@ -25,7 +25,8 @@
 the Out-Of-Spec bit. Following table summarizes the exported sysfs files:
 
 temp1_input	 - Core temperature (in millidegrees Celsius).
-temp1_crit	 - Maximum junction temperature  (in millidegrees Celsius).
+temp1_max	 - All cooling devices should be turned on (on Core2).
+temp1_crit	 - Maximum junction temperature (in millidegrees Celsius).
 temp1_crit_alarm - Set when Out-of-spec bit is set, never clears.
 		   Correct CPU operation is no longer guaranteed.
 temp1_label	 - Contains string "Core X", where X is processor

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Jean Delvare

2007-12-16 11:34 UTC #1 lore raw

Hi Rudolf,

Quoted 4 lines · Rudolf Marek

Attached patch enables driver to export the target temperature. This temperature is calibrated into each CPU, and serves as some indicator to cooling platform. All fans should spin at full speed if the temperature is reached.

Quoted 8 lines

Signed-off-by Rudolf Marek <[email protected]>
I managed to get answer from Intel at:
http://softwarecommunity.intel.com/isn/Community/en-US/forums/permalink/30228436/30230975/ShowThread.aspx#30230975
Can someone test if it works please? The lines in the patch are longer than 80 chars, imho it is allowed now?

Only if folding the line in question would hinder code readability. This doesn't seem to be the case here so I think you should fold (most of) the lines.

Quoted 5 lines

coretemp-isa-0000
Adapter: ISA adapter
Core 0: +36.0°C (high = +65.0°C, crit = +85.0°C)
I have in plan to add Penryn CPUs to driver once I know the details.

I can only test on my T2600 which does not have the feature. So all I can say is that there is no regression there.

Quoted 43 lines

Index: linux-2.6.23.9/drivers/hwmon/coretemp.c
=================================> --- linux-2.6.23.9.orig/drivers/hwmon/coretemp.c 2007-12-01 12:28:08.000000000 +0100
+++ linux-2.6.23.9/drivers/hwmon/coretemp.c 2007-12-01 21:29:10.000000000 +0100

drivers/hwmon/coretemp.c +7 -3

@@ -38,7 +38,7 @@
 
 #define DRVNAME	"coretemp"
 
-typedef enum { SHOW_TEMP, SHOW_TJMAX, SHOW_LABEL, SHOW_NAME } SHOW;
+typedef enum { SHOW_TEMP, SHOW_TJMAX, SHOW_TTARGET, SHOW_LABEL, SHOW_NAME } SHOW;
 
 /*
  * Functions declaration

@@ -55,6 +55,7 @@
 	unsigned long last_updated;	/* in jiffies */
 	int temp;
 	int tjmax;
+	int ttarget;
 	u8 alarm;
 };

@@ -95,9 +96,10 @@
 
 	if (attr->index = SHOW_TEMP)
 		err = data->valid ? sprintf(buf, "%d\n", data->temp) : -EAGAIN;
-	else
+	else if (attr->index = SHOW_TJMAX)
 		err = sprintf(buf, "%d\n", data->tjmax);
-
+	else
+		err = sprintf(buf, "%d\n", data->ttarget);
 	return err;
 }

@@ -105,6 +107,8 @@
 			  SHOW_TEMP);
 static SENSOR_DEVICE_ATTR(temp1_crit, S_IRUGO, show_temp, NULL,
 			  SHOW_TJMAX);
+static SENSOR_DEVICE_ATTR(temp1_max, S_IRUGO, show_temp, NULL,
+			  SHOW_TTARGET);
 static DEVICE_ATTR(temp1_crit_alarm, S_IRUGO, show_alarm, NULL);
 static SENSOR_DEVICE_ATTR(temp1_label, S_IRUGO, show_name, NULL, SHOW_LABEL);
 static SENSOR_DEVICE_ATTR(name, S_IRUGO, show_name, NULL, SHOW_NAME);

drivers/hwmon/coretemp.c line 230

Quoted 4 lines

drivers/hwmon/coretemp.c +1 -0

@@ -223,10 +227,27 @@
 			 "temperature might be wrong!\n");
 	}
 
+	data->ttarget = data->tjmax;

This initialization isn't needed. data->ttarget is only read if the temp1_max is created, and in this case you overwrite the value below.

drivers/hwmon/coretemp.c line 233

Quoted 3 lines

drivers/hwmon/coretemp.c +3 -0

+
+	/* read the still undocumented IA32_TEMPERATURE_TARGET it exists
+	   on older CPUs but not here */

I can't really make sense of the second part of this comment, please clarify.

Quoted 4 lines

drivers/hwmon/coretemp.c +4 -0

+	if (c->x86_model > 0xe) {
+		err = rdmsr_safe_on_cpu(data->id, 0x1a2, &eax, &edx);
+		if (err) {
+			dev_warn(&pdev->dev, "Unable to read IA32_TEMPERATURE_TARGET MSR\n");

drivers/hwmon/coretemp.c line 247

Quoted 10 lines

drivers/hwmon/coretemp.c +9 -0

+		} else {
+			data->ttarget = data->tjmax - (((eax >> 8) & 0xff) * 1000);
+			err = device_create_file(&pdev->dev,
+							&sensor_dev_attr_temp1_max.dev_attr);
+			if (err)
+				goto exit_free;
+		}
+	}
+
 	platform_set_drvdata(pdev, data);

You have a race condition here, the driver data should be set before creating any sysfs file, otherwise dev_get_drvdata() might be called before it is set.

Quoted 37 lines

drivers/hwmon/coretemp.c +4 -1

 	if ((err = sysfs_create_group(&pdev->dev.kobj, &coretemp_group)))
-		goto exit_free;
+		goto exit_dev;
 
 	data->class_dev = hwmon_device_register(&pdev->dev);
 	if (IS_ERR(data->class_dev)) {

@@ -240,6 +261,8 @@
 
 exit_class:
 	sysfs_remove_group(&pdev->dev.kobj, &coretemp_group);
+exit_dev:
+	device_remove_file(&pdev->dev, &sensor_dev_attr_temp1_max.dev_attr);
 exit_free:
 	kfree(data);
 exit:

@@ -252,6 +275,7 @@
 
 	hwmon_device_unregister(data->class_dev);
 	sysfs_remove_group(&pdev->dev.kobj, &coretemp_group);
+	device_remove_file(&pdev->dev, &sensor_dev_attr_temp1_max.dev_attr);
 	platform_set_drvdata(pdev, NULL);
 	kfree(data);
 	return 0;

Index: linux-2.6.23.9/Documentation/hwmon/coretemp
=================================> --- linux-2.6.23.9.orig/Documentation/hwmon/coretemp 2007-12-01 21:25:17.000000000 +0100
+++ linux-2.6.23.9/Documentation/hwmon/coretemp 2007-12-01 21:28:35.000000000 +0100

Documentation/hwmon/coretemp +2 -1

@@ -25,7 +25,8 @@
 the Out-Of-Spec bit. Following table summarizes the exported sysfs files:
 
 temp1_input	 - Core temperature (in millidegrees Celsius).
-temp1_crit	 - Maximum junction temperature  (in millidegrees Celsius).
+temp1_max	 - All cooling devices should be turned on (on Core2).
+temp1_crit	 - Maximum junction temperature (in millidegrees Celsius).
 temp1_crit_alarm - Set when Out-of-spec bit is set, never clears.
 		   Correct CPU operation is no longer guaranteed.
 temp1_label	 - Contains string "Core X", where X is processor

Signature · 6 lines

Jean Delvare

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors