Rocky Phagura has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42289 )
Change subject: soc/intel/xeon_sp: Enable PMC support ......................................................................
Patch Set 6:
(3 comments)
https://review.coreboot.org/c/coreboot/+/42289/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/42289/3//COMMIT_MSG@19 PS3, Line 19: select CPU_INTEL_COMMON_SMM
CB:45041 will select CPU_INTEL_COMMON_SMM, but the other two symbols still need to be selected somew […]
Yes. I will be adding SMI handler in upcoming patches after this patch is approved/merged.
https://review.coreboot.org/c/coreboot/+/42289/5/src/soc/intel/xeon_sp/inclu... File src/soc/intel/xeon_sp/include/soc/pm.h:
https://review.coreboot.org/c/coreboot/+/42289/5/src/soc/intel/xeon_sp/inclu... PS5, Line 93: __packed
Looks like it was marked as packed at some point. […]
I'll keep as is so it says consistent with other projects.
https://review.coreboot.org/c/coreboot/+/42289/5/src/soc/intel/xeon_sp/pmc.c File src/soc/intel/xeon_sp/pmc.c:
https://review.coreboot.org/c/coreboot/+/42289/5/src/soc/intel/xeon_sp/pmc.c... PS5, Line 99: reg32 = ALIGN_UP(reg32, 12);
I see that bit 0 is hardwired to zero already (indicates this base address corresponds to a memory s […]
Correct. And the other bits are reserved and by default are 00 as per HW spec. So we really don't need to do anything here. This saves code and space.