Change in ...coreboot[master]: drivers/gpio_keys: Remove redundant is_wakeup_source flag
Hello Karthikeyan Ramasubramanian,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/31258
to review the following change.
Change subject: drivers/gpio_keys: Remove redundant is_wakeup_source flag ......................................................................
drivers/gpio_keys: Remove redundant is_wakeup_source flag
"is_wakeup_source" flag is used to indicate if the concerned device can trigger a wakeup. This flag is redundant with the "wake" GPE event definition. So remove the redundant flag and use the "wake" GPE event to mark the wakeup source.
BUG=None BRANCH=None TEST=Boot to ChromeOS. Ensure that the device is marked as wakeup-source in SSDT if wake GPE is configured. Ensure that the system can suspend and the device acts as a wakeup source
Change-Id: I99237323639df1cb72e3a81bcfed869900a2eefa Signed-off-by: Karthikeyan Ramasubramanian kramasub@google.com --- M src/drivers/generic/gpio_keys/chip.h M src/drivers/generic/gpio_keys/gpio_keys.c M src/mainboard/google/octopus/variants/bobba/overridetree.cb 3 files changed, 1 insertion(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/58/31258/1
diff --git a/src/drivers/generic/gpio_keys/chip.h b/src/drivers/generic/gpio_keys/chip.h index 9d33b4f..08acfeb 100644 --- a/src/drivers/generic/gpio_keys/chip.h +++ b/src/drivers/generic/gpio_keys/chip.h @@ -50,8 +50,6 @@ uint32_t linux_input_type; /* Descriptive name of the key */ const char *label; - /* Can this key wake-up the system? */ - bool is_wakeup_source; /* Wake GPE */ unsigned int wake; /* Trigger for Wakeup Event Action as defined in EV_ACT_* enum */ diff --git a/src/drivers/generic/gpio_keys/gpio_keys.c b/src/drivers/generic/gpio_keys/gpio_keys.c index f9869d6..6e18a8a 100644 --- a/src/drivers/generic/gpio_keys/gpio_keys.c +++ b/src/drivers/generic/gpio_keys/gpio_keys.c @@ -40,10 +40,8 @@ key->linux_input_type); if (key->label) acpi_dp_add_string(dsd, "label", key->label); - if (key->is_wakeup_source) - acpi_dp_add_integer(dsd, "wakeup-source", - key->is_wakeup_source); if (key->wake) { + acpi_dp_add_integer(dsd, "wakeup-source", 1); acpigen_write_PRW(key->wake, 3); acpi_dp_add_integer(dsd, "wakeup-event-action", key->wakeup_event_action); diff --git a/src/mainboard/google/octopus/variants/bobba/overridetree.cb b/src/mainboard/google/octopus/variants/bobba/overridetree.cb index cd26fab..0230a29 100644 --- a/src/mainboard/google/octopus/variants/bobba/overridetree.cb +++ b/src/mainboard/google/octopus/variants/bobba/overridetree.cb @@ -104,7 +104,6 @@ register "key.linux_code" = "SW_PEN_INSERTED" register "key.linux_input_type" = "EV_SW" register "key.label" = ""pen_eject"" - register "key.is_wakeup_source" = "1" register "key.wake" = "GPE0_DW2_04" register "key.wakeup_event_action" = "EV_ACT_DEASSERTED" device generic 0 on end
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31258 )
Change subject: drivers/gpio_keys: Remove redundant is_wakeup_source flag ......................................................................
Patch Set 1: Code-Review+2
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31258 )
Change subject: drivers/gpio_keys: Remove redundant is_wakeup_source flag ......................................................................
drivers/gpio_keys: Remove redundant is_wakeup_source flag
"is_wakeup_source" flag is used to indicate if the concerned device can trigger a wakeup. This flag is redundant with the "wake" GPE event definition. So remove the redundant flag and use the "wake" GPE event to mark the wakeup source.
BUG=None BRANCH=None TEST=Boot to ChromeOS. Ensure that the device is marked as wakeup-source in SSDT if wake GPE is configured. Ensure that the system can suspend and the device acts as a wakeup source
Change-Id: I99237323639df1cb72e3a81bcfed869900a2eefa Signed-off-by: Karthikeyan Ramasubramanian kramasub@google.com Reviewed-on: https://review.coreboot.org/c/31258 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com --- M src/drivers/generic/gpio_keys/chip.h M src/drivers/generic/gpio_keys/gpio_keys.c M src/mainboard/google/octopus/variants/bobba/overridetree.cb 3 files changed, 1 insertion(+), 6 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/drivers/generic/gpio_keys/chip.h b/src/drivers/generic/gpio_keys/chip.h index 9d33b4f..08acfeb 100644 --- a/src/drivers/generic/gpio_keys/chip.h +++ b/src/drivers/generic/gpio_keys/chip.h @@ -50,8 +50,6 @@ uint32_t linux_input_type; /* Descriptive name of the key */ const char *label; - /* Can this key wake-up the system? */ - bool is_wakeup_source; /* Wake GPE */ unsigned int wake; /* Trigger for Wakeup Event Action as defined in EV_ACT_* enum */ diff --git a/src/drivers/generic/gpio_keys/gpio_keys.c b/src/drivers/generic/gpio_keys/gpio_keys.c index f9869d6..6e18a8a 100644 --- a/src/drivers/generic/gpio_keys/gpio_keys.c +++ b/src/drivers/generic/gpio_keys/gpio_keys.c @@ -40,10 +40,8 @@ key->linux_input_type); if (key->label) acpi_dp_add_string(dsd, "label", key->label); - if (key->is_wakeup_source) - acpi_dp_add_integer(dsd, "wakeup-source", - key->is_wakeup_source); if (key->wake) { + acpi_dp_add_integer(dsd, "wakeup-source", 1); acpigen_write_PRW(key->wake, 3); acpi_dp_add_integer(dsd, "wakeup-event-action", key->wakeup_event_action); diff --git a/src/mainboard/google/octopus/variants/bobba/overridetree.cb b/src/mainboard/google/octopus/variants/bobba/overridetree.cb index cd26fab..0230a29 100644 --- a/src/mainboard/google/octopus/variants/bobba/overridetree.cb +++ b/src/mainboard/google/octopus/variants/bobba/overridetree.cb @@ -104,7 +104,6 @@ register "key.linux_code" = "SW_PEN_INSERTED" register "key.linux_input_type" = "EV_SW" register "key.label" = ""pen_eject"" - register "key.is_wakeup_source" = "1" register "key.wake" = "GPE0_DW2_04" register "key.wakeup_event_action" = "EV_ACT_DEASSERTED" device generic 0 on end
-
Furquan Shaikh (Code Review) -
Karthik Ramasubramanian (Code Review)
-
Patrick Georgi (Code Review)