Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46091 )
Change subject: mb/intel/adlrvp: Add ADL-P romstage mainboard code ......................................................................
Patch Set 8: Code-Review+1
(3 comments)
https://review.coreboot.org/c/coreboot/+/46091/8/src/mainboard/intel/adlrvp/... File src/mainboard/intel/adlrvp/romstage_fsp_params.c:
https://review.coreboot.org/c/coreboot/+/46091/8/src/mainboard/intel/adlrvp/... PS8, Line 16: uintptr_t why is this an uintptr_t? it's not really a pointer to anything
https://review.coreboot.org/c/coreboot/+/46091/8/src/mainboard/intel/adlrvp/... PS8, Line 23: spd_index = (board_id & BIT_4_0) & BIT_2_0; Hmm, this is essentially the same as `board_id & BIT_2_0`. Is it correct?
https://review.coreboot.org/c/coreboot/+/46091/7/src/mainboard/intel/adlrvp/... File src/mainboard/intel/adlrvp/variants/adlrvp_p/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/46091/7/src/mainboard/intel/adlrvp/... PS7, Line 8: register "SaGv" = "SaGv_Disabled"
Tim, Default is subject to change over FSP version, in latest code I'm seeing default is 0x5 enable […]
Sounds good.