Change in coreboot[master]: soc/intel/alderlake/bootblock: Do initial SoC commit till bootblock
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44857 )
Change subject: soc/intel/alderlake/bootblock: Do initial SoC commit till bootblock ......................................................................
Patch Set 2:
(10 comments)
https://review.coreboot.org/c/coreboot/+/44857/2/src/soc/intel/alderlake/Kco... File src/soc/intel/alderlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/44857/2/src/soc/intel/alderlake/Kco... PS2, Line 131: 0xe00
0x1400 is typical
Ack
https://review.coreboot.org/c/coreboot/+/44857/2/src/soc/intel/alderlake/boo... File src/soc/intel/alderlake/bootblock/cpu.c:
https://review.coreboot.org/c/coreboot/+/44857/2/src/soc/intel/alderlake/boo... PS2, Line 16: only booting from SPI device
"SPI flash"
Ack
https://review.coreboot.org/c/coreboot/+/44857/2/src/soc/intel/alderlake/boo... File src/soc/intel/alderlake/bootblock/pch.c:
https://review.coreboot.org/c/coreboot/+/44857/2/src/soc/intel/alderlake/boo... PS2, Line 102: uint32_t dmi_control = pcr_read32(PID_DMI, PCR_DMI_DMICTL);
nit: const
Ack
https://review.coreboot.org/c/coreboot/+/44857/2/src/soc/intel/alderlake/boo... File src/soc/intel/alderlake/bootblock/report_platform.c:
https://review.coreboot.org/c/coreboot/+/44857/2/src/soc/intel/alderlake/boo... PS2, Line 5: 619362
This is the ADL-S EDS
doc is same
https://review.coreboot.org/c/coreboot/+/44857/2/src/soc/intel/alderlake/boo... PS2, Line 99: PCI_DEVICE_ID_INTEL_ADL_S_GT1
Would any ADL-P SoC have the ADL-S GT1 IGD ?
Ack
https://review.coreboot.org/c/coreboot/+/44857/2/src/soc/intel/alderlake/boo... PS2, Line 136: aes = (cpu_feature_flag & CPUID_AES) ? 1 : 0; : txt = (cpu_feature_flag & CPUID_SMX) ? 1 : 0; : vt = (cpu_feature_flag & CPUID_VMX) ? 1 : 0;
suggestion: you could use !!(cpu_feature_flag & CPUID... […]
Ack
https://review.coreboot.org/c/coreboot/+/44857/2/src/soc/intel/alderlake/inc... File src/soc/intel/alderlake/include/soc/espi.h:
https://review.coreboot.org/c/coreboot/+/44857/2/src/soc/intel/alderlake/inc... PS2, Line 5: 621483
this is ADL-S, is there going to be a separate EDS for -P?
doc is same
https://review.coreboot.org/c/coreboot/+/44857/2/src/soc/intel/alderlake/inc... PS2, Line 12: #include <stdint.h>
not used here
Ack
https://review.coreboot.org/c/coreboot/+/44857/2/src/soc/intel/alderlake/inc... File src/soc/intel/alderlake/include/soc/iomap.h:
https://review.coreboot.org/c/coreboot/+/44857/2/src/soc/intel/alderlake/inc... PS2, Line 65:
What about IOM_BASE_ADDRESS ?
Ack
https://review.coreboot.org/c/coreboot/+/44857/2/src/soc/intel/alderlake/inc... File src/soc/intel/alderlake/include/soc/pcr_ids.h:
https://review.coreboot.org/c/coreboot/+/44857/2/src/soc/intel/alderlake/inc... PS2, Line 14: */
SDCX controller ?
actually its unused un tgl too, i will remove from tgl too
-
Subrata Banik (Code Review)