Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37685 )
Change subject: soc/intel/cannonlake: Move GPIO PM configuration to soc level ......................................................................
Patch Set 15: Code-Review+2
(5 comments)
Thanks for the patience with the back and forth Eric and Subrata!
Eric: some minor nits, but otherwise LGTM.
https://review.coreboot.org/c/coreboot/+/37685/15//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37685/15//COMMIT_MSG@13 PS15, Line 13: measure power consumption under s0ix Can you please confirm the following things: 1. Check that GPIO PM bits are configured as per the board configuration in devicetree even after jumping to OS. This can be done by dumping MISCCFG registers for all communities
2. Check that GPIO PM bits are configured as per the board configuration in devicetree after a cycle of S0ix enter/exit. Dump MISCCFG registers for all communities before entering S0ix and after exiting S0ix.
https://review.coreboot.org/c/coreboot/+/37685/15/src/soc/intel/cannonlake/a... File src/soc/intel/cannonlake/acpi/gpio.asl:
https://review.coreboot.org/c/coreboot/+/37685/15/src/soc/intel/cannonlake/a... PS15, Line 188: Enable GPIO Power Management bits and save the current setting nit: Order is not correct ;). It should be "Save current setting of GPIO Power Management bits and enable all Power Management bits for all communities"
https://review.coreboot.org/c/coreboot/+/37685/15/src/soc/intel/cannonlake/a... PS15, Line 192: * nit: space before *
https://review.coreboot.org/c/coreboot/+/37685/15/src/soc/intel/cannonlake/a... File src/soc/intel/cannonlake/acpi/lpit.asl:
https://review.coreboot.org/c/coreboot/+/37685/15/src/soc/intel/cannonlake/a... PS15, Line 79: Enable GPIO PM with MISCCFG_ENABLE_GPIO_PM_CONFIG and : * save the current PM bits nit: Order is reversed.
https://review.coreboot.org/c/coreboot/+/37685/15/src/soc/intel/common/acpi/... File src/soc/intel/common/acpi/platform.asl:
https://review.coreboot.org/c/coreboot/+/37685/15/src/soc/intel/common/acpi/... PS15, Line 46: Enable GPIO PM with MISCCFG_ENABLE_GPIO_PM_CONFIG and : * save the current PM bits nit: order is reversed.