Subrata Banik 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:
(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
Ack
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
Ack
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?
Ack
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)
Yes, that sounds good.
Ack
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
Ack
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
Ack