[L] Change in coreboot[main]: mb/lenovo: Add ThinkCentre M900 (Skylake/LGA 1151)

[Angel Pons (Code Review)]

Attention is currently required from: Alexander Couzens, Felix Singer, Jan Philipp Groß, Nicholas Chin, Paul Menzel.

Angel Pons has posted comments on this change by Nicholas Chin. ( https://review.coreboot.org/c/coreboot/+/74187?usp=email )

Change subject: mb/lenovo: Add ThinkCentre M900 (Skylake/LGA 1151)
......................................................................

Patch Set 4:

(10 comments)

File src/mainboard/lenovo/m900/Kconfig:

https://review.coreboot.org/c/coreboot/+/74187/comment/fe3851c4_23376c1b?usp... :
PS4, Line 36: config PRERAM_CBMEM_CONSOLE_SIZE
            : 	default 0xd00
Why is the default value not suitable?

File src/mainboard/lenovo/m900/acpi/dptf.asl:

PS4: 
Was this tested?

File src/mainboard/lenovo/m900/bootblock.c:

https://review.coreboot.org/c/coreboot/+/74187/comment/cf7ac9e2_f5b643d4?usp... :
PS4, Line 9: /* Change to NCT6687D_SP1 to use COM2 header */
Should probably be a Kconfig

File src/mainboard/lenovo/m900/cmos.layout:

https://review.coreboot.org/c/coreboot/+/74187/comment/672321c4_890e29fb?usp... :
PS4, Line 24: 408	1	e	1	nmi
Skylake doesn't implement this option, IIRC.

https://review.coreboot.org/c/coreboot/+/74187/comment/af644bc0_1476c065?usp... :
PS4, Line 28: ChromeOS
Hmmmmm... I think vboot doesn't use this CMOS field anymore?

File src/mainboard/lenovo/m900/devicetree.cb:

https://review.coreboot.org/c/coreboot/+/74187/comment/6573803a_c3329e7f?usp... :
PS4, Line 20: 	# SLP_S3 Minimum Assertion Width. Values 0: 60us, 1: 1ms, 2: 50ms, 3: 2s
For another patch: provide an enum in SoC code

https://review.coreboot.org/c/coreboot/+/74187/comment/720f0217_331ac8e0?usp... :
PS4, Line 38: 	register "SendVrMbxCmd" = "2"
I'm pretty sure this is copy-pasta from a Chromebook that no one knows what it does. I've had no problems removing it on the HP 280 G2.

File src/mainboard/lenovo/m900/dsdt.asl:

https://review.coreboot.org/c/coreboot/+/74187/comment/f2ab0b6b_a669c7cc?usp... :
PS4, Line 25: /* Image processing unit */
            : 			#include <soc/intel/skylake/acpi/ipu.asl>
I am pretty sure IPU is not used on a desktop. I'm not even sure if it exists on PCH-H platforms.

File src/mainboard/lenovo/m900/ramstage.c:

https://review.coreboot.org/c/coreboot/+/74187/comment/8dbecf32_2c75c1e2?usp... :
PS4, Line 12: 	params->CdClock = 3;
What does this do? Is this needed?

File src/mainboard/lenovo/m900/romstage.c:

https://review.coreboot.org/c/coreboot/+/74187/comment/eaa2cfe4_dac75737?usp... :
PS4, Line 21: assert(sizeof(mem_cfg->RcompResistor) == sizeof(rcomp_resistors));
            : 	assert(sizeof(mem_cfg->RcompTarget) == sizeof(rcomp_targets));
nit: move just above corresponding memcpy()?