[lm-sensors] [PATCH 1/4] hwmon: (k8temp) adapt cpuid handling,

[lm-sensors] [PATCH 1/4] hwmon: (k8temp) adapt cpuid handling,

[Andreas Herrmann]

Signed-off-by: Andreas Herrmann <andreas.herrmann3@amd.com>
---
 drivers/hwmon/k8temp.c |   27 ++++++++++++---------------
 1 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/hwmon/k8temp.c b/drivers/hwmon/k8temp.c
index 3b90384..712c208 100644
--- a/drivers/hwmon/k8temp.c
+++ b/drivers/hwmon/k8temp.c
@@ -31,6 +31,7 @@
 #include <linux/hwmon-sysfs.h>
 #include <linux/err.h>
 #include <linux/mutex.h>
+#include <asm/processor.h>
 
 #define TEMP_FROM_REG(val)	(((((val) >> 16) & 0xff) - 49) * 1000)
 #define REG_TEMP	0xe4
@@ -48,7 +49,6 @@ struct k8temp_data {
 	/* registers values */
 	u8 sensorsp;		/* sensor presence bits - SEL_CORE & SEL_PLACE */
 	u32 temp[2][2];		/* core, place */
-	u8 fam;
 };
 
 static struct k8temp_data *k8temp_update_device(struct device *dev)
@@ -143,30 +143,27 @@ static int __devinit k8temp_probe(struct pci_dev *pdev,
 	int err;
 	u8 scfg;
 	u32 temp;
+	u8 model, stepping;
 	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, &cpuid);
-
-	data->fam = (cpuid & 0x00000f00) >> 8;
-	data->fam += (cpuid & 0x0ff00000) >> 20;
+	model = boot_cpu_data.x86_model;
+	stepping = boot_cpu_data.x86_mask;
 
-	switch (data->fam) {
+	switch (boot_cpu_data.x86) {
 	case 0xf:
+		/* feature available since SH-C0, exclude older revisions */
+		if (((model = 4) && (stepping = 0)) ||
+		    ((model = 5) && ((stepping = 0) || (stepping = 1)))) {
+			err = -ENODEV;
+			goto exit;
+		}
 		dev_warn(&pdev->dev, "Temperature readouts might be wrong"
-					" - check errata #141\n");
+			 " - check erratum #141\n");
 		break;
 	}
 
-- 
1.6.0.4




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

Re: [lm-sensors] [PATCH 1/4] hwmon: (k8temp) adapt cpuid handling,

[Rudolf Marek]

Hello,

Sorry for the delay.

> Signed-off-by: Andreas Herrmann <andreas.herrmann3@amd.com>

Maybe also some desc. Well the reason why the code below looks like is that

1) avoid very old systems - the idea that revs of CPU will be similar/same
2) get the cpuid from PCI. This is good, because some CPU may have the problem, 
some other may not.

Your patch reduces this to boot CPU checks. The idea was to have the driver know 
exact CPUID for each PCI driver instance. It was prepared to situation when some 
CPU steppings will have the fix, some not.

Please tell me why it is not necessary or why you changed that ;)

Thanks,
Rudolf




> ---
>  drivers/hwmon/k8temp.c |   27 ++++++++++++---------------
>  1 files changed, 12 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/hwmon/k8temp.c b/drivers/hwmon/k8temp.c
> index 3b90384..712c208 100644
> --- a/drivers/hwmon/k8temp.c
> +++ b/drivers/hwmon/k8temp.c
> @@ -31,6 +31,7 @@
>  #include <linux/hwmon-sysfs.h>
>  #include <linux/err.h>
>  #include <linux/mutex.h>
> +#include <asm/processor.h>
>  
>  #define TEMP_FROM_REG(val)	(((((val) >> 16) & 0xff) - 49) * 1000)
>  #define REG_TEMP	0xe4
> @@ -48,7 +49,6 @@ struct k8temp_data {
>  	/* registers values */
>  	u8 sensorsp;		/* sensor presence bits - SEL_CORE & SEL_PLACE */
>  	u32 temp[2][2];		/* core, place */
> -	u8 fam;
>  };
>  
>  static struct k8temp_data *k8temp_update_device(struct device *dev)
> @@ -143,30 +143,27 @@ static int __devinit k8temp_probe(struct pci_dev *pdev,
>  	int err;
>  	u8 scfg;
>  	u32 temp;
> +	u8 model, stepping;
>  	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, &cpuid);
> -
> -	data->fam = (cpuid & 0x00000f00) >> 8;
> -	data->fam += (cpuid & 0x0ff00000) >> 20;
> +	model = boot_cpu_data.x86_model;
> +	stepping = boot_cpu_data.x86_mask;
>  
> -	switch (data->fam) {
> +	switch (boot_cpu_data.x86) {
>  	case 0xf:
> +		/* feature available since SH-C0, exclude older revisions */
> +		if (((model = 4) && (stepping = 0)) ||
> +		    ((model = 5) && ((stepping = 0) || (stepping = 1)))) {
> +			err = -ENODEV;
> +			goto exit;
> +		}
>  		dev_warn(&pdev->dev, "Temperature readouts might be wrong"
> -					" - check errata #141\n");
> +			 " - check erratum #141\n");
>  		break;
>  	}
>  
> -- 1.6.0.4 


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

Re: [lm-sensors] [PATCH 1/4] hwmon: (k8temp) adapt cpuid handling,

[Andreas Herrmann]

On Tue, Dec 09, 2008 at 11:12:39PM +0100, Rudolf Marek wrote:
> Hello,
>
> Sorry for the delay.
>
>> Signed-off-by: Andreas Herrmann <andreas.herrmann3@amd.com>
>
> Maybe also some desc. Well the reason why the code below looks like is that
>
> 1) avoid very old systems - the idea that revs of CPU will be similar/same
> 2) get the cpuid from PCI. This is good, because some CPU may have the 
> problem, some other may not.

Jean gave examples of a systems where the PCI will not work. Thus
it isn't a reliable way to get CPUID. By problem you mean certain
erratum.  But this can only happen on systems with an unspported mixed
silicon setup.  E.g. combining K8 RevE with K8 RevF which is
(according to AMD's documentation) not a supported setup.

> Your patch reduces this to boot CPU checks. The idea was to have the driver 
> know exact CPUID for each PCI driver instance. It was prepared to situation 
> when some CPU steppings will have the fix, some not.
>
> Please tell me why it is not necessary or why you changed that ;)

For mixed silicon systems certain restrictions apply.
The allowed mixed silicon revisions for K8 and family 0x10 show
that it completely suffices for k8temp to check the revision of
the boot CPU.

E.g. if boot CPU is >= K8 RevC all other processors should be >=RevC <=RevE
If boot CPU is RevF all other processors should be RevF.
(Similar for family 0x10 RevB or RevC.)

And last not least K8 RevG is desktop/mobile only -- so no mixed setup
here at all.

Thus all relevant CPUID checks in k8temp can reliably be performed
with boot CPU information and no additional CPUID evaluation code is
needed in k8temp.


Regards,
Andreas



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

Re: [lm-sensors] [PATCH 1/4] hwmon: (k8temp) adapt cpuid handling,

[Andreas Herrmann]

On Wed, Dec 10, 2008 at 11:03:33AM +0100, Andreas Herrmann wrote:
> On Tue, Dec 09, 2008 at 11:12:39PM +0100, Rudolf Marek wrote:
> > Please tell me why it is not necessary or why you changed that ;)

To be more specific.

There are following CPUID checks necessary in k8temp:

>= K8 RevC

to check for support of temperature reporting at all.

>= RevF

to check for erratum #141

>= RevG (desktop)

to check for temperature skew

(In the future we might have additional checks for
family 0x10 >= RevC to check for erratum 319.)

This devides K8 CPUs into 4 classes (and family 10h CPUs into
2 classes).

Now it is important to notice that (by chance) you can't combine
processors of different classes in a mixed silicon setup.
See the relevant CPU revision guides for those CPUs.
E.g., combining RevC with RevF is not supported.

Thus it suffices to check CPUID of boot CPU.
We don't need to care about unsupported processor combinations.


Regards,
Andreas



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

Re: [lm-sensors] [PATCH 1/4] hwmon: (k8temp) adapt cpuid handling,

[Rudolf Marek]

Hi,

Sorry for the delay, I was at the mountains.

Thank you for detailed clarification.

Rudolf

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

Re: [lm-sensors] [PATCH 1/4] hwmon: (k8temp) adapt cpuid handling,

[Rudolf Marek]

Maybe please put there some short comment that:

/* enough is to check for boot CPU, mixed CPU revs are not supported */

Acked-by: Rudolf Marek <r.marek@assembler.cz>

Thanks,
Rudolf

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