EricR Lai 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 14:
(7 comments)
https://review.coreboot.org/c/coreboot/+/37685/13//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37685/13//COMMIT_MSG@9 PS13, Line 9: Enable/disable GPIO clock gating when enter/exit s0ix is common : request on CNL/CML. Move it from board level to soc level.
This description does not really capture the changes that are being done in this CL.
Done
https://review.coreboot.org/c/coreboot/+/37685/13/src/soc/intel/cannonlake/a... File src/soc/intel/cannonlake/acpi/gpio.asl:
https://review.coreboot.org/c/coreboot/+/37685/13/src/soc/intel/cannonlake/a... PS13, Line 171: Store (GPID (Local0), Local1) : Store (PCRR (Local1, GPIO_MISCCFG), Index(GPMB, Local0))
Can this be simply written as: […]
Done
https://review.coreboot.org/c/coreboot/+/37685/13/src/soc/intel/cannonlake/a... PS13, Line 192: /* Store current setting */ : SGPM ()
Why is store called within a method that is supposed to "Enable GPIO Power Management bits"? […]
Done
https://review.coreboot.org/c/coreboot/+/37685/13/src/soc/intel/cannonlake/a... File src/soc/intel/cannonlake/acpi/lpit.asl:
https://review.coreboot.org/c/coreboot/+/37685/13/src/soc/intel/cannonlake/a... PS13, Line 79: Enable GPIO PM
This is not just enable. It also saves the current state of GPIO PM bits.
Done
https://review.coreboot.org/c/coreboot/+/37685/13/src/soc/intel/cannonlake/a... PS13, Line 79: *
space before *
Done
https://review.coreboot.org/c/coreboot/+/37685/13/src/soc/intel/cannonlake/a... PS13, Line 80: OGPM
@Furquan, this is Tim wanted. I think it okay to let mainboard can choice they want this or not.
Ack
https://review.coreboot.org/c/coreboot/+/37685/13/src/soc/intel/common/acpi/... File src/soc/intel/common/acpi/platform.asl:
https://review.coreboot.org/c/coreboot/+/37685/13/src/soc/intel/common/acpi/... PS13, Line 46: *
space before *
Done