Change in coreboot[master]: Revert "mb/google/zork: Don't expose reset GPIO for touchscreen to OS"
Hello build bot (Jenkins), Raul Rangel, Furquan Shaikh, Paul Menzel, Aaron Durbin,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/43078
to review the following change.
Change subject: Revert "mb/google/zork: Don't expose reset GPIO for touchscreen to OS" ......................................................................
Revert "mb/google/zork: Don't expose reset GPIO for touchscreen to OS"
This reverts commit 728c0787f2ad742da287791bf8f606b7d5921b0b.
Reason for revert: This patch exposes a bug which leads to an invalid opcode trap in the touchscreen code. Reverting this gets the system working again, but is not a long-term solution.
BUG=b:162596241 TEST=System boots to login screen.
Change-Id: I57a070d94f961cec43834c8bedd5dafc8a54171a Signed-off-by: Martin Roth martinroth@google.com --- M src/mainboard/google/zork/variants/berknip/overridetree.cb M src/mainboard/google/zork/variants/dalboz/overridetree.cb M src/mainboard/google/zork/variants/ezkinil/overridetree.cb M src/mainboard/google/zork/variants/trembyle/overridetree.cb 4 files changed, 4 insertions(+), 16 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/78/43078/1
diff --git a/src/mainboard/google/zork/variants/berknip/overridetree.cb b/src/mainboard/google/zork/variants/berknip/overridetree.cb index 31bf643..ead39cb8 100644 --- a/src/mainboard/google/zork/variants/berknip/overridetree.cb +++ b/src/mainboard/google/zork/variants/berknip/overridetree.cb @@ -66,11 +66,8 @@ register "probed" = "1" register "irq_gpio" = "ACPI_GPIO_IRQ_EDGE_LOW(GPIO_12)" register "reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPIO_140)" - # 32ms: Rise time of the reset line - # 20ms: Firmware ready time - register "reset_delay_ms" = "32 + 20" + register "reset_delay_ms" = "20" register "has_power_resource" = "1" - register "disable_gpio_export_in_crs" = "1" device i2c 39 on end end chip drivers/i2c/generic diff --git a/src/mainboard/google/zork/variants/dalboz/overridetree.cb b/src/mainboard/google/zork/variants/dalboz/overridetree.cb index 9fe15c5..bcfff1f 100644 --- a/src/mainboard/google/zork/variants/dalboz/overridetree.cb +++ b/src/mainboard/google/zork/variants/dalboz/overridetree.cb @@ -49,11 +49,8 @@ register "probed" = "1" register "irq_gpio" = "ACPI_GPIO_IRQ_EDGE_LOW(GPIO_12)" register "reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPIO_140)" - # 32ms: Rise time of the reset line - # 20ms: Firmware ready time - register "reset_delay_ms" = "32 + 20" + register "reset_delay_ms" = "20" register "has_power_resource" = "1" - register "disable_gpio_export_in_crs" = "1" device i2c 39 on end end chip drivers/i2c/generic diff --git a/src/mainboard/google/zork/variants/ezkinil/overridetree.cb b/src/mainboard/google/zork/variants/ezkinil/overridetree.cb index 5579015..a4cdac6 100644 --- a/src/mainboard/google/zork/variants/ezkinil/overridetree.cb +++ b/src/mainboard/google/zork/variants/ezkinil/overridetree.cb @@ -68,11 +68,8 @@ register "probed" = "1" register "irq_gpio" = "ACPI_GPIO_IRQ_EDGE_LOW(GPIO_12)" register "reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPIO_140)" - # 32ms: Rise time of the reset line - # 20ms: Firmware ready time - register "reset_delay_ms" = "32 + 20" + register "reset_delay_ms" = "20" register "has_power_resource" = "1" - register "disable_gpio_export_in_crs" = "1" device i2c 39 on end end chip drivers/i2c/hid diff --git a/src/mainboard/google/zork/variants/trembyle/overridetree.cb b/src/mainboard/google/zork/variants/trembyle/overridetree.cb index bead293..4cd53ee 100644 --- a/src/mainboard/google/zork/variants/trembyle/overridetree.cb +++ b/src/mainboard/google/zork/variants/trembyle/overridetree.cb @@ -66,11 +66,8 @@ register "probed" = "1" register "irq_gpio" = "ACPI_GPIO_IRQ_EDGE_LOW(GPIO_12)" register "reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPIO_140)" - # 32ms: Rise time of the reset line - # 20ms: Firmware ready time - register "reset_delay_ms" = "32 + 20" + register "reset_delay_ms" = "20" register "has_power_resource" = "1" - register "disable_gpio_export_in_crs" = "1" device i2c 39 on end end chip drivers/i2c/generic
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43078 )
Change subject: Revert "mb/google/zork: Don't expose reset GPIO for touchscreen to OS" ......................................................................
Patch Set 1: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43078 )
Change subject: Revert "mb/google/zork: Don't expose reset GPIO for touchscreen to OS" ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/43078/1/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/berknip/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/43078/1/src/mainboard/google/zork/v... PS1, Line 71: 32 + 20 Should we keep the delay as is?
Hello build bot (Jenkins), Raul Rangel, Furquan Shaikh, Marshall Dawson, Paul Menzel, Rob Barnes, Eric Peers, Aaron Durbin, Felix Held,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43078
to look at the new patch set (#2).
Change subject: mb/google/zork: Revert Don't expose reset GPIO for touchscreen to OS ......................................................................
mb/google/zork: Revert Don't expose reset GPIO for touchscreen to OS
This reverts the code from commit 728c0787f2 that removes the reset GPIO from the touchscreen ACPI interface.
That patch exposes a bug which leads to an invalid opcode trap in the touchscreen code. Reverting this gets the system working again, but is not a long-term solution.
BUG=b:162596241 TEST=System boots to login screen.
Change-Id: I57a070d94f961cec43834c8bedd5dafc8a54171a Signed-off-by: Martin Roth martinroth@google.com --- M src/mainboard/google/zork/variants/berknip/overridetree.cb M src/mainboard/google/zork/variants/dalboz/overridetree.cb M src/mainboard/google/zork/variants/ezkinil/overridetree.cb M src/mainboard/google/zork/variants/trembyle/overridetree.cb 4 files changed, 0 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/78/43078/2
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43078 )
Change subject: mb/google/zork: Revert Don't expose reset GPIO for touchscreen to OS ......................................................................
Patch Set 2:
(1 comment)
Updated the commit message as well since it's only a partial revert.
https://review.coreboot.org/c/coreboot/+/43078/1/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/berknip/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/43078/1/src/mainboard/google/zork/v... PS1, Line 71: 32 + 20
Should we keep the delay as is?
Done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43078 )
Change subject: mb/google/zork: Revert Don't expose reset GPIO for touchscreen to OS ......................................................................
Patch Set 2: Code-Review+2
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43078 )
Change subject: mb/google/zork: Revert Don't expose reset GPIO for touchscreen to OS ......................................................................
Patch Set 2: Code-Review+2
Aaron Durbin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/43078 )
Change subject: mb/google/zork: Revert Don't expose reset GPIO for touchscreen to OS ......................................................................
mb/google/zork: Revert Don't expose reset GPIO for touchscreen to OS
This reverts the code from commit 728c0787f2 that removes the reset GPIO from the touchscreen ACPI interface.
That patch exposes a bug which leads to an invalid opcode trap in the touchscreen code. Reverting this gets the system working again, but is not a long-term solution.
BUG=b:162596241 TEST=System boots to login screen.
Change-Id: I57a070d94f961cec43834c8bedd5dafc8a54171a Signed-off-by: Martin Roth martinroth@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/43078 Reviewed-by: Furquan Shaikh furquan@google.com Reviewed-by: Aaron Durbin adurbin@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/google/zork/variants/berknip/overridetree.cb M src/mainboard/google/zork/variants/dalboz/overridetree.cb M src/mainboard/google/zork/variants/ezkinil/overridetree.cb M src/mainboard/google/zork/variants/trembyle/overridetree.cb 4 files changed, 0 insertions(+), 4 deletions(-)
Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved Furquan Shaikh: Looks good to me, approved
diff --git a/src/mainboard/google/zork/variants/berknip/overridetree.cb b/src/mainboard/google/zork/variants/berknip/overridetree.cb index 58f5844..a469fb9 100644 --- a/src/mainboard/google/zork/variants/berknip/overridetree.cb +++ b/src/mainboard/google/zork/variants/berknip/overridetree.cb @@ -78,7 +78,6 @@ # 20ms: Firmware ready time register "reset_delay_ms" = "32 + 20" register "has_power_resource" = "1" - register "disable_gpio_export_in_crs" = "1" device i2c 39 on end end chip drivers/i2c/generic diff --git a/src/mainboard/google/zork/variants/dalboz/overridetree.cb b/src/mainboard/google/zork/variants/dalboz/overridetree.cb index 8735a06..9148936 100644 --- a/src/mainboard/google/zork/variants/dalboz/overridetree.cb +++ b/src/mainboard/google/zork/variants/dalboz/overridetree.cb @@ -53,7 +53,6 @@ # 20ms: Firmware ready time register "reset_delay_ms" = "32 + 20" register "has_power_resource" = "1" - register "disable_gpio_export_in_crs" = "1" device i2c 39 on end end chip drivers/i2c/generic diff --git a/src/mainboard/google/zork/variants/ezkinil/overridetree.cb b/src/mainboard/google/zork/variants/ezkinil/overridetree.cb index 6a88b78..6b4331f 100644 --- a/src/mainboard/google/zork/variants/ezkinil/overridetree.cb +++ b/src/mainboard/google/zork/variants/ezkinil/overridetree.cb @@ -80,7 +80,6 @@ # 20ms: Firmware ready time register "reset_delay_ms" = "32 + 20" register "has_power_resource" = "1" - register "disable_gpio_export_in_crs" = "1" device i2c 39 on end end chip drivers/i2c/hid diff --git a/src/mainboard/google/zork/variants/trembyle/overridetree.cb b/src/mainboard/google/zork/variants/trembyle/overridetree.cb index 9e7660b..fb922b9 100644 --- a/src/mainboard/google/zork/variants/trembyle/overridetree.cb +++ b/src/mainboard/google/zork/variants/trembyle/overridetree.cb @@ -76,7 +76,6 @@ # 20ms: Firmware ready time register "reset_delay_ms" = "32 + 20" register "has_power_resource" = "1" - register "disable_gpio_export_in_crs" = "1" device i2c 39 on end end chip drivers/i2c/generic
Furquan Shaikh has created a revert of this change. ( https://review.coreboot.org/c/coreboot/+/43078 )
Change subject: mb/google/zork: Revert Don't expose reset GPIO for touchscreen to OS ......................................................................
-
Aaron Durbin (Code Review) -
Furquan Shaikh (Code Review)
-
Martin Roth (Code Review)