Change in coreboot[master]: mb/supermicro/x11-lga1151v2-series: Add support for X11SCH-F
Attention is currently required from: Felix Singer, Christian Walter, Angel Pons, Arthur Heymans. Michael Niewöhner 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:
(16 comments)
File Documentation/mainboard/supermicro/x11-lga1151v2-series/x11-lga1151v2-series.md:
https://review.coreboot.org/c/coreboot/+/37441/comment/337de7a8_d210319e PS67, Line 34: - Fix TODOs mentioned in code are there any? guess that was copypasta from x11-lga1151
File MAINTAINERS:
https://review.coreboot.org/c/coreboot/+/37441/comment/64312263_611d8f94 PS67, Line 438: s missing trailing slash
File src/mainboard/supermicro/x11-lga1151v2-series/Kconfig:
https://review.coreboot.org/c/coreboot/+/37441/comment/c8deb5df_9f85cc49 PS67, Line 13: SUPERIO_ASPEED_COMMON_PRE_RAM selected by SUPERIO_ASPEED_AST2400
https://review.coreboot.org/c/coreboot/+/37441/comment/1e5d295d_dc282d47 PS67, Line 15: select NO_FADT_8042
File src/mainboard/supermicro/x11-lga1151v2-series/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/37441/comment/a8458d59_870bc358 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
https://review.coreboot.org/c/coreboot/+/37441/comment/08919c8c_6968cb8f PS67, Line 38: nit: tab
https://review.coreboot.org/c/coreboot/+/37441/comment/ddbf4b0c_2c34a93c PS67, Line 48: ataPortsEnable" = "{ \ : [0] = 1, \ : [1] = 1, \ : [2] = 1, \ : [3] = 1, \ : [4] = 1, \ : [5] = 1, \ : [6] = 1, \ : [7] = 1, \ : }" no escaping needed
https://review.coreboot.org/c/coreboot/+/37441/comment/8ec41e9c_c4eb0b33 PS67, Line 92: device pci 1f.2 off end # PMC is the pmc really disabled on vendor fw?
File src/mainboard/supermicro/x11-lga1151v2-series/ramstage.c:
https://review.coreboot.org/c/coreboot/+/37441/comment/9cd7654c_96c3ff46 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. see for example: src/mainboard/supermicro/x11-lga1151-series/mainboard.c
File src/mainboard/supermicro/x11-lga1151v2-series/romstage.c:
https://review.coreboot.org/c/coreboot/+/37441/comment/ad999e59_10ff1e60 PS67, Line 38: .ect = 0, defaults to 0, doesn't it?
File src/mainboard/supermicro/x11-lga1151v2-series/variants/x11sch-f/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/37441/comment/e67afe3e_7727a25e PS64, Line 6:
*poke*
sry for the delay. it's not required, but makes matching acpi code easier, when gpe mapping matches.
File src/mainboard/supermicro/x11-lga1151v2-series/variants/x11sch-f/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/37441/comment/0422c813_e6ff3650 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.
https://review.coreboot.org/c/coreboot/+/37441/comment/94d68938_174eb6f5 PS67, Line 58: On to avoid coalescing? what for?
https://review.coreboot.org/c/coreboot/+/37441/comment/87790c3a_1569bd98 PS67, Line 75: On to avoid coalescing? what for?
https://review.coreboot.org/c/coreboot/+/37441/comment/e4c2f5f7_4d008702 PS67, Line 87: smbios_slot_desc "SlotTypeM2Socket3" "SlotLengthOther" : "M.2-P_1" "SlotDataBusWidth4X" : iirc multiline for smbios_slot_desc doesn't work, does it?
https://review.coreboot.org/c/coreboot/+/37441/comment/9be5c6f4_639a916c PS67, Line 94: smbios_slot_desc "SlotTypeM2Socket3" "SlotLengthOther" : "M.2-P_2" "SlotDataBusWidth4X" : iirc multiline for smbios_slot_desc doesn't work, does it?
-
Michael Niewöhner (Code Review)