HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38548 )
Change subject: soc/intel: Add Intel SkyLake Scalable Processor support ......................................................................
Patch Set 10: Code-Review-1
(2 comments)
https://review.coreboot.org/c/coreboot/+/38548/10/src/soc/intel/skylake_sp/a... File src/soc/intel/skylake_sp/acpi.c:
https://review.coreboot.org/c/coreboot/+/38548/10/src/soc/intel/skylake_sp/a... PS10, Line 18: #include <assert.h> : #include <arch/acpi.h> : #include <arch/cpu.h> : #include <arch/acpigen.h> : #include <arch/ioapic.h> : #include <arch/smp/mpspec.h> : #include <bootstate.h> : #include <string.h> : #include <cf9_reset.h> : #include <cpu/intel/turbo.h> : #include <cpu/x86/msr.h> : #include <cpu/x86/smm.h> : #include <intelblocks/acpi.h> : #include <intelblocks/msr.h> : #include <intelblocks/pmclib.h> : #include <device/pci.h> : #include <cbmem.h> : #include <soc/acpi.h> : #include <soc/cpu.h> : #include <soc/soc_util.h> : #include <soc/pm.h> : #include <soc/pmc.h> : #include <soc/pci_devs.h> : #include <soc/soc_util.h> : #include <soc/hob_iiouds.h> : #include <soc/hob_memmap.h> : #include "chip.h" Are you really using all of those includes ? Please include what you use. (same for the other files)
https://review.coreboot.org/c/coreboot/+/38548/10/src/soc/intel/skylake_sp/a... PS10, Line 240: Please use 'Device ()' instead of 'Processor ()' the use of 'Processor ()' is deprecated -> https://review.coreboot.org/c/coreboot/+/37876