Change in ...coreboot[master]: mainboard/google/hatch: Enable PEN_EJECT_L as wake & notify source.
Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32487
Change subject: mainboard/google/hatch: Enable PEN_EJECT_L as wake & notify source. ......................................................................
mainboard/google/hatch: Enable PEN_EJECT_L as wake & notify source.
Updated GPP_A8 to be a GPI and SCI source, to support both wake and notifications.
BUG=b:128941098 BRANCH=none TEST=Compiles, simulated pen eject with PCH_INT_L signal. Both evtest and waking from s0ix confirm this works.
Change-Id: I080fb3cbfb3e2f55209ca31824b00ca820d70f78 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/mainboard/google/hatch/variants/baseboard/gpio.c M src/mainboard/google/hatch/variants/hatch/overridetree.cb M src/mainboard/google/hatch/variants/hatch_whl/overridetree.cb 3 files changed, 8 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/32487/1
diff --git a/src/mainboard/google/hatch/variants/baseboard/gpio.c b/src/mainboard/google/hatch/variants/baseboard/gpio.c index c12aa64..e4d5a3a 100644 --- a/src/mainboard/google/hatch/variants/baseboard/gpio.c +++ b/src/mainboard/google/hatch/variants/baseboard/gpio.c @@ -30,8 +30,8 @@ PAD_CFG_GPI_INT(GPP_A6, NONE, PLTRST, LEVEL), /* A7 : PP3300_SOC_A */ PAD_NC(GPP_A7, NONE), - /* A8 : EMR_GARAGE_DET */ - PAD_CFG_GPI_GPIO_DRIVER(GPP_A8, NONE, DEEP), + /* A8 : PEN_GARAGE_DET_L */ + PAD_CFG_GPI_GPIO_DRIVER_SCI(GPP_A8, NONE, DEEP, LEVEL, INVERT), /* A9 : ESPI_CLK */ /* A10 : FPMCU_PCH_BOOT1 */ PAD_CFG_GPO(GPP_A10, 0, DEEP), diff --git a/src/mainboard/google/hatch/variants/hatch/overridetree.cb b/src/mainboard/google/hatch/variants/hatch/overridetree.cb index 13e7766..2c06d5c 100644 --- a/src/mainboard/google/hatch/variants/hatch/overridetree.cb +++ b/src/mainboard/google/hatch/variants/hatch/overridetree.cb @@ -86,7 +86,9 @@ end chip drivers/generic/gpio_keys register "name" = ""PENH"" - register "gpio" = "ACPI_GPIO_INPUT_ACTIVE_HIGH(GPP_A8)" + register "gpio" = "ACPI_GPIO_IRQ_EDGE_LOW(GPP_A8)" + register "key.wake" = "GPE0_DW0_8" + register "key.wakeup_event_action" = "EV_ACT_ASSERTED" register "key.dev_name" = ""EJCT"" register "key.linux_code" = "SW_PEN_INSERTED" register "key.linux_input_type" = "EV_SW" diff --git a/src/mainboard/google/hatch/variants/hatch_whl/overridetree.cb b/src/mainboard/google/hatch/variants/hatch_whl/overridetree.cb index 15b93f3..6d3e6fd 100644 --- a/src/mainboard/google/hatch/variants/hatch_whl/overridetree.cb +++ b/src/mainboard/google/hatch/variants/hatch_whl/overridetree.cb @@ -71,7 +71,9 @@ end chip drivers/generic/gpio_keys register "name" = ""PENH"" - register "gpio" = "ACPI_GPIO_INPUT_ACTIVE_HIGH(GPP_A8)" + register "gpio" = "ACPI_GPIO_IRQ_EDGE_LOW(GPP_A8)" + register "key.wake" = "GPE0_DW0_8" + register "key.wakeup_event_action" = "EV_ACT_ASSERTED" register "key.dev_name" = ""EJCT"" register "key.linux_code" = "SW_PEN_INSERTED" register "key.linux_input_type" = "EV_SW"
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32487 )
Change subject: mainboard/google/hatch: Enable PEN_EJECT_L as wake & notify source. ......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/#/c/32487/1/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/32487/1/src/mainboard/google/hatch/variants/... PS1, Line 34: INVERT Why are we applying the invert here?
https://review.coreboot.org/#/c/32487/1/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/hatch/overridetree.cb:
https://review.coreboot.org/#/c/32487/1/src/mainboard/google/hatch/variants/... PS1, Line 90: GPE0_DW0_8 GPE0_DW0_08
https://review.coreboot.org/#/c/32487/1/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/hatch_whl/overridetree.cb:
https://review.coreboot.org/#/c/32487/1/src/mainboard/google/hatch/variants/... PS1, Line 74: ACPI_GPIO_IRQ_EDGE_LOW This should be active high right? i.e. when pen is inserted, the signal would be read as high? some discussion here: https://review.coreboot.org/c/coreboot/+/31815
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32487 )
Change subject: mainboard/google/hatch: Enable PEN_EJECT_L as wake & notify source. ......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/#/c/32487/1/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/32487/1/src/mainboard/google/hatch/variants/... PS1, Line 34: INVERT
Why are we applying the invert here?
Ah, I see. I read your comment below. Incorrect net name, bummer. Fixed.
https://review.coreboot.org/#/c/32487/1/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/hatch/overridetree.cb:
https://review.coreboot.org/#/c/32487/1/src/mainboard/google/hatch/variants/... PS1, Line 90: GPE0_DW0_8
GPE0_DW0_08
Done
https://review.coreboot.org/#/c/32487/1/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/hatch_whl/overridetree.cb:
https://review.coreboot.org/#/c/32487/1/src/mainboard/google/hatch/variants/... PS1, Line 74: ACPI_GPIO_IRQ_EDGE_LOW
This should be active high right? i.e. […]
Ack
Hello Rajat Jain, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32487
to look at the new patch set (#2).
Change subject: mainboard/google/hatch: Enable PEN_EJECT_L as wake & notify source. ......................................................................
mainboard/google/hatch: Enable PEN_EJECT_L as wake & notify source.
Updated GPP_A8 to be a GPI and SCI source, to support both wake and notifications.
BUG=b:128941098 BRANCH=none TEST=Compiles, simulated pen eject with PCH_INT_L signal. Both evtest and waking from s0ix confirm this works.
Change-Id: I080fb3cbfb3e2f55209ca31824b00ca820d70f78 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/mainboard/google/hatch/variants/baseboard/gpio.c M src/mainboard/google/hatch/variants/hatch/overridetree.cb M src/mainboard/google/hatch/variants/hatch_whl/overridetree.cb 3 files changed, 8 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/32487/2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32487 )
Change subject: mainboard/google/hatch: Enable PEN_EJECT_L as wake & notify source. ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/#/c/32487/4/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/hatch_whl/overridetree.cb:
https://review.coreboot.org/#/c/32487/4/src/mainboard/google/hatch/variants/... PS4, Line 74: ACPI_GPIO_IRQ_EDGE_HIGH I was thinking about this some more and shouldn't this be ACPI_GPIO_IRQ_EDGE_BOTH?
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32487 )
Change subject: mainboard/google/hatch: Enable PEN_EJECT_L as wake & notify source. ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/#/c/32487/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/32487/4//COMMIT_MSG@15 PS4, Line 15: and waking from s0ix confirm this works. You can also check "/proc/interrupts" and there should be an interrupt for gpio_keys registered there. It should increment as you toggle the GPIO.
Hello Rajat Jain, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32487
to look at the new patch set (#6).
Change subject: mainboard/google/hatch: Enable PEN_EJECT_L as wake & notify source. ......................................................................
mainboard/google/hatch: Enable PEN_EJECT_L as wake & notify source.
Updated GPP_A8 to be a GPI and SCI source, to support both wake and notifications.
BUG=b:128941098 BRANCH=none TEST=Compiles, simulated pen eject with PCH_INT_L signal. Both evtest and waking from s0ix confirm this works. The output of /proc/interrupts confirms the correct interrupt is triggered.
Change-Id: I080fb3cbfb3e2f55209ca31824b00ca820d70f78 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/mainboard/google/hatch/variants/baseboard/gpio.c M src/mainboard/google/hatch/variants/hatch/overridetree.cb M src/mainboard/google/hatch/variants/hatch_whl/overridetree.cb 3 files changed, 8 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/32487/6
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32487 )
Change subject: mainboard/google/hatch: Enable PEN_EJECT_L as wake & notify source. ......................................................................
Patch Set 6: Code-Review+2
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32487 )
Change subject: mainboard/google/hatch: Enable PEN_EJECT_L as wake & notify source. ......................................................................
mainboard/google/hatch: Enable PEN_EJECT_L as wake & notify source.
Updated GPP_A8 to be a GPI and SCI source, to support both wake and notifications.
BUG=b:128941098 BRANCH=none TEST=Compiles, simulated pen eject with PCH_INT_L signal. Both evtest and waking from s0ix confirm this works. The output of /proc/interrupts confirms the correct interrupt is triggered.
Change-Id: I080fb3cbfb3e2f55209ca31824b00ca820d70f78 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/32487 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com --- M src/mainboard/google/hatch/variants/baseboard/gpio.c M src/mainboard/google/hatch/variants/hatch/overridetree.cb M src/mainboard/google/hatch/variants/hatch_whl/overridetree.cb 3 files changed, 8 insertions(+), 4 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/mainboard/google/hatch/variants/baseboard/gpio.c b/src/mainboard/google/hatch/variants/baseboard/gpio.c index 198d930..32526cc 100644 --- a/src/mainboard/google/hatch/variants/baseboard/gpio.c +++ b/src/mainboard/google/hatch/variants/baseboard/gpio.c @@ -30,8 +30,8 @@ PAD_CFG_GPI_INT(GPP_A6, NONE, PLTRST, LEVEL), /* A7 : PP3300_SOC_A */ PAD_NC(GPP_A7, NONE), - /* A8 : EMR_GARAGE_DET */ - PAD_CFG_GPI_GPIO_DRIVER(GPP_A8, NONE, DEEP), + /* A8 : PEN_GARAGE_DET_L */ + PAD_CFG_GPI_GPIO_DRIVER_SCI(GPP_A8, NONE, DEEP, LEVEL, NONE), /* A9 : ESPI_CLK */ /* A10 : FPMCU_PCH_BOOT1 */ PAD_CFG_GPO(GPP_A10, 0, DEEP), diff --git a/src/mainboard/google/hatch/variants/hatch/overridetree.cb b/src/mainboard/google/hatch/variants/hatch/overridetree.cb index 4b1b8d8..42752e6 100644 --- a/src/mainboard/google/hatch/variants/hatch/overridetree.cb +++ b/src/mainboard/google/hatch/variants/hatch/overridetree.cb @@ -86,7 +86,9 @@ end chip drivers/generic/gpio_keys register "name" = ""PENH"" - register "gpio" = "ACPI_GPIO_INPUT_ACTIVE_HIGH(GPP_A8)" + register "gpio" = "ACPI_GPIO_IRQ_EDGE_BOTH(GPP_A8)" + register "key.wake" = "GPE0_DW0_08" + register "key.wakeup_event_action" = "EV_ACT_ASSERTED" register "key.dev_name" = ""EJCT"" register "key.linux_code" = "SW_PEN_INSERTED" register "key.linux_input_type" = "EV_SW" diff --git a/src/mainboard/google/hatch/variants/hatch_whl/overridetree.cb b/src/mainboard/google/hatch/variants/hatch_whl/overridetree.cb index c5d5964..115a513 100644 --- a/src/mainboard/google/hatch/variants/hatch_whl/overridetree.cb +++ b/src/mainboard/google/hatch/variants/hatch_whl/overridetree.cb @@ -71,7 +71,9 @@ end chip drivers/generic/gpio_keys register "name" = ""PENH"" - register "gpio" = "ACPI_GPIO_INPUT_ACTIVE_HIGH(GPP_A8)" + register "gpio" = "ACPI_GPIO_IRQ_EDGE_BOTH(GPP_A8)" + register "key.wake" = "GPE0_DW0_08" + register "key.wakeup_event_action" = "EV_ACT_ASSERTED" register "key.dev_name" = ""EJCT"" register "key.linux_code" = "SW_PEN_INSERTED" register "key.linux_input_type" = "EV_SW"
-
Furquan Shaikh (Code Review) -
Patrick Georgi (Code Review)
-
Tim Wawrzynczak (Code Review)