Attention is currently required from: Angel Pons.
Keith Hui has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/73690 )
Change subject: nb/intel/snb: Normalize spd_addresses in devicetree ......................................................................
Patch Set 5:
(3 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/73690/comment/366e94a2_65efea37 PS3, Line 15: to native raminit as well.
Note that things get weird when using SPD files, as they're handled in C code. […]
Without knowing what Haswell does, I was at first thinking about using 0x01-0x0f for index into spd.bin in cbfs, and 0x50-0x5f for SPD addresses for the slots. (Doing this in devicetree doesn't allow for runtime dependencies. Read on.)
I couldn't read too deep into Haswell, but it looks like it uses a SPD address of 0xff to represent SPD file. However, I also looked at two sandybridge boards with onboard memory (samsung/lumpy and lenovo/s230u), and both check GPIOs at runtime to select the right SPD data. This requires runtime hooks anyway, and would take this patch beyond my original scope. Not to mention I would be deprecating one hook, only to have to create a new one for these sort-of-common use cases.
In short, I prefer to limit this patch to boards with only socketed DIMMs. Leave boards using SPD files to use mainboard_get_spd() and mainboard_fill_pei_data(). I should document somewhere that sandybridge boards have to either implement mainboard_get_spd() (and mfpd() if it doesn't force native raminit) or enter SPD addresses in devicetree. Then again, when was the last time we got a new sandybridge board? (p8z77-m you say? That sounds familiar :P )
https://review.coreboot.org/c/coreboot/+/73690/comment/f1154b77_99137900 PS3, Line 17: Declaring SPD addresses in devicetree was added with commit : 5709e03 (nb/intel/sandybridge: Migrate MRC settings to devicetree)
very stupid nit: avoid splitting "commit" and the hash so that it appears as a link on Gerrit: […]
Done
https://review.coreboot.org/c/coreboot/+/73690/comment/cf80be78_45fec521 PS3, Line 18: 5709e03
Hash should be at least 12 characters: commit 5709e03613b3
Done
-
Keith Hui (Code Review)