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 4:
(6 comments)
https://review.coreboot.org/c/coreboot/+/44163/4/src/soc/intel/alderlake/Kco... File src/soc/intel/alderlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/44163/4/src/soc/intel/alderlake/Kco... PS4, Line 4: select INTEL_CAR_NEM
Is there a reason this isn't underneath `CPU_SPECIFIC_OPTIONS` ?
Ack
https://review.coreboot.org/c/coreboot/+/44163/4/src/soc/intel/alderlake/Kco... PS4, Line 72: default 0x8000
Why is this is half of the heap size used in TGL (0x10000) ?
0x8000 was good so far bt as u asked, making this same as TGL
https://review.coreboot.org/c/coreboot/+/44163/4/src/soc/intel/alderlake/boo... File src/soc/intel/alderlake/bootblock/cpu.c:
https://review.coreboot.org/c/coreboot/+/44163/4/src/soc/intel/alderlake/boo... PS4, Line 10: #include <soc/bootblock.h>
looks unused
needed for "bootblock_cpu_init" function declaration (bootblock.h line 7)
https://review.coreboot.org/c/coreboot/+/44163/4/src/soc/intel/alderlake/boo... File src/soc/intel/alderlake/bootblock/pch.c:
https://review.coreboot.org/c/coreboot/+/44163/4/src/soc/intel/alderlake/boo... PS4, Line 28: #define PCR_PSF3_TO_SHDW_PMC_REG_BASE 0x1100 : #define PCR_PSFX_TO_SHDW_BAR0 0 : #define PCR_PSFX_TO_SHDW_BAR1 0x4 : #define PCR_PSFX_TO_SHDW_BAR2 0x8 : #define PCR_PSFX_TO_SHDW_BAR3 0xC : #define PCR_PSFX_TO_SHDW_BAR4 0x10 : #define PCR_PSFX_TO_SHDW_PCIEN_IOEN 0x01 : #define PCR_PSFX_T0_SHDW_PCIEN 0x1C
Can we line up the definitions on the right?
Ack
https://review.coreboot.org/c/coreboot/+/44163/4/src/soc/intel/alderlake/boo... PS4, Line 59: Enable Bus Master
The next line does not seem to enable bus mastering
Ack
https://review.coreboot.org/c/coreboot/+/44163/4/src/soc/intel/alderlake/boo... File src/soc/intel/alderlake/bootblock/report_platform.c:
https://review.coreboot.org/c/coreboot/+/44163/4/src/soc/intel/alderlake/boo... PS4, Line 17: #include <intelblocks/mp_init.h>
Not used here, `name. […]
needed for CPUID_ALDERLAKE_S_A0 and CPUID_ALDERLAKE_P_A0 macro