Hi Andreas,
On Tue, 16 Dec 2008 00:12:13 +0100, Andreas Herrmann wrote:
> Current Temperature for K8 RevG desktop CPUs is a "normalized value"
> which can be below ambient temperature.
>
> As a consequence lots of RevG systems report temperatures like:
>
> sensors
> k8temp-pci-00c3
> Adapter: PCI adapter
> Core0 Temp:
> +17C
> Core0 Temp:
> +3C
> Core1 Temp:
> +21C
> Core1 Temp:
> +5C
>
> being quite below ambient temperature.
> There are even reports of negative temperature values.
>
> This patch corrects the temperature reporting of k8temp for
> RevG desktop CPUs.
Thanks for working on this, that was very needed.
> Cc: Rudolf Marek <r.marek at assembler.cz>
> Signed-off-by: Andreas Herrmann <andreas.herrmann3 at amd.com>
> ---
> drivers/hwmon/k8temp.c | 17 +++++++++++++++--
> 1 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hwmon/k8temp.c b/drivers/hwmon/k8temp.c
> index ff12281..b138f79 100644
> --- a/drivers/hwmon/k8temp.c
> +++ b/drivers/hwmon/k8temp.c
> @@ -49,6 +49,7 @@ struct k8temp_data {
> u8 sensorsp; /* sensor presence bits - SEL_CORE & SEL_PLACE */
> u32 temp[2][2]; /* core, place */
> u8 swap_core_select; /* meaning of SEL_CORE is inverted */
> + u32 temp_offset;
> };
>
> static struct k8temp_data *k8temp_update_device(struct device *dev)
> @@ -116,13 +117,15 @@ static ssize_t show_temp(struct device *dev,
> to_sensor_dev_attr_2(devattr);
> int core = attr->nr;
> int place = attr->index;
> + int temp;
> struct k8temp_data *data = k8temp_update_device(dev);
>
> if (data->swap_core_select)
> core = core ? 0 : 1;
>
> - return sprintf(buf, "%d\n",
> - TEMP_FROM_REG(data->temp[core][place]));
> + temp = TEMP_FROM_REG(data->temp[core][place]) + data->temp_offset;
> +
> + return sprintf(buf, "%d\n", temp);
> }
>
> /* core, place */
> @@ -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))
> + /*
> + * 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?
> +
> break;
> }
>
--
Jean Delvare
