Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31902 )
Change subject: soc/intel/cannonlake: Clear SUS_PWR_FLR status bit ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/#/c/31902/3/src/soc/intel/cannonlake/pmutil.c File src/soc/intel/cannonlake/pmutil.c:
https://review.coreboot.org/#/c/31902/3/src/soc/intel/cannonlake/pmutil.c@14... PS3, Line 146: pmc_clear_suspwrflr I am wondering if this should be more than just suspwrflr clear i.e. pmc_clear_pmcon_sts and then clear: GBL_RST_STS SUS_PWR_FLR PWR_FLR
Also, why not just move this function to pmc.c. That way you don't have to expose it in the header file.
https://review.coreboot.org/#/c/31902/3/src/soc/intel/cannonlake/pmutil.c@15... PS3, Line 150: pmc_mmio_regs() + GEN_PMCON_A + 2 Just read the whole thing and update the right bits (preserving the ones that don't need to be reset)