Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46231 )
Change subject: soc/intel/xeon_sp: Enable SMI handler ......................................................................
Patch Set 16: Code-Review+1
(5 comments)
https://review.coreboot.org/c/coreboot/+/46231/16/src/soc/intel/xeon_sp/Kcon... File src/soc/intel/xeon_sp/Kconfig:
https://review.coreboot.org/c/coreboot/+/46231/16/src/soc/intel/xeon_sp/Kcon... PS16, Line 66: REG_SCRIPT W-why?
https://review.coreboot.org/c/coreboot/+/46231/16/src/soc/intel/xeon_sp/skx/... File src/soc/intel/xeon_sp/skx/cpu.c:
https://review.coreboot.org/c/coreboot/+/46231/16/src/soc/intel/xeon_sp/skx/... PS16, Line 15: Any reason to have a space here?
https://review.coreboot.org/c/coreboot/+/46231/16/src/soc/intel/xeon_sp/skx/... PS16, Line 215: 1 << 12 `4 * KiB` would also work and IMHO would be more readable
https://review.coreboot.org/c/coreboot/+/46231/16/src/soc/intel/xeon_sp/smih... File src/soc/intel/xeon_sp/smihandler.c:
https://review.coreboot.org/c/coreboot/+/46231/16/src/soc/intel/xeon_sp/smih... PS16, Line 2: /* This file is part of the coreboot project. */ This comment should be gone
https://review.coreboot.org/c/coreboot/+/46231/16/src/soc/intel/xeon_sp/smih... PS16, Line 14: #include <chip.h> Any reason to include so much stuff?