[coreboot-gerrit] Change in coreboot[master]: lapic/apic_timer.c: Provide a tsc_freq_mhz for platforms usi...

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/19504 )

Change subject: lapic/apic_timer.c: Provide a tsc_freq_mhz for platforms using LAPIC_UDELAY
......................................................................


Patch Set 3:

(3 comments)

https://review.coreboot.org/#/c/19504/3/src/cpu/x86/lapic/apic_timer.c
File src/cpu/x86/lapic/apic_timer.c:

Line 85: unsigned long tsc_freq_mhz(void)
> I would prefer to export get_fsb() and place this somewhere about
Seems like a good idea. A quick look told me most/all? core i CPU since sandy bridge have 100MHz base clk so I guess things could be more unified.


Line 90: 	msr = rdmsr(IA32_PERF_STS);
> Is this available on P4???
Made my computer choke for a few minutes but I did found it "MSRs in the Pentium® 4 and Intel® Xeon® Processors" table in Intel 64 and IA32 Architectures Software Develper's manual.


Line 101: 	} else {
> Can drop the `else`. I guess checkpatch would complain?
I think it would...


-- 
To view, visit https://review.coreboot.org/19504
To unsubscribe, visit https://review.coreboot.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic643681c3b9646bf7efbcd786c35a9beda9afc49
Gerrit-PatchSet: 3
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Arthur Heymans <arthur at aheymans.xyz>
Gerrit-Reviewer: Arthur Heymans <arthur at aheymans.xyz>
Gerrit-Reviewer: Nico Huber <nico.h at gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter at users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply at coreboot.org>
Gerrit-HasComments: Yes