[lm-sensors] [PATCH] k8temp - Add the diode offset to reported

[lm-sensors] [PATCH] k8temp - Add the diode offset to reported

[Amul Shah]

This patch adds support to include the CPU diode sensor offset when
reporting CPU temperature.

Please see http://www.lm-sensors.org/ticket/2200 for the full problem
description.

Signed-off-by: Amul Shah <amul.shah@unisys.com>
---
diff -uprN kernel.orig/drivers/hwmon/k8temp.c kernel/drivers/hwmon/k8temp.c
--- kernel.orig/drivers/hwmon/k8temp.c	2007-04-12 23:15:02.000000000 -0400
+++ kernel/drivers/hwmon/k8temp.c	2007-04-16 14:15:06.000000000 -0400
@@ -33,6 +33,8 @@
 #include <linux/mutex.h>
 
 #define TEMP_FROM_REG(val)	(((((val) >> 16) & 0xff) - 49) * 1000)
+#define OFFSET_FROM_REG(val)	((((val) >> 8) & 0x3f)? \
+				((11 - (((val) >> 8) & 0x3f)) * 1000):0)
 #define REG_TEMP	0xe4
 #define SEL_PLACE	0x40
 #define SEL_CORE	0x04
@@ -117,7 +119,8 @@ static ssize_t show_temp(struct device *
 	struct k8temp_data *data = k8temp_update_device(dev);
 
 	return sprintf(buf, "%d\n",
-		       TEMP_FROM_REG(data->temp[core][place]));
+		       TEMP_FROM_REG(data->temp[core][place]) +
+		       OFFSET_FROM_REG(data->temp[core][place]));
 }
 
 /* core, place */



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

Re: [lm-sensors] [PATCH] k8temp - Add the diode offset to reported

[Amul Shah]

On Tue, 2007-04-17 at 18:24 +0200, Rudolf Marek wrote:
> Hello all,
> 
> Amul Shah wrote:
> > This patch adds support to include the CPU diode sensor offset when
> > reporting CPU temperature.
> > 
> > Please see http://www.lm-sensors.org/ticket/2200 for the full problem
> > description.
> 
> Well I dont think this is correct. I believe that diode offset register is
> related to analog diode output not the digital sensor. Also I know that AMD is
> using my driver and did not make any changes. This leads me to a conclusion that
> the sensor simply does not work correctly and this error might be only
> compensated for analog part and not the digital. IIRC in some errata AMD states
> that the digital temperature sensor might be wrong and urges users to use the
> analog output.
> 
> What do you think?
> 
Well, I was looking to see what you think. :)  Thanks for the
information.  I'll go look for the errata and drop that information into
the ticket.  After that we can close it as works as designed. :)  No
need to fix what isn't broken.

thanks,
Amul


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

Re: [lm-sensors] [PATCH] k8temp - Add the diode offset to reported

[Rudolf Marek]

Hello all,

Amul Shah wrote:
> This patch adds support to include the CPU diode sensor offset when
> reporting CPU temperature.
> 
> Please see http://www.lm-sensors.org/ticket/2200 for the full problem
> description.

Well I dont think this is correct. I believe that diode offset register is
related to analog diode output not the digital sensor. Also I know that AMD is
using my driver and did not make any changes. This leads me to a conclusion that
the sensor simply does not work correctly and this error might be only
compensated for analog part and not the digital. IIRC in some errata AMD states
that the digital temperature sensor might be wrong and urges users to use the
analog output.

What do you think?

Thanks,

Rudolf

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

Re: [lm-sensors] [PATCH] k8temp - Add the diode offset to reported

[Jean Delvare]

Hi Amul,

On Tue, 17 Apr 2007 13:49:20 +0000, Amul Shah wrote:
> On Tue, 2007-04-17 at 18:24 +0200, Rudolf Marek wrote:
> > Hello all,
> > 
> > Amul Shah wrote:
> > > This patch adds support to include the CPU diode sensor offset when
> > > reporting CPU temperature.
> > > 
> > > Please see http://www.lm-sensors.org/ticket/2200 for the full problem
> > > description.
> > 
> > Well I dont think this is correct. I believe that diode offset register is
> > related to analog diode output not the digital sensor. Also I know that AMD is
> > using my driver and did not make any changes. This leads me to a conclusion that
> > the sensor simply does not work correctly and this error might be only
> > compensated for analog part and not the digital. IIRC in some errata AMD states
> > that the digital temperature sensor might be wrong and urges users to use the
> > analog output.
> > 
> > What do you think?
>
> Well, I was looking to see what you think. :)  Thanks for the
> information.  I'll go look for the errata and drop that information into
> the ticket.  After that we can close it as works as designed. :)  No
> need to fix what isn't broken.

What CPU version do you have exactly? You may want to take a look at
erratum 154 of document "Revision Guide for AMD Athlon™ 64 and AMD
Opteron™ Processors":
http://www.amd.com/us-en/assets/content_type/white_papers_and_tech_docs/25759.pdf

It mentions a 10 degrees C diode offset error. Only the JH-E6 revision
is affected.

-- 
Jean Delvare

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

Re: [lm-sensors] [PATCH] k8temp - Add the diode offset to reported

[Philip Pokorny]

Rudolf Marek wrote:

>Hello all,
>
>Amul Shah wrote:
>  
>
>>This patch adds support to include the CPU diode sensor offset when
>>reporting CPU temperature.
>>
>>Please see http://www.lm-sensors.org/ticket/2200 for the full problem
>>description.
>>    
>>
>
>Well I dont think this is correct. I believe that diode offset register is
>related to analog diode output not the digital sensor. Also I know that AMD is
>using my driver and did not make any changes. This leads me to a conclusion that
>the sensor simply does not work correctly and this error might be only
>compensated for analog part and not the digital. IIRC in some errata AMD states
>that the digital temperature sensor might be wrong and urges users to use the
>analog output.
>  
>

I think the digital sensor is just an embedded temperature sensor chip 
reading from a diode in a similar location to that of the analog diode.

In fact, some of the early Intel Xeon chips actually had Maxim 
temperature chips on the PCB substrate.  The Intel Xeon SMBus sensor 
spec was identical to an ADM1021 if I remember correctly.  They just 
marked some of the registers as "reserved" that otherwise had 
interesting information in them if you could identify the actual vendor 
that Intel chose that day to stick on the CPU substrate.

Clearly, this AMD digital sensor is not as transparent as that, but it 
is reasonable that the offset would need to be applied.

:v)

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