Julien Viard de Galbert has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/25439 )
Change subject: soc/intel/denverton_ns: Initialize thermal configuration ......................................................................
Patch Set 21:
(4 comments)
As suggested by Jay Talbott, denverton has the feature always available, no need to check the cpuid(1) result, this simplifies the code ;)
https://review.coreboot.org/c/coreboot/+/25439/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/25439/4//COMMIT_MSG@8 PS4, Line 8:
Add few lines info/detail on, why this code change required and what kind of problem we are solving […]
When I started this, the denverton port was just the code drop by Intel to test FSP, it's missing a lot of initialization. So it's not fixing a problem it's just enabling features of the SoC.
https://review.coreboot.org/c/coreboot/+/25439/3/src/soc/intel/denverton_ns/... File src/soc/intel/denverton_ns/cpu.c:
https://review.coreboot.org/c/coreboot/+/25439/3/src/soc/intel/denverton_ns/... PS3, Line 81: msr.lo = 0; : msr.hi = 0;
Already set above, right?
yes thanks
https://review.coreboot.org/c/coreboot/+/25439/4/src/soc/intel/denverton_ns/... File src/soc/intel/denverton_ns/cpu.c:
https://review.coreboot.org/c/coreboot/+/25439/4/src/soc/intel/denverton_ns/... PS4, Line 84: (1 << 29)
Is there a macro for this bit? […]
Right the feature bit should always be set ... I don't remember where I found the general algorithm either other soc code or generic intel doc ...
I simplified the code and added macros.
https://review.coreboot.org/c/coreboot/+/25439/13/src/soc/intel/denverton_ns... File src/soc/intel/denverton_ns/cpu.c:
https://review.coreboot.org/c/coreboot/+/25439/13/src/soc/intel/denverton_ns... PS13, Line 82: msr.lo &= (1 << 3); /* Clear TM enable */
Ping
Ack
-
Julien Viard de Galbert (Code Review)