Attention is currently required from: Alexander Couzens, Keith Hui, Martin L Roth, Nicholas Chin.
Angel Pons has posted comments on this change by Keith Hui. ( https://review.coreboot.org/c/coreboot/+/79025?usp=email )
Change subject: nb/intel/haswell: Move SPD addresses to devicetree ......................................................................
Patch Set 4:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/79025/comment/9822745b_11c88302?usp... : PS4, Line 14: Patch also covers recently added Z97 boards using Broadwell MRC. I had to make a separate follow-up commit for Sandy Bridge because the original refactoring commits had already been submitted. See CB:49088 as to how I'd handle it.
Yes, it's go code, but it's not that hard to read. Plus, the changes done are in line with what is done to the mainboards, so reviewing them is not that hard. I'd prefer to include the autoport changes in this commit, and briefly mention so in the commit message, e.g.:
Also, update util/autoport accordingly.
File src/northbridge/intel/haswell/broadwell_mrc/raminit.c:
https://review.coreboot.org/c/coreboot/+/79025/comment/9a28cde2_43fe140e?usp... : PS4, Line 377: struct spd_info spdi = {0}; : if (CONFIG(HAVE_SPD_IN_CBFS)) { : /* Obtain the SPD addresses from mainboard code */ : mb_get_spd_map(&spdi); : } else { : /* Boards without memory down: Obtain the SPD addresses from devicetree */ : memcpy(spdi.addresses, cfg->spd_addresses, ARRAY_SIZE(spdi.addresses)); : } :
Nice idea, but the code are separate enough this helper function will need to go into a separate C f […]
Yeah, it's to avoid having redundant copies of the source code that may end up out of sync. I will also move things like `report_memory_config()` (or whatever it's called).
The file could be named something like `raminit_shared.c`, for example (IIRC, Sandybridge uses this name for stuff shared between MRC and NRI).