Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34724 )
Change subject: soc/intel/common: Implement power-failure-state handling ......................................................................
Patch Set 5:
(5 comments)
https://review.coreboot.org/c/coreboot/+/34724/5/src/soc/intel/common/block/... File src/soc/intel/common/block/include/intelblocks/pmclib.h:
https://review.coreboot.org/c/coreboot/+/34724/5/src/soc/intel/common/block/... PS5, Line 222: ,
no param name?
Gone
(It seems to be a general pattern to omit the parameter's name if its purpose is clear from the type.)
https://review.coreboot.org/c/coreboot/+/34724/5/src/soc/intel/common/block/... PS5, Line 223: pmc_set_power_failure_state
Can you please add description to each of the functions as to how they differ? I believe one is a ca […]
Done
https://review.coreboot.org/c/coreboot/+/34724/5/src/soc/intel/common/block/... PS5, Line 223: )
same here?
Gone
https://review.coreboot.org/c/coreboot/+/34724/4/src/soc/intel/common/block/... File src/soc/intel/common/block/pmc/pmclib.c:
https://review.coreboot.org/c/coreboot/+/34724/4/src/soc/intel/common/block/... PS4, Line 602: state == MAINBOARD_POWER_STATE_ON
It makes sense to squash the two CLs together since it is easier to see what the overall logic is. […]
Done
https://review.coreboot.org/c/coreboot/+/34724/5/src/soc/intel/common/block/... File src/soc/intel/common/block/pmc/pmclib.c:
https://review.coreboot.org/c/coreboot/+/34724/5/src/soc/intel/common/block/... PS5, Line 583: void pmc_set_power_failure_state(const pci_devfn_t dev)
It's passed on to pmc_soc_set_afterg3_en(). In `skylake/pmc.c`, it's still […]
Done
-
Nico Huber (Code Review)