diff mbox

[1/1] cpu: msr: disabled test for non-Intel/AMD architectures

Message ID 1323109229-15402-2-git-send-email-colin.king@canonical.com
State Accepted
Headers show

Commit Message

Colin Ian King Dec. 5, 2011, 6:20 p.m. UTC 
From: Colin Ian King <colin.king@canonical.com>

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 src/cpu/msr/msr.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

Comments

Keng-Yu Lin Dec. 7, 2011, 9:30 a.m. UTC | #1 
Not sure why the mail is rejected. see if forward can work.


---------- Forwarded message ----------
From: Keng-Yu Lin <kengyu@canonical.com>
Date: 2011/12/7
Subject: Re: [PATCH 1/1] cpu: msr: disabled test for non-Intel/AMD architectures
To: fwts-devel@lists.ubuntu.com


Ack from me. and also tested building without an error.
Since the patch is from Colin, it may make sense to count as two Acks.
So the patch is applied in git://kernel.ubuntu.com/lexical/fwts.git

於 2011年12月06日 02:20, Colin King 提到:
> From: Colin Ian King<colin.king at canonical.com>
>
> Signed-off-by: Colin Ian King<colin.king at canonical.com>
> ---
>   src/cpu/msr/msr.c |    8 ++++++++
>   1 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/src/cpu/msr/msr.c b/src/cpu/msr/msr.c
> index d2c16a8..06a756f 100644
> --- a/src/cpu/msr/msr.c
> +++ b/src/cpu/msr/msr.c
> @@ -19,6 +19,8 @@
>
>   #include "fwts.h"
>
> +#ifdef FWTS_ARCH_INTEL
> +
>   typedef void (*msr_callback_check)(fwts_framework *fw, uint64_t val);
>
>   static int ncpus;
> @@ -32,6 +34,10 @@ static int msr_init(fwts_framework *fw)
>               fwts_log_error(fw, "Cannot get CPU info");
>               return FWTS_ERROR;
>       }
> +     if (cpuinfo->vendor_id == NULL) {
> +             fwts_log_error(fw, "Cannot get CPU vendor_id");
> +             return FWTS_ERROR;
> +     }
>       intel_cpu = strstr(cpuinfo->vendor_id, "Intel") != NULL;
>       amd_cpu = strstr(cpuinfo->vendor_id, "AuthenticAMD") != NULL;
>
> @@ -555,3 +561,5 @@ static fwts_framework_ops msr_ops = {
>   };
>
>   FWTS_REGISTER(msr,&msr_ops, FWTS_TEST_ANYTIME, FWTS_BATCH | FWTS_ROOT_PRIV);
> +
> +#endif
Colin Ian King Dec. 7, 2011, 9:34 a.m. UTC | #2 
On 07/12/11 09:25, Keng-Yu Lin wrote:
> Ack from me. and also tested building without an error.
> Since the patch is from Colin, it may make sense to count as two Acks.
Nah, I need 2 Acks like anyone else. :-)
> So the patch is applied in git://kernel.ubuntu.com/lexical/fwts.git
>
> 於 2011年12月06日 02:20, Colin King 提到:
>> From: Colin Ian King<colin.king at canonical.com>
>>
>> Signed-off-by: Colin Ian King<colin.king at canonical.com>
>> ---
>>   src/cpu/msr/msr.c |    8 ++++++++
>>   1 files changed, 8 insertions(+), 0 deletions(-)
>>
>> diff --git a/src/cpu/msr/msr.c b/src/cpu/msr/msr.c
>> index d2c16a8..06a756f 100644
>> --- a/src/cpu/msr/msr.c
>> +++ b/src/cpu/msr/msr.c
>> @@ -19,6 +19,8 @@
>>
>>   #include "fwts.h"
>>
>> +#ifdef FWTS_ARCH_INTEL
>> +
>>   typedef void (*msr_callback_check)(fwts_framework *fw, uint64_t val);
>>
>>   static int ncpus;
>> @@ -32,6 +34,10 @@ static int msr_init(fwts_framework *fw)
>>   		fwts_log_error(fw, "Cannot get CPU info");
>>   		return FWTS_ERROR;
>>   	}
>> +	if (cpuinfo->vendor_id == NULL) {
>> +		fwts_log_error(fw, "Cannot get CPU vendor_id");
>> +		return FWTS_ERROR;
>> +	}
>>   	intel_cpu = strstr(cpuinfo->vendor_id, "Intel") != NULL;
>>   	amd_cpu = strstr(cpuinfo->vendor_id, "AuthenticAMD") != NULL;
>>
>> @@ -555,3 +561,5 @@ static fwts_framework_ops msr_ops = {
>>   };
>>
>>   FWTS_REGISTER(msr,&msr_ops, FWTS_TEST_ANYTIME, FWTS_BATCH | FWTS_ROOT_PRIV);
>> +
>> +#endif
Keng-Yu Lin Dec. 7, 2011, 9:37 a.m. UTC | #3 
2011/12/7 Colin Ian King <colin.king@canonical.com>:
> On 07/12/11 09:25, Keng-Yu Lin wrote:
>> Ack from me. and also tested building without an error.
>> Since the patch is from Colin, it may make sense to count as two Acks.
> Nah, I need 2 Acks like anyone else. :-)

As your wish :-)  the patch is temporarily reverted in my tree:
git://kernel.ubuntu.com/lexical/fwts.git
Colin Ian King Dec. 7, 2011, 9:42 a.m. UTC | #4 
On 07/12/11 09:37, Keng-Yü Lin wrote:
> 2011/12/7 Colin Ian King<colin.king@canonical.com>:
>> On 07/12/11 09:25, Keng-Yu Lin wrote:
>>> Ack from me. and also tested building without an error.
>>> Since the patch is from Colin, it may make sense to count as two Acks.
>> Nah, I need 2 Acks like anyone else. :-)
> As your wish :-)  the patch is temporarily reverted in my tree:
> git://kernel.ubuntu.com/lexical/fwts.git
I think we have quite a few patches in the queue, I've Ack'd some, who 
else can Ack the patches?

Colin
Keng-Yu Lin Dec. 7, 2011, 9:58 a.m. UTC | #5 
> I think we have quite a few patches in the queue, I've Ack'd some, who else
> can Ack the patches?

I will be looking at these patches.
Jeremy Kerr Dec. 9, 2011, 4:22 a.m. UTC | #6 
Hi Colin,

> From: Colin Ian King <colin.king@canonical.com>
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

For single-patch series, it's probably better just to include the
changelog in this patch, rather than as the series header. Makes it
easier for the committer to just git-am the patch.

> --- a/src/cpu/msr/msr.c
> +++ b/src/cpu/msr/msr.c
> @@ -19,6 +19,8 @@
>
>  #include "fwts.h"
>
> +#ifdef FWTS_ARCH_INTEL
> +
>  typedef void (*msr_callback_check)(fwts_framework *fw, uint64_t val);
>
>  static int ncpus;

This is going to result in an #ifdef mess; maybe we need
msr-${FWTS_ARCH}.c instead?

Cheers,


Jeremy
Colin Ian King Dec. 9, 2011, 8:20 a.m. UTC | #7 
On 09/12/11 04:22, Jeremy Kerr wrote:
> Hi Colin,
>
>> From: Colin Ian King<colin.king@canonical.com>
>>
>> Signed-off-by: Colin Ian King<colin.king@canonical.com>
> For single-patch series, it's probably better just to include the
> changelog in this patch, rather than as the series header. Makes it
> easier for the committer to just git-am the patch.
OK, I was just following the kernel-team protocol, and they seemed happy 
with that methodology. Will do next time.
>
>> --- a/src/cpu/msr/msr.c
>> +++ b/src/cpu/msr/msr.c
>> @@ -19,6 +19,8 @@
>>
>>   #include "fwts.h"
>>
>> +#ifdef FWTS_ARCH_INTEL
>> +
>>   typedef void (*msr_callback_check)(fwts_framework *fw, uint64_t val);
>>
>>   static int ncpus;
> This is going to result in an #ifdef mess; maybe we need
> msr-${FWTS_ARCH}.c instead?
Ultimately, I agree, but this should be looked at on a different spin.  
I really can't care about how this is done, non-Intel architectures are 
something that I wasn't considering when I first wrote fwts and most of 
the tests are x86 specific, so just getting the critter to build on 
non-ARM was a hack, which I agree needs fixing it a more elegant way, 
but not by me, I can't devote the cycles at the moment to this.
> Cheers,
>
>
> Jeremy
Alex Hung Jan. 3, 2012, 8:27 a.m. UTC | #8 
Hi,

The patch looks good. Ack from me.

On 12/06/2011 02:20 AM, Colin King wrote:
> From: Colin Ian King<colin.king@canonical.com>
>
> Signed-off-by: Colin Ian King<colin.king@canonical.com>
> ---
>   src/cpu/msr/msr.c |    8 ++++++++
>   1 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/src/cpu/msr/msr.c b/src/cpu/msr/msr.c
> index d2c16a8..06a756f 100644
> --- a/src/cpu/msr/msr.c
> +++ b/src/cpu/msr/msr.c
> @@ -19,6 +19,8 @@
>
>   #include "fwts.h"
>
> +#ifdef FWTS_ARCH_INTEL
> +
>   typedef void (*msr_callback_check)(fwts_framework *fw, uint64_t val);
>
>   static int ncpus;
> @@ -32,6 +34,10 @@ static int msr_init(fwts_framework *fw)
>   		fwts_log_error(fw, "Cannot get CPU info");
>   		return FWTS_ERROR;
>   	}
> +	if (cpuinfo->vendor_id == NULL) {
> +		fwts_log_error(fw, "Cannot get CPU vendor_id");
> +		return FWTS_ERROR;
> +	}
>   	intel_cpu = strstr(cpuinfo->vendor_id, "Intel") != NULL;
>   	amd_cpu = strstr(cpuinfo->vendor_id, "AuthenticAMD") != NULL;
>
> @@ -555,3 +561,5 @@ static fwts_framework_ops msr_ops = {
>   };
>
>   FWTS_REGISTER(msr,&msr_ops, FWTS_TEST_ANYTIME, FWTS_BATCH | FWTS_ROOT_PRIV);
> +
> +#endif
diff mbox

Patch

diff --git a/src/cpu/msr/msr.c b/src/cpu/msr/msr.c
index d2c16a8..06a756f 100644
--- a/src/cpu/msr/msr.c
+++ b/src/cpu/msr/msr.c
@@ -19,6 +19,8 @@ 
 
 #include "fwts.h"
 
+#ifdef FWTS_ARCH_INTEL
+
 typedef void (*msr_callback_check)(fwts_framework *fw, uint64_t val);
 
 static int ncpus;
@@ -32,6 +34,10 @@  static int msr_init(fwts_framework *fw)
 		fwts_log_error(fw, "Cannot get CPU info");
 		return FWTS_ERROR;
 	}
+	if (cpuinfo->vendor_id == NULL) {
+		fwts_log_error(fw, "Cannot get CPU vendor_id");
+		return FWTS_ERROR;
+	}
 	intel_cpu = strstr(cpuinfo->vendor_id, "Intel") != NULL;
 	amd_cpu = strstr(cpuinfo->vendor_id, "AuthenticAMD") != NULL;
 
@@ -555,3 +561,5 @@  static fwts_framework_ops msr_ops = {
 };
 
 FWTS_REGISTER(msr, &msr_ops, FWTS_TEST_ANYTIME, FWTS_BATCH | FWTS_ROOT_PRIV);
+
+#endif