Yan Liu has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43175 )
Change subject: mb/mainboard/dedede: support touchpad for Boten ......................................................................
mb/mainboard/dedede: support touchpad for Boten
Add device tree for touchpad
BUG=b:160664447 BRANCH=NONE TEST=build bios and verify touchpad function for boten
Signed-off-by: Yan Liu yan.liu@bitland.corp-partner.google.com Change-Id: Ib0c692bd7f997e9a90de646b807c6c1b37a834d2 --- M src/mainboard/google/dedede/variants/boten/overridetree.cb 1 file changed, 167 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/43175/1
diff --git a/src/mainboard/google/dedede/variants/boten/overridetree.cb b/src/mainboard/google/dedede/variants/boten/overridetree.cb index 1e75864..fea72ef 100644 --- a/src/mainboard/google/dedede/variants/boten/overridetree.cb +++ b/src/mainboard/google/dedede/variants/boten/overridetree.cb @@ -35,5 +35,171 @@ .speed = I2C_SPEED_FAST, }, }" - device domain 0 on end + + # USB Port Configuration + register "usb2_ports[0]" = "USB2_PORT_MID(OC_SKIP)" # Type-C Port C0 + register "usb2_ports[1]" = "USB2_PORT_MID(OC_SKIP)" # Type-C Port C1 + register "usb2_ports[2]" = "USB2_PORT_MID(OC_SKIP)" # Type-A Port A0 + register "usb2_ports[3]" = "USB2_PORT_MID(OC_SKIP)" # Type-A Port A1 + register "usb2_ports[4]" = "USB2_PORT_MID(OC_SKIP)" # Discrete Bluetooth + register "usb2_ports[5]" = "USB2_PORT_EMPTY" # Not Used + register "usb2_ports[6]" = "USB2_PORT_EMPTY" # Not Used + register "usb2_ports[7]" = "USB2_PORT_MID(OC_SKIP)" # Integrated Bluetooth + + register "usb3_ports[0]" = "USB3_PORT_DEFAULT(OC_SKIP)" # USB3/2 Type-C Port C0 + register "usb3_ports[1]" = "USB3_PORT_DEFAULT(OC_SKIP)" # USB3/2 Type-C Port C1 + register "usb3_ports[2]" = "USB3_PORT_DEFAULT(OC_SKIP)" # USB3/1 Type-A Port A0 + register "usb3_ports[3]" = "USB3_PORT_DEFAULT(OC_SKIP)" # USB3/1 Type-A Port A1 + register "usb3_ports[4]" = "USB3_PORT_EMPTY" # Not Used + register "usb3_ports[5]" = "USB3_PORT_EMPTY" # Not Used + + device domain 0 on + device pci 00.0 on end # Host Bridge + device pci 02.0 on end # Integrated Graphics Device + device pci 04.0 on end # SA Thermal device + device pci 05.0 off end # IPU + device pci 09.0 off end # Intel Trace Hub + device pci 12.6 off end # GSPI 2 + device pci 14.0 on + chip drivers/usb/acpi + register "desc" = ""Root Hub"" + register "type" = "UPC_TYPE_HUB" + device usb 0.0 on + chip drivers/usb/acpi + register "desc" = ""Left Type-C Port"" + register "type" = "UPC_TYPE_C_USB2_SS_SWITCH" + register "group" = "ACPI_PLD_GROUP(1, 1)" + device usb 2.0 on end + end + chip drivers/usb/acpi + register "desc" = ""Right Type-C Port"" + register "type" = "UPC_TYPE_C_USB2_SS_SWITCH" + register "group" = "ACPI_PLD_GROUP(2, 1)" + device usb 2.1 on end + end + chip drivers/usb/acpi + register "desc" = ""Left Type-A Port"" + register "type" = "UPC_TYPE_A" + register "group" = "ACPI_PLD_GROUP(1, 2)" + device usb 2.2 on end + end + chip drivers/usb/acpi + register "desc" = ""Right Type-A Port"" + register "type" = "UPC_TYPE_A" + register "group" = "ACPI_PLD_GROUP(2, 2)" + device usb 2.3 on end + end + chip drivers/usb/acpi + device usb 2.4 off end + end + chip drivers/usb/acpi + device usb 2.5 off end + end + chip drivers/usb/acpi + device usb 2.6 off end + end + chip drivers/usb/acpi + register "desc" = ""Bluetooth"" + register "type" = "UPC_TYPE_INTERNAL" + register "reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_H19)" + device usb 2.7 on end + end + chip drivers/usb/acpi + register "desc" = ""Left Type-C Port"" + register "type" = "UPC_TYPE_C_USB2_SS_SWITCH" + register "group" = "ACPI_PLD_GROUP(1, 1)" + device usb 3.0 on end + end + chip drivers/usb/acpi + register "desc" = ""Right Type-C Port"" + register "type" = "UPC_TYPE_C_USB2_SS_SWITCH" + register "group" = "ACPI_PLD_GROUP(2, 1)" + device usb 3.1 on end + end + chip drivers/usb/acpi + register "desc" = ""Left Type-A Port"" + register "type" = "UPC_TYPE_USB3_A" + register "group" = "ACPI_PLD_GROUP(1, 2)" + device usb 3.2 on end + end + chip drivers/usb/acpi + register "desc" = ""Right Type-A Port"" + register "type" = "UPC_TYPE_USB3_A" + register "group" = "ACPI_PLD_GROUP(2, 2)" + device usb 3.3 on end + end + end + end + end # USB xHCI + device pci 14.1 off end # USB xDCI (OTG) + device pci 14.2 off end # PMC SRAM + chip drivers/intel/wifi + register "wake" = "GPE0_PME_B0" + device pci 14.3 on end # CNVi wifi + end + device pci 14.5 on end # SDCard + device pci 15.0 on + chip drivers/i2c/generic + register "hid" = ""ELAN0000"" + register "desc" = ""ELAN Touchpad"" + register "irq" = "ACPI_IRQ_WAKE_EDGE_LOW(GPP_B3_IRQ)" + register "wake" = "GPE0_DW0_03" + register "probed" = "1" + device i2c 15 on end + end + chip drivers/i2c/hid + register "generic.hid" = ""PNP0C50"" + register "generic.desc" = ""Synaptics Touchpad"" + register "generic.irq" = "ACPI_IRQ_WAKE_EDGE_LOW(GPP_B3_IRQ)" + register "generic.wake" = "GPE0_DW0_03" + register "generic.probed" = "1" + register "hid_desc_reg_offset" = "0x20" + device i2c 0x2c on end + end + end # I2C 0 + device pci 15.1 on end # I2C 1 + device pci 15.2 on end # I2C 2 + device pci 15.3 on end # I2C 3 + device pci 16.0 on end # HECI 1 + device pci 16.1 off end # HECI 2 + device pci 16.4 off end # HECI 3 + device pci 16.5 off end # HECI 4 + device pci 17.0 off end # SATA + device pci 19.0 on end # I2C 4 + device pci 19.1 off end # I2C 5 + device pci 19.2 on end # UART 2 + device pci 1a.0 on end # eMMC + device pci 1c.0 off end # PCI Express Root Port 1 + device pci 1c.1 off end # PCI Express Root Port 2 + device pci 1c.2 off end # PCI Express Root Port 3 + device pci 1c.3 off end # PCI Express Root Port 4 + device pci 1c.4 off end # PCI Express Root Port 5 + device pci 1c.5 off end # PCI Express Root Port 6 + device pci 1c.6 off end # PCI Express Root Port 7 + # External PCIe port 4 is mapped to PCIe Root port 8 + device pci 1c.7 on end # PCI Express Root Port 8 - WLAN + device pci 1e.0 off end # UART 0 + device pci 1e.1 off end # UART 1 + device pci 1e.2 on + chip drivers/spi/acpi + register "hid" = "ACPI_DT_NAMESPACE_HID" + register "compat_string" = ""google,cr50"" + register "irq" = "ACPI_IRQ_EDGE_LOW(GPP_B4_IRQ)" + device spi 0 on end + end + end # GSPI 0 + device pci 1e.3 off end # GSPI 1 + device pci 1f.0 on + chip ec/google/chromeec + device pnp 0c09.0 on end + end + end # eSPI Interface + device pci 1f.1 on end # P2SB + device pci 1f.2 hidden end # Power Management Controller + device pci 1f.3 off end # Intel HDA/cAVS + device pci 1f.4 off end # SMBus + device pci 1f.5 on end # PCH SPI + device pci 1f.7 off end # Intel Trace Hub + end + end
Peichao Li has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43175 )
Change subject: mb/mainboard/dedede: support touchpad for Boten ......................................................................
Patch Set 2:
(7 comments)
https://review.coreboot.org/c/coreboot/+/43175/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43175/2//COMMIT_MSG@7 PS2, Line 7: support touchpad for Boten Update device tree for Boten
https://review.coreboot.org/c/coreboot/+/43175/2/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/boten/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/43175/2/src/mainboard/google/dedede... PS2, Line 41: "USB2_PORT_MID(OC_SKIP)" # Type-C Port C1 For Boten project, there is no C port in the DB. close it.
https://review.coreboot.org/c/coreboot/+/43175/2/src/mainboard/google/dedede... PS2, Line 50: "USB3_PORT_DEFAULT(OC_SKIP)" # USB3/2 Type-C Port C1 No C port in the DB.
https://review.coreboot.org/c/coreboot/+/43175/2/src/mainboard/google/dedede... PS2, Line 75: register "desc" = ""Right Type-C Port"" please close it
https://review.coreboot.org/c/coreboot/+/43175/2/src/mainboard/google/dedede... PS2, Line 114: register "desc" = ""Right Type-C Port"" please close it
https://review.coreboot.org/c/coreboot/+/43175/2/src/mainboard/google/dedede... PS2, Line 161: device pci 15.2 on end # I2C 2 Please add touch screen description.
https://review.coreboot.org/c/coreboot/+/43175/2/src/mainboard/google/dedede... PS2, Line 169: device pci 19.1 off end # I2C 5 Please enable this port since for P sensor
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43175 )
Change subject: mb/mainboard/dedede: support touchpad for Boten ......................................................................
Patch Set 2:
(3 comments)
https://review.coreboot.org/c/coreboot/+/43175/2/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/boten/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/43175/2/src/mainboard/google/dedede... PS2, Line 39: # USB Port Configuration : register "usb2_ports[0]" = "USB2_PORT_MID(OC_SKIP)" # Type-C Port C0 : register "usb2_ports[1]" = "USB2_PORT_MID(OC_SKIP)" # Type-C Port C1 : register "usb2_ports[2]" = "USB2_PORT_MID(OC_SKIP)" # Type-A Port A0 : register "usb2_ports[3]" = "USB2_PORT_MID(OC_SKIP)" # Type-A Port A1 : register "usb2_ports[4]" = "USB2_PORT_MID(OC_SKIP)" # Discrete Bluetooth : register "usb2_ports[5]" = "USB2_PORT_EMPTY" # Not Used : register "usb2_ports[6]" = "USB2_PORT_EMPTY" # Not Used : register "usb2_ports[7]" = "USB2_PORT_MID(OC_SKIP)" # Integrated Bluetooth No need to re-configure everything in devicetree. Override only those whose configurations differ from baseboard devicetree. Also do that in a separate CL so that this change covers only about enabling trackpad.
https://review.coreboot.org/c/coreboot/+/43175/2/src/mainboard/google/dedede... PS2, Line 49: register "usb3_ports[0]" = "USB3_PORT_DEFAULT(OC_SKIP)" # USB3/2 Type-C Port C0 : register "usb3_ports[1]" = "USB3_PORT_DEFAULT(OC_SKIP)" # USB3/2 Type-C Port C1 : register "usb3_ports[2]" = "USB3_PORT_DEFAULT(OC_SKIP)" # USB3/1 Type-A Port A0 : register "usb3_ports[3]" = "USB3_PORT_DEFAULT(OC_SKIP)" # USB3/1 Type-A Port A1 : register "usb3_ports[4]" = "USB3_PORT_EMPTY" # Not Used : register "usb3_ports[5]" = "USB3_PORT_EMPTY" # Not Used No need to re-configure everything in devicetree. Override only those whose configurations differ from baseboard devicetree. Also do that in a separate CL so that this change covers only about enabling trackpad.
https://review.coreboot.org/c/coreboot/+/43175/2/src/mainboard/google/dedede... PS2, Line 142: chip drivers/i2c/generic : register "hid" = ""ELAN0000"" : register "desc" = ""ELAN Touchpad"" : register "irq" = "ACPI_IRQ_WAKE_EDGE_LOW(GPP_B3_IRQ)" : register "wake" = "GPE0_DW0_03" : register "probed" = "1" : device i2c 15 on end : end : chip drivers/i2c/hid : register "generic.hid" = ""PNP0C50"" : register "generic.desc" = ""Synaptics Touchpad"" : register "generic.irq" = "ACPI_IRQ_WAKE_EDGE_LOW(GPP_B3_IRQ)" : register "generic.wake" = "GPE0_DW0_03" : register "generic.probed" = "1" : register "hid_desc_reg_offset" = "0x20" : device i2c 0x2c on end : end Which of these 2 trackpads is used and verified in Boten. Add the configuration for that trackpad alone in this change. Move everything else to a separate CL of their own.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43175 )
Change subject: mb/mainboard/dedede: support touchpad for Boten ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43175/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43175/2//COMMIT_MSG@7 PS2, Line 7: mainboard google
Hello build bot (Jenkins), Furquan Shaikh, Henry Sun, Justin TerAvest, Marco Chen,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43175
to look at the new patch set (#3).
Change subject: mb/mainboard/dedede: Update device tree for Boten ......................................................................
mb/mainboard/dedede: Update device tree for Boten
Add device tree for touchpad
BUG=b:160664447 BRANCH=NONE TEST=build bios and verify touchpad function for boten
Signed-off-by: Yan Liu yan.liu@bitland.corp-partner.google.com Change-Id: Ib0c692bd7f997e9a90de646b807c6c1b37a834d2 --- M src/mainboard/google/dedede/variants/boten/overridetree.cb 1 file changed, 144 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/43175/3
Hello build bot (Jenkins), Furquan Shaikh, Henry Sun, Justin TerAvest, Marco Chen,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43175
to look at the new patch set (#4).
Change subject: mb/mainboard/dedede: Update device tree for Boten ......................................................................
mb/mainboard/dedede: Update device tree for Boten
Add device tree for touchpad
BUG=b:160664447 BRANCH=NONE TEST=build bios and verify touchpad function for boten
Signed-off-by: Yan Liu yan.liu@bitland.corp-partner.google.com Change-Id: Ib0c692bd7f997e9a90de646b807c6c1b37a834d2 --- M src/mainboard/google/dedede/variants/boten/overridetree.cb 1 file changed, 125 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/43175/4
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43175 )
Change subject: mb/mainboard/dedede: Update device tree for Boten ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/43175/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43175/4//COMMIT_MSG@7 PS4, Line 7: mb/mainboard/dedede: Update device tree for Boten mb/google/dedede/var/boten: Update devicetree
https://review.coreboot.org/c/coreboot/+/43175/4//COMMIT_MSG@9 PS4, Line 9: Add device tree for touchpad It looks to me, you are adding more then just an entry for the touchpad. Please mention everything.
Hello build bot (Jenkins), Furquan Shaikh, Henry Sun, Justin TerAvest, Marco Chen,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43175
to look at the new patch set (#5).
Change subject: mb/google/dedede/var/boten: Update devicetree ......................................................................
mb/google/dedede/var/boten: Update devicetree
Add device tree for touchpad, touch screen, audio jack, enable USB2_[5][6] and I2C5
BUG=b:160664447 BRANCH=NONE TEST=build bios and verify touchpad function for boten
Signed-off-by: Yan Liu yan.liu@bitland.corp-partner.google.com Change-Id: Ib0c692bd7f997e9a90de646b807c6c1b37a834d2 --- M src/mainboard/google/dedede/variants/boten/overridetree.cb 1 file changed, 125 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/43175/5
Marco Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43175 )
Change subject: mb/google/dedede/var/boten: Update devicetree ......................................................................
Patch Set 5:
(2 comments)
Hi Peichao,
We will need your support to review the CL and have +1 first. Thanks.
https://review.coreboot.org/c/coreboot/+/43175/5/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/boten/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/43175/5/src/mainboard/google/dedede... PS5, Line 47: register "usb2_ports[7]" = "USB2_PORT_MID(OC_SKIP)" # Integrated Bluetooth As comments in previous runs, some definition like usb2_ports[0-3|7] are already in baseboard/devicetree.cb so you don't need to re-specify here. Only the variant should be specified here.
https://review.coreboot.org/c/coreboot/+/43175/5/src/mainboard/google/dedede... PS5, Line 54: register "usb3_ports[5]" = "USB3_PORT_EMPTY" # Not Used Same as here and others.
Ben Kao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43175 )
Change subject: mb/google/dedede/var/boten: Update devicetree ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/c/coreboot/+/43175/5/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/boten/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/43175/5/src/mainboard/google/dedede... PS5, Line 58: [PchSerialIoIndexI2C1] = PchSerialIoPci, If you don't use I2C1, it can be set to "PchSerialIoDisabled" and set "device pci 15.1 off end # I2C 1" in the below.
https://review.coreboot.org/c/coreboot/+/43175/5/src/mainboard/google/dedede... PS5, Line 60: [PchSerialIoIndexI2C3] = PchSerialIoPci, If you don't use I2C3, it can be set to "PchSerialIoDisabled" and set "device pci 15.3 off end # I2C 3" in the below.
Ben Kao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43175 )
Change subject: mb/google/dedede/var/boten: Update devicetree ......................................................................
Patch Set 5:
(4 comments)
https://review.coreboot.org/c/coreboot/+/43175/5/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/boten/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/43175/5/src/mainboard/google/dedede... PS5, Line 12: #| I2C1 | Digitizer | You can remove it, if Boten doesn't use I2C1.
https://review.coreboot.org/c/coreboot/+/43175/5/src/mainboard/google/dedede... PS5, Line 14: #| I2C3 | Camera | You can remove it, if Boten doesn't use I2C3.
https://review.coreboot.org/c/coreboot/+/43175/5/src/mainboard/google/dedede... PS5, Line 25: .i2c[1] = { : .speed = I2C_SPEED_FAST, : }, You can remove it, if Boten doesn't use I2C1.
https://review.coreboot.org/c/coreboot/+/43175/5/src/mainboard/google/dedede... PS5, Line 31: .i2c[3] = { : .speed = I2C_SPEED_FAST, : }, You can remove it, if Boten doesn't use I2C3.
Hello build bot (Jenkins), Furquan Shaikh, Henry Sun, Justin TerAvest, Marco Chen, Peichao Li,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43175
to look at the new patch set (#6).
Change subject: mb/google/dedede/var/boten: Update devicetree ......................................................................
mb/google/dedede/var/boten: Update devicetree
Add device tree for touchpad, touch screen, audio jack, enable USB2_[5][6] and I2C5
BUG=b:160664447 BRANCH=NONE TEST=build bios and verify touchpad function for boten
Signed-off-by: Yan Liu yan.liu@bitland.corp-partner.google.com Change-Id: Ib0c692bd7f997e9a90de646b807c6c1b37a834d2 --- M src/mainboard/google/dedede/variants/boten/overridetree.cb 1 file changed, 115 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/43175/6
Peichao Li has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43175 )
Change subject: mb/google/dedede/var/boten: Update devicetree ......................................................................
Patch Set 8: Code-Review+1
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43175 )
Change subject: mb/google/dedede/var/boten: Update devicetree ......................................................................
Patch Set 8:
(3 comments)
https://review.coreboot.org/c/coreboot/+/43175/8/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/boten/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/43175/8/src/mainboard/google/dedede... PS8, Line 42: [PchSerialIoIndexI2C5] = PchSerialIoPci, Is I2C5 used? If so, can you please add in the above table regarding the use-case for I2C5 and also enable the pci device corresponding to I2C5. Otherwise configure it as PchSerialIoDisabled.
https://review.coreboot.org/c/coreboot/+/43175/8/src/mainboard/google/dedede... PS8, Line 52: Right Type-A Port How many USB-A and USB-C ports? This is overriding the Right Type-C port with Type-A port. Do you need to turn off the original Right Type-A port (ports 2.3 & 3.3)
https://review.coreboot.org/c/coreboot/+/43175/8/src/mainboard/google/dedede... PS8, Line 83: 15 Nit: 0x15. Same for all the I2C addresses below for consistency.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43175 )
Change subject: mb/google/dedede/var/boten: Update devicetree ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43175/9//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43175/9//COMMIT_MSG@9 PS9, Line 9: Add device tree for touchpad, touch screen, audio jack, enable USB2_[5][6] and I2C5 Maybe:
Add devicetree entries for touchpad, touch screen, audio jack, and enable USB2_[5][6] and I2C5.
Marco Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43175 )
Change subject: mb/google/dedede/var/boten: Update devicetree ......................................................................
Patch Set 9:
(3 comments)
https://review.coreboot.org/c/coreboot/+/43175/9/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/boten/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/43175/9/src/mainboard/google/dedede... PS9, Line 56: end Do we want to change the desc of usb 2.3 from Right Type-A Port to LTE?
https://review.coreboot.org/c/coreboot/+/43175/9/src/mainboard/google/dedede... PS9, Line 72: end Do we want to change the desc of usb 3.3 from Right Type-A Port to LTE?
https://review.coreboot.org/c/coreboot/+/43175/9/src/mainboard/google/dedede... PS9, Line 108: register "generic.enable_delay_ms" = "12" Do we have stop_gpio / report_en for this Goodix controller?
Marco Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43175 )
Change subject: mb/google/dedede/var/boten: Update devicetree ......................................................................
Patch Set 9:
(2 comments)
https://review.coreboot.org/c/coreboot/+/43175/8/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/boten/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/43175/8/src/mainboard/google/dedede... PS8, Line 42: [PchSerialIoIndexI2C5] = PchSerialIoPci,
Is I2C5 used? If so, can you please add in the above table regarding the use-case for I2C5 and also […]
As I know, this one will be planed to P-Sensor later.
https://review.coreboot.org/c/coreboot/+/43175/8/src/mainboard/google/dedede... PS8, Line 52: Right Type-A Port
How many USB-A and USB-C ports? This is overriding the Right Type-C port with Type-A port. […]
I have other comment to suggest them to be amend for LTE usage.
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43175 )
Change subject: mb/google/dedede/var/boten: Update devicetree ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43175/9/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/boten/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/43175/9/src/mainboard/google/dedede... PS9, Line 108: register "generic.enable_delay_ms" = "12"
Do we have stop_gpio / report_en for this Goodix controller?
As per the GT7375P Programming guide,
1) Time between enable (AVDD) and reset needs to be >= 10 ms i.e. enable_delay_ms >= 10 ms 2) Time between reset and stop (Report_En) needs to be >= 10 ms i.e. reset_delay_ms >= 10 ms 3) Time between reset and HID (I2C transaction) needs to be >= 120 ms i.e. (reset_delay_ms + stop_delay_ms) >= 120 ms 4) Time between disabling stop and reset off needs to be >= 1 ms i.e. stop_off_delay_ms >= 1ms 5) Time between reset off and enable off needs to be >= 1ms i.e. reset_off_delay_ms >= 1ms.
Based on that two observations: a) Both reset_delay_ms and stop_delay_ms are incorporated into reset_delay_ms itself leaving no delay between Report Enable and I2C transaction. Not sure if it is fine. b) Missing configuration for stop_off_delay_ms.
But every other google board using the Goodix controller use the same configuration as here. It does not hurt to double-confirm with the Goodix team to avoid copy-paste error.
Marco Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43175 )
Change subject: mb/google/dedede/var/boten: Update devicetree ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43175/9/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/boten/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/43175/9/src/mainboard/google/dedede... PS9, Line 108: register "generic.enable_delay_ms" = "12"
As per the GT7375P Programming guide, […]
https://review.coreboot.org/c/coreboot/+/43153 has the similar discuss as well and we should figure out whether Report_En is used in sub-board of touch controller since it is not defined here actually. Could ODM team provide the info as well.
Yan Liu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43175 )
Change subject: mb/google/dedede/var/boten: Update devicetree ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43175/9/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/boten/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/43175/9/src/mainboard/google/dedede... PS9, Line 108: register "generic.enable_delay_ms" = "12"
Report_En(GP_A11) was unused.
Hello build bot (Jenkins), Furquan Shaikh, Henry Sun, Justin TerAvest, Marco Chen, Peichao Li, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43175
to look at the new patch set (#10).
Change subject: mb/google/dedede/var/boten: Update devicetree ......................................................................
mb/google/dedede/var/boten: Update devicetree
Add devicetree entries for touchpad, touch screen, audio jack, and enable USB2_[5][6] and I2C5
BUG=b:160664447 BRANCH=NONE TEST=build bios and verify touchpad function for boten
Signed-off-by: Yan Liu yan.liu@bitland.corp-partner.google.com Change-Id: Ib0c692bd7f997e9a90de646b807c6c1b37a834d2 --- M src/mainboard/google/dedede/variants/boten/overridetree.cb 1 file changed, 127 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/43175/10
Hello build bot (Jenkins), Furquan Shaikh, Henry Sun, Justin TerAvest, Marco Chen, Peichao Li, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43175
to look at the new patch set (#11).
Change subject: mb/google/dedede/var/boten: Update devicetree ......................................................................
mb/google/dedede/var/boten: Update devicetree
Add devicetree entries for touchpad, touch screen, audio jack, and enable USB2_[5][6] and I2C5
BUG=b:160664447 BRANCH=NONE TEST=build bios and verify touchpad function for boten
Signed-off-by: Yan Liu yan.liu@bitland.corp-partner.google.com Change-Id: Ib0c692bd7f997e9a90de646b807c6c1b37a834d2 --- M src/mainboard/google/dedede/variants/boten/overridetree.cb 1 file changed, 127 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/43175/11
Ben Kao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43175 )
Change subject: mb/google/dedede/var/boten: Update devicetree ......................................................................
Patch Set 12:
(2 comments)
https://review.coreboot.org/c/coreboot/+/43175/12/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/boten/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/43175/12/src/mainboard/google/deded... PS12, Line 14: #+-------------------+---------------------------+ Should this be added? or you can add this in https://review.coreboot.org/c/coreboot/+/43478 ?
#| I2C5 | P-sensor |
https://review.coreboot.org/c/coreboot/+/43175/12/src/mainboard/google/deded... PS12, Line 29: }" Should this be added? or you can add this in https://review.coreboot.org/c/coreboot/+/43478 ?
.i2c[5] = { .speed = I2C_SPEED_FAST, },
Peichao Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43175 )
Change subject: mb/google/dedede/var/boten: Update devicetree ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43175/12/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/boten/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/43175/12/src/mainboard/google/deded... PS12, Line 14: #+-------------------+---------------------------+
Should this be added? or you can add this in https://review.coreboot.org/c/coreboot/+/43478 ? […]
Hi Ben, P-sensor CL: 43478 need to be rebased on it.
Peichao Wang has uploaded a new patch set (#13) to the change originally created by Yan Liu. ( https://review.coreboot.org/c/coreboot/+/43175 )
Change subject: mb/google/dedede/var/boten: Update devicetree ......................................................................
mb/google/dedede/var/boten: Update devicetree
Update devicetree for touchpad, touch screen, audio jack, and enable USB2_[5][6] and I2C5
BUG=b:160664447 BRANCH=NONE TEST=FW_NAME="boten" emerge-dedede coreboot chromeos-bootimage
Signed-off-by: Yan Liu yan.liu@bitland.corp-partner.google.com Change-Id: Ib0c692bd7f997e9a90de646b807c6c1b37a834d2 --- M src/mainboard/google/dedede/variants/boten/overridetree.cb 1 file changed, 107 insertions(+), 16 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/43175/13
Peichao Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43175 )
Change subject: mb/google/dedede/var/boten: Update devicetree ......................................................................
Patch Set 13: Code-Review+1
Peichao Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43175 )
Change subject: mb/google/dedede/var/boten: Update devicetree ......................................................................
Patch Set 13:
(31 comments)
https://review.coreboot.org/c/coreboot/+/43175/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43175/2//COMMIT_MSG@7 PS2, Line 7: support touchpad for Boten
Update device tree for Boten
Done
https://review.coreboot.org/c/coreboot/+/43175/2//COMMIT_MSG@7 PS2, Line 7: mainboard
Done
https://review.coreboot.org/c/coreboot/+/43175/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43175/4//COMMIT_MSG@7 PS4, Line 7: mb/mainboard/dedede: Update device tree for Boten
mb/google/dedede/var/boten: Update devicetree
Done
https://review.coreboot.org/c/coreboot/+/43175/4//COMMIT_MSG@9 PS4, Line 9: Add device tree for touchpad
Done
Done
https://review.coreboot.org/c/coreboot/+/43175/4//COMMIT_MSG@9 PS4, Line 9: Add device tree for touchpad
It looks to me, you are adding more then just an entry for the touchpad. Please mention everything.
Done
https://review.coreboot.org/c/coreboot/+/43175/9//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43175/9//COMMIT_MSG@9 PS9, Line 9: Add device tree for touchpad, touch screen, audio jack, enable USB2_[5][6] and I2C5
Maybe: […]
Done
https://review.coreboot.org/c/coreboot/+/43175/2/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/boten/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/43175/2/src/mainboard/google/dedede... PS2, Line 41: "USB2_PORT_MID(OC_SKIP)" # Type-C Port C1
For Boten project, there is no C port in the DB. close it.
Done
https://review.coreboot.org/c/coreboot/+/43175/2/src/mainboard/google/dedede... PS2, Line 39: # USB Port Configuration : register "usb2_ports[0]" = "USB2_PORT_MID(OC_SKIP)" # Type-C Port C0 : register "usb2_ports[1]" = "USB2_PORT_MID(OC_SKIP)" # Type-C Port C1 : register "usb2_ports[2]" = "USB2_PORT_MID(OC_SKIP)" # Type-A Port A0 : register "usb2_ports[3]" = "USB2_PORT_MID(OC_SKIP)" # Type-A Port A1 : register "usb2_ports[4]" = "USB2_PORT_MID(OC_SKIP)" # Discrete Bluetooth : register "usb2_ports[5]" = "USB2_PORT_EMPTY" # Not Used : register "usb2_ports[6]" = "USB2_PORT_EMPTY" # Not Used : register "usb2_ports[7]" = "USB2_PORT_MID(OC_SKIP)" # Integrated Bluetooth
No need to re-configure everything in devicetree. […]
Done
https://review.coreboot.org/c/coreboot/+/43175/2/src/mainboard/google/dedede... PS2, Line 50: "USB3_PORT_DEFAULT(OC_SKIP)" # USB3/2 Type-C Port C1
No C port in the DB.
Done
https://review.coreboot.org/c/coreboot/+/43175/2/src/mainboard/google/dedede... PS2, Line 49: register "usb3_ports[0]" = "USB3_PORT_DEFAULT(OC_SKIP)" # USB3/2 Type-C Port C0 : register "usb3_ports[1]" = "USB3_PORT_DEFAULT(OC_SKIP)" # USB3/2 Type-C Port C1 : register "usb3_ports[2]" = "USB3_PORT_DEFAULT(OC_SKIP)" # USB3/1 Type-A Port A0 : register "usb3_ports[3]" = "USB3_PORT_DEFAULT(OC_SKIP)" # USB3/1 Type-A Port A1 : register "usb3_ports[4]" = "USB3_PORT_EMPTY" # Not Used : register "usb3_ports[5]" = "USB3_PORT_EMPTY" # Not Used
No need to re-configure everything in devicetree. […]
Done
https://review.coreboot.org/c/coreboot/+/43175/2/src/mainboard/google/dedede... PS2, Line 75: register "desc" = ""Right Type-C Port""
please close it
Done
https://review.coreboot.org/c/coreboot/+/43175/2/src/mainboard/google/dedede... PS2, Line 114: register "desc" = ""Right Type-C Port""
please close it
Done
https://review.coreboot.org/c/coreboot/+/43175/2/src/mainboard/google/dedede... PS2, Line 142: chip drivers/i2c/generic : register "hid" = ""ELAN0000"" : register "desc" = ""ELAN Touchpad"" : register "irq" = "ACPI_IRQ_WAKE_EDGE_LOW(GPP_B3_IRQ)" : register "wake" = "GPE0_DW0_03" : register "probed" = "1" : device i2c 15 on end : end : chip drivers/i2c/hid : register "generic.hid" = ""PNP0C50"" : register "generic.desc" = ""Synaptics Touchpad"" : register "generic.irq" = "ACPI_IRQ_WAKE_EDGE_LOW(GPP_B3_IRQ)" : register "generic.wake" = "GPE0_DW0_03" : register "generic.probed" = "1" : register "hid_desc_reg_offset" = "0x20" : device i2c 0x2c on end : end
Which of these 2 trackpads is used and verified in Boten. […]
Done
https://review.coreboot.org/c/coreboot/+/43175/2/src/mainboard/google/dedede... PS2, Line 161: device pci 15.2 on end # I2C 2
Please add touch screen description.
Done
https://review.coreboot.org/c/coreboot/+/43175/2/src/mainboard/google/dedede... PS2, Line 169: device pci 19.1 off end # I2C 5
Please enable this port since for P sensor
Done
https://review.coreboot.org/c/coreboot/+/43175/5/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/boten/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/43175/5/src/mainboard/google/dedede... PS5, Line 12: #| I2C1 | Digitizer |
You can remove it, if Boten doesn't use I2C1.
Done
https://review.coreboot.org/c/coreboot/+/43175/5/src/mainboard/google/dedede... PS5, Line 14: #| I2C3 | Camera |
You can remove it, if Boten doesn't use I2C3.
Done
https://review.coreboot.org/c/coreboot/+/43175/5/src/mainboard/google/dedede... PS5, Line 25: .i2c[1] = { : .speed = I2C_SPEED_FAST, : },
You can remove it, if Boten doesn't use I2C1.
Done
https://review.coreboot.org/c/coreboot/+/43175/5/src/mainboard/google/dedede... PS5, Line 31: .i2c[3] = { : .speed = I2C_SPEED_FAST, : },
You can remove it, if Boten doesn't use I2C3.
Done
https://review.coreboot.org/c/coreboot/+/43175/5/src/mainboard/google/dedede... PS5, Line 47: register "usb2_ports[7]" = "USB2_PORT_MID(OC_SKIP)" # Integrated Bluetooth
As comments in previous runs, some definition like usb2_ports[0-3|7] are already in baseboard/device […]
Done
https://review.coreboot.org/c/coreboot/+/43175/5/src/mainboard/google/dedede... PS5, Line 54: register "usb3_ports[5]" = "USB3_PORT_EMPTY" # Not Used
Same as here and others.
Done
https://review.coreboot.org/c/coreboot/+/43175/5/src/mainboard/google/dedede... PS5, Line 58: [PchSerialIoIndexI2C1] = PchSerialIoPci,
If you don't use I2C1, it can be set to "PchSerialIoDisabled" and set […]
Done
https://review.coreboot.org/c/coreboot/+/43175/5/src/mainboard/google/dedede... PS5, Line 60: [PchSerialIoIndexI2C3] = PchSerialIoPci,
If you don't use I2C3, it can be set to "PchSerialIoDisabled" and set […]
Done
https://review.coreboot.org/c/coreboot/+/43175/8/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/boten/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/43175/8/src/mainboard/google/dedede... PS8, Line 42: [PchSerialIoIndexI2C5] = PchSerialIoPci,
As I know, this one will be planed to P-Sensor later.
Done
https://review.coreboot.org/c/coreboot/+/43175/8/src/mainboard/google/dedede... PS8, Line 52: Right Type-A Port
I have other comment to suggest them to be amend for LTE usage.
Done
https://review.coreboot.org/c/coreboot/+/43175/8/src/mainboard/google/dedede... PS8, Line 83: 15
Nit: 0x15. Same for all the I2C addresses below for consistency.
Done
https://review.coreboot.org/c/coreboot/+/43175/9/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/boten/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/43175/9/src/mainboard/google/dedede... PS9, Line 56: end
Do we want to change the desc of usb 2. […]
Done
https://review.coreboot.org/c/coreboot/+/43175/9/src/mainboard/google/dedede... PS9, Line 72: end
Do we want to change the desc of usb 3. […]
Done
https://review.coreboot.org/c/coreboot/+/43175/9/src/mainboard/google/dedede... PS9, Line 108: register "generic.enable_delay_ms" = "12"
Report_En(GP_A11) was unused.
Done
https://review.coreboot.org/c/coreboot/+/43175/12/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/boten/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/43175/12/src/mainboard/google/deded... PS12, Line 14: #+-------------------+---------------------------+
Hi Ben, P-sensor CL: 43478 need to be rebased on it.
Done
https://review.coreboot.org/c/coreboot/+/43175/12/src/mainboard/google/deded... PS12, Line 29: }"
Should this be added? or you can add this in https://review.coreboot.org/c/coreboot/+/43478 ? […]
Done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43175 )
Change subject: mb/google/dedede/var/boten: Update devicetree ......................................................................
Patch Set 13: Code-Review+1
LGTM. I will let Karthik +2 this.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43175 )
Change subject: mb/google/dedede/var/boten: Update devicetree ......................................................................
Patch Set 13: Code-Review+1
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43175 )
Change subject: mb/google/dedede/var/boten: Update devicetree ......................................................................
Patch Set 13: Code-Review-1
Everything introduced in this change has already landed in coreboot - CB:45732. This can be abandoned.
Peichao Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43175 )
Change subject: mb/google/dedede/var/boten: Update devicetree ......................................................................
Patch Set 13: Code-Review-1
Stefan Reinauer has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/43175?usp=email )
Change subject: mb/google/dedede/var/boten: Update devicetree ......................................................................
Abandoned
-
Angel Pons (Code Review) -
Ben Kao (Code Review)
-
Furquan Shaikh (Code Review)
-
Karthik Ramasubramanian (Code Review)
-
Marco Chen (Code Review)
-
Paul Menzel (Code Review)
-
Peichao Li (Code Review)
-
Peichao Wang (Code Review)
-
Stefan Reinauer (Code Review)
-
Yan Liu (Code Review)