Change in coreboot[master]: ec/google/chromeec: Add support for USB Type-C alternate mode control
John Zhao has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/40424 )
Change subject: ec/google/chromeec: Add support for USB Type-C alternate mode control ......................................................................
ec/google/chromeec: Add support for USB Type-C alternate mode control
PMC mux control can be used for swapping the USB data role and for setting connector to act in alternate modes.
BUG=b:151646486 BRANCH=None TEST=Verify USB Type-C alternate mode (DP or TBT) along with PMC mux control.
Change-Id: I8c872537d9c633f8eb4238fd9b023577c4570a5a Signed-off-by: John Zhao john.zhao@intel.com --- M src/ec/google/chromeec/acpi/cros_ec.asl 1 file changed, 113 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/40424/1
diff --git a/src/ec/google/chromeec/acpi/cros_ec.asl b/src/ec/google/chromeec/acpi/cros_ec.asl index 3e9b773..a8d859f 100644 --- a/src/ec/google/chromeec/acpi/cros_ec.asl +++ b/src/ec/google/chromeec/acpi/cros_ec.asl @@ -56,4 +56,117 @@ { Return (0xB) } + + Device (USBC) + { + Name (_HID, "GOOG0014") + Name (_DDN, "ChromeOS Embedded Controller USB Type-C Control") + + /* + * Each connector shall have its own ACPI device entry + * (node), under the actual interface device. + */ + Device (CON0) + { + Name (_ADR, 0) + Name (_DSD, Package () { + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), + Package() { + Package () { "usb-role-switch", + _SB.PCI0.PMC.MUX.CON0 }, + Package () { "mode-switch", + _SB.PCI0.PMC.MUX.CON0 }, + Package () { "orientation-switch", + _SB.PCI0.PMC.MUX.CON0 }, + Package () { "power-role", "dual" }, + Package () { "data-role", "dual" }, + } + }) + + /* + * Each supported alternate mode. Use _STA() to disable/enable + * them. + */ + + /* Thunderbolt 3 alternate mode */ + Device (TBT3) + { + Name (_ADR, 0) + + Name (_DSD, Package () { + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), + Package() { + Package () { "svid", 0x8087 }, + Package () { "vdo", 1 }, + } + }) + + Method (_STA, 0, NotSerialized) + { + Return (0x0F) + } + } + + /* Display Port alternate mode */ + Device (DP) + { + Name (_ADR, 1) + Name (_DSD, Package () { + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), + Package() { + Package () { "svid", 0xff01 }, + Package () { "vdo", 0x1c46 }, + } + }) + } + } + + Device (CON1) + { + Name (_ADR, 1) + Name (_DSD, Package () { + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), + Package() { + Package () { "usb-role-switch", + _SB.PCI0.PMC.MUX.CON1 }, + Package () { "mode-switch", + _SB.PCI0.PMC.MUX.CON1 }, + Package () { "orientation-switch", + _SB.PCI0.PMC.MUX.CON1 }, + Package () { "power-role", "dual" }, + Package () { "data-role", "dual" }, + }, + }) + + Device (TBT3) + { + Name (_ADR, 0) + + Name (_DSD, Package () { + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), + Package() { + Package () {"svid", 0x8087}, + Package () {"vdo", 1}, + } + }) + + Method (_STA, 0, NotSerialized) + { + Return (0x0F) + } + } + + Device (DP) + { + Name (_ADR, 1) + Name (_DSD, Package () { + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), + Package() { + Package () { "svid", 0xff01 }, + Package () { "vdo", 0x1c46 }, + } + }) + } + } + } }
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40424 )
Change subject: ec/google/chromeec: Add support for USB Type-C alternate mode control ......................................................................
Patch Set 1:
(1 comment)
This change is ready for review.
https://review.coreboot.org/c/coreboot/+/40424/1/src/ec/google/chromeec/acpi... File src/ec/google/chromeec/acpi/cros_ec.asl:
https://review.coreboot.org/c/coreboot/+/40424/1/src/ec/google/chromeec/acpi... PS1, Line 60: Device (USBC)
Most of the work here is already done in: https://review.coreboot.org/cgit/coreboot. […]
That and we can't add this unconditionally. This is a shared ASL file.
Furquan, we probably want to generate this stuff at runtime vs being static. I'm not sure if Tim wants to venture into this. I think we've hit our saturation point for static asl.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40424 )
Change subject: ec/google/chromeec: Add support for USB Type-C alternate mode control ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40424/1/src/ec/google/chromeec/acpi... File src/ec/google/chromeec/acpi/cros_ec.asl:
https://review.coreboot.org/c/coreboot/+/40424/1/src/ec/google/chromeec/acpi... PS1, Line 60: Device (USBC)
That and we can't add this unconditionally. This is a shared ASL file. […]
Yes, the plan is to generate this stuff at runtime. In fact, https://review.coreboot.org/cgit/coreboot.git/tree/src/ec/google/chromeec/ec... already takes care of generating the connector nodes with _DSD in SSDT.
However, there is still more work required to be done here.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40424 )
Change subject: ec/google/chromeec: Add support for USB Type-C alternate mode control ......................................................................
Patch Set 1:
(1 comment)
This change is ready for review.
https://review.coreboot.org/c/coreboot/+/40424/1/src/ec/google/chromeec/acpi... File src/ec/google/chromeec/acpi/cros_ec.asl:
https://review.coreboot.org/c/coreboot/+/40424/1/src/ec/google/chromeec/acpi... PS1, Line 60: Device (USBC)
Yes, the plan is to generate this stuff at runtime. In fact, https://review.coreboot. […]
I'd be happy to take a look once we have an idea what the generated ASL should look like.
Prashant Malani has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40424 )
Change subject: ec/google/chromeec: Add support for USB Type-C alternate mode control ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40424/1/src/ec/google/chromeec/acpi... File src/ec/google/chromeec/acpi/cros_ec.asl:
https://review.coreboot.org/c/coreboot/+/40424/1/src/ec/google/chromeec/acpi... PS1, Line 82: Package () { "data-role", "dual" }, port-number also needs to specified as a property (This is of course, handled by the SSDT generation code)
https://review.coreboot.org/c/coreboot/+/40424/1/src/ec/google/chromeec/acpi... PS1, Line 91: /* Thunderbolt 3 alternate mode */ : Device (TBT3) : { : Name (_ADR, 0) : : Name (_DSD, Package () { : ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), : Package() { : Package () { "svid", 0x8087 }, : Package () { "vdo", 1 }, : } : }) : : Method (_STA, 0, NotSerialized) : { : Return (0x0F) : } : } : : /* Display Port alternate mode */ : Device (DP) : { : Name (_ADR, 1) : Name (_DSD, Package () { : ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), : Package() { : Package () { "svid", 0xff01 }, : Package () { "vdo", 0x1c46 }, : } : }) : } We've still not decided how to define alternate mode support, so we should leave this out for upstream patches (for now)
Prashant Malani has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40424 )
Change subject: ec/google/chromeec: Add support for USB Type-C alternate mode control ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40424/1/src/ec/google/chromeec/acpi... File src/ec/google/chromeec/acpi/cros_ec.asl:
https://review.coreboot.org/c/coreboot/+/40424/1/src/ec/google/chromeec/acpi... PS1, Line 74: Package https://review.coreboot.org/c/coreboot/+/40425/1/src/mainboard/google/voltee... adds these objects.
I would think this cros_ec file should be made board specific, and should only include the switch definitions? These will then get combined with the dynamically generated connector nodes?
So, something like:
Device (USBC) { Name (_HID, "GOOG0014") Name (_DDN, "ChromeOS Embedded Controller USB Type-C Control")
/* * Each connector shall have its own ACPI device entry * (node), under the actual interface device. */ Device (CON0) { Name (_ADR, 0) Name (_DSD, Package () { ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), Package() { Package () { "usb-role-switch", _SB.PCI0.PMC.MUX.CON0 }, Package () { "mode-switch", _SB.PCI0.PMC.MUX.CON0 }, Package () { "orientation-switch", _SB.PCI0.PMC.MUX.CON0 }, } }) }
Device (CON1) { Name (_ADR, 1) Name (_DSD, Package () { ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), Package() { Package () { "usb-role-switch", _SB.PCI0.PMC.MUX.CON1 }, Package () { "mode-switch", _SB.PCI0.PMC.MUX.CON1 }, Package () { "orientation-switch", _SB.PCI0.PMC.MUX.CON1 }, }, })
} }
Not sure how dynamically generated + statically defined ASL is handled in the kernel, so my apologies if the above is incorrect.
Of course, the above is just a proposal for an interim solution. Eventually, all the properties should be dynamically generated.
Prashant Malani has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40424 )
Change subject: ec/google/chromeec: Add support for USB Type-C alternate mode control ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40424/1/src/ec/google/chromeec/acpi... File src/ec/google/chromeec/acpi/cros_ec.asl:
https://review.coreboot.org/c/coreboot/+/40424/1/src/ec/google/chromeec/acpi... PS1, Line 74: Package
https://review.coreboot.org/c/coreboot/+/40425/1/src/mainboard/google/voltee.... […]
Update: as defined above, the merging of SSDT generated ASL and DSDT asl does not work. The kernel only seems to see the DSDT version.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40424 )
Change subject: ec/google/chromeec: Add support for USB Type-C alternate mode control ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40424/1/src/ec/google/chromeec/acpi... File src/ec/google/chromeec/acpi/cros_ec.asl:
https://review.coreboot.org/c/coreboot/+/40424/1/src/ec/google/chromeec/acpi... PS1, Line 74: Package
Update: as defined above, the merging of SSDT generated ASL and DSDT asl does not work. […]
Thanks for confirming, that is what I expected.
Hello build bot (Jenkins), Tim Wawrzynczak, Duncan Laurie,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40424
to look at the new patch set (#2).
Change subject: ec/google/chromeec: Add support for USB Type-C alternate mode control ......................................................................
ec/google/chromeec: Add support for USB Type-C alternate mode control
PMC mux control can be used for swapping the USB data role and for setting connector to act in alternate modes.
BUG=b:151646486 BRANCH=None TEST=Verify USB Type-C alternate mode (DP or TBT) along with PMC mux control.
Change-Id: I8c872537d9c633f8eb4238fd9b023577c4570a5a Signed-off-by: John Zhao john.zhao@intel.com --- M src/ec/google/chromeec/acpi/cros_ec.asl 1 file changed, 113 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/40424/2
Prashant Malani has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40424 )
Change subject: ec/google/chromeec: Add support for USB Type-C alternate mode control ......................................................................
Patch Set 2:
(3 comments)
https://review.coreboot.org/c/coreboot/+/40424/1/src/ec/google/chromeec/acpi... File src/ec/google/chromeec/acpi/cros_ec.asl:
https://review.coreboot.org/c/coreboot/+/40424/1/src/ec/google/chromeec/acpi... PS1, Line 74: Package
Thanks for confirming, that is what I expected.
Ack
https://review.coreboot.org/c/coreboot/+/40424/2/src/ec/google/chromeec/acpi... File src/ec/google/chromeec/acpi/cros_ec.asl:
https://review.coreboot.org/c/coreboot/+/40424/2/src/ec/google/chromeec/acpi... PS2, Line 62: GOOG0013 The device ID should be GOOG0014
https://review.coreboot.org/c/coreboot/+/40424/2/src/ec/google/chromeec/acpi... PS2, Line 71: Name (_ADR, 0) : Name (_DSD, Package () { : ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), : Package() { : Package () { "usb-role-switch", : _SB.PCI0.PMC.MUX.CON0 }, : Package () { "mode-switch", : _SB.PCI0.PMC.MUX.CON0 }, : Package () { "orientation-switch", : _SB.PCI0.PMC.MUX.CON0 }, : Package () { "power-role", "dual" }, : Package () { "data-role", "dual" }, : } : }) : Please check with twawrzynczak@.
These properties will be getting auto-generated, so instead ensure that the correct board-file/devicetree settings are enabled for this board.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40424 )
Change subject: ec/google/chromeec: Add support for USB Type-C alternate mode control ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40424/2/src/ec/google/chromeec/acpi... File src/ec/google/chromeec/acpi/cros_ec.asl:
https://review.coreboot.org/c/coreboot/+/40424/2/src/ec/google/chromeec/acpi... PS2, Line 71: Name (_ADR, 0) : Name (_DSD, Package () { : ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), : Package() { : Package () { "usb-role-switch", : _SB.PCI0.PMC.MUX.CON0 }, : Package () { "mode-switch", : _SB.PCI0.PMC.MUX.CON0 }, : Package () { "orientation-switch", : _SB.PCI0.PMC.MUX.CON0 }, : Package () { "power-role", "dual" }, : Package () { "data-role", "dual" }, : } : }) :
Please check with twawrzynczak@. […]
John, the patch I've got in flight that enables auto-generation of those properties is CB:40618. Could you rebase on top of that? It will probably depend on having the PMC MUX driver set up in the SSDT as well, so it can be found in the devicetree.
John Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40424 )
Change subject: ec/google/chromeec: Add support for USB Type-C alternate mode control ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40424/2/src/ec/google/chromeec/acpi... File src/ec/google/chromeec/acpi/cros_ec.asl:
https://review.coreboot.org/c/coreboot/+/40424/2/src/ec/google/chromeec/acpi... PS2, Line 71: Name (_ADR, 0) : Name (_DSD, Package () { : ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), : Package() { : Package () { "usb-role-switch", : _SB.PCI0.PMC.MUX.CON0 }, : Package () { "mode-switch", : _SB.PCI0.PMC.MUX.CON0 }, : Package () { "orientation-switch", : _SB.PCI0.PMC.MUX.CON0 }, : Package () { "power-role", "dual" }, : Package () { "data-role", "dual" }, : } : }) :
John, the patch I've got in flight that enables auto-generation of those properties is CB:40618. […]
Sure, thanks. I will refer to CB:40618. Let us wait the final clarification for HID, GOOG0014 or GOOG0013.
Prashant Malani has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40424 )
Change subject: ec/google/chromeec: Add support for USB Type-C alternate mode control ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40424/2/src/ec/google/chromeec/acpi... File src/ec/google/chromeec/acpi/cros_ec.asl:
https://review.coreboot.org/c/coreboot/+/40424/2/src/ec/google/chromeec/acpi... PS2, Line 71: Name (_ADR, 0) : Name (_DSD, Package () { : ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), : Package() { : Package () { "usb-role-switch", : _SB.PCI0.PMC.MUX.CON0 }, : Package () { "mode-switch", : _SB.PCI0.PMC.MUX.CON0 }, : Package () { "orientation-switch", : _SB.PCI0.PMC.MUX.CON0 }, : Package () { "power-role", "dual" }, : Package () { "data-role", "dual" }, : } : }) :
Sure, thanks. I will refer to CB:40618. […]
It's GOOG0014. We'd communicated the change to Mika a while back. It's also in the upstream driver code: https://elixir.bootlin.com/linux/v5.7-rc2/source/drivers/platform/chrome/cro...
Hello build bot (Jenkins), Tim Wawrzynczak, Duncan Laurie,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40424
to look at the new patch set (#3).
Change subject: ec/google/chromeec: Add support for USB Type-C alternate mode control ......................................................................
ec/google/chromeec: Add support for USB Type-C alternate mode control
PMC mux control can be used for swapping the USB data role and for setting connector to act in alternate modes.
BUG=b:151646486 BRANCH=None TEST=Verify USB Type-C alternate mode (DP or TBT) along with PMC mux control.
Change-Id: I8c872537d9c633f8eb4238fd9b023577c4570a5a Signed-off-by: John Zhao john.zhao@intel.com --- M src/ec/google/chromeec/acpi/cros_ec.asl 1 file changed, 113 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/40424/3
John Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40424 )
Change subject: ec/google/chromeec: Add support for USB Type-C alternate mode control ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40424/2/src/ec/google/chromeec/acpi... File src/ec/google/chromeec/acpi/cros_ec.asl:
https://review.coreboot.org/c/coreboot/+/40424/2/src/ec/google/chromeec/acpi... PS2, Line 71: Name (_ADR, 0) : Name (_DSD, Package () { : ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), : Package() { : Package () { "usb-role-switch", : _SB.PCI0.PMC.MUX.CON0 }, : Package () { "mode-switch", : _SB.PCI0.PMC.MUX.CON0 }, : Package () { "orientation-switch", : _SB.PCI0.PMC.MUX.CON0 }, : Package () { "power-role", "dual" }, : Package () { "data-role", "dual" }, : } : }) :
It's GOOG0014. We'd communicated the change to Mika a while back. […]
yes, thanks for the clarification. I also got confirmation GOOG0013 was claimed for audio and GOOG0014 is for the mux agent.
We are working with DSDT version and will consider to refer to SSDT (CB:40618).
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40424 )
Change subject: ec/google/chromeec: Add support for USB Type-C alternate mode control ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40424/2/src/ec/google/chromeec/acpi... File src/ec/google/chromeec/acpi/cros_ec.asl:
https://review.coreboot.org/c/coreboot/+/40424/2/src/ec/google/chromeec/acpi... PS2, Line 71: Name (_ADR, 0) : Name (_DSD, Package () { : ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), : Package() { : Package () { "usb-role-switch", : _SB.PCI0.PMC.MUX.CON0 }, : Package () { "mode-switch", : _SB.PCI0.PMC.MUX.CON0 }, : Package () { "orientation-switch", : _SB.PCI0.PMC.MUX.CON0 }, : Package () { "power-role", "dual" }, : Package () { "data-role", "dual" }, : } : }) :
yes, thanks for the clarification. […]
Sounds good, let me know if you'd like me to take a look.
Stefan Reinauer has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/40424?usp=email )
Change subject: ec/google/chromeec: Add support for USB Type-C alternate mode control ......................................................................
Abandoned
-
Aaron Durbin (Code Review) -
Furquan Shaikh (Code Review)
-
John Zhao (Code Review)
-
Prashant Malani (Code Review)
-
Stefan Reinauer (Code Review)
-
Tim Wawrzynczak (Code Review)