Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34423 )
Change subject: soc/amd/picasso: Add FSP support for including AGESA ......................................................................
Patch Set 27:
(6 comments)
PTAL
https://review.coreboot.org/c/coreboot/+/34423/21/src/soc/amd/picasso/romsta... File src/soc/amd/picasso/romstage.c:
https://review.coreboot.org/c/coreboot/+/34423/21/src/soc/amd/picasso/romsta... PS21, Line 39: clear_agesa_mtrrs
Wouldn't it be more clear if MTRR's are cleared in one function and restored (to some extend) in a d […]
Done
https://review.coreboot.org/c/coreboot/+/34423/21/src/soc/amd/picasso/romsta... PS21, Line 83: TODO: make AGESA leave the MTRRs alone
Looks like this has been a problem ever since AGESA has been integrated in coreboot...
Ack
https://review.coreboot.org/c/coreboot/+/34423/21/src/soc/amd/picasso/romsta... PS21, Line 91: mem_top - 16 * MiB
needs to be aligned to its size.
Removed all MTRR caching from this patch.
https://review.coreboot.org/c/coreboot/+/34423/21/src/soc/amd/picasso/romsta... PS21, Line 103: tseg_base
needs to be aligned to its size.
Ack
https://review.coreboot.org/c/coreboot/+/34423/21/src/soc/amd/picasso/romsta... PS21, Line 120: config != NULL
Die earlier if that is not the case? That allows to reduce the indentation.
Done
https://review.coreboot.org/c/coreboot/+/34423/21/src/soc/amd/picasso/romsta... PS21, Line 120: config != NULL
Die earlier if that is not the case? That allows to reduce the indentation.
Done