Change in coreboot[master]: sb/intel/lynxpoint/acpi: Add missing USB ports

[Angel Pons (Code Review)]

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46959 )

Change subject: sb/intel/lynxpoint/acpi: Add missing USB ports
......................................................................

sb/intel/lynxpoint/acpi: Add missing USB ports

Broadwell has these, so add them to Lynxpoint as well.

Change-Id: Id66f169070cdfe3a6d166ca18916d4ddaf4a5fea
Signed-off-by: Angel Pons 
---
M src/southbridge/intel/lynxpoint/acpi/xhci.asl
1 file changed, 3 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/46959/1

diff --git a/src/southbridge/intel/lynxpoint/acpi/xhci.asl b/src/southbridge/intel/lynxpoint/acpi/xhci.asl
index fbeb562..2b3d433 100644
--- a/src/southbridge/intel/lynxpoint/acpi/xhci.asl
+++ b/src/southbridge/intel/lynxpoint/acpi/xhci.asl
@@ -338,9 +338,12 @@
    	Device (PRT5) { Name (_ADR, 5) } // USB Port 4
    	Device (PRT6) { Name (_ADR, 6) } // USB Port 5
    	Device (PRT7) { Name (_ADR, 7) } // USB Port 6
+		Device (PRT8) { Name (_ADR, 8) } // USB Port 7
    	Device (SSP1) { Name (_ADR, 10) } // USB Port 10
    	Device (SSP2) { Name (_ADR, 11) } // USB Port 11
    	Device (SSP3) { Name (_ADR, 12) } // USB Port 12
    	Device (SSP4) { Name (_ADR, 13) } // USB Port 13
+		Device (SSP5) { Name (_ADR, 14) } // USB Port 14
+		Device (SSP6) { Name (_ADR, 15) } // USB Port 15
    }
 }

[Angel Pons (Code Review)]

Hello build bot (Jenkins), Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/46959

to look at the new patch set (#5).

Change subject: sb/intel/lynxpoint/acpi: Add missing USB ports
......................................................................

sb/intel/lynxpoint/acpi: Add missing USB ports

Broadwell has these, so add them to Lynxpoint as well.

Change-Id: Id66f169070cdfe3a6d166ca18916d4ddaf4a5fea
Signed-off-by: Angel Pons 
---
M src/southbridge/intel/lynxpoint/acpi/xhci.asl
1 file changed, 3 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/46959/5

[Tim Wawrzynczak (Code Review)]

Attention is currently required from: Nico Huber, Angel Pons, Arthur Heymans.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46959 )

Change subject: sb/intel/lynxpoint/acpi: Add missing USB ports
......................................................................

Patch Set 11:

(1 comment)

File src/southbridge/intel/lynxpoint/acpi/xhci.asl:

https://review.coreboot.org/c/coreboot/+/46959/comment/9fbbf199_d4c97a02 
PS11, Line 346: 	Device (SSP6) { Name (_ADR, 15) } // USB Port 15
doc #486708 says Lynx Point can have up to 14 usb ports

[Angel Pons (Code Review)]

Attention is currently required from: Nico Huber, Tim Wawrzynczak, Arthur Heymans.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46959 )

Change subject: sb/intel/lynxpoint/acpi: Add missing USB ports
......................................................................

Patch Set 11:

(1 comment)

File src/southbridge/intel/lynxpoint/acpi/xhci.asl:

https://review.coreboot.org/c/coreboot/+/46959/comment/be310c5c_6c358278 
PS11, Line 346: 	Device (SSP6) { Name (_ADR, 15) } // USB Port 15
Yeah, and 486708 Rev2.0 section `5.21 xHCI Controller (D20:F0)` also says the following:
> Some USB 3.0 motherboard down devices do not require support for USB 2.0 speed and it is possible to route only the SuperSpeed signals, as allowed by the USB 3.0 specification. In this special case, USB 2.0 and USB 3.0 signals will not need to be paired together, thereby allowing support for more than 14 USB connections.

This means there can be more than 14 USB ports in some cases, though I would expect this to be a highly unusual configuration. Moreover, the number of USB ports depends on PCH-H vs PCH-LP (known at compile-time), and also on HSIO configuration (mainboard-specific).

I've primarily added these devices to synchronize with Broadwell ASL, since I want to unify them someday.

Maybe I should move the USB settings from romstage to the devicetree, and use that to create the device nodes through acpigen. This would also allow decoupling USB settings from pei_data.h (the MRC ABI), which is desired because Haswell and Broadwell MRCs have incompatible ABIs. And since I don't need to use the Haswell MRC anymore, not depending on its ABI is also a plus. But this would be best done separately.

[Angel Pons (Code Review)]

Attention is currently required from: Nico Huber, Tim Wawrzynczak, Arthur Heymans.
Hello build bot (Jenkins), Nico Huber, Tim Wawrzynczak, Arthur Heymans, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/46959

to look at the new patch set (#12).

Change subject: sb/intel/lynxpoint/acpi: Add missing USB ports
......................................................................

sb/intel/lynxpoint/acpi: Add missing USB ports

Broadwell has these devices, so add them to Lynx Point as well. This is
done in preparation to have Broadwell boards use Lynx Point ACPI code.

Change-Id: Id66f169070cdfe3a6d166ca18916d4ddaf4a5fea
Signed-off-by: Angel Pons 
---
M src/southbridge/intel/lynxpoint/acpi/xhci.asl
1 file changed, 3 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/46959/12

[Paul Menzel (Code Review)]

Attention is currently required from: Nico Huber, Tim Wawrzynczak, Angel Pons, Arthur Heymans.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46959 )

Change subject: sb/intel/lynxpoint/acpi: Add missing USB ports
......................................................................

Patch Set 12: Code-Review+1

[Tim Wawrzynczak (Code Review)]

Attention is currently required from: Nico Huber, Angel Pons, Arthur Heymans.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46959 )

Change subject: sb/intel/lynxpoint/acpi: Add missing USB ports
......................................................................

Patch Set 12: Code-Review+2

(1 comment)

File src/southbridge/intel/lynxpoint/acpi/xhci.asl:

https://review.coreboot.org/c/coreboot/+/46959/comment/7c276dc8_ba94af49 
PS11, Line 346: 	Device (SSP6) { Name (_ADR, 15) } // USB Port 15
> Yeah, and 486708 Rev2.0 section `5.21 xHCI Controller (D20:F0)` also says the following: […]

Ah yes, that would be very unusual, and probably not technically "USB" standard, to have a port that only supports SS but not HS...

+1 to more usage of acpigen vs. static ASL

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/46959 )

Change subject: sb/intel/lynxpoint/acpi: Add missing USB ports
......................................................................

sb/intel/lynxpoint/acpi: Add missing USB ports

Broadwell has these devices, so add them to Lynx Point as well. This is
done in preparation to have Broadwell boards use Lynx Point ACPI code.

Change-Id: Id66f169070cdfe3a6d166ca18916d4ddaf4a5fea
Signed-off-by: Angel Pons 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/46959
Reviewed-by: Paul Menzel 
Reviewed-by: Tim Wawrzynczak 
Tested-by: build bot (Jenkins) 
---
M src/southbridge/intel/lynxpoint/acpi/xhci.asl
1 file changed, 3 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Paul Menzel: Looks good to me, but someone else must approve
  Tim Wawrzynczak: Looks good to me, approved

diff --git a/src/southbridge/intel/lynxpoint/acpi/xhci.asl b/src/southbridge/intel/lynxpoint/acpi/xhci.asl
index a65b1f6..9233163 100644
--- a/src/southbridge/intel/lynxpoint/acpi/xhci.asl
+++ b/src/southbridge/intel/lynxpoint/acpi/xhci.asl
@@ -337,9 +337,12 @@
    	Device (PRT5) { Name (_ADR, 5) } // USB Port 4
    	Device (PRT6) { Name (_ADR, 6) } // USB Port 5
    	Device (PRT7) { Name (_ADR, 7) } // USB Port 6
+		Device (PRT8) { Name (_ADR, 8) } // USB Port 7
    	Device (SSP1) { Name (_ADR, 10) } // USB Port 10
    	Device (SSP2) { Name (_ADR, 11) } // USB Port 11
    	Device (SSP3) { Name (_ADR, 12) } // USB Port 12
    	Device (SSP4) { Name (_ADR, 13) } // USB Port 13
+		Device (SSP5) { Name (_ADR, 14) } // USB Port 14
+		Device (SSP6) { Name (_ADR, 15) } // USB Port 15
    }
 }