Change in coreboot[master]: mb/google/hatch: Remove GPIO_DRIVER from pen eject GPIO configuration
Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35459 )
Change subject: mb/google/hatch: Remove GPIO_DRIVER from pen eject GPIO configuration ......................................................................
mb/google/hatch: Remove GPIO_DRIVER from pen eject GPIO configuration
A closer read of the EDS indicates that when GPIO Driver mode is selected, GPIO input event updates are limited to GPI_STS only. GPI_GPE_STS updates are therefore masked, and we don't want to enable this behavior. It masks the GPE and does not allow us to see this GPE as a wake source, obscuring the reason that the system woke up.
BUG=b:132981083 BRANCH=none TEST=Wake up system from S0ix using pen eject, verify that mosys eventlog shows GPE#8 as the S0ix wakeup source.
Change-Id: If017e12e23134f5cfed7cbb6047cc9badd9bf7e8 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/mainboard/google/hatch/variants/baseboard/gpio.c 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/35459/1
diff --git a/src/mainboard/google/hatch/variants/baseboard/gpio.c b/src/mainboard/google/hatch/variants/baseboard/gpio.c index fcb1a61..1414776 100644 --- a/src/mainboard/google/hatch/variants/baseboard/gpio.c +++ b/src/mainboard/google/hatch/variants/baseboard/gpio.c @@ -31,7 +31,7 @@ /* A7 : PP3300_SOC_A */ PAD_NC(GPP_A7, NONE), /* A8 : PEN_GARAGE_DET_L (wake) */ - PAD_CFG_GPI_GPIO_DRIVER_SCI(GPP_A8, NONE, DEEP, LEVEL, NONE), + PAD_CFG_GPI_SCI(GPP_A8, NONE, DEEP, LEVEL, NONE), /* A9 : ESPI_CLK */ /* A10 : FPMCU_PCH_BOOT1 */ PAD_CFG_GPO(GPP_A10, 0, DEEP),
Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35459 )
Change subject: mb/google/hatch: Remove GPIO_DRIVER from pen eject GPIO configuration ......................................................................
Patch Set 1: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35459 )
Change subject: mb/google/hatch: Remove GPIO_DRIVER from pen eject GPIO configuration ......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/c/coreboot/+/35459/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35459/1//COMMIT_MSG@18 PS1, Line 18: . What about S3? That should work too.
https://review.coreboot.org/c/coreboot/+/35459/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/35459/1/src/mainboard/google/hatch/... PS1, Line 34: PAD_CFG_GPI_SCI Just a note: If any variants were relying on this pin for OS notifications, then that wouldn't work anymore. It would be good to confirm that all variants have this dual-routed. And add the other pin for OS notifications.
https://review.coreboot.org/c/coreboot/+/35459/1/src/mainboard/google/hatch/... PS1, Line 34: LEVEL This needs to be EDGE_SINGLE. Else, the device will wake up whenever the pen is left in the garage.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35459 )
Change subject: mb/google/hatch: Remove GPIO_DRIVER from pen eject GPIO configuration ......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/c/coreboot/+/35459/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35459/1//COMMIT_MSG@18 PS1, Line 18: .
What about S3? That should work too.
Done
https://review.coreboot.org/c/coreboot/+/35459/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/35459/1/src/mainboard/google/hatch/... PS1, Line 34: PAD_CFG_GPI_SCI
Just a note: If any variants were relying on this pin for OS notifications, then that wouldn't work […]
At the time when we figured out the single-routing wouldn't work, I emailed all of the EEs and filed bugs for all the variants at the time to make sure they dual-routed the signal. I can check on the new ones that have come up since then.
https://review.coreboot.org/c/coreboot/+/35459/1/src/mainboard/google/hatch/... PS1, Line 34: LEVEL
This needs to be EDGE_SINGLE. Else, the device will wake up whenever the pen is left in the garage.
It wakes up if the pen is left out of the garage, will fix.
Hello Paul Fagerburg, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35459
to look at the new patch set (#2).
Change subject: mb/google/hatch: Remove GPIO_DRIVER from pen eject GPIO configuration ......................................................................
mb/google/hatch: Remove GPIO_DRIVER from pen eject GPIO configuration
A closer read of the EDS indicates that when GPIO Driver mode is selected, GPIO input event updates are limited to GPI_STS only. GPI_GPE_STS updates are therefore masked, and we don't want to enable this behavior. It masks the GPE and does not allow us to see this GPE as a wake source, obscuring the reason that the system woke up.
Also switch the IRQ from level-triggered to edge-triggered, otherwise the system will auto-wake from any sleep state when the pen is ejected from the garage.
BUG=b:132981083 BRANCH=none TEST=Wake up system from S0ix using pen eject, verify that mosys eventlog shows GPE#8 as the S0ix wakeup source. Wake up system from S3 via pen eject, and verify that the wakeup source shows as GPE#8.
Change-Id: If017e12e23134f5cfed7cbb6047cc9badd9bf7e8 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/mainboard/google/hatch/variants/baseboard/gpio.c 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/35459/2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35459 )
Change subject: mb/google/hatch: Remove GPIO_DRIVER from pen eject GPIO configuration ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35459/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/35459/1/src/mainboard/google/hatch/... PS1, Line 34: LEVEL
It wakes up if the pen is left out of the garage, will fix.
Ah yes, it is marked SCI without invert. It will wake up if left out of garage.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35459 )
Change subject: mb/google/hatch: Remove GPIO_DRIVER from pen eject GPIO configuration ......................................................................
Patch Set 2: Code-Review+2
Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35459 )
Change subject: mb/google/hatch: Remove GPIO_DRIVER from pen eject GPIO configuration ......................................................................
Patch Set 2: Code-Review+2
-
Furquan Shaikh (Code Review) -
Paul Fagerburg (Code Review)
-
Tim Wawrzynczak (Code Review)