Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34145 )
Change subject: src/cpu/intel: Add sanity check for cpu turbo mode capability ......................................................................
Patch Set 5:
(3 comments)
https://review.coreboot.org/c/coreboot/+/34145/5/src/cpu/intel/turbo/turbo.c File src/cpu/intel/turbo/turbo.c:
https://review.coreboot.org/c/coreboot/+/34145/5/src/cpu/intel/turbo/turbo.c... PS5, Line 107: cpuid_regs = cpuid(CPUID_LEAF_PM); : turbo_cap = !!(cpuid_regs.eax & PM_CAP_TURBO_MODE); Why is this function checking cap again since get_turbo_state() already does that?
https://review.coreboot.org/c/coreboot/+/34145/5/src/cpu/intel/turbo/turbo.c... PS5, Line 112: turbo_state = TURBO_UNAVAILABLE; get_turbo_state() in line 101 ensures that turbo_state is not TURBO_UNAVAILABLE. Isn't this check redundant?
https://review.coreboot.org/c/coreboot/+/34145/5/src/cpu/intel/turbo/turbo.c... PS5, Line 120: turbo_state = get_global_turbo_state(); Else case doesn't make much sense. It gets global state to turbo_state and writes back turbo_state to global state in the next line?