Attention is currently required from: Alexander Couzens, Elyes Haouas, Keith Hui, Martin L Roth.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/76962?usp=email )
Change subject: SNB+MRC boards: Do not redo PEI data struct in hook ......................................................................
Patch Set 5: Code-Review+1
(2 comments)
File src/mainboard/asus/p8x7x-series/variants/p8z77-m_pro/early_init.c:
https://review.coreboot.org/c/coreboot/+/76962/comment/6488b053_49726d05 : PS4, Line 16: #include <northbridge/intel/sandybridge/pei_data.h>
Part of my goal with this is to have all the boards include only raminit. […]
On Haswell, mainboards fill in structures, chipset code consumes them to fill in the pei_data structure (for MRC) or uses them directly (for native raminit). It was easier to do because native raminit didn't exist back then. None of the headers included (directly or indirectly) by mainboards contains pei_data stuff.
File src/mainboard/google/butterfly/early_init.c:
https://review.coreboot.org/c/coreboot/+/76962/comment/93a3fa83_60468d8a : PS5, Line 100: /* TODO: Native raminit only uses 1333. Reconcile. */ If memory serves right, native raminit had issues on Google boards. No idea about details, though.
Looks like the long-term plan is to set this in a single place. You can simply use a ternary operator, something like this:
``` pei_data->max_ddr3_freq = CONFIG(USE_NATIVE_RAMINIT) ? 1333 : 1600; ```
And yes, this also works for devicetree "register"s:
``` register "max_mem_clock_mhz" = "CONFIG(USE_NATIVE_RAMINIT) ? 666 : 800" ```
It works because SCONFIG converts that line into `.max_mem_clock_mhz = CONFIG(USE_NATIVE_RAMINIT) ? 666 : 800,` (inside a struct initialiser), and the ternary is a constant expression (the condition and both results are constant).