Re: [lm-sensors] [PATCH 3/3] hwmon: (k8temp) fix temperature

[Andreas Herrmann <andreas.herrmann3@amd.com>]

On Fri, Jan 09, 2009 at 02:34:39PM +0100, Jean Delvare wrote:
> > @@ -176,6 +179,16 @@ static int __devinit k8temp_probe(struct pci_dev *pdev,
> >  				 "wrong - check erratum #141\n");
> >  		}
> >  
> > +		if (((model >= 0x68) && (model != 0xc1)) &&
> > +		    !(model = 0x68) && !(model = 0x6c) &&
> > +		    !(model = 0x7c))
> 
> This test is pretty confusing, with these extra parentheses and the mix
> of (a != b) and !(a = b). What about the following instead? As far as
> I can see, it leads to the same results, but is much more readable:
> 
> 		if (model >= 0x69 &&
> 		    !(model = 0xc1 || model = 0x6c || model = 0x7c))
> 

Well, right you are.

When I wrote the check I had following in mind:

  Check for RevG:

  ((model >= 0x68) && (model != 0xc1)) // excluding ATHLON64 FX, "JH-F3"

  and after that exclude all mobile parts:

  !(model = 0x68) && !(model = 0x6c) && !(model = 0x7c))

Of course this can be simplified.

> > +			/*
> > +			 * RevG desktop CPUs (i.e. no socket S1G1 parts)
> > +			 * need additional offset, otherwise reported
> > +			 * temperature is below ambient temperature
> > +			 */
> > +			data->temp_offset = 21000;
> 
> If you apply the same offset to all sensors, you'll still obtain
> something odd:
> 
>  k8temp-pci-00c3
>  Adapter: PCI adapter
>  Core0 Temp:
>               +38°C
>  Core0 Temp:
>               +24°C
>  Core1 Temp:
>               +42°C
>  Core1 Temp:
>               +26°C
> 
> That's not terribly realistic, is it? Unless both sensors for a given
> core are very far apart -
> but I suspect each core is pretty small,
> isn't it?

"Far" is relative, isn't it ;-)

Guess this is from an idle system.
Please bring load on both cores and look at the temperature
differences again.




Regards,

Andreas



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