Frank Wu has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33808
Change subject: mb/google/hatch: Set GPP_D9 as enable pin for Goodix Touch Screen ......................................................................
mb/google/hatch: Set GPP_D9 as enable pin for Goodix Touch Screen
Goodix touchscreen cannot work in normal mode because PP3300_TOUCHSCREEN_DX dropped. Configure GPP_D9 as enable pin in the devicetree.cb to fix the power sequence.
BUG=b:135287161 BRANCH=None TEST=local build and measure sequence with Goodix touch screen
Change-Id: I33140869990aa4715c780b0fa322921e450530ef Signed-off-by: Frank Wu frank_wu@compal.corp-partner.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, 6 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/33808/1
diff --git a/src/mainboard/google/hatch/variants/baseboard/gpio.c b/src/mainboard/google/hatch/variants/baseboard/gpio.c index e309175..e92d93b 100644 --- a/src/mainboard/google/hatch/variants/baseboard/gpio.c +++ b/src/mainboard/google/hatch/variants/baseboard/gpio.c @@ -195,8 +195,8 @@ PAD_NC(GPP_D7, NONE), /* D8 : WWAN_CONFIG_3 */ PAD_NC(GPP_D8, NONE), - /* D9 : GPP_D9 ==> NC */ - PAD_NC(GPP_D9, NONE), + /* D9 : GPP_D9 ==> EN_PP3300_DX_TOUCHSCREEN */ + PAD_CFG_GPO(GPP_D9, 0, DEEP), /* D10 : GPP_D10 ==> NC */ PAD_NC(GPP_D10, NONE), /* D11 : GPP_D11 ==> NC */ diff --git a/src/mainboard/google/hatch/variants/hatch/overridetree.cb b/src/mainboard/google/hatch/variants/hatch/overridetree.cb index d676843..fd2d061 100644 --- a/src/mainboard/google/hatch/variants/hatch/overridetree.cb +++ b/src/mainboard/google/hatch/variants/hatch/overridetree.cb @@ -91,6 +91,8 @@ "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_D15)" register "generic.reset_delay_ms" = "10" register "generic.reset_off_delay_ms" = "1" + register "generic.enable_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPP_D9)" + register "generic.enable_delay_ms" = "10" register "generic.has_power_resource" = "1" register "hid_desc_reg_offset" = "0x01" device i2c 5d on end diff --git a/src/mainboard/google/hatch/variants/hatch_whl/overridetree.cb b/src/mainboard/google/hatch/variants/hatch_whl/overridetree.cb index 373438c..c3a1b2e 100644 --- a/src/mainboard/google/hatch/variants/hatch_whl/overridetree.cb +++ b/src/mainboard/google/hatch/variants/hatch_whl/overridetree.cb @@ -82,6 +82,8 @@ "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_D15)" register "generic.reset_delay_ms" = "10" register "generic.reset_off_delay_ms" = "1" + register "generic.enable_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPP_D9)" + register "generic.enable_delay_ms" = "10" register "generic.has_power_resource" = "1" register "hid_desc_reg_offset" = "0x01" device i2c 5d on end
Philip Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33808 )
Change subject: mb/google/hatch: Set GPP_D9 as enable pin for Goodix Touch Screen ......................................................................
Patch Set 1: Code-Review+1
Philip Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33808 )
Change subject: mb/google/hatch: Set GPP_D9 as enable pin for Goodix Touch Screen ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/33808/1/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/33808/1/src/mainboard/google/hatch/variants/... PS1, Line 199: 0 Do we need the internal pull-down mentioned in the bug link?
Frank Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33808 )
Change subject: mb/google/hatch: Set GPP_D9 as enable pin for Goodix Touch Screen ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/33808/1/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/33808/1/src/mainboard/google/hatch/variants/... PS1, Line 199: 0
Do we need the internal pull-down mentioned in the bug link?
I provide local build GPP_D9 fw with/without PU_DOWN_20K and our HW team is measuring the waveform. The status is also updated in issue tracker.
Hello EricR Lai, Paul Fagerburg, Philip Chen, Tim Wawrzynczak, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33808
to look at the new patch set (#2).
Change subject: mb/google/hatch: Set GPP_D9 as enable pin for Goodix Touch Screen ......................................................................
mb/google/hatch: Set GPP_D9 as enable pin for Goodix Touch Screen
Goodix touchscreen cannot work in normal mode because PP3300_TOUCHSCREEN_DX dropped. Configure GPP_D9 as enable pin in the devicetree.cb to fix the power sequence.
BUG=b:135287161 BRANCH=None TEST=local build and measure sequence with Goodix touch screen
Change-Id: I33140869990aa4715c780b0fa322921e450530ef Signed-off-by: Frank Wu frank_wu@compal.corp-partner.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(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/33808/2
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33808 )
Change subject: mb/google/hatch: Set GPP_D9 as enable pin for Goodix Touch Screen ......................................................................
Patch Set 2: Code-Review+2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33808 )
Change subject: mb/google/hatch: Set GPP_D9 as enable pin for Goodix Touch Screen ......................................................................
Patch Set 2: Code-Review+1
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33808 )
Change subject: mb/google/hatch: Set GPP_D9 as enable pin for Goodix Touch Screen ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/33808/2/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/hatch_whl/overridetree.cb:
https://review.coreboot.org/#/c/33808/2/src/mainboard/google/hatch/variants/... PS2, Line 84: 3 Why did this change?
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33808 )
Change subject: mb/google/hatch: Set GPP_D9 as enable pin for Goodix Touch Screen ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/33808/2/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/hatch_whl/overridetree.cb:
https://review.coreboot.org/#/c/33808/2/src/mainboard/google/hatch/variants/... PS2, Line 84: 3
Why did this change?
if we set 1ms, the scope capture is under 1ms. so increase to 3. This is follow octopus setting.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33808 )
Change subject: mb/google/hatch: Set GPP_D9 as enable pin for Goodix Touch Screen ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/33808/2/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/hatch_whl/overridetree.cb:
https://review.coreboot.org/#/c/33808/2/src/mainboard/google/hatch/variants/... PS2, Line 84: 3
if we set 1ms, the scope capture is under 1ms. so increase to 3. This is follow octopus setting.
Please update commit message to capture this change as well.
EricR Lai has uploaded a new patch set (#3) to the change originally created by Frank Wu. ( https://review.coreboot.org/c/coreboot/+/33808 )
Change subject: mb/google/hatch: Set GPP_D9 as enable pin for Goodix Touch Screen and increase reset off delay time ......................................................................
mb/google/hatch: Set GPP_D9 as enable pin for Goodix Touch Screen and increase reset off delay time
Goodix touchscreen cannot work in normal mode because PP3300_TOUCHSCREEN_DX dropped. Configure GPP_D9 as enable pin in the devicetree.cb to fix the power sequence. Increase reset_off_delay time from 1ms to 3ms to met the HW requirement
BUG=b:135287161 BRANCH=None TEST=local build and measure sequence with Goodix touch screen
Change-Id: I33140869990aa4715c780b0fa322921e450530ef Signed-off-by: Frank Wu frank_wu@compal.corp-partner.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(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/33808/3
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33808 )
Change subject: mb/google/hatch: Set GPP_D9 as enable pin for Goodix Touch Screen and increase reset off delay time ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/#/c/33808/2/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/hatch_whl/overridetree.cb:
https://review.coreboot.org/#/c/33808/2/src/mainboard/google/hatch/variants/... PS2, Line 84: 3
Please update commit message to capture this change as well.
Done
EricR Lai has uploaded a new patch set (#4) to the change originally created by Frank Wu. ( https://review.coreboot.org/c/coreboot/+/33808 )
Change subject: mb/google/hatch: Set GPP_D9 as enable pin for Goodix Touch Screen and increase reset off delay time ......................................................................
mb/google/hatch: Set GPP_D9 as enable pin for Goodix Touch Screen and increase reset off delay time
Goodix touchscreen cannot work in normal mode because PP3300_TOUCHSCREEN_DX dropped. Configure GPP_D9 as enable pin in the devicetree.cb to fix the power sequence. Increase reset_off_delay time from 1ms to 3ms to met the HW requirement.
BUG=b:135287161 BRANCH=None TEST=local build and measure sequence with Goodix touch screen
Change-Id: I33140869990aa4715c780b0fa322921e450530ef Signed-off-by: Frank Wu frank_wu@compal.corp-partner.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(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/33808/4
Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33808 )
Change subject: mb/google/hatch: Set GPP_D9 as enable pin for Goodix Touch Screen and increase reset off delay time ......................................................................
Patch Set 4: Code-Review+1
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33808 )
Change subject: mb/google/hatch: Set GPP_D9 as enable pin for Goodix Touch Screen and increase reset off delay time ......................................................................
Patch Set 4: Code-Review+1
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33808 )
Change subject: mb/google/hatch: Set GPP_D9 as enable pin for Goodix Touch Screen and increase reset off delay time ......................................................................
Patch Set 4: Code-Review+2
Frank Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33808 )
Change subject: mb/google/hatch: Set GPP_D9 as enable pin for Goodix Touch Screen and increase reset off delay time ......................................................................
Patch Set 4:
Patch Set 4: Code-Review+2
Furquan@,
Could you help merge the CL for touchscreen solution? Thank you.
Furquan Shaikh has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/33808 )
Change subject: mb/google/hatch: Set GPP_D9 as enable pin for Goodix Touch Screen and increase reset off delay time ......................................................................
mb/google/hatch: Set GPP_D9 as enable pin for Goodix Touch Screen and increase reset off delay time
Goodix touchscreen cannot work in normal mode because PP3300_TOUCHSCREEN_DX dropped. Configure GPP_D9 as enable pin in the devicetree.cb to fix the power sequence. Increase reset_off_delay time from 1ms to 3ms to met the HW requirement.
BUG=b:135287161 BRANCH=None TEST=local build and measure sequence with Goodix touch screen
Change-Id: I33140869990aa4715c780b0fa322921e450530ef Signed-off-by: Frank Wu frank_wu@compal.corp-partner.google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/33808 Reviewed-by: Paul Fagerburg pfagerburg@chromium.org Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org Reviewed-by: Furquan Shaikh furquan@google.com Reviewed-by: EricR Lai ericr_lai@compal.corp-partner.google.com Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net Tested-by: build bot (Jenkins) no-reply@coreboot.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(-)
Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Furquan Shaikh: Looks good to me, approved EricR Lai: Looks good to me, approved Tim Wawrzynczak: Looks good to me, but someone else must approve Paul Fagerburg: Looks good to me, but someone else must approve
diff --git a/src/mainboard/google/hatch/variants/baseboard/gpio.c b/src/mainboard/google/hatch/variants/baseboard/gpio.c index 528ab02..1da3d75 100644 --- a/src/mainboard/google/hatch/variants/baseboard/gpio.c +++ b/src/mainboard/google/hatch/variants/baseboard/gpio.c @@ -181,8 +181,8 @@ PAD_NC(GPP_D7, NONE), /* D8 : WWAN_CONFIG_3 */ PAD_NC(GPP_D8, NONE), - /* D9 : GPP_D9 ==> NC */ - PAD_NC(GPP_D9, NONE), + /* D9 : GPP_D9 ==> EN_PP3300_DX_TOUCHSCREEN */ + PAD_CFG_GPO(GPP_D9, 0, DEEP), /* D10 : GPP_D10 ==> NC */ PAD_NC(GPP_D10, NONE), /* D11 : GPP_D11 ==> NC */ diff --git a/src/mainboard/google/hatch/variants/hatch/overridetree.cb b/src/mainboard/google/hatch/variants/hatch/overridetree.cb index 2582940..18878ce 100644 --- a/src/mainboard/google/hatch/variants/hatch/overridetree.cb +++ b/src/mainboard/google/hatch/variants/hatch/overridetree.cb @@ -90,7 +90,9 @@ register "generic.reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_D15)" register "generic.reset_delay_ms" = "10" - register "generic.reset_off_delay_ms" = "1" + register "generic.reset_off_delay_ms" = "3" + register "generic.enable_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPP_D9)" + register "generic.enable_delay_ms" = "12" register "generic.has_power_resource" = "1" register "hid_desc_reg_offset" = "0x01" device i2c 5d on end diff --git a/src/mainboard/google/hatch/variants/hatch_whl/overridetree.cb b/src/mainboard/google/hatch/variants/hatch_whl/overridetree.cb index 373438c..0321867 100644 --- a/src/mainboard/google/hatch/variants/hatch_whl/overridetree.cb +++ b/src/mainboard/google/hatch/variants/hatch_whl/overridetree.cb @@ -81,7 +81,9 @@ register "generic.reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_D15)" register "generic.reset_delay_ms" = "10" - register "generic.reset_off_delay_ms" = "1" + register "generic.reset_off_delay_ms" = "3" + register "generic.enable_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPP_D9)" + register "generic.enable_delay_ms" = "12" register "generic.has_power_resource" = "1" register "hid_desc_reg_offset" = "0x01" device i2c 5d on end