Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33759 )
Change subject: soc/amd/picasso: Create a hybrid romstage to begin in DRAM ......................................................................
Patch Set 13:
(3 comments)
Is the name hibrid_romstage going to be final? I remember some discussions about using some different name. If so, shouldn't you change file name and the commit message?>
https://review.coreboot.org/c/coreboot/+/33759/13/src/soc/amd/picasso/early_... File src/soc/amd/picasso/early_stack.S:
https://review.coreboot.org/c/coreboot/+/33759/13/src/soc/amd/picasso/early_... PS13, Line 31: Extra space.
https://review.coreboot.org/c/coreboot/+/33759/13/src/soc/amd/picasso/early_... PS13, Line 32: * may be enabled. This could be made more elegant for more Current limit is 96 columns, please use full new size. Same in next comment.
https://review.coreboot.org/c/coreboot/+/33759/13/src/soc/amd/picasso/hybrid... File src/soc/amd/picasso/hybrid_romstage.c:
https://review.coreboot.org/c/coreboot/+/33759/13/src/soc/amd/picasso/hybrid... PS13, Line 4: * Copyright (C) 2019 Advanced Micro Devices, Inc. In romstage.h you placed this line after previous copyright, here you placed it before. Examining other code, it looks like placing newer copyrights after older copyrights is the norm. Please change order.
-
Richard Spiegel (Code Review)