Attention is currently required from: Angel Pons, Arthur Heymans, Christian Walter, Johnny Lin, Jonathan Zhang, Morgan Jang, Shuo Liu, Tim Chu.
Patrick Rudolph has posted comments on this change by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/85557?usp=email )
Change subject: soc/intel/xeon_sp: Merge SKX and CPX ......................................................................
Patch Set 5:
(5 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/85557/comment/e94ea908_da7af6ab?usp... : PS5, Line 10: - Create a new 14nm folder
Is there an Intel name for the "Xeon-SP SPR and CPX" group?
I'm not aware of a name for the group. Intel naming is broken for CPX and intentionally not used.Intel marketing team named both the 14nm CPX and 10nm ICX the *3rd gen Xeon-SP*. Datasheets contain those names only and no code-names, thus it's hard to tell which is which.
CPX and ICX are very different from silicon POV, while SKX and CPX are very similar. To prevent confusion we should stay with the 14nm model. It perfectly describes which silicon it refers to.
https://review.coreboot.org/c/coreboot/+/85557/comment/62d2b068_cf760b45?usp... : PS5, Line 18: TEST: Still boots on ocp/tiogapass.
Should this be boot-tested on CPX too?
Yes, but not sure if we have such a test system.
File src/soc/intel/xeon_sp/14nm/acpi/gpio.asl:
PS5:
Should we still place the 'acpi' ahead of '14nm (or gen1)' to be consistent?
That's not done for spr either. With this change only common code resides in `src/soc/intel/xeon_sp/acpi`, which makes more sense.
File src/soc/intel/xeon_sp/cpx/Kconfig:
https://review.coreboot.org/c/coreboot/+/85557/comment/9f027e18_f778ee83?usp... : PS5, Line 5: select SOC_INTEL_XEON_SP_14NM
I don't know what GEN1 and GEN3 correspond to, SKX and CPX respectively?
GEN1 is SKX, but GEN3 isn't usable. It's CPX and ICX.
File src/soc/intel/xeon_sp/skx/Kconfig:
https://review.coreboot.org/c/coreboot/+/85557/comment/32e1bdb2_71ee294b?usp... : PS5, Line 5: select SOC_INTEL_XEON_SP_14NM
SOC_INTEL_XEON_SP_GEN1?
Doesn't apply to CPX.