From patchwork Thu Nov 21 14:36:05 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Colin Ian King X-Patchwork-Id: 293159 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) by ozlabs.org (Postfix) with ESMTP id AEDB82C00BA for ; Fri, 22 Nov 2013 01:36:16 +1100 (EST) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.76) (envelope-from ) id 1VjVMa-0000PP-2f; Thu, 21 Nov 2013 14:36:12 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by huckleberry.canonical.com with esmtp (Exim 4.76) (envelope-from ) id 1VjVMU-0000P9-AL for fwts-devel@lists.ubuntu.com; Thu, 21 Nov 2013 14:36:06 +0000 Received: from cpc3-craw6-2-0-cust180.croy.cable.virginm.net ([77.100.248.181] helo=localhost) by youngberry.canonical.com with esmtpsa (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1VjVMU-0003ZN-6F for fwts-devel@lists.ubuntu.com; Thu, 21 Nov 2013 14:36:06 +0000 From: Colin King To: fwts-devel@lists.ubuntu.com Subject: [PATCH] cpu: maxfreq: report maxfreq and tidy up the test (LP: #1253658) Date: Thu, 21 Nov 2013 14:36:05 +0000 Message-Id: <1385044565-17603-1-git-send-email-colin.king@canonical.com> X-Mailer: git-send-email 1.8.3.2 X-BeenThere: fwts-devel@lists.ubuntu.com X-Mailman-Version: 2.1.14 Precedence: list List-Id: Firmware Test Suite Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: fwts-devel-bounces@lists.ubuntu.com Sender: fwts-devel-bounces@lists.ubuntu.com From: Colin Ian King Add some feedback on the maxfreq as it is useful to know. Also, tidy up the test in a few ways: Use bools rather than ints for true/false settings. Reduce the nesting as this makes the code harder to read. Fix the source into 80 columns. Signed-off-by: Colin Ian King Acked-by: Ivan Hu Acked-by: Alex Hung --- src/cpu/maxfreq/maxfreq.c | 128 ++++++++++++++++++++++++++-------------------- 1 file changed, 73 insertions(+), 55 deletions(-) diff --git a/src/cpu/maxfreq/maxfreq.c b/src/cpu/maxfreq/maxfreq.c index 51ed1e8..be9ba43 100644 --- a/src/cpu/maxfreq/maxfreq.c +++ b/src/cpu/maxfreq/maxfreq.c @@ -20,6 +20,8 @@ #ifdef FWTS_ARCH_INTEL +#include +#include #include #include #include @@ -50,16 +52,13 @@ static double maxfreq_max(const char *str) static int maxfreq_test1(fwts_framework *fw) { - DIR *dir; + double *cpufreq; + int cpu, cpus; + bool advice = false, passed = true, cpufreqs_read = false; struct dirent *entry; - int cpus; - int cpufreqs_read = 0; - int cpu; + DIR *dir; fwts_list_link *item; fwts_list *cpuinfo; - double *cpufreq; - int failed = 0; - int advice = 0; fwts_log_info(fw, "This test checks the maximum CPU frequency as detected by " @@ -96,7 +95,7 @@ static int maxfreq_test1(fwts_framework *fw) cpufreq[cpu] *= 1000000.0; if (strstr(str, "MHz")) cpufreq[cpu] *= 1000.0; - cpufreqs_read++; + cpufreqs_read = true; } else cpufreq[cpu] = -1.0; @@ -106,8 +105,10 @@ static int maxfreq_test1(fwts_framework *fw) } fwts_list_free(cpuinfo, free); - if (cpufreqs_read == 0) { - fwts_skipped(fw, "Cannot read CPU frequencies from %s, this generally happens on AMD CPUs, skipping test.", CPU_INFO_PATH); + if (!cpufreqs_read) { + fwts_skipped(fw, + "Cannot read CPU frequencies from %s, this generally " + "happens on AMD CPUs, skipping test.", CPU_INFO_PATH); free(cpufreq); return FWTS_SKIP; } @@ -122,59 +123,76 @@ static int maxfreq_test1(fwts_framework *fw) } do { + char path[PATH_MAX]; + char *data; + double maxfreq, maxfreq_ghz, cpufreq_ghz; + int cpunum; + entry = readdir(dir); - if (entry && strlen(entry->d_name)>2) { - char path[PATH_MAX]; - char *data; - - snprintf(path, sizeof(path), - CPU_FREQ_PATH "/%s/cpufreq/scaling_available_frequencies", - entry->d_name); - if ((data = fwts_get(path)) != NULL) { - double maxfreq = maxfreq_max(data); - int cpunum = atoi(entry->d_name + 3); - if (maxfreq < 0.0) { - fwts_failed(fw, LOG_LEVEL_MEDIUM, - "CPUFreqReadFailed", - "Cannot read cpu frequency from %s for CPU %s\n", - CPU_FREQ_PATH, entry->d_name); - failed++; - } else { - double maxfreq_ghz = maxfreq / 1000000.0; - double cpufreq_ghz = cpufreq[cpunum] / 1000000.0; - - if (fabs(maxfreq_ghz - cpufreq_ghz) > (maxfreq_ghz * 0.005)) { - failed++; - fwts_failed(fw, - LOG_LEVEL_MEDIUM, - "CPUFreqSpeedMismatch", - "Maximum scaling frequency %f GHz do not match expected frequency %f GHz\n", - maxfreq_ghz, cpufreq_ghz); - if (!advice) { - advice++; - fwts_advice(fw, - "The maximum scaling frequency %f GHz for CPU %d configured by the BIOS in %s " - "does not match the expected maximum CPU frequency %f GHz " - "that the CPU can run at. This usually indicates a misconfiguration of " - "the ACPI _PSS (Performance Supported States) object. This is described in " - "section 8.4.4.2 of the APCI specification.", - (double)maxfreq/1000000.0, cpunum, path, - (double)cpufreq[cpunum]/1000000.0); - } - else - fwts_advice(fw, "See advice for previous CPU."); - } - } + if (!entry || entry->d_name[0] == '.' || + strlen(entry->d_name) < 3) + continue; + + snprintf(path, sizeof(path), + CPU_FREQ_PATH "/%s/cpufreq/scaling_available_frequencies", + entry->d_name); + + if ((data = fwts_get(path)) == NULL) + continue; + maxfreq = maxfreq_max(data); + free(data); + + cpunum = atoi(entry->d_name + 3); + if (maxfreq < 0.0) { + fwts_failed(fw, LOG_LEVEL_MEDIUM, + "CPUFreqReadFailed", + "Cannot read cpu frequency from %s for CPU %s.", + CPU_FREQ_PATH, entry->d_name); + passed = false; + continue; + } + + maxfreq_ghz = maxfreq / 1000000.0; + cpufreq_ghz = cpufreq[cpunum] / 1000000.0; + + if (fabs(maxfreq_ghz - cpufreq_ghz) > (maxfreq_ghz * 0.005)) { + passed = false; + fwts_failed(fw, + LOG_LEVEL_MEDIUM, + "CPUFreqSpeedMismatch", + "Maximum scaling frequency %f GHz do not match " + "expected frequency %f GHz.", + maxfreq_ghz, cpufreq_ghz); + if (!advice) { + advice = true; + fwts_advice(fw, + "The maximum scaling frequency %f GHz " + "for CPU %d configured by the BIOS in " + "%s does not match the expected " + "maximum CPU frequency %f GHz that " + "the CPU can run at. This usually " + "indicates a misconfiguration of the " + "ACPI _PSS (Performance Supported " + "States) object. This is described in " + "section 8.4.4.2 of the APCI " + "specification.", + (double)maxfreq/1000000.0, cpunum, path, + (double)cpufreq[cpunum]/1000000.0); + } else { + fwts_advice(fw, "See advice for previous CPU."); } - free(data); + } else { + fwts_log_info(fw, + "CPU %d maximum frequency %f GHz is sane.", + cpunum, maxfreq_ghz); } } while (entry); - if (!failed) + if (passed) fwts_passed(fw, "%d CPUs passed the maximum frequency check.", cpus); free(cpufreq); - closedir(dir); + (void)closedir(dir); return FWTS_OK; }