Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40386 )
Change subject: soc/intel/xeon_sp/cpx: Finalize PCU configuration ......................................................................
Patch Set 27:
(4 comments)
https://review.coreboot.org/c/coreboot/+/40386/27/src/soc/intel/xeon_sp/cpx/... File src/soc/intel/xeon_sp/cpx/soc_util.c:
https://review.coreboot.org/c/coreboot/+/40386/27/src/soc/intel/xeon_sp/cpx/... PS27, Line 265: pcu
nit: uppercase `PCU`
Done
https://review.coreboot.org/c/coreboot/+/40386/27/src/soc/intel/xeon_sp/cpx/... PS27, Line 304: uint32_t sbsp_socket_id = 0;
Looks like this can be const. […]
Yes, it may not be socket 0. I will add a comment about it. Right now our focus is on one socket server platform, so we will not get to this part for some time.
https://review.coreboot.org/c/coreboot/+/40386/27/src/soc/intel/xeon_sp/cpx/... PS27, Line 307: According to the BIOS writer's guide, this needs to be set on non-SBSP : * first, before set on SBSP.
How about: […]
Done
https://review.coreboot.org/c/coreboot/+/40386/27/src/soc/intel/xeon_sp/cpx/... PS27, Line 315: set_bios_init_completion_for_package(sbsp_socket_id);
And maybe add a comment here: […]
Done
-
Jonathan Zhang (Code Review)