Tristan Corrick has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43094 )
Change subject: nb/intel/haswell: Add `mb_post_raminit_setup` function ......................................................................
Patch Set 2:
Patch Set 2:
Patch Set 1: Code-Review+2
(1 comment)
Perhaps a name such as `mb_romstage_final` would be more appropriate? If the T440p PEG code runs after raminit, it will have to come after `haswell_unhide_peg` too.
I used the same name as in x4x, and to preserve the original behavior (which may be wrong) I placed the code so that it runs at the same point as before (only difference being the postcode call, which doesn't really matter). In any case, once I turn romstage_common inside-out in CB:43108 it will be easier to move the function call to a better place.
Yes, the behaviour is preserved. My thought was that the T440p code relies on more than just coming after raminit, since the PEG devices need to be unhidden first. The name `mb_post_raminit_setup` doesn't suggest that one could always rely on it being executed after the PEG devices are unhidden. However, I think a name such as `mb_romstage_final` shows that one can rely on it running after all the common romstage code, including the PEG devices being unhidden.
When thinking of future boards that might use it, I think it's much more likely to have use cases that depend on running after all the common romstage code than immediately after raminit and before the end of common romstage.
What I could see potentially happening in the future is it being decided to move `mb_post_raminit_setup` immediately after raminit, for whatever reason, which would break the T440p PEG code as I understand it. Unlikely? Perhaps. But I don't see a downside to changing the name.