Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38824 )
Change subject: soc/intel/apollolake: Display apollolake platform information ......................................................................
Patch Set 4:
(7 comments)
For the future the name mapping should be done differently. Some text file, or use of some database, so it does not need to be in code, and even duplicated.
https://review.coreboot.org/c/coreboot/+/38824/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38824/4//COMMIT_MSG@7 PS4, Line 7: apollolake Not needed, as it’s in the prefix.
https://review.coreboot.org/c/coreboot/+/38824/4//COMMIT_MSG@9 PS4, Line 9: apollolake Apollo Lake
https://review.coreboot.org/c/coreboot/+/38824/4//COMMIT_MSG@19 PS4, Line 19: Geminilake It’s the successor of Apollo Lake.
https://review.coreboot.org/c/coreboot/+/38824/4/src/soc/intel/apollolake/re... File src/soc/intel/apollolake/report_platform.c:
https://review.coreboot.org/c/coreboot/+/38824/4/src/soc/intel/apollolake/re... PS4, Line 79: u32 i The counting variable should be just `unsigned int i`.
https://review.coreboot.org/c/coreboot/+/38824/4/src/soc/intel/apollolake/re... PS4, Line 81: int vt, txt, aes; Why not boolean type? Also, rename to `vt_supported`, and so on.
https://review.coreboot.org/c/coreboot/+/38824/4/src/soc/intel/apollolake/re... PS4, Line 102: BIOS_DEBUG I’d make that BIOS_INFO.
`src/commonlib/include/commonlib/loglevel.h` has definitions, but it does not really apply to informational messages.
https://review.coreboot.org/c/coreboot/+/38824/4/src/soc/intel/apollolake/re... PS4, Line 117: int unsigned int