Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33624 )
Change subject: soc/amd/stoneyridge: Change code to accommodate merlinfalcon SOC ......................................................................
Patch Set 9:
(3 comments)
https://review.coreboot.org/c/coreboot/+/33624/9//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/33624/9//COMMIT_MSG@9 PS9, Line 9: carrizo CPU in a SOC presentation I would prefer you don't mention this. AMD Embedded has been very careful to use their own branding. Carrizo is also in an SOC. You don't even need to say "as stoneyridge...". You could mention that Merlin Falcon is Family 15h Models 60h-6Fh. Stoney Ridge is Family 15h models 70h-7Fh, so you're making this code backward compatible to support Merlin Falcon.
https://review.coreboot.org/c/coreboot/+/33624/9//COMMIT_MSG@12 PS9, Line 12: merlinfalcon You also might want to develop the habit of capitalizing the first letters of codenames, e.g. "Merlin Falcon". There is no merlinfalcon SOC. And all lower case for directory names, e.g. stoneyridge.
https://review.coreboot.org/c/coreboot/+/33624/9/src/soc/amd/stoneyridge/chi... File src/soc/amd/stoneyridge/chip.h:
https://review.coreboot.org/c/coreboot/+/33624/9/src/soc/amd/stoneyridge/chi... PS9, Line 29: MAX_DIMMS_PER_CH Are you certain of this? Can you direct me to the document? I only see 2 DIMMs in the BKDG. If you picked it up from the 00660F01 code, it could be wrong.