Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37441 )
Change subject: src/mainboard/supermicro/x11-lga1151v2-series: Add Support for X11SCH-F ......................................................................
Patch Set 51:
(18 comments)
https://review.coreboot.org/c/coreboot/+/37441/51//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37441/51//COMMIT_MSG@7 PS51, Line 7: src/mainboard/supermicro/x11-lga1151v2-series: Add Support for X11SCH-F
mb/supermicro/x11-lga1151v2-series: Add support for X11SCH-F
https://review.coreboot.org/c/coreboot/+/37441/51//COMMIT_MSG@15 PS51, Line 15: DevTree I’d spell it devicetree.
https://review.coreboot.org/c/coreboot/+/37441/51//COMMIT_MSG@16 PS51, Line 16: Linux What version (and what payload)?
https://review.coreboot.org/c/coreboot/+/37441/51//COMMIT_MSG@17 PS51, Line 17: Slots slots
https://review.coreboot.org/c/coreboot/+/37441/51//COMMIT_MSG@21 PS51, Line 21: * Proper ACPI Tables What does this mean?
https://review.coreboot.org/c/coreboot/+/37441/51//COMMIT_MSG@22 PS51, Line 22: * Windows Support What version, and what payload (with version)?
https://review.coreboot.org/c/coreboot/+/37441/51//COMMIT_MSG@25 PS51, Line 25: Tested with Intel Xeon E-2186G and 64 GB ECC RAM. How long does coreboot take to execute?
https://review.coreboot.org/c/coreboot/+/37441/51//COMMIT_MSG@26 PS51, Line 26: Please mention, why the code can’t be shared with the x11-lga1151 series code.
https://review.coreboot.org/c/coreboot/+/37441/51/Documentation/mainboard/in... File Documentation/mainboard/index.md:
https://review.coreboot.org/c/coreboot/+/37441/51/Documentation/mainboard/in... PS51, Line 150: LGA1151V2 v2
https://review.coreboot.org/c/coreboot/+/37441/51/Documentation/mainboard/su... File Documentation/mainboard/supermicro/x11-lga1151v2-series/x11-lga1151v2-series.md:
https://review.coreboot.org/c/coreboot/+/37441/51/Documentation/mainboard/su... PS51, Line 1: LGA1151V2 v2?
https://review.coreboot.org/c/coreboot/+/37441/51/Documentation/mainboard/su... PS51, Line 3: series series boards
https://review.coreboot.org/c/coreboot/+/37441/51/Documentation/mainboard/su... PS51, Line 18: - [Intel FSP2.0] can not be removed as long as there is no free replacement Please add a dot at the end of sentences.
https://review.coreboot.org/c/coreboot/+/37441/51/Documentation/mainboard/su... PS51, Line 29: - MRC caching does not work on cold boot with Intel SPS (see [Intel FSP2.0]) 1. Please add a dot/period at the end of sentences. 2. Please mention the result: The full RAM training of 64 GB ECC RAM lasts X seconds.
https://review.coreboot.org/c/coreboot/+/37441/51/Documentation/mainboard/su... PS51, Line 52: https://www.supermicro.com/en/products/motherboards?pro=cpu%3DUP%26processor... That URL does not work for me.
https://review.coreboot.org/c/coreboot/+/37441/51/Documentation/mainboard/su... PS51, Line 52: LGA1151 v2
https://review.coreboot.org/c/coreboot/+/37441/51/Documentation/mainboard/su... File Documentation/mainboard/supermicro/x11-lga1151v2-series/x11sch-f/x11sch-f.md:
https://review.coreboot.org/c/coreboot/+/37441/51/Documentation/mainboard/su... PS51, Line 3: This section details how to run coreboot on the [Supermicro X11SCH-F]. … The -F denotes that a BMC is part of the device.
https://review.coreboot.org/c/coreboot/+/37441/51/Documentation/mainboard/su... PS51, Line 30: - TPM on TPM expansion header Mention the patching of IFD as written in the commit message?
https://review.coreboot.org/c/coreboot/+/37441/51/Documentation/mainboard/su... PS51, Line 34: See general issue section. Where is that section? Maybe link to it?