Attention is currently required from: Alexander Couzens, Nicholas Chin, Paul Menzel.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74187?usp=email )
Change subject: mb/lenovo: Add Thinkcentre M900 (Skylake/LGA 1151) ......................................................................
Patch Set 2:
(8 comments)
Patchset:
PS2: Only looked very quickly over it and found some nits.
File src/mainboard/lenovo/m900/Kconfig:
https://review.coreboot.org/c/coreboot/+/74187/comment/7f685ab4_2775415e : PS2, Line 5: select BOARD_ROMSIZE_KB_16384 : select HAVE_ACPI_RESUME : select HAVE_ACPI_TABLES : select HAVE_OPTION_TABLE : select HAVE_CMOS_DEFAULT : select INTEL_GMA_HAVE_VBT : select INTEL_INT15 : select SOC_INTEL_SKYLAKE : select SKYLAKE_SOC_PCH_H : select SUPERIO_NUVOTON_COMMON_PRE_RAM : select MAINBOARD_HAS_LIBGFXINIT ``` select BOARD_ROMSIZE_KB_16384 select HAVE_ACPI_RESUME select HAVE_ACPI_TABLES select HAVE_CMOS_DEFAULT select HAVE_OPTION_TABLE select INTEL_GMA_HAVE_VBT select INTEL_INT15 select MAINBOARD_HAS_LIBGFXINIT select SKYLAKE_SOC_PCH_H select SOC_INTEL_SKYLAKE select SUPERIO_NUVOTON_COMMON_PRE_RAM ```
https://review.coreboot.org/c/coreboot/+/74187/comment/a1bd1ed6_c17fc8cd : PS2, Line 27: hex Remove
File src/mainboard/lenovo/m900/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/74187/comment/1a49d6bb_6a6ba396 : PS2, Line 21: r Move that under iGPU
https://review.coreboot.org/c/coreboot/+/74187/comment/c26d4242_0b9029c2 : PS2, Line 155: # Set LPC Serial IRQ mode Seems superfluous too
https://review.coreboot.org/c/coreboot/+/74187/comment/47034b3b_9ddcb1cf : PS2, Line 157: Remove superfluous lines
https://review.coreboot.org/c/coreboot/+/74187/comment/0cfa94ce_276f7705 : PS2, Line 159: # LPC Interface It's already in line 150
File src/mainboard/lenovo/m900/mainboard.c:
https://review.coreboot.org/c/coreboot/+/74187/comment/0bd34beb_b25e3700 : PS2, Line 1: /* SPDX-License-Identifier: GPL-2.0-or-later */ Would you mind using that for the other files as well?