All of lore.kernel.org
 help / color / mirror / Atom feed
* [lm-sensors] [PATCH] Add support for Penryn mobile CPUs
@ 2009-07-06 16:39 Rudolf Marek
  2009-07-07 11:26 ` Jean Delvare
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Rudolf Marek @ 2009-07-06 16:39 UTC (permalink / raw)
  To: lm-sensors

[-- Attachment #1: Type: text/plain, Size: 557 bytes --]

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

Hello,

Following patch adds support for mobile Penryn CPUs. Intel documents this
poorly. I asked the Coretemp author for some help. This is totally untested and
may not work. Please test!

Signed-off-by: Rudolf Marek <r.marek@assembler.cz>


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

iEYEARECAAYFAkpSKLgACgkQ3J9wPJqZRNV86gCfbEsfdUfaEEaaS5gsoosTxYDr
gWcAoImIpffHIUxRtCJ13GCfMr5jPkly
=2to8
-----END PGP SIGNATURE-----

[-- Attachment #2: add_mobile_penryn.patch --]
[-- Type: text/x-diff, Size: 3117 bytes --]

Index: linux-2.6.30.1/drivers/hwmon/coretemp.c
===================================================================
--- linux-2.6.30.1.orig/drivers/hwmon/coretemp.c	2009-07-06 18:25:46.934758067 +0200
+++ linux-2.6.30.1/drivers/hwmon/coretemp.c	2009-07-06 18:35:38.602757417 +0200
@@ -157,6 +157,7 @@
 	/* The 100C is default for both mobile and non mobile CPUs */
 
 	int tjmax = 100000;
+	int tjmax_ee = 85000;
 	int usemsr_ee = 1;
 	int err;
 	u32 eax, edx;
@@ -175,20 +176,31 @@
 	}
 
 	if ((c->x86_model > 0xe) && (usemsr_ee)) {
-
+		u8 platform_id;
 		/* Now we can detect the mobile CPU using Intel provided table
 		   http://softwarecommunity.intel.com/Wiki/Mobility/720.htm
 		   For Core2 cores, check MSR 0x17, bit 28 1 = Mobile CPU
 		*/
 
 		err = rdmsr_safe_on_cpu(id, 0x17, &eax, &edx);
+		/* Platform ID bits 52:50 (EDX starts at bit 32) */
+		platform_id = (edx >> 18) & 0x7;
 		if (err) {
 			dev_warn(dev,
 				 "Unable to access MSR 0x17, assuming desktop"
 				 " CPU\n");
 			usemsr_ee = 0;
-		} else if (!(eax & 0x10000000)) {
+		/* trust bit 28 up to Penryn, I could not find any documentation on that
+		   if you happen to know someone at Intel please ask */
+
+		} else if ((!(eax & 0x10000000)) && (c->x86_model < 0x17)) {
 			usemsr_ee = 0;
+		/* Mobile Penryn CPU seems to be platform ID 7 or 5 (guesswork) */
+		} else if ((c->x86_model == 0x17) &&
+				((platform_id == 5) || (platform_id == 7))) {
+			/* If MSR EE bit is set, set it to 90C otherwise 105C */
+			tjmax_ee = 90000;
+			tjmax = 105000;
 		}
 	}
 
@@ -200,7 +212,7 @@
 				 "Unable to access MSR 0xEE, for Tjmax, left"
 				 " at default");
 		} else if (eax & 0x40000000) {
-			tjmax = 85000;
+			tjmax = tjmax_ee;
 		}
 	} else {
 		dev_warn(dev, "Using relative temperature scale!\n");
@@ -420,7 +432,9 @@
 	for_each_online_cpu(i) {
 		struct cpuinfo_x86 *c = &cpu_data(i);
 
-		/* check if family 6, models 0xe, 0xf, 0x16, 0x17, 0x1A, 0x1c */
+		/* check if family 6, models 0xe (Pentium M DC),
+		  0xf (Core 2 DC 65nm), 0x16 (Core 2 SC 65nm), 0x17 (Penryn 45nm)
+		  0x1a (Nehalem), 0x1c (Atom) */
 		if ((c->cpuid_level < 0) || (c->x86 != 0x6) ||
 		    !((c->x86_model == 0xe) || (c->x86_model == 0xf) ||
 			(c->x86_model == 0x16) || (c->x86_model == 0x17) ||
Index: linux-2.6.30.1/Documentation/hwmon/coretemp
===================================================================
--- linux-2.6.30.1.orig/Documentation/hwmon/coretemp	2009-07-06 18:35:53.765759446 +0200
+++ linux-2.6.30.1/Documentation/hwmon/coretemp	2009-07-06 18:36:39.817757139 +0200
@@ -4,7 +4,9 @@
 Supported chips:
   * All Intel Core family
     Prefix: 'coretemp'
-    CPUID: family 0x6, models 0xe, 0xf, 0x16, 0x17, 0x1c (Atom)
+    CPUID: family 0x6, models 0xe (Pentium M DC), 0xf (Core 2 DC 65nm),
+                              0x16 (Core 2 SC 65nm), 0x17 (Penryn 45nm),
+                              0x1a (Nehalem), 0x1c (Atom).
     Datasheet: Intel 64 and IA-32 Architectures Software Developer's Manual
                Volume 3A: System Programming Guide
                http://softwarecommunity.intel.com/Wiki/Mobility/720.htm

[-- Attachment #3: add_mobile_penryn.patch.sig --]
[-- Type: application/octet-stream, Size: 72 bytes --]

[-- Attachment #4: Type: text/plain, Size: 153 bytes --]

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [lm-sensors] [PATCH] Add support for Penryn mobile CPUs
  2009-07-06 16:39 [lm-sensors] [PATCH] Add support for Penryn mobile CPUs Rudolf Marek
@ 2009-07-07 11:26 ` Jean Delvare
  2009-07-08 20:40 ` Rudolf Marek
  2009-09-20  8:49 ` Jean Delvare
  2 siblings, 0 replies; 4+ messages in thread
From: Jean Delvare @ 2009-07-07 11:26 UTC (permalink / raw)
  To: lm-sensors

Hi Ruik,

On Mon, 06 Jul 2009 18:39:20 +0200, Rudolf Marek wrote:
> Following patch adds support for mobile Penryn CPUs. Intel documents this
> poorly. I asked the Coretemp author for some help. This is totally untested and
> may not work. Please test!
> 
> Signed-off-by: Rudolf Marek <r.marek@assembler.cz>

> Index: linux-2.6.30.1/drivers/hwmon/coretemp.c
> =================================> --- linux-2.6.30.1.orig/drivers/hwmon/coretemp.c	2009-07-06 18:25:46.934758067 +0200
> +++ linux-2.6.30.1/drivers/hwmon/coretemp.c	2009-07-06 18:35:38.602757417 +0200
> @@ -157,6 +157,7 @@
>  	/* The 100C is default for both mobile and non mobile CPUs */
>  
>  	int tjmax = 100000;
> +	int tjmax_ee = 85000;
>  	int usemsr_ee = 1;
>  	int err;
>  	u32 eax, edx;
> @@ -175,20 +176,31 @@
>  	}
>  
>  	if ((c->x86_model > 0xe) && (usemsr_ee)) {
> -
> +		u8 platform_id;
>  		/* Now we can detect the mobile CPU using Intel provided table
>  		   http://softwarecommunity.intel.com/Wiki/Mobility/720.htm
>  		   For Core2 cores, check MSR 0x17, bit 28 1 = Mobile CPU
>  		*/
>  
>  		err = rdmsr_safe_on_cpu(id, 0x17, &eax, &edx);
> +		/* Platform ID bits 52:50 (EDX starts at bit 32) */
> +		platform_id = (edx >> 18) & 0x7;
>  		if (err) {

Logic is invalid: if there's an error then the value of edx is
undefined.

>  			dev_warn(dev,
>  				 "Unable to access MSR 0x17, assuming desktop"
>  				 " CPU\n");
>  			usemsr_ee = 0;
> -		} else if (!(eax & 0x10000000)) {
> +		/* trust bit 28 up to Penryn, I could not find any documentation on that
> +		   if you happen to know someone at Intel please ask */
> +

Indentation is pretty messy, and first comment line is too long.

> +		} else if ((!(eax & 0x10000000)) && (c->x86_model < 0x17)) {

It would make more sense to check model first and eax second IMHO.

>  			usemsr_ee = 0;
> +		/* Mobile Penryn CPU seems to be platform ID 7 or 5 (guesswork) */

Line too long.

> +		} else if ((c->x86_model = 0x17) &&
> +				((platform_id = 5) || (platform_id = 7))) {
> +			/* If MSR EE bit is set, set it to 90C otherwise 105C */
> +			tjmax_ee = 90000;
> +			tjmax = 105000;
>  		}
>  	}
>  
> @@ -200,7 +212,7 @@
>  				 "Unable to access MSR 0xEE, for Tjmax, left"
>  				 " at default");
>  		} else if (eax & 0x40000000) {
> -			tjmax = 85000;
> +			tjmax = tjmax_ee;
>  		}
>  	} else {
>  		dev_warn(dev, "Using relative temperature scale!\n");
> @@ -420,7 +432,9 @@
>  	for_each_online_cpu(i) {
>  		struct cpuinfo_x86 *c = &cpu_data(i);
>  
> -		/* check if family 6, models 0xe, 0xf, 0x16, 0x17, 0x1A, 0x1c */
> +		/* check if family 6, models 0xe (Pentium M DC),
> +		  0xf (Core 2 DC 65nm), 0x16 (Core 2 SC 65nm), 0x17 (Penryn 45nm)

Line too long.

> +		  0x1a (Nehalem), 0x1c (Atom) */
>  		if ((c->cpuid_level < 0) || (c->x86 != 0x6) ||
>  		    !((c->x86_model = 0xe) || (c->x86_model = 0xf) ||
>  			(c->x86_model = 0x16) || (c->x86_model = 0x17) ||
> Index: linux-2.6.30.1/Documentation/hwmon/coretemp
> =================================> --- linux-2.6.30.1.orig/Documentation/hwmon/coretemp	2009-07-06 18:35:53.765759446 +0200
> +++ linux-2.6.30.1/Documentation/hwmon/coretemp	2009-07-06 18:36:39.817757139 +0200
> @@ -4,7 +4,9 @@
>  Supported chips:
>    * All Intel Core family
>      Prefix: 'coretemp'
> -    CPUID: family 0x6, models 0xe, 0xf, 0x16, 0x17, 0x1c (Atom)
> +    CPUID: family 0x6, models 0xe (Pentium M DC), 0xf (Core 2 DC 65nm),
> +                              0x16 (Core 2 SC 65nm), 0x17 (Penryn 45nm),
> +                              0x1a (Nehalem), 0x1c (Atom).
>      Datasheet: Intel 64 and IA-32 Architectures Software Developer's Manual
>                 Volume 3A: System Programming Guide
>                 http://softwarecommunity.intel.com/Wiki/Mobility/720.htm


-- 
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] 4+ messages in thread

* Re: [lm-sensors] [PATCH] Add support for Penryn mobile CPUs
  2009-07-06 16:39 [lm-sensors] [PATCH] Add support for Penryn mobile CPUs Rudolf Marek
  2009-07-07 11:26 ` Jean Delvare
@ 2009-07-08 20:40 ` Rudolf Marek
  2009-09-20  8:49 ` Jean Delvare
  2 siblings, 0 replies; 4+ messages in thread
From: Rudolf Marek @ 2009-07-08 20:40 UTC (permalink / raw)
  To: lm-sensors

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

> Logic is invalid: if there's an error then the value of edx is
> undefined.


Yes I know, but there was no place I could create this variable. Perhaps I need
to re-think it a bit. Maybe on sunday evening :/

Anyway thanks for the review! I will try to fix it somehow.

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

iEYEARECAAYFAkpVBCUACgkQ3J9wPJqZRNXSIQCfYZdeRqvNDsbbU/navL/P332x
BuoAn2aQNMCTsx0Vors75IlnZPZqtK1J
=7GW2
-----END PGP SIGNATURE-----

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [lm-sensors] [PATCH] Add support for Penryn mobile CPUs
  2009-07-06 16:39 [lm-sensors] [PATCH] Add support for Penryn mobile CPUs Rudolf Marek
  2009-07-07 11:26 ` Jean Delvare
  2009-07-08 20:40 ` Rudolf Marek
@ 2009-09-20  8:49 ` Jean Delvare
  2 siblings, 0 replies; 4+ messages in thread
From: Jean Delvare @ 2009-09-20  8:49 UTC (permalink / raw)
  To: lm-sensors

Hi Rudolf,

On Wed, 08 Jul 2009 22:40:05 +0200, Rudolf Marek wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> > Logic is invalid: if there's an error then the value of edx is
> > undefined.
> 
> 
> Yes I know, but there was no place I could create this variable. Perhaps I need
> to re-think it a bit. Maybe on sunday evening :/
> 
> Anyway thanks for the review! I will try to fix it somehow.

Any news about this? I'd like to push this to Linus, it's been delayed
way too long already.

-- 
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] 4+ messages in thread

end of thread, other threads:[~2009-09-20  8:49 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-07-06 16:39 [lm-sensors] [PATCH] Add support for Penryn mobile CPUs Rudolf Marek
2009-07-07 11:26 ` Jean Delvare
2009-07-08 20:40 ` Rudolf Marek
2009-09-20  8:49 ` Jean Delvare

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.