coretemp support for Core i5 CPU

coretemp support for Core i5 CPU

[Robert Hancock]

Trying to load the coretemp driver for CPU temperature monitoring on a 
Core i5 750 CPU gives:

coretemp: Unknown CPU model 1e
coretemp: Unknown CPU model 1e
coretemp: Unknown CPU model 1e
coretemp: Unknown CPU model 1e

Looks like this model needs to be added to the list in 
drivers/hwmon/coretemp.c. Though this doesn't really seem like a very 
good approach as we'll keep having to update this driver whenever a new 
CPU model gets released. Intel has a CPUID field that indicates if the 
thermal sensor is supported: "The processor supports a digital thermal 
sensor if CPUID.06H.EAX[0] = 1." We should likely be using this instead 
of hard-coded CPU model checks..

[PATCH] coretemp: add support for Core i5 CPU

[Robert Hancock]

Add coretemp support for Core i5 (Lynnfield) CPUs with model 0x1E.

Signed-off-by: Robert Hancock <hancockrwd@gmail.com>

---

This minimal patch works to add support for these CPUs, though we should likely
still look into using the CPUID flags instead of the model check.

diff --git a/drivers/hwmon/coretemp.c b/drivers/hwmon/coretemp.c
index 93c1722..a9a21dc 100644
--- a/drivers/hwmon/coretemp.c
+++ b/drivers/hwmon/coretemp.c
@@ -413,11 +413,11 @@ static int __init coretemp_init(void)
 	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, 0x1E */
 		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 == 0x1E))) {
 
 			/* supported CPU not found, but report the unknown
 			   family 6 CPU */

Re: [PATCH] coretemp: add support for Core i5 CPU

[Jean Delvare]

On Sun, 20 Sep 2009 12:19:54 -0600, Robert Hancock wrote:
> Add coretemp support for Core i5 (Lynnfield) CPUs with model 0x1E.
> 
> Signed-off-by: Robert Hancock <hancockrwd@gmail.com>
> 
> ---
> 
> This minimal patch works to add support for these CPUs, though we should likely
> still look into using the CPUID flags instead of the model check.
> 
> diff --git a/drivers/hwmon/coretemp.c b/drivers/hwmon/coretemp.c
> index 93c1722..a9a21dc 100644
> --- a/drivers/hwmon/coretemp.c
> +++ b/drivers/hwmon/coretemp.c
> @@ -413,11 +413,11 @@ static int __init coretemp_init(void)
>  	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, 0x1E */
>  		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 == 0x1E))) {
>  
>  			/* supported CPU not found, but report the unknown
>  			   family 6 CPU */

We've received a similar patch from Intel themselves a few days ago,
and I've just pushed it upstream.

-- 
Jean Delvare

Re: [lm-sensors] coretemp support for Core i5 CPU

[Jean Delvare]

Hi Robert,

On Sun, 20 Sep 2009 01:47:43 -0600, Robert Hancock wrote:
> Trying to load the coretemp driver for CPU temperature monitoring on a 
> Core i5 750 CPU gives:
> 
> coretemp: Unknown CPU model 1e
> coretemp: Unknown CPU model 1e
> coretemp: Unknown CPU model 1e
> coretemp: Unknown CPU model 1e
> 
> Looks like this model needs to be added to the list in 
> drivers/hwmon/coretemp.c. Though this doesn't really seem like a very 
> good approach as we'll keep having to update this driver whenever a new 
> CPU model gets released. Intel has a CPUID field that indicates if the 
> thermal sensor is supported: "The processor supports a digital thermal 
> sensor if CPUID.06H.EAX[0] = 1." We should likely be using this instead 
> of hard-coded CPU model checks..

Supporting the digital thermal sensor is one thing, but it is totally
insufficient. If you look at the coretemp driver, you'll see we need to
figure out a number of calibration values which are model-dependent. So
we will need to check this part before adding support for new CPU
models anyway.

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

Re: [lm-sensors] coretemp support for Core i5 CPU

[Robert Hancock]

On Sun, Sep 20, 2009 at 1:16 PM, Jean Delvare <khali@linux-fr.org> wrote:
> Hi Robert,
>
> On Sun, 20 Sep 2009 01:47:43 -0600, Robert Hancock wrote:
>> Trying to load the coretemp driver for CPU temperature monitoring on a
>> Core i5 750 CPU gives:
>>
>> coretemp: Unknown CPU model 1e
>> coretemp: Unknown CPU model 1e
>> coretemp: Unknown CPU model 1e
>> coretemp: Unknown CPU model 1e
>>
>> Looks like this model needs to be added to the list in
>> drivers/hwmon/coretemp.c. Though this doesn't really seem like a very
>> good approach as we'll keep having to update this driver whenever a new
>> CPU model gets released. Intel has a CPUID field that indicates if the
>> thermal sensor is supported: "The processor supports a digital thermal
>> sensor if CPUID.06H.EAX[0] = 1." We should likely be using this instead
>> of hard-coded CPU model checks..
>
> Supporting the digital thermal sensor is one thing, but it is totally
> insufficient. If you look at the coretemp driver, you'll see we need to
> figure out a number of calibration values which are model-dependent. So
> we will need to check this part before adding support for new CPU
> models anyway.

Well, I see there is the TCC activation temperature which the reading
is relative to, which is detected on the mobile CPUs via an MSR read,
so it would only be a problem if they changed this temperature on
other CPUs..

It might make sense to warn "unknown CPU model, temperature reading
may be suspect" or something, but I should think it should still load
as the temperature reading is likely still correct.

Re: [lm-sensors] coretemp support for Core i5 CPU

[Jean Delvare]

On Sun, 20 Sep 2009 16:53:53 -0600, Robert Hancock wrote:
> On Sun, Sep 20, 2009 at 1:16 PM, Jean Delvare <khali@linux-fr.org> wrote:
> > Supporting the digital thermal sensor is one thing, but it is totally
> > insufficient. If you look at the coretemp driver, you'll see we need to
> > figure out a number of calibration values which are model-dependent. So
> > we will need to check this part before adding support for new CPU
> > models anyway.
> 
> Well, I see there is the TCC activation temperature which the reading
> is relative to, which is detected on the mobile CPUs via an MSR read,
> so it would only be a problem if they changed this temperature on
> other CPUs..

They actually did change it on almost every CPU model so far. I fail to
see why that would suddenly change (even though I'd really love it.)

> It might make sense to warn "unknown CPU model, temperature reading
> may be suspect" or something, but I should think it should still load
> as the temperature reading is likely still correct.

Our experience is the exact opposite. The temperature reading is likely
not correct. And we prefer not telling the user anything than to tell
him/her something incorrect. Most users won't notice warnings in their
kernel logs, all they see is a CPU temperature value in their desktop
tray area.

I hope Intel will finally take over the driver maintenance and deal
with their own mess. It has been a pain for us to maintain since day 2.

-- 
Jean Delvare