Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42737 )
Change subject: soc/intel/cannonlake: Add UWES ASL into xhci.asl ......................................................................
soc/intel/cannonlake: Add UWES ASL into xhci.asl
Align support for enable wake-on-usb attach/detach as was introduced in Skylake in `commit 3bfe3404df32ca226c624be0435c640bf1ebeae7`.
This adds the USB Wake Enable Setup (UWES) ASL blocks required to inform the OS about plug wake events bits being set in the PORTSCN register configured by devicetree.
BUG=b:159187889 BRANCH=none TEST=none
Change-Id: I6c63d226e5acadff04486c8a6764fb715a0ac051 Signed-off-by: Edward O'Callaghan quasisec@google.com --- M src/soc/intel/cannonlake/acpi/xhci.asl 1 file changed, 63 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/42737/1
diff --git a/src/soc/intel/cannonlake/acpi/xhci.asl b/src/soc/intel/cannonlake/acpi/xhci.asl index d5187e6..fabe1f1 100644 --- a/src/soc/intel/cannonlake/acpi/xhci.asl +++ b/src/soc/intel/cannonlake/acpi/xhci.asl @@ -2,6 +2,61 @@
#include <soc/gpe.h>
+/* + * USB Port Wake Enable (UPWE) on usb attach/detach + * Arg0 - Port Number + * Arg1 - Port 1 Status and control offset + * Arg2 - xHCI Memory-mapped address + */ +Method (UPWE, 3, Serialized) +{ + /* Local0 = Arg1 + ((Arg0 - 1) * 0x10) */ + Add (Arg1, Multiply (Subtract (Arg0, 1), 0x10), Local0) + + /* Map ((XMEM << 16) + Local0 in PSCR */ + OperationRegion (PSCR, SystemMemory, + Add (ShiftLeft (Arg2, 16), Local0), 0x10) + Field (PSCR, DWordAcc, NoLock, Preserve) + { + PSCT, 32, + } + Store(PSCT, Local0) + /* + * And port status/control reg with RO and RWS bits + * RO bits: 0, 2:3, 10:13, 24, 28:30 + * RWS bits: 5:9, 14:16, 25:27 + */ + And (Local0, ~0x80FE0012, Local0) + /* Set WCE and WDE bits */ + Or (Local0, 0x6000000, Local0) + Store(Local0, PSCT) +} + +/* + * USB Wake Enable Setup (UWES) + * Arg0 - Port enable bitmap + * Arg1 - Port 1 Status and control offset + * Arg2 - xHCI Memory-mapped address + */ +Method (UWES, 3, Serialized) +{ + Store (Arg0, Local0) + + While (One) { + FindSetRightBit (Local0, Local1) + If (LEqual (Local1, Zero)) { + Break + } + UPWE (Local1, Arg1, Arg2) + /* + * Clear the lowest set bit in Local0 since it was + * processed. + * Local0 = Local0 & (Local0 - 1) + */ + And (Local0, Subtract (Local0, 1), Local0) + } +} + /* XHCI Controller 0:14.0 */
Device (XHCI) @@ -10,6 +65,14 @@
Name (_PRW, Package () { GPE0_PME_B0, 3 })
+ Name (_DSW, 3) + { + Store (Arg0, PMEE) + UWES (And (\U2WE, 0x3FF), 0x480, XMEM) + UWES (And (\U3WE, 0x3F ), 0x540, XMEM) + + } + Name (_S3D, 3) /* D3 supported in S3 */ Name (_S0W, 3) /* D3 can wake device in S0 */ Name (_S3W, 3) /* D3 can wake system from S3 */
Hello build bot (Jenkins), Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42737
to look at the new patch set (#2).
Change subject: soc/intel/cannonlake: Add UWES ASL into xhci.asl ......................................................................
soc/intel/cannonlake: Add UWES ASL into xhci.asl
Align support for enable wake-on-usb attach/detach as was introduced in Skylake in `commit 3bfe3404df32ca226c624be0435c640bf1ebeae7`.
This adds the USB Wake Enable Setup (UWES) ASL blocks required to inform the OS about plug wake events bits being set in the PORTSCN register configured by devicetree.
BUG=b:159187889 BRANCH=none TEST=none
Change-Id: I6c63d226e5acadff04486c8a6764fb715a0ac051 Signed-off-by: Edward O'Callaghan quasisec@google.com --- M src/soc/intel/cannonlake/acpi/xhci.asl 1 file changed, 71 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/42737/2
Hello build bot (Jenkins), Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42737
to look at the new patch set (#3).
Change subject: soc/intel/cannonlake: Add UWES ASL into xhci.asl ......................................................................
soc/intel/cannonlake: Add UWES ASL into xhci.asl
Align support for enable wake-on-usb attach/detach as was introduced in Skylake in `commit 3bfe3404df32ca226c624be0435c640bf1ebeae7`.
This adds the USB Wake Enable Setup (UWES) ASL blocks required to inform the OS about plug wake events bits being set in the PORTSCN register configured by devicetree.
BUG=b:159187889 BRANCH=none TEST=none
Change-Id: I6c63d226e5acadff04486c8a6764fb715a0ac051 Signed-off-by: Edward O'Callaghan quasisec@google.com --- M src/soc/intel/cannonlake/acpi/xhci.asl 1 file changed, 80 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/42737/3
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42737 )
Change subject: soc/intel/cannonlake: Add UWES ASL into xhci.asl ......................................................................
Patch Set 3:
(6 comments)
https://review.coreboot.org/c/coreboot/+/42737/3/src/soc/intel/cannonlake/ac... File src/soc/intel/cannonlake/acpi/xhci.asl:
https://review.coreboot.org/c/coreboot/+/42737/3/src/soc/intel/cannonlake/ac... PS3, Line 29: ~0x80FE0012 symbolic constant would be nice (RO_BITS_OFF_MASK or similar)
https://review.coreboot.org/c/coreboot/+/42737/3/src/soc/intel/cannonlake/ac... PS3, Line 31: 0x6000000 symbolic constant would be nice (WAKE_ON_CONNECT_DISCONNECT_ENABLE or similar)
https://review.coreboot.org/c/coreboot/+/42737/3/src/soc/intel/cannonlake/ac... PS3, Line 70: 0x480 symbolic constant for this would be nice (PORTSCN_OFFSET or similar)
https://review.coreboot.org/c/coreboot/+/42737/3/src/soc/intel/cannonlake/ac... PS3, Line 71: 0x540 symbolic constant for this would be nice (PORTSCXUSB3_OFFSET or similar)
https://review.coreboot.org/c/coreboot/+/42737/3/src/soc/intel/cannonlake/ac... PS3, Line 82: DVID, 16, /* VENDORID */ this is unused
https://review.coreboot.org/c/coreboot/+/42737/3/src/soc/intel/cannonlake/ac... PS3, Line 88: OperationRegion (XREG, SystemMemory, : Add (ShiftLeft (XMEM, 16), 0x8000), 0x200) : Field (XREG, DWordAcc, Lock, Preserve) : { : Offset (0x1c4), /* USB2PMCTRL */ : , 2, : UPSW, 2, /* U2PSUSPGP */ : } this whole operation region is unused
Sam McNally has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42737 )
Change subject: soc/intel/cannonlake: Add UWES ASL into xhci.asl ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42737/3/src/soc/intel/cannonlake/ac... File src/soc/intel/cannonlake/acpi/xhci.asl:
https://review.coreboot.org/c/coreboot/+/42737/3/src/soc/intel/cannonlake/ac... PS3, Line 70: 0x3FF There are 12 USB2 ports listed below (compared to 10 for skylake) so I suspect this should be 0xFFF for cannonlake.
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42737 )
Change subject: soc/intel/cannonlake: Add UWES ASL into xhci.asl ......................................................................
Patch Set 3:
Please use the new ASL syntax.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42737 )
Change subject: soc/intel/cannonlake: Add UWES ASL into xhci.asl ......................................................................
Patch Set 3:
(7 comments)
https://review.coreboot.org/c/coreboot/+/42737/3/src/soc/intel/cannonlake/ac... File src/soc/intel/cannonlake/acpi/xhci.asl:
https://review.coreboot.org/c/coreboot/+/42737/3/src/soc/intel/cannonlake/ac... PS3, Line 29: ~0x80FE0012
symbolic constant would be nice (RO_BITS_OFF_MASK or similar)
Done
https://review.coreboot.org/c/coreboot/+/42737/3/src/soc/intel/cannonlake/ac... PS3, Line 31: 0x6000000
symbolic constant would be nice (WAKE_ON_CONNECT_DISCONNECT_ENABLE or similar)
Done
https://review.coreboot.org/c/coreboot/+/42737/3/src/soc/intel/cannonlake/ac... PS3, Line 70: 0x3FF
There are 12 USB2 ports listed below (compared to 10 for skylake) so I suspect this should be 0xFFF […]
Done
https://review.coreboot.org/c/coreboot/+/42737/3/src/soc/intel/cannonlake/ac... PS3, Line 70: 0x480
symbolic constant for this would be nice (PORTSCN_OFFSET or similar)
Done
https://review.coreboot.org/c/coreboot/+/42737/3/src/soc/intel/cannonlake/ac... PS3, Line 71: 0x540
symbolic constant for this would be nice (PORTSCXUSB3_OFFSET or similar)
Done
https://review.coreboot.org/c/coreboot/+/42737/3/src/soc/intel/cannonlake/ac... PS3, Line 82: DVID, 16, /* VENDORID */
this is unused
Done
https://review.coreboot.org/c/coreboot/+/42737/3/src/soc/intel/cannonlake/ac... PS3, Line 88: OperationRegion (XREG, SystemMemory, : Add (ShiftLeft (XMEM, 16), 0x8000), 0x200) : Field (XREG, DWordAcc, Lock, Preserve) : { : Offset (0x1c4), /* USB2PMCTRL */ : , 2, : UPSW, 2, /* U2PSUSPGP */ : }
this whole operation region is unused
Done
Hello Shelley Chen, build bot (Jenkins), Daniel Kurtz, Furquan Shaikh, Tim Wawrzynczak, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42737
to look at the new patch set (#4).
Change subject: soc/intel/cannonlake: Add UWES ASL into xhci.asl ......................................................................
soc/intel/cannonlake: Add UWES ASL into xhci.asl
Align support for enable wake-on-usb attach/detach as was introduced in Skylake in `commit 3bfe3404df32ca226c624be0435c640bf1ebeae7`.
This adds the USB Wake Enable Setup (UWES) ASL blocks required to inform the OS about plug wake events bits being set in the PORTSCN register configured by devicetree.
BUG=b:159187889 BRANCH=none TEST=none
Change-Id: I6c63d226e5acadff04486c8a6764fb715a0ac051 Signed-off-by: Edward O'Callaghan quasisec@google.com --- M src/soc/intel/cannonlake/acpi/xhci.asl 1 file changed, 75 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/42737/4
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42737 )
Change subject: soc/intel/cannonlake: Add UWES ASL into xhci.asl ......................................................................
Patch Set 4:
(9 comments)
I think Patrick is right, we should switch to use new ASL syntax when possible
https://review.coreboot.org/c/coreboot/+/42737/4/src/soc/intel/cannonlake/ac... File src/soc/intel/cannonlake/acpi/xhci.asl:
https://review.coreboot.org/c/coreboot/+/42737/4/src/soc/intel/cannonlake/ac... PS4, Line 20: Add (Arg1, Multiply (Subtract (Arg0, 1), 0x10), Local0) With new ASL syntax, I believe your comment can be the actual code now: Local0 = Arg1 + ((Arg0 - 1) * 0x10)
https://review.coreboot.org/c/coreboot/+/42737/4/src/soc/intel/cannonlake/ac... PS4, Line 35: And (Local0, RO_BITS_OFF_MASK, Local0) new syntax: Local0 = Local0 & RO_BITS_OFF_MASK
https://review.coreboot.org/c/coreboot/+/42737/4/src/soc/intel/cannonlake/ac... PS4, Line 37: Or (Local0, WAKE_ON_CONNECT_DISCONNECT_ENABLE, Local0) new syntax: Local0 = Local0 | WAKE_ON_CONNECT_DISCONNECT_ENABLE
https://review.coreboot.org/c/coreboot/+/42737/4/src/soc/intel/cannonlake/ac... PS4, Line 38: Store(Local0, PSCT) PSCT = Local0
https://review.coreboot.org/c/coreboot/+/42737/4/src/soc/intel/cannonlake/ac... PS4, Line 49: Store (Arg0, Local0) Local0 = Arg0
https://review.coreboot.org/c/coreboot/+/42737/4/src/soc/intel/cannonlake/ac... PS4, Line 53: If (LEqual (Local1, Zero)) If (Local1 == Zero)
https://review.coreboot.org/c/coreboot/+/42737/4/src/soc/intel/cannonlake/ac... PS4, Line 62: And (Local0, Subtract (Local0, 1), Local0) Local0 = Local0 & (Local0 - 1)
https://review.coreboot.org/c/coreboot/+/42737/4/src/soc/intel/cannonlake/ac... PS4, Line 76: nd (\U2WE, 0xFFF) \U2WE & 0xFFF
https://review.coreboot.org/c/coreboot/+/42737/4/src/soc/intel/cannonlake/ac... PS4, Line 77: And (\U3WE, 0x3F ) \U3WE & 0x3F
Hello Shelley Chen, build bot (Jenkins), Daniel Kurtz, Furquan Shaikh, Tim Wawrzynczak, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42737
to look at the new patch set (#5).
Change subject: soc/intel/cannonlake: Add UWES ASL into xhci.asl ......................................................................
soc/intel/cannonlake: Add UWES ASL into xhci.asl
Align support for enable wake-on-usb attach/detach as was introduced in Skylake in `commit 3bfe3404df32ca226c624be0435c640bf1ebeae7`.
This adds the USB Wake Enable Setup (UWES) ASL blocks required to inform the OS about plug wake events bits being set in the PORTSCN register configured by devicetree.
BUG=b:159187889 BRANCH=none TEST=none
Change-Id: I6c63d226e5acadff04486c8a6764fb715a0ac051 Signed-off-by: Edward O'Callaghan quasisec@google.com --- M src/soc/intel/cannonlake/acpi/xhci.asl 1 file changed, 74 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/42737/5
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42737 )
Change subject: soc/intel/cannonlake: Add UWES ASL into xhci.asl ......................................................................
Patch Set 4:
(9 comments)
Patch Set 4:
(9 comments)
I think Patrick is right, we should switch to use new ASL syntax when possible
Thank you for showing me this! Done.
https://review.coreboot.org/c/coreboot/+/42737/4/src/soc/intel/cannonlake/ac... File src/soc/intel/cannonlake/acpi/xhci.asl:
https://review.coreboot.org/c/coreboot/+/42737/4/src/soc/intel/cannonlake/ac... PS4, Line 20: Add (Arg1, Multiply (Subtract (Arg0, 1), 0x10), Local0)
With new ASL syntax, I believe your comment can be the actual code now: […]
Done
https://review.coreboot.org/c/coreboot/+/42737/4/src/soc/intel/cannonlake/ac... PS4, Line 35: And (Local0, RO_BITS_OFF_MASK, Local0)
new syntax: […]
Done
https://review.coreboot.org/c/coreboot/+/42737/4/src/soc/intel/cannonlake/ac... PS4, Line 37: Or (Local0, WAKE_ON_CONNECT_DISCONNECT_ENABLE, Local0)
new syntax: […]
Done
https://review.coreboot.org/c/coreboot/+/42737/4/src/soc/intel/cannonlake/ac... PS4, Line 38: Store(Local0, PSCT)
PSCT = Local0
Done
https://review.coreboot.org/c/coreboot/+/42737/4/src/soc/intel/cannonlake/ac... PS4, Line 49: Store (Arg0, Local0)
Local0 = Arg0
Done
https://review.coreboot.org/c/coreboot/+/42737/4/src/soc/intel/cannonlake/ac... PS4, Line 53: If (LEqual (Local1, Zero))
If (Local1 == Zero)
Done
https://review.coreboot.org/c/coreboot/+/42737/4/src/soc/intel/cannonlake/ac... PS4, Line 62: And (Local0, Subtract (Local0, 1), Local0)
Local0 = Local0 & (Local0 - 1)
Done
https://review.coreboot.org/c/coreboot/+/42737/4/src/soc/intel/cannonlake/ac... PS4, Line 76: nd (\U2WE, 0xFFF)
\U2WE & 0xFFF
Done
https://review.coreboot.org/c/coreboot/+/42737/4/src/soc/intel/cannonlake/ac... PS4, Line 77: And (\U3WE, 0x3F )
\U3WE & 0x3F
Done
Hello Shelley Chen, build bot (Jenkins), Daniel Kurtz, Furquan Shaikh, Tim Wawrzynczak, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42737
to look at the new patch set (#6).
Change subject: soc/intel/cannonlake: Add UWES ASL into xhci.asl ......................................................................
soc/intel/cannonlake: Add UWES ASL into xhci.asl
Align support for enable wake-on-usb attach/detach as was introduced in Skylake in `commit 3bfe3404df32ca226c624be0435c640bf1ebeae7`.
This adds the USB Wake Enable Setup (UWES) ASL blocks required to inform the OS about plug wake events bits being set in the PORTSCN register configured by devicetree.
BUG=b:159187889 BRANCH=none TEST=none
Change-Id: I6c63d226e5acadff04486c8a6764fb715a0ac051 Signed-off-by: Edward O'Callaghan quasisec@google.com --- M src/soc/intel/cannonlake/acpi/xhci.asl 1 file changed, 73 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/42737/6
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42737 )
Change subject: soc/intel/cannonlake: Add UWES ASL into xhci.asl ......................................................................
Patch Set 6: Code-Review+2
Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/coreboot/+/42737 )
Change subject: soc/intel/cannonlake: Add UWES ASL into xhci.asl ......................................................................
soc/intel/cannonlake: Add UWES ASL into xhci.asl
Align support for enable wake-on-usb attach/detach as was introduced in Skylake in `commit 3bfe3404df32ca226c624be0435c640bf1ebeae7`.
This adds the USB Wake Enable Setup (UWES) ASL blocks required to inform the OS about plug wake events bits being set in the PORTSCN register configured by devicetree.
BUG=b:159187889 BRANCH=none TEST=none
Change-Id: I6c63d226e5acadff04486c8a6764fb715a0ac051 Signed-off-by: Edward O'Callaghan quasisec@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/42737 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/soc/intel/cannonlake/acpi/xhci.asl 1 file changed, 73 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Tim Wawrzynczak: Looks good to me, approved
diff --git a/src/soc/intel/cannonlake/acpi/xhci.asl b/src/soc/intel/cannonlake/acpi/xhci.asl index d5187e6..7d89665 100644 --- a/src/soc/intel/cannonlake/acpi/xhci.asl +++ b/src/soc/intel/cannonlake/acpi/xhci.asl @@ -2,6 +2,65 @@
#include <soc/gpe.h>
+#define PORTSCN_OFFSET 0x480 +#define PORTSCXUSB3_OFFSET 0x540 + +#define WAKE_ON_CONNECT_DISCONNECT_ENABLE 0x6000000 +#define RO_BITS_OFF_MASK ~0x80FE0012 + +/* + * USB Port Wake Enable (UPWE) on usb attach/detach + * Arg0 - Port Number + * Arg1 - Port 1 Status and control offset + * Arg2 - xHCI Memory-mapped address + */ +Method (UPWE, 3, Serialized) +{ + Local0 = Arg1 + ((Arg0 - 1) * 0x10) + + /* Map ((XMEM << 16) + Local0 in PSCR */ + OperationRegion (PSCR, SystemMemory, + Add (ShiftLeft (Arg2, 16), Local0), 0x10) + Field (PSCR, DWordAcc, NoLock, Preserve) + { + PSCT, 32, + } + Local0 = PSCT + /* + * And port status/control reg with RO and RWS bits + * RO bits: 0, 2:3, 10:13, 24, 28:30 + * RWS bits: 5:9, 14:16, 25:27 + */ + Local0 = Local0 & RO_BITS_OFF_MASK + /* Set WCE and WDE bits */ + Local0 = Local0 | WAKE_ON_CONNECT_DISCONNECT_ENABLE + PSCT = Local0 +} + +/* + * USB Wake Enable Setup (UWES) + * Arg0 - Port enable bitmap + * Arg1 - Port 1 Status and control offset + * Arg2 - xHCI Memory-mapped address + */ +Method (UWES, 3, Serialized) +{ + Local0 = Arg0 + + While (One) { + FindSetRightBit (Local0, Local1) + If (Local1 == Zero) { + Break + } + UPWE (Local1, Arg1, Arg2) + /* + * Clear the lowest set bit in Local0 since it was + * processed. + */ + Local0 = Local0 & (Local0 - 1) + } +} + /* XHCI Controller 0:14.0 */
Device (XHCI) @@ -10,10 +69,24 @@
Name (_PRW, Package () { GPE0_PME_B0, 3 })
+ Method (_DSW, 3) + { + UWES ((\U2WE & 0xFFF), PORTSCN_OFFSET, XMEM) + UWES ((\U3WE & 0x3F ), PORTSCXUSB3_OFFSET, XMEM) + } + Name (_S3D, 3) /* D3 supported in S3 */ Name (_S0W, 3) /* D3 can wake device in S0 */ Name (_S3W, 3) /* D3 can wake system from S3 */
+ OperationRegion (XPRT, PCI_Config, 0x00, 0x100) + Field (XPRT, AnyAcc, NoLock, Preserve) + { + Offset (0x10), + , 16, + XMEM, 16, /* MEM_BASE */ + } + Method (_PS0, 0, Serialized) {
-
Edward O'Callaghan (Code Review) -
Patrick Rudolph (Code Review)
-
Sam McNally (Code Review)
-
Tim Wawrzynczak (Code Review)