Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44163 )
Change subject: soc/intel/alderlake/bootblock: Do initial SoC commit till bootblock ......................................................................
Patch Set 3: Code-Review+1
(6 comments)
https://review.coreboot.org/c/coreboot/+/44163/3/src/soc/intel/alderlake/boo... File src/soc/intel/alderlake/bootblock/cpu.c:
https://review.coreboot.org/c/coreboot/+/44163/3/src/soc/intel/alderlake/boo... PS3, Line 17: temporarily : * cacheing on temporary caching of
https://review.coreboot.org/c/coreboot/+/44163/3/src/soc/intel/alderlake/boo... PS3, Line 20: This assumption will not hold good for APL/GLK platform where boot : * from eMMC is also possible options. drop this part? This isn't APL
https://review.coreboot.org/c/coreboot/+/44163/3/src/soc/intel/alderlake/boo... File src/soc/intel/alderlake/bootblock/pch.c:
https://review.coreboot.org/c/coreboot/+/44163/3/src/soc/intel/alderlake/boo... PS3, Line 52: BIT 1-2 Bit 1? Bit 2? Or both?
https://review.coreboot.org/c/coreboot/+/44163/2/src/soc/intel/alderlake/boo... File src/soc/intel/alderlake/bootblock/report_platform.c:
https://review.coreboot.org/c/coreboot/+/44163/2/src/soc/intel/alderlake/boo... PS2, Line 192: microcode_ver.lo = 0; : microcode_ver.hi = 0; : wrmsr(BIOS_SIGN_ID, microcode_ver); : cpu_id = cpu_get_cpuid(); : microcode_ver = rdmsr(BIOS_SIGN_ID)
valid point, shall i address that in separate CL for all possible intel SoC ?
Yes, that sounds good.
https://review.coreboot.org/c/coreboot/+/44163/3/src/soc/intel/alderlake/inc... File src/soc/intel/alderlake/include/soc/p2sb.h:
https://review.coreboot.org/c/coreboot/+/44163/3/src/soc/intel/alderlake/inc... PS3, Line 4: Tiger Hmmmmmmmm
https://review.coreboot.org/c/coreboot/+/44163/3/src/soc/intel/alderlake/inc... File src/soc/intel/alderlake/include/soc/pcr_ids.h:
https://review.coreboot.org/c/coreboot/+/44163/3/src/soc/intel/alderlake/inc... PS3, Line 14: Move this blank line above the comment