John Zhao has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/40425 )
Change subject: mb/google/volteer: Add support for USB Type-C ports ......................................................................
mb/google/volteer: Add support for USB Type-C ports
Two usb type-c ports under the actual Mux device. Each port has its own ACPI device entry (node). These nodes are the ones that the USB Type-C port/connnector devices will refer to in order to configure the mux.
BUG=b:151646486 BRANCH=None TEST=Verify USB Type-C alternate mode (DP or TBT) along wth PMC control.
Change-Id: I4b443a1ca1c5361652a69586b340b1c95e9c3f06 Signed-off-by: John Zhao john.zhao@intel.com --- A src/mainboard/google/volteer/acpi/usbc.asl M src/mainboard/google/volteer/dsdt.asl 2 files changed, 46 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/25/40425/1
diff --git a/src/mainboard/google/volteer/acpi/usbc.asl b/src/mainboard/google/volteer/acpi/usbc.asl new file mode 100644 index 0000000..b185b2f --- /dev/null +++ b/src/mainboard/google/volteer/acpi/usbc.asl @@ -0,0 +1,43 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* This file is part of the coreboot project. */ + +Scope (_SB.PCI0.PMC.MUX) { + /* + * Mux driver looks for usb type-C devices underneath it through + * _DSD and _ADR, where _ADR specifies the device address on the + * parent bus. CON0 is representing the first type-C port and CON1 + * is representing the second. + */ + Device (CON0) + { + Name (_ADR, 0) + /* + * These properties should have the values that the driver needs + * to supply to the PMC via IPC when the muxes are being + * configured. + */ + Name (_DSD, Package () { + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), + Package() { + Package () { "usb2-port", 6 }, + Package () { "usb3-port", 3 }, + Package () { "sbu-orientation", "normal" }, + Package () { "hsl-orientation", "normal" }, + }, + }) + } + + Device (CON1) + { + Name (_ADR, 1) + Name (_DSD, Package () { + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), + Package() { + Package () { "usb2-port", 5 }, + Package () { "usb3-port", 2 }, + Package () { "sbu-orientation", "normal" }, + Package () { "hsl-orientation", "normal" }, + }, + }) + } +} diff --git a/src/mainboard/google/volteer/dsdt.asl b/src/mainboard/google/volteer/dsdt.asl index 640f7cd..1888267 100644 --- a/src/mainboard/google/volteer/dsdt.asl +++ b/src/mainboard/google/volteer/dsdt.asl @@ -50,4 +50,7 @@ /* Camera */ #include <soc/intel/tigerlake/acpi/ipu.asl> #include "acpi/mipi_camera.asl" + + /* usbc */ + #include "acpi/usbc.asl" }
Prashant Malani has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40425 )
Change subject: mb/google/volteer: Add support for USB Type-C ports ......................................................................
Patch Set 1:
(1 comment)
This change is ready for review.
https://review.coreboot.org/c/coreboot/+/40425/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/acpi/usbc.asl:
PS1:
can we generate this using a new "chip drivers/intel/tcss" […]
+1 would love for this to be auto-generated.
Prashant Malani has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40425 )
Change subject: mb/google/volteer: Add support for USB Type-C ports ......................................................................
Patch Set 1:
Hi John, is this patch ready for review?
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40425 )
Change subject: mb/google/volteer: Add support for USB Type-C ports ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40425/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/acpi/usbc.asl:
PS1:
+1 would love for this to be auto-generated.
Couple of things. +1 to Caveh, this pretty much has to be generated at runtime. Also, why are the sbu-orientation and hsl-orientation static (i.e. a Package)? Are they intended to just be the orientations discovered at boot time, or should this be a Method, which can determine the correct setting when it's called?
Prashant Malani has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40425 )
Change subject: mb/google/volteer: Add support for USB Type-C ports ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40425/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/acpi/usbc.asl:
PS1:
Couple of things. +1 to Caveh, this pretty much has to be generated at runtime. […]
BTW, for the specific case of Volteer: both the parade part and burnside bridge have similar expected behaviour: - SBU lines lane reversal handled by the retimer. - HS (i.e SSTx/SSRx) lines lane reversal handled by AP (TCSS)
So it doesn't look like we will need runtime tweaking of those two properties for Volteer (that's not to say this will be the case for other retimers / board configurations)
https://review.coreboot.org/c/coreboot/+/40425/1/src/mainboard/google/voltee... PS1, Line 24: "normal" Is this the terminology being used by the kernel code which will be upstreamed?
https://github.com/krohei/linux/commit/c9dbafc424ad075aa424037440af01d60e92a... suggests enums/ints are being used.
Hello build bot (Jenkins), Furquan Shaikh, Tim Wawrzynczak, Duncan Laurie,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40425
to look at the new patch set (#2).
Change subject: mb/google/volteer: Add support for USB Type-C ports ......................................................................
mb/google/volteer: Add support for USB Type-C ports
Two usb type-c ports under the actual mux device. Each port has its own ACPI device entry (node). These nodes are the ones that the USB Type-C port/connector devices will refer to in order to configure the mux.
BUG=b:151646486 BRANCH=None TEST=Verify the scope of PMC.MUX CONx in the DSDT.
Change-Id: I4b443a1ca1c5361652a69586b340b1c95e9c3f06 Signed-off-by: John Zhao john.zhao@intel.com --- A src/mainboard/google/volteer/acpi/usbc.asl M src/mainboard/google/volteer/dsdt.asl 2 files changed, 46 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/25/40425/2
John Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40425 )
Change subject: mb/google/volteer: Add support for USB Type-C ports ......................................................................
Patch Set 2:
Patch Set 1:
(1 comment)
This change is ready for review.
Sorry being late to reply. It is not yet ready as still in the process of verification with updated kernel.
John Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40425 )
Change subject: mb/google/volteer: Add support for USB Type-C ports ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40425/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/acpi/usbc.asl:
PS1:
BTW, for the specific case of Volteer: both the parade part and burnside bridge have similar expecte […]
Maybe "chip drivers/intel/pmc"?
https://review.coreboot.org/c/coreboot/+/40425/1/src/mainboard/google/voltee... PS1, Line 24: "normal"
Is this the terminology being used by the kernel code which will be upstreamed? […]
Heikki's comment: "Those names are now reserved for generic references that point to the actual usb3 and usb2 ports. Here we supply the port indexes that the mux-agent driver needs to use when communicating with the PMC".
There will be minor update for ports properties: usb2-port -> usb2-port-number usb3-port -> usb3-port-number
Hello build bot (Jenkins), Furquan Shaikh, Tim Wawrzynczak, Duncan Laurie,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40425
to look at the new patch set (#3).
Change subject: mb/google/volteer: Add support for USB Type-C ports ......................................................................
mb/google/volteer: Add support for USB Type-C ports
Two usb type-c ports under the actual mux device. Each port has its own ACPI device entry (node). These nodes are the ones that the USB Type-C port/connector devices will refer to in order to configure the mux.
BUG=b:151646486 BRANCH=None TEST=Verify the scope of PMC.MUX CONx in the DSDT.
Change-Id: I4b443a1ca1c5361652a69586b340b1c95e9c3f06 Signed-off-by: John Zhao john.zhao@intel.com --- A src/mainboard/google/volteer/acpi/usbc.asl M src/mainboard/google/volteer/dsdt.asl 2 files changed, 46 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/25/40425/3
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40425 )
Change subject: mb/google/volteer: Add support for USB Type-C ports ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40425/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/acpi/usbc.asl:
PS1:
Maybe "chip drivers/intel/pmc"?
Sounds good!
John Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40425 )
Change subject: mb/google/volteer: Add support for USB Type-C ports ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40425/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/acpi/usbc.asl:
https://review.coreboot.org/c/coreboot/+/40425/1/src/mainboard/google/voltee... PS1, Line 24: "normal"
Heikki's comment: "Those names are now reserved for generic references that point to the actual usb3 […]
For "normal" description, as quoted from Heikki "the value must be a string for those properties, so it's correct as-is. We just convert the strings to integers in the code."
John Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40425 )
Change subject: mb/google/volteer: Add support for USB Type-C ports ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40425/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/acpi/usbc.asl:
PS1:
Sounds good!
Drop "chip drivers/intel/pmc" since pmc is hidden and resource is inaccessible. Back to Caveh's "drivers/intel/tcss" through xhci's device ID "PCI_DEVICE_ID_INTEL_TGP_TCSS_XHCI -> 0x9A13". USB properties of usb2-port-number/usb3-port-number/sbu-orientation/hsl-orientation are assigned from devicetree's xhci(00.0d.00). With the generated SSDT PMC.MUX.CONx along with SSDT LPCB.EC0.CREC.USBC, the preliminary test shows USBC(GOOG0014) and PMC(INTC1026) are probed successfully.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40425 )
Change subject: mb/google/volteer: Add support for USB Type-C ports ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40425/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/acpi/usbc.asl:
PS1:
Drop "chip drivers/intel/pmc" since pmc is hidden and resource is inaccessible. Back to Caveh's "drivers/intel/tcss"
Makes sense.
With the generated SSDT PMC.MUX.CONx along with SSDT LPCB.EC0.CREC.USBC, the preliminary test shows USBC(GOOG0014) and PMC(INTC1026) are probed successfully.
Great to hear!
Prashant Malani has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40425 )
Change subject: mb/google/volteer: Add support for USB Type-C ports ......................................................................
Patch Set 3:
(1 comment)
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40425/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/acpi/usbc.asl:
PS1:
With the generated SSDT PMC.MUX.CONx along with SSDT LPCB.EC0.CREC.USBC, the preliminary test shows USBC(GOOG0014) and PMC(INTC1026) are probed successfully.
Have you verified that the upstream Mux agent driver is probed successfully? HID = INTC105C. FWIU that is the driver that uses these properties. This driver is not in chromeos-5.4 yet, so you will have to verify it locally.
John Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40425 )
Change subject: mb/google/volteer: Add support for USB Type-C ports ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40425/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/acpi/usbc.asl:
PS1:
With the generated SSDT PMC.MUX.CONx along with SSDT LPCB.EC0.CREC. […]
We used the kernel 5.4 plus class driver (being assigned GOOG0013) which replaces the extcon driver and the followings coreboot configuration. #1 Upstream ToT with SSDT USBC (GOOG0014), change it to be GOOG0013 to align with kernel. #2 CB:38777 for DSDT PMC:INTC1026 and MUX:INTC105C. #3 SSDT version PMC.MUX.CONx for USB Type-c ports. dmesg shows GOOG0013 and INTC1026 are probed successfully. GOOG0013/INTC1026/INTC105C are shown from /sys/bus/platform/devices. USBC GOOG0013 and PMC.MUX.CONx are only shown in the SSDT.dsl. INTC1026 and INTC105C are shown in the DSDT.dsl.
Note: GOOG0014 will be applied instead of the temporarily GOOG0013.
Prashant Malani has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40425 )
Change subject: mb/google/volteer: Add support for USB Type-C ports ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40425/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/acpi/usbc.asl:
PS1:
We used the kernel 5. […]
Thanks for clarifying John. I think you would need to pick (and enable) the following driver, to ensure that the properties for ACPI HID=INTC105C are being parsed correctly:
https://elixir.bootlin.com/linux/v5.7-rc2/source/drivers/usb/typec/mux/intel...
John Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40425 )
Change subject: mb/google/volteer: Add support for USB Type-C ports ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40425/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/acpi/usbc.asl:
https://review.coreboot.org/c/coreboot/+/40425/1/src/mainboard/google/voltee... PS1, Line 22: Package () { "usb2-port", 6 }, : Package () { "usb3-port", 3 },
c0: […]
Ack
Hello build bot (Jenkins), Furquan Shaikh, Tim Wawrzynczak, Duncan Laurie,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40425
to look at the new patch set (#4).
Change subject: mb/google/volteer: Add support for USB Type-C ports ......................................................................
mb/google/volteer: Add support for USB Type-C ports
Two usb type-c ports under the actual mux device. Each port has its own ACPI device entry (node). These nodes are the ones that the USB Type-C port/connector devices will refer to in order to configure the mux.
BUG=b:151646486 BRANCH=None TEST=Verify the scope of PMC.MUX CONx in the DSDT.
Change-Id: I4b443a1ca1c5361652a69586b340b1c95e9c3f06 Signed-off-by: John Zhao john.zhao@intel.com --- A src/mainboard/google/volteer/acpi/usbc.asl M src/mainboard/google/volteer/dsdt.asl 2 files changed, 46 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/25/40425/4
Vijay P Hiremath has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40425 )
Change subject: mb/google/volteer: Add support for USB Type-C ports ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40425/4/src/mainboard/google/voltee... File src/mainboard/google/volteer/acpi/usbc.asl:
https://review.coreboot.org/c/coreboot/+/40425/4/src/mainboard/google/voltee... PS4, Line 24: normal if this info is not based on the dynamic "CC_orienation" then the fields need to be like these "fixed" or "follow_cc_line"
Attention is currently required from: John Zhao. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40425 )
Change subject: mb/google/volteer: Add support for USB Type-C ports ......................................................................
Patch Set 4:
(1 comment)
Patchset:
PS4: I don't think this patch is required anymore
John Zhao has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/40425 )
Change subject: mb/google/volteer: Add support for USB Type-C ports ......................................................................
Abandoned
The settings had been moved to SSDT.