http://defect.opensolaris.org/bz/show_bug.cgi?id=6534
Summary: Implement code review feedback for Seth Goldberg
Classification: Development
Product: power-mgmt
Version: unspecified
Platform: Other
OS/Version: Solaris
Status: NEW
Severity: minor
Priority: P3
Component: c-state
AssignedTo: bill.holler at sun.com
ReportedBy: bill.holler at sun.com
CC: tesla-dev at opensolaris.org
hpet_acpi.h:
Still misspelling femto as "FEMPTO" in a number of places.
Line 224 "LACPI" should be "LAPIC"
Line 252: Is 10 "enough" ?
Line 253: Why "3"?
Non-capitalizations of "hpet"
hpet.h:
Line 68: What is "unix.so" ?
cpupm_mach.h:
Change multiple occurrences of "it's" to "its" (i.e. "some of it's
processors")
mp_machdep.c:
Remove "XXX"'ed statement from hpet definition
What's up with the XXX DTRACE_PROBE1 didn't work stuff?
/power saving/power-saving/
wtf is XXX_highbit() ? :)
cpupm_throttle.c:
/power manage/power-manage/
cpupm_mach.c:
s/%90/90%/
What about the XXX @ mcpu c-state?
XXX @ PAD-specific interfaces?
s/book keeping/bookkeeping/
s/Often wakeup/Wakeup often/ ?
cpu_idle.c:
s/See if there's any other/See if there are any other/
(plus other instances of "there's" -> "there are")
s/trys/tries/
asymmetric usage of "lAPIC" and "LAPIC" -- please pick one :)
Lines 350,392: If it's dead, remove it.
Line 498: s/Today all Intel's processor/Today, all Intel processors/
...and... s/C3 share cache/C3 shared cache/
Line 803: was a final decision made here?
Line 836 s/for it/for them/
cpu_acpi.c:
Garbled statement:
+ * if there have duplicate entries, we keep the
"If there are duplicate entries" maybe ?
+ * last one. This fixes:
+ * 1) some buggy BIOS has total duplicate entries.
"Some buggy BIOSes have"...
+ * we got a valid entry, count it up
Please rephrase "count it up"
apic_regops.c:
This should no longer be necessary. (the line of code above the line you
added is the fix!)
apic.c:
s/Set timer far into future/Set timer far into the future/
uncapitalize "Current Count" & no space between nano and seconds.
hpet_acpi.c:
Again, lAPIC & LAPIC -- pick one :) .
You can use "IOAPIC" instead of "I/O APIC" :) .
s/possibly shared/possibly-shared/
Isn't the plural of proxy "proxies" ?
s/possibly stuck/possibly-stuck/
hpet_checksum_table : do we REALLY not have another 8-bit checksum routine
elsewhere in the kernel that you can use instead of creating another one? :)
"Guaranty" is misspelled all over the place. It's "Guarantee".
s/written to/written-to/
s/states main counter/states that the main counter/
This comment needs a re-write:
633 * The I/O APIC line (vector) is programmed in ioapic_init_intr() called
634 * from apic_picinit() psm_ops apic_ops entry point after we return from
635 * apic_init() psm_ops entry point.
s/irq/IRQ/ -- capitalization varies -- please be consistent.
934 * The order of these opperations is important to avoid
it's "operations"
Is it really OK to try to grab a lock with interrupts disabled?
941 cli();
942 while (!mutex_tryenter(&hpet_proxy_lock)) {
hpet_isr() clears interrupts directly (instead of saving the flags and
restoring them on exit) -- I hope this doesn't cause problems in the future.
s/level triggered/level-triggered/
1053 * Returns B_FLASE if the HPET could not be programmed to interrupt
before
"B_FALSE"
Please restate this:
1092 * There are no CPUs to program the HPET for.
DJ Jazzy HPET and the Fresh Prince?:
1094 * Letting the HPET timer rap around to the
current
1177 * A critical section exists between when the HPET is programed
"programmed"
1280 * Do not enable a LAPIC Timer than was initially disabled.
"that was initially disabled"
1311 #include <sys/clock.h>
EWW. Please put this at the top of the file with the other #includes.
--
Configure bugmail: http://defect.opensolaris.org/bz/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.
