[lm-sensors] [PATCH] Add support for Atom CPUs

[lm-sensors] [PATCH] Add support for Atom CPUs

[Rudolf Marek]

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

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

Hello,

Following patch adds support for Atom CPUs. Intel documents TjMax at 90C but
some Atoms may have 125C (this is undocumented speculation). The variable is
renamed, because next patch use this.

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

Please test this!

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

iEYEARECAAYFAkpSJ5QACgkQ3J9wPJqZRNU3jwCgm2dIVplikohpmBHd9FPyIECj
mY8AnjSRQoKdl7VKZJawJ+l6NH6I5LKI
=1Mg+
-----END PGP SIGNATURE-----

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

Index: linux-2.6.30.1/drivers/hwmon/coretemp.c
===================================================================
--- linux-2.6.30.1.orig/drivers/hwmon/coretemp.c	2009-07-06 17:07:16.458757525 +0200
+++ linux-2.6.30.1/drivers/hwmon/coretemp.c	2009-07-06 18:25:46.934758067 +0200
@@ -157,17 +157,24 @@
 	/* The 100C is default for both mobile and non mobile CPUs */
 
 	int tjmax = 100000;
-	int ismobile = 1;
+	int usemsr_ee = 1;
 	int err;
 	u32 eax, edx;
 
 	/* Early chips have no MSR for TjMax */
 
 	if ((c->x86_model == 0xf) && (c->x86_mask < 4)) {
-		ismobile = 0;
+		usemsr_ee = 0;
 	}
 
-	if ((c->x86_model > 0xe) && (ismobile)) {
+	/* Atoms seems to have TjMax at 90C */
+
+	if (c->x86_model == 0x1c) {
+		usemsr_ee = 0;
+		tjmax = 90000;
+	}
+
+	if ((c->x86_model > 0xe) && (usemsr_ee)) {
 
 		/* Now we can detect the mobile CPU using Intel provided table
 		   http://softwarecommunity.intel.com/Wiki/Mobility/720.htm
@@ -179,13 +186,13 @@
 			dev_warn(dev,
 				 "Unable to access MSR 0x17, assuming desktop"
 				 " CPU\n");
-			ismobile = 0;
+			usemsr_ee = 0;
 		} else if (!(eax & 0x10000000)) {
-			ismobile = 0;
+			usemsr_ee = 0;
 		}
 	}
 
-	if (ismobile) {
+	if (usemsr_ee) {
 
 		err = rdmsr_safe_on_cpu(id, 0xee, &eax, &edx);
 		if (err) {
@@ -413,11 +420,11 @@
 	for_each_online_cpu(i) {
 		struct cpuinfo_x86 *c = &cpu_data(i);
 
-		/* check if family 6, models 0xe, 0xf, 0x16, 0x17, 0x1A */
+		/* check if family 6, models 0xe, 0xf, 0x16, 0x17, 0x1A, 0x1c */
 		if ((c->cpuid_level < 0) || (c->x86 != 0x6) ||
 		    !((c->x86_model == 0xe) || (c->x86_model == 0xf) ||
 			(c->x86_model == 0x16) || (c->x86_model == 0x17) ||
-			(c->x86_model == 0x1A))) {
+			(c->x86_model == 0x1A) || (c->x86_model == 0x1c))) {
 
 			/* supported CPU not found, but report the unknown
 			   family 6 CPU */
Index: linux-2.6.30.1/Documentation/hwmon/coretemp
===================================================================
--- linux-2.6.30.1.orig/Documentation/hwmon/coretemp	2009-07-06 18:31:56.262759644 +0200
+++ linux-2.6.30.1/Documentation/hwmon/coretemp	2009-07-06 18:32:20.302461349 +0200
@@ -4,7 +4,7 @@
 Supported chips:
   * All Intel Core family
     Prefix: 'coretemp'
-    CPUID: family 0x6, models 0xe, 0xf, 0x16, 0x17
+    CPUID: family 0x6, models 0xe, 0xf, 0x16, 0x17, 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
Index: linux-2.6.30.1/drivers/hwmon/Kconfig
===================================================================
--- linux-2.6.30.1.orig/drivers/hwmon/Kconfig	2009-07-06 18:32:31.974757310 +0200
+++ linux-2.6.30.1/drivers/hwmon/Kconfig	2009-07-06 18:33:48.566759515 +0200
@@ -402,12 +402,12 @@
 	  will be called gl520sm.
 
 config SENSORS_CORETEMP
-	tristate "Intel Core (2) Duo/Solo temperature sensor"
+	tristate "Intel Core/Core2/Atom temperature sensor"
 	depends on X86 && EXPERIMENTAL
 	help
 	  If you say yes here you get support for the temperature
-	  sensor inside your CPU. Supported all are all known variants
-	  of Intel Core family.
+	  sensor inside your CPU. Most of the family 6 CPUs
+	  are supported. Check documentation/driver for details.
 
 config SENSORS_IBMAEM
 	tristate "IBM Active Energy Manager temperature/power sensors and control"

[-- Attachment #3: add_atoms.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

Re: [lm-sensors] [PATCH] Add support for Atom CPUs

[Rudolf Marek]

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

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

Hi all,

Thanks Martin for a test. It seems that TAGET MSR is not implemented, also
driver incorrectly reports that it uses relative temperature scale. Fix it with
updated patch.

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

iEYEARECAAYFAkpSYLEACgkQ3J9wPJqZRNX2qwCfeF0+WAidbXs/cterief56qV/
i4EAn1zchUyNP54Qas3KEqV5RGGi40Kj
=ye/T
-----END PGP SIGNATURE-----

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

Index: linux-2.6.30.1/drivers/hwmon/coretemp.c
===================================================================
--- linux-2.6.30.1.orig/drivers/hwmon/coretemp.c	2009-07-06 17:07:16.458757525 +0200
+++ linux-2.6.30.1/drivers/hwmon/coretemp.c	2009-07-06 22:34:59.417758968 +0200
@@ -157,17 +157,24 @@
 	/* The 100C is default for both mobile and non mobile CPUs */
 
 	int tjmax = 100000;
-	int ismobile = 1;
+	int usemsr_ee = 1;
 	int err;
 	u32 eax, edx;
 
 	/* Early chips have no MSR for TjMax */
 
 	if ((c->x86_model == 0xf) && (c->x86_mask < 4)) {
-		ismobile = 0;
+		usemsr_ee = 0;
 	}
 
-	if ((c->x86_model > 0xe) && (ismobile)) {
+	/* Atoms seems to have TjMax at 90C */
+
+	if (c->x86_model == 0x1c) {
+		usemsr_ee = 0;
+		tjmax = 90000;
+	}
+
+	if ((c->x86_model > 0xe) && (usemsr_ee)) {
 
 		/* Now we can detect the mobile CPU using Intel provided table
 		   http://softwarecommunity.intel.com/Wiki/Mobility/720.htm
@@ -179,13 +186,13 @@
 			dev_warn(dev,
 				 "Unable to access MSR 0x17, assuming desktop"
 				 " CPU\n");
-			ismobile = 0;
+			usemsr_ee = 0;
 		} else if (!(eax & 0x10000000)) {
-			ismobile = 0;
+			usemsr_ee = 0;
 		}
 	}
 
-	if (ismobile) {
+	if (usemsr_ee) {
 
 		err = rdmsr_safe_on_cpu(id, 0xee, &eax, &edx);
 		if (err) {
@@ -195,7 +202,8 @@
 		} else if (eax & 0x40000000) {
 			tjmax = 85000;
 		}
-	} else {
+	/* if we dont use msr EE it means we are desktop CPU (with exeception of Atom) */
+	} else if (tjmax == 100000) {
 		dev_warn(dev, "Using relative temperature scale!\n");
 	}
 
@@ -248,9 +256,9 @@
 	platform_set_drvdata(pdev, data);
 
 	/* read the still undocumented IA32_TEMPERATURE_TARGET it exists
-	   on older CPUs but not in this register */
+	   on older CPUs but not in this register, Atoms don't have it too */
 
-	if (c->x86_model > 0xe) {
+	if ((c->x86_model > 0xe) && (c->x86_model != 0x1c)) {
 		err = rdmsr_safe_on_cpu(data->id, 0x1a2, &eax, &edx);
 		if (err) {
 			dev_warn(&pdev->dev, "Unable to read"
@@ -413,11 +421,11 @@
 	for_each_online_cpu(i) {
 		struct cpuinfo_x86 *c = &cpu_data(i);
 
-		/* check if family 6, models 0xe, 0xf, 0x16, 0x17, 0x1A */
+		/* check if family 6, models 0xe, 0xf, 0x16, 0x17, 0x1A, 0x1c */
 		if ((c->cpuid_level < 0) || (c->x86 != 0x6) ||
 		    !((c->x86_model == 0xe) || (c->x86_model == 0xf) ||
 			(c->x86_model == 0x16) || (c->x86_model == 0x17) ||
-			(c->x86_model == 0x1A))) {
+			(c->x86_model == 0x1A) || (c->x86_model == 0x1c))) {
 
 			/* supported CPU not found, but report the unknown
 			   family 6 CPU */
Index: linux-2.6.30.1/Documentation/hwmon/coretemp
===================================================================
--- linux-2.6.30.1.orig/Documentation/hwmon/coretemp	2009-07-06 18:31:56.262759644 +0200
+++ linux-2.6.30.1/Documentation/hwmon/coretemp	2009-07-06 22:31:01.905898848 +0200
@@ -4,7 +4,7 @@
 Supported chips:
   * All Intel Core family
     Prefix: 'coretemp'
-    CPUID: family 0x6, models 0xe, 0xf, 0x16, 0x17
+    CPUID: family 0x6, models 0xe, 0xf, 0x16, 0x17, 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
Index: linux-2.6.30.1/drivers/hwmon/Kconfig
===================================================================
--- linux-2.6.30.1.orig/drivers/hwmon/Kconfig	2009-07-06 18:32:31.974757310 +0200
+++ linux-2.6.30.1/drivers/hwmon/Kconfig	2009-07-06 18:33:48.566759515 +0200
@@ -402,12 +402,12 @@
 	  will be called gl520sm.
 
 config SENSORS_CORETEMP
-	tristate "Intel Core (2) Duo/Solo temperature sensor"
+	tristate "Intel Core/Core2/Atom temperature sensor"
 	depends on X86 && EXPERIMENTAL
 	help
 	  If you say yes here you get support for the temperature
-	  sensor inside your CPU. Supported all are all known variants
-	  of Intel Core family.
+	  sensor inside your CPU. Most of the family 6 CPUs
+	  are supported. Check documentation/driver for details.
 
 config SENSORS_IBMAEM
 	tristate "IBM Active Energy Manager temperature/power sensors and control"

[-- Attachment #3: add_atoms.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

Re: [lm-sensors] [PATCH] Add support for Atom CPUs

[Jean Delvare]

Hi Rudolf,

On Mon, 06 Jul 2009 22:38:10 +0200, Rudolf Marek wrote:
> Thanks Martin for a test. It seems that TAGET MSR is not implemented, also
> driver incorrectly reports that it uses relative temperature scale. Fix it with
> updated patch.
> 
> 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 17:07:16.458757525 +0200
> +++ linux-2.6.30.1/drivers/hwmon/coretemp.c	2009-07-06 22:34:59.417758968 +0200
> @@ -157,17 +157,24 @@
>  	/* The 100C is default for both mobile and non mobile CPUs */
>  
>  	int tjmax = 100000;
> -	int ismobile = 1;
> +	int usemsr_ee = 1;
>  	int err;
>  	u32 eax, edx;
>  
>  	/* Early chips have no MSR for TjMax */
>  
>  	if ((c->x86_model = 0xf) && (c->x86_mask < 4)) {
> -		ismobile = 0;
> +		usemsr_ee = 0;
>  	}
>  
> -	if ((c->x86_model > 0xe) && (ismobile)) {
> +	/* Atoms seems to have TjMax at 90C */
> +
> +	if (c->x86_model = 0x1c) {
> +		usemsr_ee = 0;
> +		tjmax = 90000;
> +	}
> +
> +	if ((c->x86_model > 0xe) && (usemsr_ee)) {
>  
>  		/* Now we can detect the mobile CPU using Intel provided table
>  		   http://softwarecommunity.intel.com/Wiki/Mobility/720.htm
> @@ -179,13 +186,13 @@
>  			dev_warn(dev,
>  				 "Unable to access MSR 0x17, assuming desktop"
>  				 " CPU\n");
> -			ismobile = 0;
> +			usemsr_ee = 0;
>  		} else if (!(eax & 0x10000000)) {
> -			ismobile = 0;
> +			usemsr_ee = 0;
>  		}
>  	}
>  
> -	if (ismobile) {
> +	if (usemsr_ee) {
>  
>  		err = rdmsr_safe_on_cpu(id, 0xee, &eax, &edx);
>  		if (err) {
> @@ -195,7 +202,8 @@
>  		} else if (eax & 0x40000000) {
>  			tjmax = 85000;
>  		}
> -	} else {
> +	/* if we dont use msr EE it means we are desktop CPU (with exeception of Atom) */

Line too long.

> +	} else if (tjmax = 100000) {
>  		dev_warn(dev, "Using relative temperature scale!\n");
>  	}
>  

I think function adjust_tjmax() has reached its limit of readability. I
don't understand why it needs to be so complex. For example, the case
of the Atom is very simply always return 90 degree C. So why not just:

	/* Atoms seems to have TjMax at 90C */
	if (c->x86_model = 0x1c)
		return 90000;

at the beginning of the function, and be done with that case? And I
suspect the rest can be simplified a bit too.

> @@ -248,9 +256,9 @@
>  	platform_set_drvdata(pdev, data);
>  
>  	/* read the still undocumented IA32_TEMPERATURE_TARGET it exists
> -	   on older CPUs but not in this register */
> +	   on older CPUs but not in this register, Atoms don't have it too */

s/too/either/

>  
> -	if (c->x86_model > 0xe) {
> +	if ((c->x86_model > 0xe) && (c->x86_model != 0x1c)) {
>  		err = rdmsr_safe_on_cpu(data->id, 0x1a2, &eax, &edx);
>  		if (err) {
>  			dev_warn(&pdev->dev, "Unable to read"
> @@ -413,11 +421,11 @@
>  	for_each_online_cpu(i) {
>  		struct cpuinfo_x86 *c = &cpu_data(i);
>  
> -		/* check if family 6, models 0xe, 0xf, 0x16, 0x17, 0x1A */
> +		/* check if family 6, models 0xe, 0xf, 0x16, 0x17, 0x1A, 0x1c */
>  		if ((c->cpuid_level < 0) || (c->x86 != 0x6) ||
>  		    !((c->x86_model = 0xe) || (c->x86_model = 0xf) ||
>  			(c->x86_model = 0x16) || (c->x86_model = 0x17) ||
> -			(c->x86_model = 0x1A))) {
> +			(c->x86_model = 0x1A) || (c->x86_model = 0x1c))) {
>  
>  			/* supported CPU not found, but report the unknown
>  			   family 6 CPU */

sensors-detect needs to be updated as well.

> Index: linux-2.6.30.1/Documentation/hwmon/coretemp
> =================================> --- linux-2.6.30.1.orig/Documentation/hwmon/coretemp	2009-07-06 18:31:56.262759644 +0200
> +++ linux-2.6.30.1/Documentation/hwmon/coretemp	2009-07-06 22:31:01.905898848 +0200
> @@ -4,7 +4,7 @@
>  Supported chips:
>    * All Intel Core family
>      Prefix: 'coretemp'
> -    CPUID: family 0x6, models 0xe, 0xf, 0x16, 0x17
> +    CPUID: family 0x6, models 0xe, 0xf, 0x16, 0x17, 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
> Index: linux-2.6.30.1/drivers/hwmon/Kconfig
> =================================> --- linux-2.6.30.1.orig/drivers/hwmon/Kconfig	2009-07-06 18:32:31.974757310 +0200
> +++ linux-2.6.30.1/drivers/hwmon/Kconfig	2009-07-06 18:33:48.566759515 +0200
> @@ -402,12 +402,12 @@
>  	  will be called gl520sm.
>  
>  config SENSORS_CORETEMP
> -	tristate "Intel Core (2) Duo/Solo temperature sensor"
> +	tristate "Intel Core/Core2/Atom temperature sensor"
>  	depends on X86 && EXPERIMENTAL
>  	help
>  	  If you say yes here you get support for the temperature
> -	  sensor inside your CPU. Supported all are all known variants
> -	  of Intel Core family.
> +	  sensor inside your CPU. Most of the family 6 CPUs
> +	  are supported. Check documentation/driver for details.
>  
>  config SENSORS_IBMAEM
>  	tristate "IBM Active Energy Manager temperature/power sensors and control"


-- 
Jean Delvare

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

Re: [lm-sensors] [PATCH] Add support for Atom CPUs

[Rudolf Marek]

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

>> +	/* if we dont use msr EE it means we are desktop CPU (with exeception of Atom) */
> 
> Line too long.

Well I thought the 80 lines limit is not so strict anymore.

> 
>> +	} else if (tjmax = 100000) {
>>  		dev_warn(dev, "Using relative temperature scale!\n");
>>  	}
>>  
> 
> I think function adjust_tjmax() has reached its limit of readability. I
> don't understand why it needs to be so complex. For example, the case
> of the Atom is very simply always return 90 degree C. So why not just:

Yes right, but it is going to be complex for penryns. I will try to simplify it.

> 
> 	/* Atoms seems to have TjMax at 90C */
> 	if (c->x86_model = 0x1c)
> 		return 90000;

Because of multiple returns? If you are OK with multiple returns I will fix it.

> 
> at the beginning of the function, and be done with that case? And I
> suspect the rest can be simplified a bit too.
> 
>> @@ -248,9 +256,9 @@
>>  	platform_set_drvdata(pdev, data);
>>  
>>  	/* read the still undocumented IA32_TEMPERATURE_TARGET it exists
>> -	   on older CPUs but not in this register */
>> +	   on older CPUs but not in this register, Atoms don't have it too */
> 
> s/too/either/

ok
> sensors-detect needs to be updated as well.

yes good catch.

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

iEYEARECAAYFAkpVA6YACgkQ3J9wPJqZRNVM6wCghNTnneKd3ta0M7X9GO+EeRNZ
OH4An3uuSHgvH/rB+8leDYf7zFAOI9mc
=HIzj
-----END PGP SIGNATURE-----

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

Re: [lm-sensors] [PATCH] Add support for Atom CPUs

[Jean Delvare]

On Wed, 08 Jul 2009 22:38:05 +0200, Rudolf Marek wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> >> +	/* if we dont use msr EE it means we are desktop CPU (with exeception of Atom) */
> > 
> > Line too long.
> 
> Well I thought the 80 lines limit is not so strict anymore.

Quoting CodingStyle:

"The only exception to this is where exceeding 80 columns significantly
increases readability and does not hide information."

Do you really believe that splitting this comment will hinder
readability?

> >> +	} else if (tjmax = 100000) {
> >>  		dev_warn(dev, "Using relative temperature scale!\n");
> >>  	}
> >>  
> > 
> > I think function adjust_tjmax() has reached its limit of readability. I
> > don't understand why it needs to be so complex. For example, the case
> > of the Atom is very simply always return 90 degree C. So why not just:
> 
> Yes right, but it is going to be complex for penryns. I will try to simplify it.
> 
> > 
> > 	/* Atoms seems to have TjMax at 90C */
> > 	if (c->x86_model = 0x1c)
> > 		return 90000;
> 
> Because of multiple returns? If you are OK with multiple returns I will fix it.

Last time I checked, the C language supported it ;) I see no point in
limiting yourself if this means your code becomes harder to read.

Thanks,
-- 
Jean Delvare

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

Re: [lm-sensors] [PATCH] Add support for Atom CPUs

[Jean Delvare]

On Mon, 06 Jul 2009 22:38:10 +0200, Rudolf Marek wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Hi all,
> 
> Thanks Martin for a test. It seems that TAGET MSR is not implemented, also
> driver incorrectly reports that it uses relative temperature scale. Fix it with
> updated patch.
> 
> Signed-off-by: Rudolf Marek <r.marek@assembler.cz>

As there is increasing pressure to get support for the Atom CPU, I've
included this patch in my unofficial hwmon tree, fixed the two
comments, and will send it to Linus for kernel 2.6.32.  I still believe
adjust_tjmax() could and should be simplified, but this is left for a
future patch.

I've also updated sensors-detect.

-- 
Jean Delvare
http://khali.linux-fr.org/wishlist.html

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