Karthik Ramasubramanian has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38855 )
Change subject: mb/google/dedede: Add USB configuration ......................................................................
mb/google/dedede: Add USB configuration
Add USB port configuration in devicetree. Configure USB Over-Current (OC) GPIOs.
BUG=None TEST=Build the mainboard.
Signed-off-by: Karthikeyan Ramasubramanian kramasub@google.com Change-Id: I19f7563013c7d702d52b7f34a207a34abe308621 --- M src/mainboard/google/dedede/variants/baseboard/devicetree.cb M src/mainboard/google/dedede/variants/baseboard/gpio.c 2 files changed, 26 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/38855/1
diff --git a/src/mainboard/google/dedede/variants/baseboard/devicetree.cb b/src/mainboard/google/dedede/variants/baseboard/devicetree.cb index 2d57a14..9dfd6b7 100644 --- a/src/mainboard/google/dedede/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/dedede/variants/baseboard/devicetree.cb @@ -19,6 +19,23 @@ register "pmc_gpe0_dw1" = "GPP_D" register "pmc_gpe0_dw2" = "GPP_H"
+ # 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)" # 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_EMPTY" # Not Used + + 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 + register "SerialIoI2cMode" = "{ [PchSerialIoIndexI2C0] = PchSerialIoPci, [PchSerialIoIndexI2C1] = PchSerialIoPci, @@ -95,7 +112,7 @@ 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 off end # USB xHCI + device pci 14.0 on end # USB xHCI device pci 14.1 off end # USB xDCI (OTG) device pci 14.2 off end # PMC SRAM device pci 14.3 off end # CNVi wifi diff --git a/src/mainboard/google/dedede/variants/baseboard/gpio.c b/src/mainboard/google/dedede/variants/baseboard/gpio.c index 8e4200a..9ee4dfc 100644 --- a/src/mainboard/google/dedede/variants/baseboard/gpio.c +++ b/src/mainboard/google/dedede/variants/baseboard/gpio.c @@ -21,6 +21,14 @@ /* A4 : ESPI_CS# */ /* A5 : ESPI_CLK */ /* A6 : ESPI_RESET_L */ + /* A12 : USB_OC1_N */ + PAD_CFG_NF(GPP_A12, NONE, DEEP, NF1), + /* A13 : USB_OC2_N */ + PAD_CFG_NF(GPP_A13, NONE, DEEP, NF1), + /* A14 : USB_OC3_N */ + PAD_CFG_NF(GPP_A14, NONE, DEEP, NF1), + /* A18 : USB_OC0_N */ + PAD_CFG_NF(GPP_A18, NONE, DEEP, NF1),
/* B4 : H1_PCH_INT_ODL */ PAD_CFG_GPI_APIC(GPP_B4, NONE, PLTRST, LEVEL, INVERT),
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38855 )
Change subject: mb/google/dedede: Add USB configuration ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38855/1/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/38855/1/src/mainboard/google/dedede... PS1, Line 23: OC_SKIP Is this correct? None of the USB ports use any OC pins?
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38855 )
Change subject: mb/google/dedede: Add USB configuration ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38855/1/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/38855/1/src/mainboard/google/dedede... PS1, Line 23: OC_SKIP
Is this correct? None of the USB ports use any OC pins?
Yes, that is correct. All the overcurrent events are handled by EC and hence none are routed to AP.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38855 )
Change subject: mb/google/dedede: Add USB configuration ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38855/1/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/38855/1/src/mainboard/google/dedede... PS1, Line 23: OC_SKIP
Yes, that is correct. All the overcurrent events are handled by EC and hence none are routed to AP.
In that case, do we need to configure the OC pins as NC instead of NF?
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38855 )
Change subject: mb/google/dedede: Add USB configuration ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38855/1/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/38855/1/src/mainboard/google/dedede... PS1, Line 23: OC_SKIP
In that case, do we need to configure the OC pins as NC instead of NF?
The concerned gpios are pulled up externally to 3.3V even though they are not used for OC events.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38855 )
Change subject: mb/google/dedede: Add USB configuration ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38855/1/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/38855/1/src/mainboard/google/dedede... PS1, Line 23: OC_SKIP
The concerned gpios are pulled up externally to 3.3V even though they are not used for OC events.
Right. My question was mostly that if there is any advantage of configuring the OC pins as NF since we know that they are not used anyways?
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38855 )
Change subject: mb/google/dedede: Add USB configuration ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38855/1/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/38855/1/src/mainboard/google/dedede... PS1, Line 23: OC_SKIP
Right. […]
I am not aware of any advantage. I can leave it as NC and we can revisit later if we see some advantage.
Hello Nick Vaccaro, Justin TerAvest, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38855
to look at the new patch set (#2).
Change subject: mb/google/dedede: Add USB configuration ......................................................................
mb/google/dedede: Add USB configuration
Add USB port configuration in devicetree. Configure USB Over-Current (OC) GPIOs.
BUG=None TEST=Build the mainboard.
Signed-off-by: Karthikeyan Ramasubramanian kramasub@google.com Change-Id: I19f7563013c7d702d52b7f34a207a34abe308621 --- M src/mainboard/google/dedede/variants/baseboard/devicetree.cb M src/mainboard/google/dedede/variants/baseboard/gpio.c 2 files changed, 26 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/38855/2
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38855 )
Change subject: mb/google/dedede: Add USB configuration ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38855/1/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/38855/1/src/mainboard/google/dedede... PS1, Line 23: OC_SKIP
I am not aware of any advantage. […]
Done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38855 )
Change subject: mb/google/dedede: Add USB configuration ......................................................................
Patch Set 2: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38855 )
Change subject: mb/google/dedede: Add USB configuration ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38855/2/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/38855/2/src/mainboard/google/dedede... PS2, Line 25: UP_20K Are these the POR states for the pads?
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38855 )
Change subject: mb/google/dedede: Add USB configuration ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38855/2/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/38855/2/src/mainboard/google/dedede... PS2, Line 22: # USB Port Configuration Is USB ACPI configuration planned in a separate CL?
Hello Nick Vaccaro, Justin TerAvest, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38855
to look at the new patch set (#3).
Change subject: mb/google/dedede: Add USB configuration ......................................................................
mb/google/dedede: Add USB configuration
Add USB port configuration in devicetree. Configure USB Over-Current (OC) GPIOs.
BUG=None TEST=Build the mainboard.
Signed-off-by: Karthikeyan Ramasubramanian kramasub@google.com Change-Id: I19f7563013c7d702d52b7f34a207a34abe308621 --- M src/mainboard/google/dedede/variants/baseboard/devicetree.cb M src/mainboard/google/dedede/variants/baseboard/gpio.c 2 files changed, 26 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/38855/3
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38855 )
Change subject: mb/google/dedede: Add USB configuration ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/38855/2/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/38855/2/src/mainboard/google/dedede... PS2, Line 22: # USB Port Configuration
Is USB ACPI configuration planned in a separate CL?
Yes
https://review.coreboot.org/c/coreboot/+/38855/2/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/38855/2/src/mainboard/google/dedede... PS2, Line 25: UP_20K
Are these the POR states for the pads?
By default, the gpio pads are neither pulled up nor pulled down. So I marked as NONE.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38855 )
Change subject: mb/google/dedede: Add USB configuration ......................................................................
Patch Set 3: Code-Review+2
Justin TerAvest has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38855 )
Change subject: mb/google/dedede: Add USB configuration ......................................................................
Patch Set 3: Code-Review+2
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38855 )
Change subject: mb/google/dedede: Add USB configuration ......................................................................
Patch Set 3: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/38855 )
Change subject: mb/google/dedede: Add USB configuration ......................................................................
mb/google/dedede: Add USB configuration
Add USB port configuration in devicetree. Configure USB Over-Current (OC) GPIOs.
BUG=None TEST=Build the mainboard.
Signed-off-by: Karthikeyan Ramasubramanian kramasub@google.com Change-Id: I19f7563013c7d702d52b7f34a207a34abe308621 Reviewed-on: https://review.coreboot.org/c/coreboot/+/38855 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com Reviewed-by: Justin TerAvest teravest@chromium.org Reviewed-by: Aamir Bohra aamir.bohra@intel.com --- M src/mainboard/google/dedede/variants/baseboard/devicetree.cb M src/mainboard/google/dedede/variants/baseboard/gpio.c 2 files changed, 26 insertions(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved Aamir Bohra: Looks good to me, approved Justin TerAvest: Looks good to me, approved
diff --git a/src/mainboard/google/dedede/variants/baseboard/devicetree.cb b/src/mainboard/google/dedede/variants/baseboard/devicetree.cb index 2d57a14..9dfd6b7 100644 --- a/src/mainboard/google/dedede/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/dedede/variants/baseboard/devicetree.cb @@ -19,6 +19,23 @@ register "pmc_gpe0_dw1" = "GPP_D" register "pmc_gpe0_dw2" = "GPP_H"
+ # 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)" # 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_EMPTY" # Not Used + + 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 + register "SerialIoI2cMode" = "{ [PchSerialIoIndexI2C0] = PchSerialIoPci, [PchSerialIoIndexI2C1] = PchSerialIoPci, @@ -95,7 +112,7 @@ 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 off end # USB xHCI + device pci 14.0 on end # USB xHCI device pci 14.1 off end # USB xDCI (OTG) device pci 14.2 off end # PMC SRAM device pci 14.3 off end # CNVi wifi diff --git a/src/mainboard/google/dedede/variants/baseboard/gpio.c b/src/mainboard/google/dedede/variants/baseboard/gpio.c index 8e4200a..71ddadb 100644 --- a/src/mainboard/google/dedede/variants/baseboard/gpio.c +++ b/src/mainboard/google/dedede/variants/baseboard/gpio.c @@ -21,6 +21,14 @@ /* A4 : ESPI_CS# */ /* A5 : ESPI_CLK */ /* A6 : ESPI_RESET_L */ + /* A12 : USB_OC1_N */ + PAD_NC(GPP_A12, NONE), + /* A13 : USB_OC2_N */ + PAD_NC(GPP_A13, NONE), + /* A14 : USB_OC3_N */ + PAD_NC(GPP_A14, NONE), + /* A18 : USB_OC0_N */ + PAD_NC(GPP_A18, NONE),
/* B4 : H1_PCH_INT_ODL */ PAD_CFG_GPI_APIC(GPP_B4, NONE, PLTRST, LEVEL, INVERT),
-
Aamir Bohra (Code Review) -
Furquan Shaikh (Code Review)
-
Justin TerAvest (Code Review)
-
Karthik Ramasubramanian (Code Review)
-
Patrick Georgi (Code Review)