Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48769 )
Change subject: mb/yanling: Add Yanling YL-KBR6L mainboard + doc ......................................................................
Patch Set 1:
(10 comments)
https://review.coreboot.org/c/coreboot/+/48769/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48769/1//COMMIT_MSG@17 PS1, Line 17: VGA Option ROM
Extracted how?
Extracting and adding vbt to the commit is also an option.
https://review.coreboot.org/c/coreboot/+/48769/1//COMMIT_MSG@25 PS1, Line 25: Crucial CT2K32G4SFD8266
Is that one DIMM with 64 GB?
It is rather 2x 32GB by searching for it on google.com
https://review.coreboot.org/c/coreboot/+/48769/1/Documentation/mainboard/yan... File Documentation/mainboard/yanling/yl-kbr6l.md:
https://review.coreboot.org/c/coreboot/+/48769/1/Documentation/mainboard/yan... PS1, Line 26: Kaby Lake FSP According to Intel FSP repository the Kaby Lake Refresh should use AmberlakeFspBinPkg https://github.com/intel/FSP#fsp-project-information. i5-8250 is Kaby Lake Refresh.
Does the platform boot with Kaby Lake FSP?
https://review.coreboot.org/c/coreboot/+/48769/1/Documentation/mainboard/yan... PS1, Line 33: VGA Option ROM is not required to boot, but if one needs graphics in pre-OS : stage, it should be included (if not using libgfxinit).
Maybe: […]
It should be with correct VBT. I don't actually remember why I have not written about VBT and GOP here.
https://review.coreboot.org/c/coreboot/+/48769/1/Documentation/mainboard/yan... PS1, Line 56: assume the same as for Protectli FW6 Does have to be the same
https://review.coreboot.org/c/coreboot/+/48769/1/Documentation/mainboard/yan... PS1, Line 61: FW6A There wouldn't be FW6A since it is Protectli platform variant, not Yanling.
https://review.coreboot.org/c/coreboot/+/48769/1/Documentation/mainboard/yan... PS1, Line 98: | PCH | Kaby Lake U w/ iHDCP2.2 Premium | Have you checked that the chipset is the same? Flashrom can detect the chipset and it's name may be different.
https://review.coreboot.org/c/coreboot/+/48769/1/src/mainboard/yanling/yl_kb... File src/mainboard/yanling/yl_kbr6l/bootblock.c:
https://review.coreboot.org/c/coreboot/+/48769/1/src/mainboard/yanling/yl_kb... PS1, Line 13: ite_reg_write(GPIO_DEV, 0x2d, 0x02); /* PCICLK 25MHz */ That will need a revisit of the Super I/O configuration for this platform. Although FW2B/FW4B had the same chip, the configuration may not be identical. Have you tried using superiotool on the original firmware?
https://review.coreboot.org/c/coreboot/+/48769/1/src/mainboard/yanling/yl_kb... File src/mainboard/yanling/yl_kbr6l/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/48769/1/src/mainboard/yanling/yl_kb... PS1, Line 33: # FSP Configuration Some of the settings probably will change the names after Amberlake FSP integration
https://review.coreboot.org/c/coreboot/+/48769/1/src/mainboard/yanling/yl_kb... PS1, Line 151: # TODO: Check why protectli used them and WiFi won't work : # for me if I set them -> err: lost pci device Well, that depends on the routing of CLRREQ signals. If you can't get it working, then just remove the lines: ``` register "PcieRpClkReqSupport[8]" = "1" # RP 9 uses CLKREQ0# register "PcieRpClkReqNumber[8]" = "0" ``` And the clock will always be on in such case and the device shouldn't get lost.