Hello Tim Chen,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/47433
to review the following change.
Change subject: mb/google/dedede/var/metaknight: Add touchscreen settings ......................................................................
mb/google/dedede/var/metaknight: Add touchscreen settings
Add Elan and Goodix touchscreen settings.
BUG=b:169813211 BRANCH=None TEST=build metaknight firmware
Change-Id: Ib2acd31a8076533c3b927d37127e7d27bac0bb57 --- M src/mainboard/google/dedede/variants/metaknight/overridetree.cb 1 file changed, 35 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/47433/1
diff --git a/src/mainboard/google/dedede/variants/metaknight/overridetree.cb b/src/mainboard/google/dedede/variants/metaknight/overridetree.cb index 556a44f..25bc866 100644 --- a/src/mainboard/google/dedede/variants/metaknight/overridetree.cb +++ b/src/mainboard/google/dedede/variants/metaknight/overridetree.cb @@ -44,6 +44,41 @@
device domain 0 on device pci 15.0 on end + device pci 15.2 on + chip drivers/i2c/hid + register "generic.hid" = ""GDIX0000"" + register "generic.desc" = ""Goodix Touchscreen"" + register "generic.irq" = "ACPI_IRQ_LEVEL_LOW(GPP_D4_IRQ)" + register "generic.probed" = "1" + register "generic.reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_D5)" + register "generic.reset_delay_ms" = "120" + register "generic.reset_off_delay_ms" = "2" + register "generic.enable_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPP_D6)" + register "generic.enable_delay_ms" = "12" + register "generic.has_power_resource" = "1" + register "generic.disable_gpio_export_in_crs" = "1" + register "hid_desc_reg_offset" = "0x01" + device i2c 0x5d on end + end + chip drivers/i2c/hid + register "generic.hid" = ""ELAN6915"" + register "generic.desc" = ""ELAN Touchscreen"" + register "generic.irq" = "ACPI_IRQ_LEVEL_LOW(GPP_D4_IRQ)" + register "generic.probed" = "1" + register "generic.reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_D5)" + register "generic.reset_delay_ms" = "20" + register "generic.reset_off_delay_ms" = "2" + register "generic.stop_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_A11)" + register "generic.stop_delay_ms" = "280" + register "generic.stop_off_delay_ms" = "2" + register "generic.enable_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPP_D6)" + register "generic.enable_delay_ms" = "1" + register "generic.has_power_resource" = "1" + register "generic.disable_gpio_export_in_crs" = "1" + register "hid_desc_reg_offset" = "0x01" + device i2c 15 on end + end + end # I2C 2 device pci 15.3 off end # I2C 3 end end
Hello build bot (Jenkins), Tim Chen,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47433
to look at the new patch set (#2).
Change subject: mb/google/dedede/var/metaknight: Add touchscreen settings ......................................................................
mb/google/dedede/var/metaknight: Add touchscreen settings
Add Elan and Goodix touchscreen settings.
BUG=b:169813211 BRANCH=None TEST=build metaknight firmware
Change-Id: Ib2acd31a8076533c3b927d37127e7d27bac0bb57 Signed-off-by: Tim Chen tim-chen@quanta.corp-partner.google.com --- M src/mainboard/google/dedede/variants/metaknight/overridetree.cb 1 file changed, 35 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/47433/2
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47433 )
Change subject: mb/google/dedede/var/metaknight: Add touchscreen settings ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47433/2/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/metaknight/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/47433/2/src/mainboard/google/dedede... PS2, Line 59: generic.disable_gpio_export_in_crs" = "1" Any specific reason to disable exporting GPIO in _CRS?
Tim Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47433 )
Change subject: mb/google/dedede/var/metaknight: Add touchscreen settings ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47433/2/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/metaknight/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/47433/2/src/mainboard/google/dedede... PS2, Line 59: generic.disable_gpio_export_in_crs" = "1"
Any specific reason to disable exporting GPIO in _CRS?
I referenced from project mogolor, https://chromium.googlesource.com/chromiumos/third_party/coreboot/+/refs/hea... .
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47433 )
Change subject: mb/google/dedede/var/metaknight: Add touchscreen settings ......................................................................
Patch Set 2: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/47433/2/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/metaknight/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/47433/2/src/mainboard/google/dedede... PS2, Line 59: generic.disable_gpio_export_in_crs" = "1"
I referenced from project mogolor, https://chromium.googlesource. […]
Ack
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47433 )
Change subject: mb/google/dedede/var/metaknight: Add touchscreen settings ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47433/2/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/metaknight/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/47433/2/src/mainboard/google/dedede... PS2, Line 59: generic.disable_gpio_export_in_crs" = "1"
Ack
FYI, this is only required for Elan screens. It's because of a quirk in the Elan driver in the kernel.
Tim Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47433 )
Change subject: mb/google/dedede/var/metaknight: Add touchscreen settings ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47433/2/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/metaknight/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/47433/2/src/mainboard/google/dedede... PS2, Line 59: generic.disable_gpio_export_in_crs" = "1"
FYI, this is only required for Elan screens. […]
Do I need to remove this setting? or this CL could be merged directly? Thank you.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47433 )
Change subject: mb/google/dedede/var/metaknight: Add touchscreen settings ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47433/2/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/metaknight/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/47433/2/src/mainboard/google/dedede... PS2, Line 59: generic.disable_gpio_export_in_crs" = "1"
Do I need to remove this setting? or this CL could be merged directly? […]
I don't think it hurts anything, it's just unnecessary.
Tim Wawrzynczak has submitted this change. ( https://review.coreboot.org/c/coreboot/+/47433 )
Change subject: mb/google/dedede/var/metaknight: Add touchscreen settings ......................................................................
mb/google/dedede/var/metaknight: Add touchscreen settings
Add Elan and Goodix touchscreen settings.
BUG=b:169813211 BRANCH=None TEST=build metaknight firmware
Change-Id: Ib2acd31a8076533c3b927d37127e7d27bac0bb57 Signed-off-by: Tim Chen tim-chen@quanta.corp-partner.google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/47433 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Karthik Ramasubramanian kramasub@google.com --- M src/mainboard/google/dedede/variants/metaknight/overridetree.cb 1 file changed, 35 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Karthik Ramasubramanian: Looks good to me, approved
diff --git a/src/mainboard/google/dedede/variants/metaknight/overridetree.cb b/src/mainboard/google/dedede/variants/metaknight/overridetree.cb index d5903f6..d455a47 100644 --- a/src/mainboard/google/dedede/variants/metaknight/overridetree.cb +++ b/src/mainboard/google/dedede/variants/metaknight/overridetree.cb @@ -64,6 +64,41 @@ end end # USB xHCI device pci 15.0 on end + device pci 15.2 on + chip drivers/i2c/hid + register "generic.hid" = ""GDIX0000"" + register "generic.desc" = ""Goodix Touchscreen"" + register "generic.irq" = "ACPI_IRQ_LEVEL_LOW(GPP_D4_IRQ)" + register "generic.probed" = "1" + register "generic.reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_D5)" + register "generic.reset_delay_ms" = "120" + register "generic.reset_off_delay_ms" = "2" + register "generic.enable_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPP_D6)" + register "generic.enable_delay_ms" = "12" + register "generic.has_power_resource" = "1" + register "generic.disable_gpio_export_in_crs" = "1" + register "hid_desc_reg_offset" = "0x01" + device i2c 0x5d on end + end + chip drivers/i2c/hid + register "generic.hid" = ""ELAN6915"" + register "generic.desc" = ""ELAN Touchscreen"" + register "generic.irq" = "ACPI_IRQ_LEVEL_LOW(GPP_D4_IRQ)" + register "generic.probed" = "1" + register "generic.reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_D5)" + register "generic.reset_delay_ms" = "20" + register "generic.reset_off_delay_ms" = "2" + register "generic.stop_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_A11)" + register "generic.stop_delay_ms" = "280" + register "generic.stop_off_delay_ms" = "2" + register "generic.enable_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPP_D6)" + register "generic.enable_delay_ms" = "1" + register "generic.has_power_resource" = "1" + register "generic.disable_gpio_export_in_crs" = "1" + register "hid_desc_reg_offset" = "0x01" + device i2c 15 on end + end + end # I2C 2 device pci 15.3 off end # I2C 3 end end
-
Karthik Ramasubramanian (Code Review) -
Tim Chen (Code Review)
-
Tim Wawrzynczak (Code Review)