Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43004 )
Change subject: soc/intel/xeon_sp: Add RTC failure checking ......................................................................
Patch Set 3: Code-Review+1
(4 comments)
https://review.coreboot.org/c/coreboot/+/43004/3/src/soc/intel/xeon_sp/inclu... File src/soc/intel/xeon_sp/include/soc/pmc.h:
https://review.coreboot.org/c/coreboot/+/43004/3/src/soc/intel/xeon_sp/inclu... PS3, Line 28: nit: two spaces here (it's a bit inside a register)
https://review.coreboot.org/c/coreboot/+/43004/3/src/soc/intel/xeon_sp/pmuti... File src/soc/intel/xeon_sp/pmutil.c:
https://review.coreboot.org/c/coreboot/+/43004/3/src/soc/intel/xeon_sp/pmuti... PS3, Line 14: int bool
https://review.coreboot.org/c/coreboot/+/43004/3/src/soc/intel/xeon_sp/pmuti... PS3, Line 16: RTC_BATTERY_DEAD Instead of masking here, I'd read the register as-is inside a variable and mask the desired bit elsewhere. This should make the lines fit within 96 characters.
https://review.coreboot.org/c/coreboot/+/43004/3/src/soc/intel/xeon_sp/pmuti... PS3, Line 16: PCH_DEVFN_PMC<<12 huh? Why this shift?
-
Angel Pons (Code Review)