* [lm-sensors] [PATCH] AGAIN: support for AMD 10H and 11H
@ 2009-10-04 17:51 Hans-Frieder Vogt
2009-10-09 15:30 ` Rudolf Marek
2009-10-21 21:08 ` Hans-Frieder Vogt
0 siblings, 2 replies; 3+ messages in thread
From: Hans-Frieder Vogt @ 2009-10-04 17:51 UTC (permalink / raw)
To: lm-sensors
Hi,
picking up the discussion that happened in May (see
http://lists.lm-sensors.org/pipermail/lm-sensors/2009-May/025896.html
and following discussion), it seems that the decision taken at the time was a
bit too general.
The latest revisions of the Revision Guide for AMD Family 10h processors (from
Rev. 3.50) show that only model 'B' of the Family 10h processors is affected by
the erratum #319 (inconsistent temperature reporting).
I have a model 'C' Family 10h processor and can confirm that the temperatures
reported by the processor do make sense.
I suggest therefore a patch based on Jasminders proposal, slightly modified to
exclude only the affected family 10h model 'B' processors, but accepting the
other models (i.e. currently 'C' and 'D').
Rudolf, what do you think?
Thanks,
Hans-Frieder
Signed-off-by: Hans-Frieder Vogt <hfvogt@gmx.net>
---
drivers/hwmon/k8temp.c | 39 +++++++++++++++++++++++++++++++++++++--
1 file changed, 37 insertions(+), 2 deletions(-)
--- a/drivers/hwmon/k8temp.c 2009-01-17 18:56:04.000000000 +0100
+++ b/drivers/hwmon/k8temp.c 2009-10-04 19:10:46.873812447 +0200
@@ -1,5 +1,6 @@
/*
* k8temp.c - Linux kernel module for hardware monitoring
+ * for AMD K8 and derivates
*
* Copyright (C) 2006 Rudolf Marek <r.marek@assembler.cz>
*
@@ -33,7 +34,7 @@
#include <linux/mutex.h>
#include <asm/processor.h>
-#define TEMP_FROM_REG(val) (((((val) >> 16) & 0xff) - 49) * 1000)
+#define REG_TCTL 0xa4
#define REG_TEMP 0xe4
#define SEL_PLACE 0x40
#define SEL_CORE 0x04
@@ -52,6 +53,14 @@ struct k8temp_data {
u32 temp_offset;
};
+static unsigned long temp_from_reg(unsigned long val)
+{
+ if (boot_cpu_data.x86 > 0xf)
+ return ((val) >> 21) * 125;
+ else
+ return ((((val) >> 16) & 0xff) - 49) * 1000;
+}
+
static struct k8temp_data *k8temp_update_device(struct device *dev)
{
struct k8temp_data *data = dev_get_drvdata(dev);
@@ -62,6 +71,11 @@ static struct k8temp_data *k8temp_update
if (!data->valid
|| time_after(jiffies, data->last_updated + HZ)) {
+ if (boot_cpu_data.x86 > 0xf) {
+ pci_read_config_dword(pdev, REG_TCTL,
+ &data->temp[0][0]);
+ goto update_done;
+ }
pci_read_config_byte(pdev, REG_TEMP, &tmp);
tmp &= ~(SEL_PLACE | SEL_CORE); /* Select sensor 0, core0 */
pci_write_config_byte(pdev, REG_TEMP, tmp);
@@ -89,6 +103,7 @@ static struct k8temp_data *k8temp_update
}
}
+update_done:
data->last_updated = jiffies;
data->valid = 1;
}
@@ -123,7 +138,7 @@ static ssize_t show_temp(struct device *
if (data->swap_core_select)
core = core ? 0 : 1;
- temp = TEMP_FROM_REG(data->temp[core][place]) + data->temp_offset;
+ temp = temp_from_reg(data->temp[core][place]) + data->temp_offset;
return sprintf(buf, "%d\n", temp);
}
@@ -138,6 +153,8 @@ static DEVICE_ATTR(name, S_IRUGO, show_n
static struct pci_device_id k8temp_ids[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_K8_NB_MISC) },
+ { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_10H_NB_MISC) },
+ { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_11H_NB_MISC) },
{ 0 },
};
@@ -158,6 +175,23 @@ static int __devinit k8temp_probe(struct
}
model = boot_cpu_data.x86_model;
+
+ if ((boot_cpu_data.x86 = 0x10) && (model = 2)) {
+ /*
+ * AMD 10H cpus rev. B report Inaccurate Temperature
+ * Measurement :
+ * http://www.amd.com/us-
en/assets/content_type/white_papers_and_tech_docs/41322.pdf
+ * Errata #319
+ */
+ dev_err(&pdev->dev, "Reported temperature may be inconsistent, "
+ "therefore rejected here - see erratum #319\n");
+ err = -ENODEV;
+ goto exit_free;
+ }
+ if (boot_cpu_data.x86 > 0xf) {
+ goto probe_done;
+ }
+
stepping = boot_cpu_data.x86_mask;
switch (boot_cpu_data.x86) {
@@ -226,6 +260,7 @@ static int __devinit k8temp_probe(struct
data->sensorsp &= ~SEL_CORE;
}
+probe_done:
data->name = "k8temp";
mutex_init(&data->update_lock);
dev_set_drvdata(&pdev->dev, data);
Hans-Frieder Vogt e-mail: hfvogt <at> gmx .dot. net
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [lm-sensors] [PATCH] AGAIN: support for AMD 10H and 11H
2009-10-04 17:51 [lm-sensors] [PATCH] AGAIN: support for AMD 10H and 11H Hans-Frieder Vogt
@ 2009-10-09 15:30 ` Rudolf Marek
2009-10-21 21:08 ` Hans-Frieder Vogt
1 sibling, 0 replies; 3+ messages in thread
From: Rudolf Marek @ 2009-10-09 15:30 UTC (permalink / raw)
To: lm-sensors
Hi all,
Thank you. It seems AMD fixed that. Maybe we would need to avoid the goto.
> if (!data->valid
> || time_after(jiffies, data->last_updated + HZ)) {
> + if (boot_cpu_data.x86 > 0xf) {
> + pci_read_config_dword(pdev, REG_TCTL,
> + &data->temp[0][0]);
> + goto update_done;
> + }
Hm goto can be used only to jump to error paths.
> +
> + if ((boot_cpu_data.x86 = 0x10) && (model = 2)) {
> + /*
> + * AMD 10H cpus rev. B report Inaccurate Temperature
> + * Measurement :
> + * http://www.amd.com/us-
> en/assets/content_type/white_papers_and_tech_docs/41322.pdf
> + * Errata #319
> + */
> + dev_err(&pdev->dev, "Reported temperature may be inconsistent, "
> + "therefore rejected here - see erratum #319\n");
Maybe the message could be - Erratum #319 detected, refusing to load.
> + err = -ENODEV;
Thank you
Rudolf
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [lm-sensors] [PATCH] AGAIN: support for AMD 10H and 11H
2009-10-04 17:51 [lm-sensors] [PATCH] AGAIN: support for AMD 10H and 11H Hans-Frieder Vogt
2009-10-09 15:30 ` Rudolf Marek
@ 2009-10-21 21:08 ` Hans-Frieder Vogt
1 sibling, 0 replies; 3+ messages in thread
From: Hans-Frieder Vogt @ 2009-10-21 21:08 UTC (permalink / raw)
To: lm-sensors
Hi,
> Thank you. It seems AMD fixed that. Maybe we would need to avoid the goto.
> > if (!data->valid
> > || time_after(jiffies, data->last_updated + HZ)) {
> > + if (boot_cpu_data.x86 > 0xf) {
> > + pci_read_config_dword(pdev, REG_TCTL,
> > + &data->temp[0][0]);
> > + goto update_done;
> > + }
>
> Hm goto can be used only to jump to error paths.
>
> > +
> > + if ((boot_cpu_data.x86 = 0x10) && (model = 2)) {
> > + /*
> > + * AMD 10H cpus rev. B report Inaccurate Temperature
> > + * Measurement :
> > + * http://www.amd.com/us-
> > en/assets/content_type/white_papers_and_tech_docs/41322.pdf
> > + * Errata #319
> > + */
> > + dev_err(&pdev->dev, "Reported temperature may be inconsistent, "
> > + "therefore rejected here - see erratum #319\n");
>
> Maybe the message could be - Erratum #319 detected, refusing to load.
>
> > + err = -ENODEV;
>
> Thank you
>
> Rudolf
thanks for your comments, Rudolf. Here is an updated patch that is a bit more
structured and only uses gotos for error paths.
Signed-off-by: Hans-Frieder Vogt <hfvogt@gmx.net>
---
drivers/hwmon/k8temp.c | 133
++++++++++++++++++++++++++++++++++++++-------------------------
1 file changed, 82 insertions(+), 51 deletions(-)
--- a/drivers/hwmon/k8temp.c 2009-03-24 21:08:16.000000000 +0100
+++ b/drivers/hwmon/k8temp.c 2009-10-21 18:43:54.720019815 +0200
@@ -1,5 +1,6 @@
/*
* k8temp.c - Linux kernel module for hardware monitoring
+ * for AMD K8 and derivates
*
* Copyright (C) 2006 Rudolf Marek <r.marek@assembler.cz>
*
@@ -33,7 +34,7 @@
#include <linux/mutex.h>
#include <asm/processor.h>
-#define TEMP_FROM_REG(val) (((((val) >> 16) & 0xff) - 49) * 1000)
+#define REG_TCTL 0xa4
#define REG_TEMP 0xe4
#define SEL_PLACE 0x40
#define SEL_CORE 0x04
@@ -52,6 +53,14 @@ struct k8temp_data {
u32 temp_offset;
};
+static unsigned long temp_from_reg(unsigned long val)
+{
+ if (boot_cpu_data.x86 > 0xf)
+ return ((val) >> 21) * 125;
+ else
+ return ((((val) >> 16) & 0xff) - 49) * 1000;
+}
+
static struct k8temp_data *k8temp_update_device(struct device *dev)
{
struct k8temp_data *data = dev_get_drvdata(dev);
@@ -62,30 +71,35 @@ static struct k8temp_data *k8temp_update
if (!data->valid
|| time_after(jiffies, data->last_updated + HZ)) {
- pci_read_config_byte(pdev, REG_TEMP, &tmp);
- tmp &= ~(SEL_PLACE | SEL_CORE); /* Select sensor 0, core0 */
- pci_write_config_byte(pdev, REG_TEMP, tmp);
- pci_read_config_dword(pdev, REG_TEMP, &data->temp[0][0]);
-
- if (data->sensorsp & SEL_PLACE) {
- tmp |= SEL_PLACE; /* Select sensor 1, core0 */
+ if (boot_cpu_data.x86 > 0xf) {
+ pci_read_config_dword(pdev, REG_TCTL,
+ &data->temp[0][0]);
+ } else {
+ pci_read_config_byte(pdev, REG_TEMP, &tmp);
+ tmp &= ~(SEL_PLACE | SEL_CORE); /* Select sensor 0, core0 */
pci_write_config_byte(pdev, REG_TEMP, tmp);
- pci_read_config_dword(pdev, REG_TEMP,
- &data->temp[0][1]);
- }
-
- if (data->sensorsp & SEL_CORE) {
- tmp &= ~SEL_PLACE; /* Select sensor 0, core1 */
- tmp |= SEL_CORE;
- pci_write_config_byte(pdev, REG_TEMP, tmp);
- pci_read_config_dword(pdev, REG_TEMP,
- &data->temp[1][0]);
+ pci_read_config_dword(pdev, REG_TEMP, &data->temp[0][0]);
if (data->sensorsp & SEL_PLACE) {
- tmp |= SEL_PLACE; /* Select sensor 1, core1 */
+ tmp |= SEL_PLACE; /* Select sensor 1, core0 */
+ pci_write_config_byte(pdev, REG_TEMP, tmp);
+ pci_read_config_dword(pdev, REG_TEMP,
+ &data->temp[0][1]);
+ }
+
+ if (data->sensorsp & SEL_CORE) {
+ tmp &= ~SEL_PLACE; /* Select sensor 0, core1 */
+ tmp |= SEL_CORE;
pci_write_config_byte(pdev, REG_TEMP, tmp);
pci_read_config_dword(pdev, REG_TEMP,
- &data->temp[1][1]);
+ &data->temp[1][0]);
+
+ if (data->sensorsp & SEL_PLACE) {
+ tmp |= SEL_PLACE; /* Select sensor 1, core1 */
+ pci_write_config_byte(pdev, REG_TEMP, tmp);
+ pci_read_config_dword(pdev, REG_TEMP,
+ &data->temp[1][1]);
+ }
}
}
@@ -123,7 +137,7 @@ static ssize_t show_temp(struct device *
if (data->swap_core_select)
core = core ? 0 : 1;
- temp = TEMP_FROM_REG(data->temp[core][place]) + data->temp_offset;
+ temp = temp_from_reg(data->temp[core][place]) + data->temp_offset;
return sprintf(buf, "%d\n", temp);
}
@@ -138,6 +152,8 @@ static DEVICE_ATTR(name, S_IRUGO, show_n
static struct pci_device_id k8temp_ids[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_K8_NB_MISC) },
+ { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_10H_NB_MISC) },
+ { PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_11H_NB_MISC) },
{ 0 },
};
@@ -189,41 +205,56 @@ static int __devinit k8temp_probe(struct
data->temp_offset = 21000;
}
- break;
- }
+ pci_read_config_byte(pdev, REG_TEMP, &scfg);
+ scfg &= ~(SEL_PLACE | SEL_CORE); /* Select sensor 0, core0 */
+ pci_write_config_byte(pdev, REG_TEMP, scfg);
+ pci_read_config_byte(pdev, REG_TEMP, &scfg);
- pci_read_config_byte(pdev, REG_TEMP, &scfg);
- scfg &= ~(SEL_PLACE | SEL_CORE); /* Select sensor 0, core0 */
- pci_write_config_byte(pdev, REG_TEMP, scfg);
- pci_read_config_byte(pdev, REG_TEMP, &scfg);
-
- if (scfg & (SEL_PLACE | SEL_CORE)) {
- dev_err(&pdev->dev, "Configuration bit(s) stuck at 1!\n");
- err = -ENODEV;
- goto exit_free;
- }
+ if (scfg & (SEL_PLACE | SEL_CORE)) {
+ dev_err(&pdev->dev, "Configuration bit(s) stuck at 1!\n");
+ err = -ENODEV;
+ goto exit_free;
+ }
- scfg |= (SEL_PLACE | SEL_CORE);
- pci_write_config_byte(pdev, REG_TEMP, scfg);
+ scfg |= (SEL_PLACE | SEL_CORE);
+ pci_write_config_byte(pdev, REG_TEMP, scfg);
- /* now we know if we can change core and/or sensor */
- pci_read_config_byte(pdev, REG_TEMP, &data->sensorsp);
+ /* now we know if we can change core and/or sensor */
+ pci_read_config_byte(pdev, REG_TEMP, &data->sensorsp);
- if (data->sensorsp & SEL_PLACE) {
- scfg &= ~SEL_CORE; /* Select sensor 1, core0 */
- pci_write_config_byte(pdev, REG_TEMP, scfg);
- pci_read_config_dword(pdev, REG_TEMP, &temp);
- scfg |= SEL_CORE; /* prepare for next selection */
- if (!((temp >> 16) & 0xff)) /* if temp is 0 -49C is not likely */
- data->sensorsp &= ~SEL_PLACE;
- }
+ if (data->sensorsp & SEL_PLACE) {
+ scfg &= ~SEL_CORE; /* Select sensor 1, core0 */
+ pci_write_config_byte(pdev, REG_TEMP, scfg);
+ pci_read_config_dword(pdev, REG_TEMP, &temp);
+ scfg |= SEL_CORE; /* prepare for next selection */
+ if (!((temp >> 16) & 0xff)) /* if temp is 0 -49C is not likely */
+ data->sensorsp &= ~SEL_PLACE;
+ }
- if (data->sensorsp & SEL_CORE) {
- scfg &= ~SEL_PLACE; /* Select sensor 0, core1 */
- pci_write_config_byte(pdev, REG_TEMP, scfg);
- pci_read_config_dword(pdev, REG_TEMP, &temp);
- if (!((temp >> 16) & 0xff)) /* if temp is 0 -49C is not likely */
- data->sensorsp &= ~SEL_CORE;
+ if (data->sensorsp & SEL_CORE) {
+ scfg &= ~SEL_PLACE; /* Select sensor 0, core1 */
+ pci_write_config_byte(pdev, REG_TEMP, scfg);
+ pci_read_config_dword(pdev, REG_TEMP, &temp);
+ if (!((temp >> 16) & 0xff)) /* if temp is 0 -49C is not likely */
+ data->sensorsp &= ~SEL_CORE;
+ }
+ break;
+ case 0x10:
+ if (model <= 2) {
+ /*
+ * AMD 10H cpus rev. B report Inaccurate Temperature
+ * Measurement :
+ * http://www.amd.com/us-
en/assets/content_type/white_papers_and_tech_docs/41322.pdf
+ * Errata #319
+ */
+ dev_err(&pdev->dev, "Erratum #319 detected, refusing to"
+ " load.\n");
+ err = -ENODEV;
+ goto exit_free;
+ }
+ break;
+ case 0x11:
+ break;
}
data->name = "k8temp";
Hans-Frieder Vogt e-mail: hfvogt <at> gmx .dot. net
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2009-10-21 21:08 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-10-04 17:51 [lm-sensors] [PATCH] AGAIN: support for AMD 10H and 11H Hans-Frieder Vogt
2009-10-09 15:30 ` Rudolf Marek
2009-10-21 21:08 ` Hans-Frieder Vogt
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.