Andrey Petrov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39601 )
Change subject: soc/intel/xeon_sp: Refactor code to allow for additional CPUs ......................................................................
Patch Set 3:
(7 comments)
thanks David. As you see I was already removing copyright notices as I was moving files. So I rebased the patch on top of Patrick's. I think I was able to revert most of the cosmetic changes, and I will submit them separately
https://review.coreboot.org/c/coreboot/+/39601/1/src/mainboard/ocp/tiogapass... File src/mainboard/ocp/tiogapass/Kconfig:
https://review.coreboot.org/c/coreboot/+/39601/1/src/mainboard/ocp/tiogapass... PS1, Line 26: select FSP_CAR
Does this change belong in this patch?
yeah. FSP_CAR should not be in motherboard in the first place
https://review.coreboot.org/c/coreboot/+/39601/1/src/soc/intel/xeon_sp/bootb... File src/soc/intel/xeon_sp/bootblock.c:
https://review.coreboot.org/c/coreboot/+/39601/1/src/soc/intel/xeon_sp/bootb... PS1, Line 22: #include <console/console.h>
Cosmetic changes should go in another patch.
Ack
https://review.coreboot.org/c/coreboot/+/39601/1/src/soc/intel/xeon_sp/inclu... File src/soc/intel/xeon_sp/include/soc/iomap.h:
PS1:
Thanks for fixing the indentation, but purely cosmetic changes should go in another patch.
Ack
https://review.coreboot.org/c/coreboot/+/39601/1/src/soc/intel/xeon_sp/inclu... File src/soc/intel/xeon_sp/include/soc/pcr_ids.h:
PS1:
purely cosmetic changes should go in another patch.
Ack
https://review.coreboot.org/c/coreboot/+/39601/1/src/soc/intel/xeon_sp/skx/M... File src/soc/intel/xeon_sp/skx/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/39601/1/src/soc/intel/xeon_sp/skx/M... PS1, Line 5: ## Copyright (C) 2019 - 2020 Facebook Inc
Get rid of these (as per the CB:39605 patch chain)
Ack
https://review.coreboot.org/c/coreboot/+/39601/1/src/soc/intel/xeon_sp/skx/h... File src/soc/intel/xeon_sp/skx/hob_display.c:
PS1:
purely cosmetic changes should go in another patch.
Ack
https://review.coreboot.org/c/coreboot/+/39601/1/src/soc/intel/xeon_sp/skx/r... File src/soc/intel/xeon_sp/skx/romstage.c:
https://review.coreboot.org/c/coreboot/+/39601/1/src/soc/intel/xeon_sp/skx/r... PS1, Line 5: * Copyright (C) 2019 - 2020 Facebook Inc
Get rid of these (as per the CB:39605 patch chain)
Ack