Attention is currently required from: Ana Carolina Cabral, Felix Held, Fred Reitberger, Jason Glenesk, Martin Roth, Matt DeVillier, Matt DeVillier, Paul Menzel.
Nick Kochlowski has posted comments on this change by Nick Kochlowski. ( https://review.coreboot.org/c/coreboot/+/84915?usp=email )
Change subject: soc/amd/phoenix/chip.c: Add PHX OpenSIL POC TP2/TP3 calls ......................................................................
Patch Set 7:
(4 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/84915/comment/ebabc531_6ab98845?usp... : PS6, Line 9: Call OpenSIL timepoint 2 right after resource allocation, : and timepoint 3 right before entering payload.
What is OpenSIL timepoint 2 and 3? Please elaborate.
Done
File src/soc/amd/phoenix/chip.c:
PS6:
yes, i agree with that there's nothing in that file that would be needed to be changed or updated fo […]
Before we commit to the decision, I wonder if it would be of more instructional value (being a POC implementation) for the three TPs to be called from within this file. It's true for FSP the later calls are done from the driver code instead of here, but they're not called from vendorcode either. What do you think?
https://review.coreboot.org/c/coreboot/+/84915/comment/2a26ce62_04eb4ac3?usp... : PS6, Line 30: !CONFIG(PLATFORM_USES_FSP2_0)
Do we not have a config for PLATFORM_USES_OPENSIL? That seems like a better choice for this. […]
Just added it to the Kconfig since we didn't have that particular config.
https://review.coreboot.org/c/coreboot/+/84915/comment/5d1fd232_5e80395f?usp... : PS6, Line 41: default: : break;
This seems a little unnecessary. […]
Out of good practice, I left the default case and added an error log although we shouldn't expect to see it used. I can remove it altogether if it really isn't needed.
-
Nick Kochlowski (Code Review)