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 at 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
