Attention is currently required from: Alexander Couzens, Nicholas Chin, Nicholas Sudsgaard, Nico Huber, Paul Menzel.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80343?usp=email )
Change subject: mainboard/lenovo: Add ThinkCentre M710s (Skylake) ......................................................................
Patch Set 8:
(14 comments)
File src/mainboard/lenovo/thinkcentre_m710s/Kconfig:
https://review.coreboot.org/c/coreboot/+/80343/comment/ee2ec00b_5230e6db : PS8, Line 7: select BOARD_ROMSIZE_KB_8192 : select SOC_INTEL_KABYLAKE : select SKYLAKE_SOC_PCH_H : select HAVE_ACPI_TABLES : select HAVE_ACPI_RESUME : select INTEL_GMA_HAVE_VBT : select MAINBOARD_HAS_LIBGFXINIT : select SOC_INTEL_COMMON_BLOCK_HDA_VERB : select SUPERIO_ITE_IT8629E : select MAINBOARD_HAS_TPM2 : s Sort alphabetically:
``` select BOARD_ROMSIZE_KB_8192 select HAVE_ACPI_RESUME select HAVE_ACPI_TABLES select INTEL_GMA_HAVE_VBT select MAINBOARD_HAS_LIBGFXINIT select MAINBOARD_HAS_TPM2 select MEMORY_MAPPED_TPM select SKYLAKE_SOC_PCH_H select SOC_INTEL_COMMON_BLOCK_HDA_VERB select SOC_INTEL_KABYLAKE select SUPERIO_ITE_IT8629E ```
File src/mainboard/lenovo/thinkcentre_m710s/bootblock.c:
https://review.coreboot.org/c/coreboot/+/80343/comment/7cd0955b_55c8c1f0 : PS8, Line 21: gpio_configure_pads(gpio_table, ARRAY_SIZE(gpio_table)); FSP might reconfigure some GPIOs. So we only do necessary GPIO configuration in early stages and do the whole configuration in ramstage after FSP ran. See the Clevo mainboards for example.
File src/mainboard/lenovo/thinkcentre_m710s/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/80343/comment/fba82b85_f49df147 : PS8, Line 2: # Model: IB250MH What does that mean?
https://review.coreboot.org/c/coreboot/+/80343/comment/35374b1d_ef75c755 : PS8, Line 19: register "usb2_ports[0]" = "USB2_PORT_MID(OC1)" # USB30A : register "usb2_ports[1]" = "USB2_PORT_MID(OC1)" # USB30B : register "usb2_ports[2]" = "USB2_PORT_MID(OC2)" # F_USB30_1A : register "usb2_ports[3]" = "USB2_PORT_MID(OC2)" # F_USB30_1B : register "usb2_ports[4]" = "USB2_PORT_MID(OC3)" # F_USB30_2A : register "usb2_ports[5]" = "USB2_PORT_MID(OC3)" # F_USB30_2B : register "usb2_ports[6]" = "USB2_PORT_MID(OC_SKIP)" # M.2 Bluetooth : register "usb2_ports[7]" = "USB2_PORT_MID(OC5)" # USB_LANA : register "usb2_ports[8]" = "USB2_PORT_MID(OC5)" # USB_LANB : register "usb2_ports[9]" = "USB2_PORT_MID(OC_SKIP)" # F_USB1 (Pins 5, 7) : register "usb2_ports[10]" = "USB2_PORT_MID(OC_SKIP)" # F_USB1 (Pins 6, 8) : # Used by the SD/MMC reader. : Please use:
``` register "usb2_ports" = "{ [0] = ... # Comment }" ```
https://review.coreboot.org/c/coreboot/+/80343/comment/331b41b7_0ec6df2e : PS8, Line 33: register "usb3_ports[0]" = "USB3_PORT_DEFAULT(OC1)" # USB30A Same here
https://review.coreboot.org/c/coreboot/+/80343/comment/bebc5679_6199074e : PS8, Line 45: register "SataPortsEnable[0]" = "true" # SATA1 Same here
https://review.coreboot.org/c/coreboot/+/80343/comment/41b5c960_fddabc07 : PS8, Line 48: register "SataPortsEnable[3]" = "false" # SATA4 (not mounted) Did you test if it works? If not, please remove.
https://review.coreboot.org/c/coreboot/+/80343/comment/1ea68353_13ed3a04 : PS8, Line 58: device ref pcie_rp6 off end # PCI (not mounted) Did you test if it works? If not, please remove.
File src/mainboard/lenovo/thinkcentre_m710s/gpio.h:
https://review.coreboot.org/c/coreboot/+/80343/comment/f348e8f8_546f1e3b : PS8, Line 27: PAD_CFG_NF(GPP_A14, NONE, DEEP, NF1), Nit: Missing description
https://review.coreboot.org/c/coreboot/+/80343/comment/d4a9b719_839b4d81 : PS8, Line 50: PAD_CFG_GPO(GPP_B11, 0, DEEP), Nit: Missing description
https://review.coreboot.org/c/coreboot/+/80343/comment/df99ba3c_4e92d8fd : PS8, Line 88: PAD_NC(GPP_C21, NONE), // UART2_TXD Does it have pads or headers for UART? Is RXD accessible as well?
https://review.coreboot.org/c/coreboot/+/80343/comment/d0e57814_34a96d9c : PS8, Line 189: PAD_CFG_NF(GPP_H3, NONE, PLTRST, NF1), Nit: Missing description.
https://review.coreboot.org/c/coreboot/+/80343/comment/3a9e5c8f_c1c3f41b : PS8, Line 208: PAD_CFG_GPI_TRIG_OWN(GPP_H22, NONE, PLTRST, OFF, ACPI), Nit: Missing description
File src/mainboard/lenovo/thinkcentre_m710s/hda_verb.c:
https://review.coreboot.org/c/coreboot/+/80343/comment/2a6e0420_5057d500 : PS8, Line 20: AZALIA_PIN_CFG(0, 0x1e, 0x411111f0), Does it not have a pin config for a display port?