David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38548 )
Change subject: soc/intel: Add Intel Xeon Scalable Processor support ......................................................................
Patch Set 43:
(7 comments)
https://review.coreboot.org/c/coreboot/+/38548/41/src/soc/intel/xeon_sp/cpu.... File src/soc/intel/xeon_sp/cpu.c:
https://review.coreboot.org/c/coreboot/+/38548/41/src/soc/intel/xeon_sp/cpu.... PS41, Line 224: */
In a follow-up patch.
Ack
https://review.coreboot.org/c/coreboot/+/38548/42/src/soc/intel/xeon_sp/incl... File src/soc/intel/xeon_sp/include/soc/irq.h:
PS42:
Are these #defines used anywhere? I see this headed included by chip.h and uncore.asl. […]
Thanks for cleaning out the unused #defines. We can add them back later as needed.
https://review.coreboot.org/c/coreboot/+/38548/42/src/soc/intel/xeon_sp/incl... File src/soc/intel/xeon_sp/include/soc/p2sb.h:
PS42:
Yes, this file is used by src/soc/intel/common/block/p2sb/p2sb.c .
Ack
https://review.coreboot.org/c/coreboot/+/38548/42/src/soc/intel/xeon_sp/incl... PS42, Line 20: #define HPTC_OFFSET 0x60
This macro is used in src/soc/intel/common/block/p2sb/p2sb.c to configure HPET.
Ack
https://review.coreboot.org/c/coreboot/+/38548/42/src/soc/intel/xeon_sp/incl... File src/soc/intel/xeon_sp/include/soc/pcr_ids.h:
https://review.coreboot.org/c/coreboot/+/38548/42/src/soc/intel/xeon_sp/incl... PS42, Line 23: #define PID_PSTH 0x89
This is for IO Trap PCRs. […]
Thanks for pointing that out, I'll check into it in more detail and submit a correction in a follow-up if needed.
https://review.coreboot.org/c/coreboot/+/38548/42/src/soc/intel/xeon_sp/incl... File src/soc/intel/xeon_sp/include/soc/pm.h:
https://review.coreboot.org/c/coreboot/+/38548/42/src/soc/intel/xeon_sp/incl... PS42, Line 41: #define PM1_CNT 0x04
SLP_EN and SLP_TYP are already defined in src/arch/x86/include/arch/acpi. […]
Ack
https://review.coreboot.org/c/coreboot/+/38548/37/src/soc/intel/xeon_sp/roms... File src/soc/intel/xeon_sp/romstage.c:
https://review.coreboot.org/c/coreboot/+/38548/37/src/soc/intel/xeon_sp/roms... PS37, Line 78: mainboard_memory_init_params(mupd);
It calls into the overridden mainboard_memory_init_params() in mb/ocp/tiogapass/romstage. […]
Oops, I must have missed that. Thanks!