Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/19067 )
Change subject: cpu/intel/model_106cx: Add tsc_freq_mhz() function
......................................................................
Patch Set 1:
(2 comments)
Maybe it is better to use MCHBAR(CLKCFG) since that one actually reflects BSEL straps. Not sure if physically possible on these CPUs but for instance on LGA775 BSEL straps can modified in some cases...
https://review.coreboot.org/#/c/19067/1/src/cpu/intel/model_106cx/Makefile.…
File src/cpu/intel/model_106cx/Makefile.inc:
PS1, Line 5: romstage-y += tsc_freq.c
: smm-y += tsc_freq.c
> Does it have any effect on these stages?
Not sure. I just copied it from other Intel Makefile.inc.
https://review.coreboot.org/#/c/19067/1/src/cpu/intel/model_106cx/tsc_freq.c
File src/cpu/intel/model_106cx/tsc_freq.c:
> This looks the same as `model_1067x/fsc_freq.c`. But right now, no
you mean model_2065x ?
--
To view, visit https://review.coreboot.org/19067
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifb6074f3f097d7e6fc501a376754a05eb9e4faa0
Gerrit-PatchSet: 1
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins)
Gerrit-HasComments: Yes
Arthur Heymans has uploaded a new change for review. ( https://review.coreboot.org/19067 )
Change subject: cpu/intel/model_106cx: Add tsc_freq_mhz() function
......................................................................
cpu/intel/model_106cx: Add tsc_freq_mhz() function
This adds a function to get TSC frequency which then gets stored in
coreboot tables. This allows to read timestamps on Intel Atom 230
using cbmem.
TESTED on d945gclf
Change-Id: Ifb6074f3f097d7e6fc501a376754a05eb9e4faa0
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
---
M src/cpu/intel/model_106cx/Makefile.inc
A src/cpu/intel/model_106cx/tsc_freq.c
2 files changed, 58 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/19067/1
diff --git a/src/cpu/intel/model_106cx/Makefile.inc b/src/cpu/intel/model_106cx/Makefile.inc
index cd753db..9624c7c 100644
--- a/src/cpu/intel/model_106cx/Makefile.inc
+++ b/src/cpu/intel/model_106cx/Makefile.inc
@@ -1,5 +1,8 @@
ramstage-y += model_106cx_init.c
subdirs-y += ../../x86/name
subdirs-y += ../common
+ramstage-y += tsc_freq.c
+romstage-y += tsc_freq.c
+smm-y += tsc_freq.c
cpu_microcode_bins += 3rdparty/blobs/cpu/intel/model_106cx/microcode.bin
diff --git a/src/cpu/intel/model_106cx/tsc_freq.c b/src/cpu/intel/model_106cx/tsc_freq.c
new file mode 100644
index 0000000..9958369
--- /dev/null
+++ b/src/cpu/intel/model_106cx/tsc_freq.c
@@ -0,0 +1,55 @@
+/*
+ * This file is part of the coreboot project.
+ *
+ * Copyright (C) 2014 Google Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <stdint.h>
+#include <cpu/x86/msr.h>
+#include <cpu/x86/tsc.h>
+#include <cpu/intel/speedstep.h>
+
+unsigned long tsc_freq_mhz(void)
+{
+ msr_t msr;
+ unsigned long fsb = 0, divisor;
+
+ msr = rdmsr(MSR_FSB_FREQ);
+ switch (msr.lo & 0x07) {
+ case 5:
+ fsb = 400;
+ break;
+ case 1:
+ fsb = 533;
+ break;
+ case 3:
+ fsb = 667;
+ break;
+ case 2:
+ fsb = 800;
+ break;
+ case 0:
+ fsb = 1067;
+ break;
+ case 4:
+ fsb = 1333;
+ break;
+ case 6:
+ fsb = 1600;
+ break;
+ }
+
+ msr = rdmsr(IA32_PERF_STS);
+ divisor = (msr.hi >> 8) & 0x1f;
+
+ return (fsb * divisor) / 4;
+}
--
To view, visit https://review.coreboot.org/19067
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifb6074f3f097d7e6fc501a376754a05eb9e4faa0
Gerrit-PatchSet: 1
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/18305 )
Change subject: nb/i945/raminit: Use common ddr2 decode functions
......................................................................
Patch Set 29: Code-Review-1
Increases raminit time from 109,759µs to 249,674µs :(
--
To view, visit https://review.coreboot.org/18305
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I97c93939d11807752797785dd88c70b43a236ee3
Gerrit-PatchSet: 29
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins)
Gerrit-Reviewer: coreboot org <coreboot.org(a)gmail.com>
Gerrit-HasComments: No
Youness Alaoui has posted comments on this change. ( https://review.coreboot.org/19066 )
Change subject: util/intelmetool: Fix access to deleted data on stack
......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/#/c/19066/1//COMMIT_MSG
Commit Message:
PS1, Line 9: pci_me_interface_scan was returning (via argument 'name') a pointer
: to the interface name which was stored in a stack variable.
: This caused part of the name to be printed as garbage stack data
: in some situations if stack data was overwritten.
> There is no reason to break the line for a new sentence, if it’s in the sam
You mean between "stack variable." and "This caused" ? I broke that line because of the 80-char limit, not because it's a new sentence. I can merge 'this' into the previous line if that's what you want.
https://review.coreboot.org/#/c/19066/1/util/intelmetool/intelmetool.c
File util/intelmetool/intelmetool.c:
PS1, Line 155: char
> I think this should be `const char`. That libpci has a weird interface,
I don't think it should be a const char, since the value of name gets modified. Unless I'm misunderstanding what the const refers to in a 'const char **' declaration.
PS1, Line 238: name
> It's never checked for a NULL-pointer, or did I miss that?
It's not checked, it's used for the printf below directly, but the value will always be set to either a static string or namebuf by pci_lookup_name, and pci_lookup_name is always called if 'dev' is returned non NULL, and 'dev' is checked. Either way, isn't printf NULL-safe when printing strings ?
--
To view, visit https://review.coreboot.org/19066
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I947a4c794ee37fe87e035593eaabcaf963b9875e
Gerrit-PatchSet: 1
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Youness Alaoui <snifikino(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com>
Gerrit-Reviewer: Youness Alaoui <snifikino(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins)
Gerrit-HasComments: Yes