Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson, Julian Schroeder. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56188 )
Change subject: soc/amd/cezanne: ACPI CPPC support for AMD ......................................................................
Patch Set 2:
(6 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/56188/comment/1f49da50_f3e96f65 PS2, Line 7: i'd add an "add" here
https://review.coreboot.org/c/coreboot/+/56188/comment/303f7e49_bb31d602 PS2, Line 9: CPPC might be good to have the acronym expanded in the commit message text. at least i had to look up what this is about
Patchset:
PS2: haven't checked the msr numbers
File src/soc/amd/cezanne/cppc.h:
PS2: this one should probably be moved to src/soc/amd/cezanne/include/soc/cppc.h
File src/soc/amd/cezanne/cppc_init.c:
PS2: i'd move the contents of this file to cppc.c. since both get only built into ramstage, i don't really see the necessity to split it into 2 files
https://review.coreboot.org/c/coreboot/+/56188/comment/1b7ec433_35e36da4 PS2, Line 33: msr.addrl = 0xc00102b0; would be good to have defines for those instead of the magic msr numbers here
-
Felix Held (Code Review)