k8temp add support for family 10h and 11h
Subsystems (2)
Rudolf Marek
maintainerPATCH 2/2 k8temp add support for family 10h and 11h
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Hello all,
I finally had time to add support to k8temp for the new family 10h and family 11h of CPUs.
The patch adds support for new fam10h and fam11h CPUs (aka Phenom and new 45nm CPUs). The patch was tested on K8 fam 0fh revE and revF CPU. I dont know if it works at all on new CPUs. Please test. You should see the max temp (always 70C and one temperature per CPU). Of course test is on your own risk ;)
Andreas, please is TcontrolMax also 70C for fam 11h CPUs?
The patch needs review too. I tried not to change many things in the driver, especially the sensorsp logic, but mine test sempron has -49 at core0 place0, which needs to be fixed by another logic which will tell driver what CPU temperatures are usable. But maybe this is for another "cleanup" patch. I think we really need to get this kind of change to kernel first.
What do you think?
Thanks,
Rudolf
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFI4/e13J9wPJqZRNURAojeAJ9OCFor1n+sDshe1gkfMOVGa1LlSwCgxEAF b7YgrXlA2eC/aWyEsnNwLGg=
=wSE1
-----END PGP SIGNATURE-----
drivers/hwmon/k8temp.c +170 -61
@@ -1,7 +1,7 @@/* * k8temp.c - Linux kernel module for hardware monitoring * -* Copyright (C) 2006 Rudolf Marek <[email protected]> +* Copyright (C) 2006, 2008 Rudolf Marek <[email protected]> * * Inspired from the w83785 and amd756 drivers. *
@@ -32,11 +32,18 @@#include <linux/err.h> #include <linux/mutex.h> -#define TEMP_FROM_REG (val) (((((val) >> 16) & 0xff) - 49) * 1000) -#define REG_TEMP 0xe4 +#define FAM_F_TEMP_FROM_REG (val) (((((val) >> 16) & 0xff) - 49) * 1000) +#define FAM_10_TEMP_FROM_REG (val) (((val) >> 21) * 125) + +#define FAM_F_REG_TEMP 0xe4 +#define FAM_10_REG_TEMP 0xa5 +#define REG_CPUID 0xfc +/* real bitpos */ #define SEL_PLACE 0x40#define SEL_CORE 0x04 +/* program logic bitpos */ +#define SEL_TEMPMAX 0x01struct k8temp_data {struct device *hwmon_dev;
@@ -45,46 +52,69 @@char valid;/* zero until following fields are valid */ unsigned long last_updated;/* in jiffies */ -/* registers values */ u8 sensorsp;/* sensor presence bits - SEL_CORE & SEL_PLACE */ -u32 temp[2 ][2 ];/* core, place */ +int temp[2 ][2 ];/* core, place */ +int tcontrol_max;u8 fam; +u32 cpuid; }; -static struct k8temp_data *k8temp_update_device (struct device *dev) +static void k8temp_update_device_fam_10 (struct device *dev) {struct k8temp_data *data =dev_get_drvdata (dev);struct pci_dev *pdev =to_pci_dev (dev); -u8 tmp; +u32 tmp;mutex_lock (&data->update_lock);if (!data->valid ||time_after (jiffies, data->last_updated +HZ )) { -pci_read_config_byte (pdev,REG_TEMP , &tmp); + +pci_read_config_dword (pdev,FAM_10_REG_TEMP , &tmp); + data->temp[0 ][0 ] =FAM_10_TEMP_FROM_REG (tmp); + + data->last_updated = jiffies; + data->valid =1 ; + } + +mutex_unlock (&data->update_lock); +} + +static void k8temp_update_device_fam_f (struct device *dev) +{ +struct k8temp_data *data =dev_get_drvdata (dev); +struct pci_dev *pdev =to_pci_dev (dev); +u32 tmp; + +mutex_lock (&data->update_lock); + +if (!data->valid + ||time_after (jiffies, data->last_updated +HZ )) { +pci_read_config_dword (pdev,FAM_F_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 ]); +pci_write_config_byte (pdev,FAM_F_REG_TEMP , tmp); +pci_read_config_dword (pdev,FAM_F_REG_TEMP , &tmp); + data->temp[0 ][0 ] =FAM_F_TEMP_FROM_REG (tmp);if (data->sensorsp &SEL_PLACE ) { 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 ]); +pci_write_config_byte (pdev,FAM_F_REG_TEMP , tmp); +pci_read_config_dword (pdev,FAM_F_REG_TEMP , &tmp); + data->temp[0 ][1 ] =FAM_F_TEMP_FROM_REG (tmp); }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_write_config_byte (pdev,FAM_F_REG_TEMP , tmp); +pci_read_config_dword (pdev,FAM_F_REG_TEMP , &tmp); + data->temp[1 ][0 ] =FAM_F_TEMP_FROM_REG (tmp);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 ]); +pci_write_config_byte (pdev,FAM_F_REG_TEMP , tmp); +pci_read_config_dword (pdev,FAM_F_REG_TEMP , &tmp); + data->temp[1 ][1 ] =FAM_F_TEMP_FROM_REG (tmp); } }
@@ -93,7 +123,6 @@ }mutex_unlock (&data->update_lock); -return data; }/*
@@ -116,15 +145,36 @@to_sensor_dev_attr_2 (devattr);int core = attr->nr;int place = attr->index; -struct k8temp_data *data =k8temp_update_device (dev); +struct k8temp_data *data =dev_get_drvdata (dev); + +switch (data->fam) { +case 0xf : +k8temp_update_device_fam_f (dev); +break ; +case 0x10 : +k8temp_update_device_fam_10 (dev); +break ; +case 0x11 : +k8temp_update_device_fam_10 (dev); +break ; + } + +return sprintf (buf,"%d\n" , data->temp[core][place]); +} + -return sprintf (buf,"%d\n" , -TEMP_FROM_REG (data->temp[core][place])); +static ssize_t show_max (struct device *dev,struct device_attribute + *devattr,char *buf) +{ +struct k8temp_data *data =dev_get_drvdata (dev); + +return sprintf (buf,"%d\n" , data->tcontrol_max); }/* core, place */ static SENSOR_DEVICE_ATTR_2(temp1_input, S_IRUGO, show_temp, NULL, 0, 0); +static SENSOR_DEVICE_ATTR(temp1_max, S_IRUGO, show_max, NULL, 0); static SENSOR_DEVICE_ATTR_2(temp2_input, S_IRUGO, show_temp, NULL, 0, 1); static SENSOR_DEVICE_ATTR_2(temp3_input, S_IRUGO, show_temp, NULL, 1, 0); static SENSOR_DEVICE_ATTR_2(temp4_input, S_IRUGO, show_temp, NULL, 1, 1);
@@ -132,65 +182,52 @@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 ,0x1203 ) }, + {PCI_DEVICE (PCI_VENDOR_ID_AMD ,0x1303 ) }, {0 }, };MODULE_DEVICE_TABLE (pci, k8temp_ids); -static int __devinit k8temp_probe (struct pci_dev *pdev, -const struct pci_device_id *id) -{ -int err; -u8 scfg; -u32 temp; - -struct k8temp_data *data; -u32 cpuid =cpuid_eax (1 ); -/* this feature should be available since SH-C0 core */ -if ((cpuid ==0xf40 ) || (cpuid ==0xf50 ) || (cpuid ==0xf51 )) { - err = -ENODEV ; - goto exit; - } - -if (!(data =kzalloc (sizeof (struct k8temp_data ),GFP_KERNEL ))) { - err = -ENOMEM ; - goto exit; - } +static int __devinit setup_fam_10 (struct pci_dev *pdev,struct k8temp_data *data) +{ +/* we need max temp */ + data->sensorsp =SEL_TEMPMAX ; + data->tcontrol_max =70000 ; -/* get real PCI based cpuid, prior revF of fam 0Fh, this reg is 0 */ -pci_read_config_dword (pdev,REG_CPUID , &cpuid); + data->name = (data->fam ==0x11 ) ?"fam11temp" :"fam10temp" ; - data->fam = (cpuid &0x00000f00 ) >>8 ; - data->fam += (cpuid &0x00f00000 ) >>20 ; +return 0 ; +} -switch (data->fam) { -case 0xf : -dev_warn (&pdev->dev,"Temperature readouts might be wrong" -" - check errata #141\n" ); - } +static int __devinit setup_fam_f (struct pci_dev *pdev,struct k8temp_data *data) +{ +u8 scfg; +u32 temp; +int err =0 ; -pci_read_config_byte (pdev,REG_TEMP , &scfg); +pci_read_config_byte (pdev,FAM_F_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_write_config_byte (pdev,FAM_F_REG_TEMP , scfg); +pci_read_config_byte (pdev,FAM_F_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; + goto exit_setup; } scfg |= (SEL_PLACE |SEL_CORE ); -pci_write_config_byte (pdev,REG_TEMP , scfg); +pci_write_config_byte (pdev,FAM_F_REG_TEMP , scfg);/* now we know if we can change core and/or sensor */ -pci_read_config_byte (pdev,REG_TEMP , &data->sensorsp); +pci_read_config_byte (pdev,FAM_F_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); +pci_write_config_byte (pdev,FAM_F_REG_TEMP , scfg); +pci_read_config_dword (pdev,FAM_F_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 ;
@@ -198,13 +235,74 @@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); +pci_write_config_byte (pdev,FAM_F_REG_TEMP , scfg); +pci_read_config_dword (pdev,FAM_F_REG_TEMP , &temp);if (!((temp >>16 ) &0xff ))/* if temp is 0 -49C is not likely */ data->sensorsp &= ~SEL_CORE ; } +/* we use sensorsp for programm logic too */ + data->sensorsp &= (SEL_PLACE |SEL_CORE ); data->name ="k8temp" ; + +exit_setup: +return err; +} + +static int __devinit k8temp_probe (struct pci_dev *pdev, +const struct pci_device_id *id) +{ +int err =0 ; + +struct k8temp_data *data; +u32 cpuid =cpuid_eax (1 ); + +/* this feature should be available since SH-C0 core */ +if ((cpuid ==0xf40 ) || (cpuid ==0xf50 ) || (cpuid ==0xf51 )) { + err = -ENODEV ; + goto exit; + } + +if (!(data =kzalloc (sizeof (struct k8temp_data ),GFP_KERNEL ))) { + err = -ENOMEM ; + goto exit; + } + +/* get real PCI based cpuid, prior revF of fam 0Fh, this reg is 0 */ +pci_read_config_dword (pdev,REG_CPUID , &data->cpuid); + + data->fam = (data->cpuid &0x00000f00 ) >>8 ; + data->fam += (data->cpuid &0x00f00000 ) >>20 ; + +switch (data->fam) { +case 0xf : +dev_warn (&pdev->dev,"Temperature readouts might be wrong" +" - check errata #141\n" ); +if ((err =setup_fam_f (pdev, data))) + goto exit_free; +break ; +case 0x10 : +dev_warn (&pdev->dev,"Temperature readouts might be wrong" +" - check errata #319\n" ); +if ((err =setup_fam_10 (pdev, data))) + goto exit_free; +break ; +case 0x11 : +if ((err =setup_fam_10 (pdev, data))) + goto exit_free; +break ; +case 0x0 : +/* mark revE fam 0fh as fam 0fh */ + data->fam =0xf ; +if ((err =setup_fam_f (pdev, data))) + goto exit_free; +break ; +default : + err = -ENODEV ; +dev_err (&pdev->dev,"Unknown family with known PCI ID\n" ); + goto exit_free; + } +mutex_init (&data->update_lock);dev_set_drvdata (&pdev->dev, data);
@@ -236,6 +334,13 @@ goto exit_remove; } +if (data->sensorsp &SEL_TEMPMAX ) { + err =device_create_file (&pdev->dev, + &sensor_dev_attr_temp1_max.dev_attr); +if (err) + goto exit_remove; + } + err =device_create_file (&pdev->dev, &dev_attr_name);if (err) goto exit_remove;
@@ -253,6 +358,8 @@device_remove_file (&pdev->dev, &sensor_dev_attr_temp1_input.dev_attr);device_remove_file (&pdev->dev, + &sensor_dev_attr_temp1_max.dev_attr); +device_remove_file (&pdev->dev, &sensor_dev_attr_temp2_input.dev_attr);device_remove_file (&pdev->dev, &sensor_dev_attr_temp3_input.dev_attr);
@@ -274,6 +381,8 @@device_remove_file (&pdev->dev, &sensor_dev_attr_temp1_input.dev_attr);device_remove_file (&pdev->dev, + &sensor_dev_attr_temp1_max.dev_attr); +device_remove_file (&pdev->dev, &sensor_dev_attr_temp2_input.dev_attr);device_remove_file (&pdev->dev, &sensor_dev_attr_temp3_input.dev_attr);
_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Re: [lm-sensors] [PATCH 2/2] k8temp add support for family 10h and
Quoted 9 lines · Rudolf Marek
I finally had time to add support to k8temp for the new family 10h and family 11h of CPUs.
The patch adds support for new fam10h and fam11h CPUs (aka Phenom and new 45nm CPUs). The patch was tested on K8 fam 0fh revE and revF CPU. I dont know if it works at all on new CPUs. Please test. You should see the max temp (always 70C and one temperature per CPU). Of course test is on your own risk ;)
Andreas, please is TcontrolMax also 70C for fam 11h CPUs?
Hmm, the "Power and Thermal Data Sheets" don't provide this info. I'll check this asap.
.. and I'll test your stuff in the next few days.
Regards,
Andreas
_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Re: [lm-sensors] [PATCH 2/2] k8temp add support for family 10h and
In reply to Rudolf MarekHi Rudolf,
I can do as many tests as you want.
I already installed k10temp (
http://alt.nntp2http.com/os/linux/2008/08/3e7de045c54f5cd5b7e563330597ab79.html
) as a test and it's working pretty fine. I have update sensors-detect to use it.
2008/10/1 Rudolf Marek <[email protected]>:
Quoted 30 lines
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1Hello all,
I finally had time to add support to k8temp for the new family 10h and family 11h of CPUs.
The patch adds support for new fam10h and fam11h CPUs (aka Phenom and new 45nm CPUs). The patch was tested on K8 fam 0fh revE and revF CPU. I dont know if it works at all on new CPUs. Please test. You should see the max temp (always 70C and one temperature per CPU). Of course test is on your own risk ;)
Andreas, please is TcontrolMax also 70C for fam 11h CPUs?
The patch needs review too. I tried not to change many things in the driver, especially the sensorsp logic, but mine test sempron has -49 at core0 place0, which needs to be fixed by another logic which will tell driver what CPU temperatures are usable. But maybe this is for another "cleanup" patch. I think we really need to get this kind of change to kernel first.
What do you think?
Thanks,
Rudolf-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
Quoted 9 lines
iD8DBQFI4/e13J9wPJqZRNURAojeAJ9OCFor1n+sDshe1gkfMOVGa1LlSwCgxEAF b7YgrXlA2eC/aWyEsnNwLGg> =wSE1
-----END PGP SIGNATURE-----_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Re: [lm-sensors] [PATCH 2/2] k8temp add support for family 10h and
In reply to Rudolf MarekHi Rudolf,
Quoted 10 lines · Rudolf Marek
I finally had time to add support to k8temp for the new family 10h and family 11h of CPUs.
The patch adds support for new fam10h and fam11h CPUs (aka Phenom and new 45nm CPUs). The patch was tested on K8 fam 0fh revE and revF CPU. I dont know if it works at all on new CPUs. Please test. You should see the max temp (always 70C and one temperature per CPU). Of course test is on your own risk ;)
Andreas, please is TcontrolMax also 70C for fam 11h CPUs?
Quoted 7 lines
The patch needs review too. I tried not to change many things in the driver, especially the sensorsp logic, but mine test sempron has -49 at core0 place0, which needs to be fixed by another logic which will tell driver what CPU temperatures are usable. But maybe this is for another "cleanup" patch. I think we really need to get this kind of change to kernel first.
What do you think?
Looking at your patch, there doesn't seem to be much in common between the family 0Fh and the family 10h. The temperature conversion formula is different, the registers are different... And I seem to understand that the family 10h has a single sensor? In the end your patch is larger than the k8temp driver itself. So I am wondering, does it really make sense to support both families with the same driver?
About the code itself (limited review as I can't apply the patch):
Quoted 13 lines
Index:
linux-2.6.27-rc7/drivers/hwmon/k8temp.c
=================================> ---linux-2.6.27-rc7.orig/drivers/hwmon/k8temp.c2008-09-28 11:13:42.000000000 +0200
+++linux-2.6.27-rc7/drivers/hwmon/k8temp.c2008-10-02 00:01:56.383473013 +0200drivers/hwmon/k8temp.c +1
-1@@ -1,7 +1,7 @@/* * k8temp.c - Linux kernel module for hardware monitoring * -* Copyright (C) 2006 Rudolf Marek <[email protected]> +* Copyright (C) 2006, 2008 Rudolf Marek <[email protected]> * * Inspired from the w83785 and amd756 drivers. *
drivers/hwmon/k8temp.c line 36
Quoted 7 lines
drivers/hwmon/k8temp.c +2
-2@@ -32,11 +32,18 @@#include <linux/err.h> #include <linux/mutex.h> -#define TEMP_FROM_REG (val) (((((val) >> 16) & 0xff) - 49) * 1000) -#define REG_TEMP 0xe4 +#define FAM_F_TEMP_FROM_REG (val) (((((val) >> 16) & 0xff) - 49) * 1000) +#define FAM_10_TEMP_FROM_REG (val) (((val) >> 21) * 125)
It's probably the right time to turn these macros into inline functions.
drivers/hwmon/k8temp.c line 46
Quoted 10 lines
drivers/hwmon/k8temp.c +7
-0+ +#define FAM_F_REG_TEMP 0xe4 +#define FAM_10_REG_TEMP 0xa5 +#define REG_CPUID 0xfc +/* real bitpos */ #define SEL_PLACE 0x40#define SEL_CORE 0x04 +/* program logic bitpos */ +#define SEL_TEMPMAX 0x01
These comments are rather obscure to me. I had to read the driver code to understand what they meant. This pretty much voids the point of comments, doesn't it? ;)
I don't think you need SEL_TEMPMAX anyway. You only use it to determine whether to create file temp1_max or not, but you could use (data->tcontrol_max != 0) or (data->fam >= 0x10) for that.
Quoted 4 lines
drivers/hwmon/k8temp.c +0
-0struct k8temp_data {struct device *hwmon_dev;
drivers/hwmon/k8temp.c line 59
Quoted 10 lines
drivers/hwmon/k8temp.c +3
-2@@ -45,46 +52,69 @@char valid;/* zero until following fields are valid */ unsigned long last_updated;/* in jiffies */ -/* registers values */ u8 sensorsp;/* sensor presence bits - SEL_CORE & SEL_PLACE */ -u32 temp[2 ][2 ];/* core, place */ +int temp[2 ][2 ];/* core, place */ +int tcontrol_max;u8 fam; +u32 cpuid;
Why do you store cpuid? You never use it.
Quoted 90 lines
drivers/hwmon/k8temp.c +38
-16}; -static struct k8temp_data *k8temp_update_device (struct device *dev) +static void k8temp_update_device_fam_10 (struct device *dev) {struct k8temp_data *data =dev_get_drvdata (dev);struct pci_dev *pdev =to_pci_dev (dev); -u8 tmp; +u32 tmp;mutex_lock (&data->update_lock);if (!data->valid ||time_after (jiffies, data->last_updated +HZ )) { -pci_read_config_byte (pdev,REG_TEMP , &tmp); + +pci_read_config_dword (pdev,FAM_10_REG_TEMP , &tmp); + data->temp[0 ][0 ] =FAM_10_TEMP_FROM_REG (tmp); + + data->last_updated = jiffies; + data->valid =1 ; + } + +mutex_unlock (&data->update_lock); +} + +static void k8temp_update_device_fam_f (struct device *dev) +{ +struct k8temp_data *data =dev_get_drvdata (dev); +struct pci_dev *pdev =to_pci_dev (dev); +u32 tmp; + +mutex_lock (&data->update_lock); + +if (!data->valid + ||time_after (jiffies, data->last_updated +HZ )) { +pci_read_config_dword (pdev,FAM_F_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 ]); +pci_write_config_byte (pdev,FAM_F_REG_TEMP , tmp); +pci_read_config_dword (pdev,FAM_F_REG_TEMP , &tmp); + data->temp[0 ][0 ] =FAM_F_TEMP_FROM_REG (tmp);if (data->sensorsp &SEL_PLACE ) { 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 ]); +pci_write_config_byte (pdev,FAM_F_REG_TEMP , tmp); +pci_read_config_dword (pdev,FAM_F_REG_TEMP , &tmp); + data->temp[0 ][1 ] =FAM_F_TEMP_FROM_REG (tmp); }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_write_config_byte (pdev,FAM_F_REG_TEMP , tmp); +pci_read_config_dword (pdev,FAM_F_REG_TEMP , &tmp); + data->temp[1 ][0 ] =FAM_F_TEMP_FROM_REG (tmp);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 ]); +pci_write_config_byte (pdev,FAM_F_REG_TEMP , tmp); +pci_read_config_dword (pdev,FAM_F_REG_TEMP , &tmp); + data->temp[1 ][1 ] =FAM_F_TEMP_FROM_REG (tmp); } }@@ -93,7 +123,6 @@ }mutex_unlock (&data->update_lock); -return data; }/* @@ -116,15 +145,36 @@to_sensor_dev_attr_2 (devattr);int core = attr->nr;int place = attr->index; -struct k8temp_data *data =k8temp_update_device (dev); +struct k8temp_data *data =dev_get_drvdata (dev); +
drivers/hwmon/k8temp.c line 159
Quoted 10 lines
drivers/hwmon/k8temp.c +10
-0+switch (data->fam) { +case 0xf : +k8temp_update_device_fam_f (dev); +break ; +case 0x10 : +k8temp_update_device_fam_10 (dev); +break ; +case 0x11 : +k8temp_update_device_fam_10 (dev); +break ;
You could merge case 0x10 and case 0x11.
Quoted 10 lines
drivers/hwmon/k8temp.c +7
-2+ } + +return sprintf (buf,"%d\n" , data->temp[core][place]); +} + -return sprintf (buf,"%d\n" , -TEMP_FROM_REG (data->temp[core][place])); +static ssize_t show_max (struct device *dev,struct device_attribute + *devattr,char *buf)
drivers/hwmon/k8temp.c line 177
Quoted 10 lines
drivers/hwmon/k8temp.c +5
-0+{ +struct k8temp_data *data =dev_get_drvdata (dev); + +return sprintf (buf,"%d\n" , data->tcontrol_max); }/* core, place */ static SENSOR_DEVICE_ATTR_2(temp1_input, S_IRUGO, show_temp, NULL, 0, 0); +static SENSOR_DEVICE_ATTR(temp1_max, S_IRUGO, show_max, NULL, 0);
This could be a simple DEVICE_ATTR() as you never use the parameter value.
drivers/hwmon/k8temp.c line 185
Quoted 8 lines
drivers/hwmon/k8temp.c +1
-0static SENSOR_DEVICE_ATTR_2(temp2_input, S_IRUGO, show_temp, NULL, 0, 1); static SENSOR_DEVICE_ATTR_2(temp3_input, S_IRUGO, show_temp, NULL, 1, 0); static SENSOR_DEVICE_ATTR_2(temp4_input, S_IRUGO, show_temp, NULL, 1, 1);@@ -132,65 +182,52 @@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 ,0x1203 ) },
PCI_DEVICE_ID_AMD_10H_NB_MISC
drivers/hwmon/k8temp.c line 186
Quoted 1 line
drivers/hwmon/k8temp.c +1
-0+ { PCI_DEVICE(PCI_VENDOR_ID_AMD, 0x1303) },
PCI_DEVICE_ID_AMD_11H_NB_MISC
Quoted 24 lines
drivers/hwmon/k8temp.c +0
-18{ 0 }, };MODULE_DEVICE_TABLE (pci, k8temp_ids); -static int __devinit k8temp_probe (struct pci_dev *pdev, -const struct pci_device_id *id) -{ -int err; -u8 scfg; -u32 temp; - -struct k8temp_data *data; -u32 cpuid =cpuid_eax (1 ); -/* this feature should be available since SH-C0 core */ -if ((cpuid =0xf40 ) || (cpuid =0xf50 ) || (cpuid =0xf51 )) { - err = -ENODEV ; - goto exit; - } - -if (!(data =kzalloc (sizeof (struct k8temp_data ),GFP_KERNEL ))) { - err = -ENOMEM ; - goto exit;
drivers/hwmon/k8temp.c
Quoted 10 lines
drivers/hwmon/k8temp.c +6
-3- } +static int __devinit setup_fam_10 (struct pci_dev *pdev,struct k8temp_data *data) +{ +/* we need max temp */ + data->sensorsp =SEL_TEMPMAX ; + data->tcontrol_max =70000 ; -/* get real PCI based cpuid, prior revF of fam 0Fh, this reg is 0 */ -pci_read_config_dword (pdev,REG_CPUID , &cpuid); + data->name = (data->fam =0x11 ) ?"fam11temp" :"fam10temp" ;
What sexy names! :/
Quoted 52 lines
drivers/hwmon/k8temp.c +15
-15- data->fam = (cpuid &0x00000f00 ) >>8 ; - data->fam += (cpuid &0x00f00000 ) >>20 ; +return 0 ; +} -switch (data->fam) { -case 0xf : -dev_warn (&pdev->dev,"Temperature readouts might be wrong" -" - check errata #141\n" ); - } +static int __devinit setup_fam_f (struct pci_dev *pdev,struct k8temp_data *data) +{ +u8 scfg; +u32 temp; +int err =0 ; -pci_read_config_byte (pdev,REG_TEMP , &scfg); +pci_read_config_byte (pdev,FAM_F_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_write_config_byte (pdev,FAM_F_REG_TEMP , scfg); +pci_read_config_byte (pdev,FAM_F_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; + goto exit_setup; } scfg |= (SEL_PLACE |SEL_CORE ); -pci_write_config_byte (pdev,REG_TEMP , scfg); +pci_write_config_byte (pdev,FAM_F_REG_TEMP , scfg);/* now we know if we can change core and/or sensor */ -pci_read_config_byte (pdev,REG_TEMP , &data->sensorsp); +pci_read_config_byte (pdev,FAM_F_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); +pci_write_config_byte (pdev,FAM_F_REG_TEMP , scfg); +pci_read_config_dword (pdev,FAM_F_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 ;@@ -198,13 +235,74 @@if (data->sensorsp &SEL_CORE ) {
drivers/hwmon/k8temp.c line 244
Quoted 10 lines
drivers/hwmon/k8temp.c +3
-2scfg &= ~SEL_PLACE ;/* Select sensor 0, core1 */ -pci_write_config_byte (pdev,REG_TEMP , scfg); -pci_read_config_dword (pdev,REG_TEMP , &temp); +pci_write_config_byte (pdev,FAM_F_REG_TEMP , scfg); +pci_read_config_dword (pdev,FAM_F_REG_TEMP , &temp);if (!((temp >>16 ) &0xff ))/* if temp is 0 -49C is not likely */ data->sensorsp &= ~SEL_CORE ; } +/* we use sensorsp for programm logic too */
Spelling: program.
drivers/hwmon/k8temp.c line 249
Quoted 5 lines
drivers/hwmon/k8temp.c +4
-0+ data->sensorsp &= (SEL_PLACE |SEL_CORE ); data->name ="k8temp" ; + +exit_setup: +return err;
Doesn't seem terribly useful, you might as well have returned the error code directly.
Quoted 7 lines
drivers/hwmon/k8temp.c +7
-0+} + +static int __devinit k8temp_probe (struct pci_dev *pdev, +const struct pci_device_id *id) +{ +int err =0 ; +
drivers/hwmon/k8temp.c line 266
Quoted 10 lines
drivers/hwmon/k8temp.c +10
-0+struct k8temp_data *data; +u32 cpuid =cpuid_eax (1 ); + +/* this feature should be available since SH-C0 core */ +if ((cpuid =0xf40 ) || (cpuid =0xf50 ) || (cpuid =0xf51 )) { + err = -ENODEV ; + goto exit; + } + +if (!(data =kzalloc (sizeof (struct k8temp_data ),GFP_KERNEL ))) {
Preferred coding style (as checkpatch.pl would have told you) is:
data = kzalloc(sizeof(struct k8temp_data), GFP_KERNEL);
if (!data) {drivers/hwmon/k8temp.c line 275
Quoted 9 lines
drivers/hwmon/k8temp.c +9
-0+ err = -ENOMEM ; + goto exit; + } + +/* get real PCI based cpuid, prior revF of fam 0Fh, this reg is 0 */ +pci_read_config_dword (pdev,REG_CPUID , &data->cpuid); + + data->fam = (data->cpuid &0x00000f00 ) >>8 ; + data->fam += (data->cpuid &0x00f00000 ) >>20 ;
Same mask bug as the previous patch had.
drivers/hwmon/k8temp.c line 151
Quoted 3 lines
drivers/hwmon/k8temp.c +3
-0+ +switch (data->fam) { +case 0xf :
And same indentation issue here.
drivers/hwmon/k8temp.c line 281
Quoted 3 lines
drivers/hwmon/k8temp.c +3
-0+dev_warn (&pdev->dev,"Temperature readouts might be wrong" +" - check errata #141\n" ); +if ((err =setup_fam_f (pdev, data)))
Here again checkpatch.pl would tell you to separate the function call and the error check. Same below.
drivers/hwmon/k8temp.c line 286
Quoted 5 lines
drivers/hwmon/k8temp.c +5
-0+ goto exit_free; +break ; +case 0x10 : +dev_warn (&pdev->dev,"Temperature readouts might be wrong" +" - check errata #319\n" );
You can save a few bytes by making the errata number a %d and passing it as parameter.
Quoted 54 lines
drivers/hwmon/k8temp.c +30
-0+if ((err =setup_fam_10 (pdev, data))) + goto exit_free; +break ; +case 0x11 : +if ((err =setup_fam_10 (pdev, data))) + goto exit_free; +break ; +case 0x0 : +/* mark revE fam 0fh as fam 0fh */ + data->fam =0xf ; +if ((err =setup_fam_f (pdev, data))) + goto exit_free; +break ; +default : + err = -ENODEV ; +dev_err (&pdev->dev,"Unknown family with known PCI ID\n" ); + goto exit_free; + } +mutex_init (&data->update_lock);dev_set_drvdata (&pdev->dev, data);@@ -236,6 +334,13 @@ goto exit_remove; } +if (data->sensorsp &SEL_TEMPMAX ) { + err =device_create_file (&pdev->dev, + &sensor_dev_attr_temp1_max.dev_attr); +if (err) + goto exit_remove; + } + err =device_create_file (&pdev->dev, &dev_attr_name);if (err) goto exit_remove;@@ -253,6 +358,8 @@device_remove_file (&pdev->dev, &sensor_dev_attr_temp1_input.dev_attr);device_remove_file (&pdev->dev, + &sensor_dev_attr_temp1_max.dev_attr); +device_remove_file (&pdev->dev, &sensor_dev_attr_temp2_input.dev_attr);device_remove_file (&pdev->dev, &sensor_dev_attr_temp3_input.dev_attr);@@ -274,6 +381,8 @@device_remove_file (&pdev->dev, &sensor_dev_attr_temp1_input.dev_attr);device_remove_file (&pdev->dev, + &sensor_dev_attr_temp1_max.dev_attr); +device_remove_file (&pdev->dev, &sensor_dev_attr_temp2_input.dev_attr);device_remove_file (&pdev->dev, &sensor_dev_attr_temp3_input.dev_attr);
Signature · 6 lines
Jean Delvare _______________________________________________ lm-sensors mailing list [email protected] http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Re: [lm-sensors] [PATCH 2/2] k8temp add support for family 10h
In reply to Rudolf MarekQuoted 9 lines · Jean Delvare
Quoted 1 line · Rudolf Marek
What do you think?
Looking at your patch, there doesn't seem to be much in common between the family 0Fh and the family 10h. The temperature conversion formula is different, the registers are different... And I seem to understand that the family 10h has a single sensor? In the end your patch is larger than the k8temp driver itself. So I am wondering, does it really make sense to support both families with the same driver?
IMHO this makes sense. The user just needs to switch on CONFIG_SENSORS_K8TEMP on a system with newer AMD CPUs (regardless whether the CPU is family 0xf, 0x10 or 0x11) and the driver is able to handle the CPU temperature sensor.
It's the same with powernow-k8. This cpufreq driver is used for all newer AMD CPU families as well.
Regards,
Andreas
_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Re: [lm-sensors] [PATCH 2/2] k8temp add support for family 10h and
In reply to Rudolf MarekHi Andreas,
Quoted 11 lines · Andreas Herrmann
Quoted 9 lines · Jean Delvare
Quoted 1 line · Rudolf Marek
What do you think?
Looking at your patch, there doesn't seem to be much in common between the family 0Fh and the family 10h. The temperature conversion formula is different, the registers are different... And I seem to understand that the family 10h has a single sensor? In the end your patch is larger than the k8temp driver itself. So I am wondering, does it really make sense to support both families with the same driver?
Quoted 4 lines
IMHO this makes sense. The user just needs to switch on
CONFIG_SENSORS_K8TEMPon a system with newer AMD CPUs (regardless whether the CPU is family0xf,0x10or0x11) and the driver is able to handle the CPU temperature sensor.
The default user really doesn't care. He/she runs a distribution with all hwmon drivers compiled as modules, and the k8temp driver loads automatically as needed. A potential fam10temp driver would also be compiled as a module by default and would also load automatically, just as the k8temp driver does. The user really doesn't care whether there is one driver or two, he/she doesn't care how the drivers are named, he/she doesn't even need to know that there _are_ drivers.
The advanced user doesn't care, either. He/she knows enough to enable the right driver in all cases.
The rationale to add support to an existing driver vs. writing a new driver is how much do these devices have in common. In the case of the K8 vs. fam10h, the answer, as far as I can see, is: not much (see details in my original reply above.)
Quoted 2 lines
It's the same with powernow-k8. This cpufreq driver is used for all newer AMD CPU families as well.
Well, maybe the cpufreq interface to fam10h CPUs is the same as those to K8 temp CPUs? In which case it makes sense to have a single driver.
Anyway, what the cpufreq people decided to do is not exactly relevant. We don't have to follow them if we don't want to.
Signature · 6 lines
Jean Delvare _______________________________________________ lm-sensors mailing list [email protected] http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Re: [lm-sensors] [PATCH 2/2] k8temp add support for family 10h and
In reply to Rudolf MarekHowdy,
I have a AMD Phenom processor and noticed your patch to get the temps for this processor. What and How do I apply this patch to in order to get this working? Or do you have a patched k8temp.c file I can overwrite in the 3.0.3 package (which I'm having trouble locating)? Thanks in advance.
Mike Doerner
_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Re: [lm-sensors] [PATCH 2/2] k8temp add support for family 10h and
In reply to Rudolf MarekAnybody there?
Mike Doerner
Quoted 10 lines · Michael R. Doerner
Howdy,
I have a AMD Phenom processor and noticed your patch to get the temps for this processor. What and How do I apply this patch to in order to get this working? Or do you have a patched
k8temp.cfile I can overwrite in the 3.0.3 package (which I'm having trouble locating)? Thanks in advance.Mike Doerner
_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Re: [lm-sensors] [PATCH 2/2] k8temp add support for family 10h and
In reply to Rudolf MarekI'm running my Fedora 11 system on an AMD Athlon 64 X2 7750 processor and Gigabyte GA-MA78GM-US2H motherboard.
Running sensors-detect(8) finds the embedded "AMD K10 thermal sensors" and reports "(driver "to be written"). In the summary it reports:
Driver `to-be-written':
* Chip `AMD K10 thermal sensors' (confidence: 9)
Note: there is no driver for AMD K10 thermal sensors yet. Check http://www.lm-sensors.org/wiki/Devices for updates.
Checking the site, I found the subject topic and wondered if the 10h family is synonymous with K10, and if the subject patch applies to my system?
The sensors(1) app appears to pick up temperature readings from my system, but I'm not able to figure out the mapping of the values I'm getting (sample below):
$ sensors
it8718-isa-0228
Adapter: ISA adapter
in0: +1.04 V (min = +0.00 V, max = +4.08 V)
in1: +1.95 V (min = +0.00 V, max = +4.08 V)
in2: +3.30 V (min = +0.00 V, max = +4.08 V)
in3: +2.96 V (min = +0.00 V, max = +4.08 V)
in4: +3.06 V (min = +0.00 V, max = +4.08 V)
in5: +4.08 V (min = +0.00 V, max = +4.08 V) ALARM
in6: +4.08 V (min = +0.00 V, max = +4.08 V) ALARM
in7: +2.03 V (min = +0.00 V, max = +4.08 V)
Vbat: +3.30 V
fan1: 2537 RPM (min = 10 RPM)
fan2: 0 RPM (min = 0 RPM)
fan3: 0 RPM (min = 0 RPM)
fan4: 0 RPM (min = 0 RPM)
temp1: +44.0°C (low = +127.0°C, high = +127.0°C) sensor
= thermistor
temp2: +37.0°C (low = +127.0°C, high = +60.0°C) sensor
= thermal diode
temp3: +46.0°C (low = +127.0°C, high = +127.0°C) sensor
= thermistor
cpu0_vid: +1.550 VCan you help me understand whether I need to wait for a K10 driver, can use the subject patch, or need nothing? If waiting for the K10 driver is the answer, can you give me any sense of when that might be available? If I can use the patch, can you guide me on how to apply it to my system?
Thanks, and best regards,
Rob Robason
Improving Software
Development
Practices and
Products
Work:
925-825-1512
Mobile:
925-348-1512
Email:
[email protected] http://www.linkedin.com/in/robrobason
Concord, CA
94518
See who we know in common
Want a signature like this?
_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Re: [lm-sensors] [PATCH 2/2] k8temp add support for family 10h and
In reply to Rudolf MarekHi Rob,
Quoted 8 lines · Rob Robason
I'm running my Fedora 11 system on an AMD Athlon 64 X2 7750 processor and Gigabyte GA-MA78GM-US2H motherboard.
Running sensors-detect(8) finds the embedded "AMD K10 thermal sensors" and reports "(driver "to be written"). In the summary it reports:Driver `to-be-written':
* Chip `AMD K10 thermal sensors' (confidence: 9)
Quoted 6 lines
Note: there is no driver for AMD K10 thermal sensors yet. Check http://www.lm-sensors.org/wiki/Devices for updates.
Checking the site, I found the subject topic and wondered if the 10h family is synonymous with K10, and if the subject patch applies to my system?
Yes, "K10" and "family 10h" are synonyms.
But anyway, the family 10h sensors are known to be unreliable so they will never be supported. I'll update sensors-detect in a minute so that it stops giving false hopes to users.
Quoted 16 lines
The sensors(1) app appears to pick up temperature readings from my system, but I'm not able to figure out the mapping of the values I'm getting (sample below):
$ sensors it8718-isa-0228 Adapter: ISA adapter in0: +1.04 V (min = +0.00 V, max = +4.08 V) in1: +1.95 V (min = +0.00 V, max = +4.08 V) in2: +3.30 V (min = +0.00 V, max = +4.08 V) in3: +2.96 V (min = +0.00 V, max = +4.08 V) in4: +3.06 V (min = +0.00 V, max = +4.08 V) in5: +4.08 V (min = +0.00 V, max = +4.08 V) ALARM in6: +4.08 V (min = +0.00 V, max = +4.08 V) ALARM in7: +2.03 V (min = +0.00 V, max = +4.08 V) Vbat: +3.30 V
Quoted 8 lines
fan1: 2537 RPM (min = 10 RPM)
fan2: 0 RPM (min = 0 RPM)
fan3: 0 RPM (min = 0 RPM)
fan4: 0 RPM (min = 0 RPM)
temp1: +44.0°C (low = +127.0°C, high = +127.0°C) sensor = thermistor
temp2: +37.0°C (low = +127.0°C, high = +60.0°C) sensor = thermal diode
temp3: +46.0°C (low = +127.0°C, high = +127.0°C) sensor = thermistor
cpu0_vid: +1.550 VYou should be able to figure out fan mappings by comparing with what the BIOS says.
For temperatures, temp2 would be the CPU temperature, while temp1 and temp3 would be mainboard temperatures. Again, comparing with what the BIOS says should help. The fact that the CPU temperature is lower than the system temperature is somewhat suspicious, although not impossible.
For voltages it's a little more difficult. Checking what the BIOS reports would definitely help. For reference, the recommended wiring for the IT8718F is:
in0 -> Vcore1
in1 -> Vcore2
in2 -> +3.3V
in3 -> +5V (scaling factor 1.68)
in4 -> +12V (scaling factor 4)
in5 -> -12V
in6 -> -5V
in7 -> +5V Stand-By (scaling factor 1.68)
But in practice most boards no longer monitor negative voltage lines any longer, and as a matter of fact your board doesn't. So you can start with:
ignore in5
ignore in6
And CPUs tend to have a single Vcore these days so in1 is more likely Vdimm. So a first approximation would be:
label in0 "Vcore"
label in1 "Vdimm"
label in2 "+3.3V"
label in3 "+5V"
label in4 "+12V"
compute in3 @ * 1.68, @ / 1.68
compute in4 @ * 4, @ / 4
Not sure about 5VSB.
If you come up with a good complete configuration, please post it so that we can add it to the wiki.
Quoted 5 lines
Can you help me understand whether I need to wait for a K10 driver, can use the subject patch, or need nothing? If waiting for the K10 driver is the answer, can you give me any sense of when that might be available? If I can use the patch, can you guide me on how to apply it to my system?
Don't wait for a patch or new driver, there won't be any. Best thing you can do is tweak your IT8718F configuration to match your mainboard.
Signature · 7 lines
Jean Delvare http://khali.linux-fr.org/wishlist.html _______________________________________________ lm-sensors mailing list [email protected] http://lists.lm-sensors.org/mailman/listinfo/lm-sensors