Change in coreboot[master]: mb/supermicro/x11-lga1151v2-series: Add support for X11SCH-F
Attention is currently required from: Felix Singer, Arthur Heymans, Michael Niewöhner. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37441 )
Change subject: mb/supermicro/x11-lga1151v2-series: Add support for X11SCH-F ......................................................................
Patch Set 67:
(17 comments)
File Documentation/mainboard/supermicro/x11-lga1151v2-series/x11-lga1151v2-series.md:
https://review.coreboot.org/c/coreboot/+/37441/comment/572cf950_0248ffcd PS67, Line 34: - Fix TODOs mentioned in code
are there any? guess that was copypasta from x11-lga1151
none explicit, but some stuff needs to be rechecked. Hmmm, but there's a FIXME on the CLKSRC stuff...
File Documentation/mainboard/supermicro/x11-lga1151v2-series/x11sch-f/x11sch-f.md:
https://review.coreboot.org/c/coreboot/+/37441/comment/1f177464_03278ff1 PS67, Line 32:
not working: BMC UI sensor readings - this is probably due to the missing "POST complete" gpio code. […]
Ack
File MAINTAINERS:
https://review.coreboot.org/c/coreboot/+/37441/comment/03f91e38_23713f68 PS67, Line 438: s
missing trailing slash
I'll add it for consistency.
File src/mainboard/supermicro/x11-lga1151v2-series/Kconfig:
https://review.coreboot.org/c/coreboot/+/37441/comment/c2b5400b_3e9d8954 PS67, Line 13: SUPERIO_ASPEED_COMMON_PRE_RAM
selected by SUPERIO_ASPEED_AST2400
Gone
https://review.coreboot.org/c/coreboot/+/37441/comment/7c3f5348_cfe6b39a PS67, Line 15:
select NO_FADT_8042
Why?
File src/mainboard/supermicro/x11-lga1151v2-series/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/37441/comment/3a8fb2d3_fce2eebe PS67, Line 12: # SPS doesn't support all commands issued by FSP
huh? heciretry is not a different command but just a for-loop for retry sending commands
This was here already when I took over the port. I imagine it's to reduce boot delays when FSP tries to run commands that SPS does not support.
https://review.coreboot.org/c/coreboot/+/37441/comment/2b72ff6c_d2b579f8 PS67, Line 38:
nit: tab
*squints* oh!
https://review.coreboot.org/c/coreboot/+/37441/comment/a8e63670_c3253fb3 PS67, Line 48: ataPortsEnable" = "{ \ : [0] = 1, \ : [1] = 1, \ : [2] = 1, \ : [3] = 1, \ : [4] = 1, \ : [5] = 1, \ : [6] = 1, \ : [7] = 1, \ : }"
no escaping needed
Gone
https://review.coreboot.org/c/coreboot/+/37441/comment/d6d446b0_b2694ce5 PS67, Line 92: device pci 1f.2 off end # PMC
is the pmc really disabled on vendor fw?
Haven't checked myself. I re-enabled it and doesn't seem to have any side-effects.
File src/mainboard/supermicro/x11-lga1151v2-series/ramstage.c:
https://review.coreboot.org/c/coreboot/+/37441/comment/a167acb0_5e2bc76f PS67, Line 7: mainboard_silicon_init_params
this function should only be used for fsp params, as it's name says. better use mainboard ops. […]
Ack
File src/mainboard/supermicro/x11-lga1151v2-series/romstage.c:
https://review.coreboot.org/c/coreboot/+/37441/comment/180c026e_7e5bccd2 PS67, Line 38: .ect = 0,
defaults to 0, doesn't it?
Yes, but the comment above is anchored to this line
File src/mainboard/supermicro/x11-lga1151v2-series/variants/x11sch-f/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/37441/comment/feaa150a_c63a79ed PS67, Line 22: USB 2.0/3.0 : : # USB OC0 : register "usb2_ports[0]" = "USB2_PORT_MID(OC0)" : register "usb2_ports[2]" = "USB2_PORT_MID(OC0)" : : # USB OC1 : register "usb2_ports[1]" = "USB2_PORT_MID(OC1)" : register "usb3_ports[3]" = "USB3_PORT_DEFAULT(OC1)" : : # USB OC2 : register "usb2_ports[3]" = "USB2_PORT_MID(OC2)" : register "usb2_ports[5]" = "USB2_PORT_MID(OC2)" : register "usb3_ports[0]" = "USB3_PORT_DEFAULT(OC2)" : register "usb3_ports[1]" = "USB3_PORT_DEFAULT(OC2)" : : # USB OC3 : register "usb2_ports[4]" = "USB2_PORT_MID(OC3)" : register "usb2_ports[6]" = "USB2_PORT_MID(OC3)" : : # USB OC4 : register "usb2_ports[7]" = "USB2_PORT_MID(OC4)" : register "usb2_ports[9]" = "USB2_PORT_MID(OC4)" : : # USB OC5 : register "usb2_ports[8]" = "USB2_PORT_MID(OC5)" : register "usb2_ports[10]" = "USB2_PORT_MID(OC5)" : register "usb3_ports[2]" = "USB3_PORT_DEFAULT(OC5)" : register "usb3_ports[4]" = "USB3_PORT_DEFAULT(OC5)" : : # USB KCS : register "usb2_ports[13]" = "USB2_PORT_MID(OC_SKIP)" : : # USB OC6/7 - not
xhci could be added below and this moved into it. not a must, though. feel free to just close.
I'll leave it here next to the PCIe settings, since I have no means to verify this.
https://review.coreboot.org/c/coreboot/+/37441/comment/a1caf1c7_2e66da06 PS67, Line 58: On to avoid coalescing?
what for?
Was there already when I took over the port. I guess vendor firmware enabled this RP to avoid coalescing, or CB:36651 hadn't landed yet. Will have to check if it can be disabled.
https://review.coreboot.org/c/coreboot/+/37441/comment/1b7179f5_5b8e4681 PS67, Line 75: On to avoid coalescing?
what for?
Ack
https://review.coreboot.org/c/coreboot/+/37441/comment/a6f56ad0_a2f380b3 PS67, Line 84: 5 4
https://review.coreboot.org/c/coreboot/+/37441/comment/c022869f_317a66a1 PS67, Line 87: smbios_slot_desc "SlotTypeM2Socket3" "SlotLengthOther" : "M.2-P_1" "SlotDataBusWidth4X" :
iirc multiline for smbios_slot_desc doesn't work, does it?
BUILD_TIMELESS=1 says it makes no difference. If multiline didn't work, why wouldn't SCONFIG complain about it? However, dmidecode data bus width looks wrong:
Handle 0x0014, DMI type 9, 19 bytes System Slot Information Designation: M.2-P_1 Type: x4 M.2 Socket 3 Current Usage: Available Length: Other Characteristics: Unknown Bus Address: 0000:00:1c.4 Data Bus Width: 1 Peer Devices: 0
Handle 0x0015, DMI type 9, 19 bytes System Slot Information Designation: M.2-P_2 Type: x4 M.2 Socket 3 Current Usage: Available Length: Other Characteristics: Unknown Bus Address: 0000:00:1d.0 Data Bus Width: 1 Peer Devices: 0
I can't test properly because there are no devices plugged in.
https://review.coreboot.org/c/coreboot/+/37441/comment/8196bd5d_3d7d7adf PS67, Line 94: smbios_slot_desc "SlotTypeM2Socket3" "SlotLengthOther" : "M.2-P_2" "SlotDataBusWidth4X" :
iirc multiline for smbios_slot_desc doesn't work, does it?
Ack
-
Angel Pons (Code Review)