Attention is currently required from: Raul Rangel, Karthik Ramasubramanian, Felix Held. Jon Murphy has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62903 )
Change subject: mb/google/skyrim: Add DXIO descriptors ......................................................................
Patch Set 8:
(11 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/62903/comment/29756271_9bfeee91 PS3, Line 13: TEST=Builds
Requested assistance from factory bring up team in testing the patch
Should get hardware today or Monday and can test patch locally as well.
File src/mainboard/google/skyrim/port_descriptors.c:
https://review.coreboot.org/c/coreboot/+/62903/comment/96406ec5_5e2c720a PS8, Line 12: 0
1
Done
https://review.coreboot.org/c/coreboot/+/62903/comment/a4956ebb_4d67f6ca PS8, Line 13: 0
1
Done
https://review.coreboot.org/c/coreboot/+/62903/comment/1c9131ad_ebb983f8 PS8, Line 24: 1
0
Done
https://review.coreboot.org/c/coreboot/+/62903/comment/e196ccb2_df7d46d8 PS8, Line 25: 1
0
Done
https://review.coreboot.org/c/coreboot/+/62903/comment/3cfecd26_e3719d29 PS8, Line 38: .start_logical_lane = 2, : .end_logical_lane = 3,
not sure if the numbers need to be swapped or not
Ack
https://review.coreboot.org/c/coreboot/+/62903/comment/a4b3c651_8272755e PS8, Line 49: {/* unused MXM */ : .engine_type = UNUSED_ENGINE, : .port_present = false, : .start_logical_lane = 4, : .end_logical_lane = 7, : .device_number = 0, : .function_number = 0, : .link_speed_capability = GEN3, : .turn_off_unused_lanes = true, : }, : { /* unused MXM2 */ : .engine_type = UNUSED_ENGINE, : .port_present = false, : .start_logical_lane = 8, : .end_logical_lane = 11, : .device_number = 0, : .function_number = 0, : .link_speed_capability = GEN3, : .turn_off_unused_lanes = true, : }, : { /* unused MXM3 */ : .engine_type = UNUSED_ENGINE, : .port_present = false, : .start_logical_lane = 12, : .end_logical_lane = 15, : .device_number = 0, : .function_number = 0, : .link_speed_capability = GEN3, : .turn_off_unused_lanes = true, : }, : { /* unused SSD2 */ : .engine_type = UNUSED_ENGINE, : .port_present = false, : .start_logical_lane = 16, : .end_logical_lane = 19, : .device_number = 0, : .function_number = 0, : .link_speed_capability = GEN3, : .turn_off_unused_lanes = true, : },
can this be removed?
Done
https://review.coreboot.org/c/coreboot/+/62903/comment/615c3662_40d92437 PS8, Line 98: DDI_HDMI
DDI_DP
Done
https://review.coreboot.org/c/coreboot/+/62903/comment/9b3e7708_da981995 PS8, Line 102: { /* DDI2 - DP (type C) */
i'm not 100% sure if ddi2 is a dummy and ddi3 and 4 are the type c ones or if ddi2 and 3 are the typ […]
Ack
File src/mainboard/google/skyrim/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/62903/comment/713d7074_678a805e PS8, Line 50: device ref gpp_bridge_0 on # WLAN : chip drivers/wifi/generic : register "wake" = "GEVENT_8" : device pci 00.0 on end : end : end : device ref gpp_bridge_1 on end # SD
wifi before sd turned out to be the correct order after figuring out how the physical to logical lan […]
Done
File src/mainboard/google/skyrim/variants/baseboard/include/baseboard/variants.h:
https://review.coreboot.org/c/coreboot/+/62903/comment/a498f1f4_b7e57335 PS8, Line 9: : #define SD_DEVFN PCIE_GPP_2_0_DEVFN : #define WLAN_DEVFN PCIE_GPP_2_1_DEVFN
swap
Done
-
Jon Murphy (Code Review)