Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39713 )
Change subject: soc/intel/xeon_sp: Add basic Cooperlake-SP support ......................................................................
Patch Set 4:
(5 comments)
https://review.coreboot.org/c/coreboot/+/39713/4/src/soc/intel/xeon_sp/cpx/a... File src/soc/intel/xeon_sp/cpx/acpi.c:
https://review.coreboot.org/c/coreboot/+/39713/4/src/soc/intel/xeon_sp/cpx/a... PS4, Line 22: unsigned long current, fits on the previous line
https://review.coreboot.org/c/coreboot/+/39713/4/src/soc/intel/xeon_sp/cpx/a... PS4, Line 81: acpi_create_madt_irqoverride((void *)current, 0, sci, sci, flags); fits on the previous line
https://review.coreboot.org/c/coreboot/+/39713/4/src/soc/intel/xeon_sp/cpx/a... PS4, Line 132: ACPI_FADT_C2_MP_SUPPORTED | ACPI_FADT_RESET_REGISTER | Can be reflowed into two lines
https://review.coreboot.org/c/coreboot/+/39713/4/src/soc/intel/xeon_sp/cpx/a... PS4, Line 167: 0x00 Any reason to have hex here?
https://review.coreboot.org/c/coreboot/+/39713/4/src/soc/intel/xeon_sp/cpx/c... File src/soc/intel/xeon_sp/cpx/chip.c:
https://review.coreboot.org/c/coreboot/+/39713/4/src/soc/intel/xeon_sp/cpx/c... PS4, Line 56: PCH-LP Uh?