Thomas 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:
(15 comments)
Patch Set 1:
I'm not sure if I have credited 3mdeb/Protectli enough and if not what should be added?
Cheers Thomas
To me it is enough to be mentioned in board documentation. Anyway 3mdeb and Protectli is in the AUTHORS file, so there isn't any distinction about single files now.
First and foremost: does it boot well?
It does boot very well, a bit too fast so sometimes an USB stick won't be recognized but that's expected I guess.
And there is one error in the boot log regarding PCH_DEVFN_THERMAL device not found (line 7), which I haven't looked into it yet:
``` 1: ME: Error Code : No Error 2: ME: Progress Phase : Host Communication 3: ME: Power Management Event : Pseudo-global reset 4: ME: Progress Phase State : Host communication established 5: ME: Power Down Mitigation : NO 6: ME: FPF status : unfused 7: ERROR: PCH_DEVFN_THERMAL device not found! 8: Finalizing SMM. 9: APMC done. ```
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 3: Protectli FW6
Please link to the page.
Your right, I'll do that.
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 […]
It works very well, yes.
Looking at the "7th-and-8th-gen" document from Intel which you mentioned in an email, Kaby Lake and Kaby Lake Refresh where in the same category.
Nevertheless I'll have a look at the AmberlakeFspBinPkg and compare the settings.
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).
It should be with correct VBT. […]
That is one of the parts which I took over one-to-one from the FW6 documenation.
I know nothing yet about GOP, had to look up what it means.
https://review.coreboot.org/c/coreboot/+/48769/1/Documentation/mainboard/yan... PS1, Line 40: The main SPI flash can be accessed using [flashrom].
Maybe clarify, that this also applies to the vendor firmware.
Again, part of the original FW6 documentation.
But yes I used to flash coreboot and the original firmware, although only the BIOS part.
So I should add that both coreboot and the vendor firmware can be flashed using internal programing?
https://review.coreboot.org/c/coreboot/+/48769/1/Documentation/mainboard/yan... PS1, Line 56: - assume the same as for Protectli FW6:
Remove the “bullet point”?
Will be done
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.
You're right, as mentioned (way) earlier, it maybe better if I remove all the pure copy-paste from FW6 and only reference to the FW6 document.
https://review.coreboot.org/c/coreboot/+/48769/1/Documentation/mainboard/yan... PS1, Line 63: return
returns
Will be corrected.
https://review.coreboot.org/c/coreboot/+/48769/1/Documentation/mainboard/yan... PS1, Line 64: or : temporarily disconnect
or *to* temporarily disconnect
"or *to* temporarily disconnect *the* DIMS*s*"?
https://review.coreboot.org/c/coreboot/+/48769/1/Documentation/mainboard/yan... PS1, Line 66: dim
What does that mean?
Copy-paste from FW6 doc, no idea.
I created a bootsplash with my red Moto Guzzi Griso motorbike and the red colors look just right to me. Maybe Michal could elaborate?
https://review.coreboot.org/c/coreboot/+/48769/1/Documentation/mainboard/yan... PS1, Line 70: - assume the same as for Protectli FW6:
Remove the “bullet point”?
Will be done.
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 […]
I ran the recommended tools before starting. And both the flashrom and "7th-and-8th-gen-core-family-mobile-u-y-processor-lines-i-o-datasheet-vol-1.pdf" let me to believe that it is indeed the same chipset.
--- flashrom.log --- DMI string chassis-type: "Desktop" DMI string system-manufacturer: "YANLING" DMI string system-product-name: "YL-KBR6L" DMI string system-version: "Ver:1.00" DMI string baseboard-manufacturer: "YANLING" DMI string baseboard-product-name: "YL-KBR6L" DMI string baseboard-version: "Ver:1.00" Found ITE Super I/O, ID 0x8613 on port 0x2e Found chipset "Intel Kaby Lake U w/ iHDCP2.2 Prem." with PCI ID 8086:9d4e. ---
https://review.coreboot.org/c/coreboot/+/48769/1/Documentation/mainboard/yan... PS1, Line 133: [Yanling N18]:https://www.ylipc.com/product/network_server_network_server/N18_Firewall_Min...
Please add a space after the colon.
I will do that.
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. […]
Yes I did use the superiotool on the original firmware but haven't look at anything else then the serial port. You are right I have to revisit that topic again.
Not sure if I'm allowed to dump some log in here:
--- superiotool.log Found ITE IT8613E (id=0x8613, rev=0x8) at 0x2e Register dump: idx 20 21 22 23 24 2b val 86 13 08 49 00 48 def 86 13 05 40 00 48 LDN 0x01 (COM1) idx 30 60 61 70 f0 val 01 03 f8 04 00 def 00 03 f8 04 00 LDN 0x04 (Environment controller) idx 30 60 61 62 63 70 f0 f1 f2 f3 f4 f5 f6 fa fb fc val 01 0a 30 0a 20 00 00 00 40 00 20 00 f0 00 00 00 def 00 02 90 02 30 09 00 00 00 00 00 NA NA 00 00 00 LDN 0x05 (Keyboard) idx 30 60 61 62 63 70 71 f0 val 01 00 60 00 64 01 02 48 def 01 00 60 00 64 01 02 48 LDN 0x06 (Mouse) idx 30 70 71 f0 val 01 0c 02 00 def 00 0c 02 00 LDN 0x07 (GPIO) idx 25 26 27 28 29 2a 2c 2d 60 61 62 63 70 71 72 73 74 b0 b1 b2 b3 b4 b8 ba bb bc bd c0 c1 c2 c3 c4 c8 c9 ca cb cc cd da db e0 e1 e2 e3 e4 ec f0 f1 f2 f3 f4 f5 f6 f7 f8 f9 fa fb val 00 f3 00 00 00 01 41 02 0a 10 0a 00 00 08 10 00 00 00 00 00 00 00 00 00 00 c0 03 01 00 00 40 00 01 00 00 00 00 00 30 44 00 00 00 00 00 00 00 00 00 00 00 00 0e 00 00 00 00 00 def 00 f3 00 00 00 01 01 00 00 00 00 00 00 00 20 38 00 00 00 00 00 00 00 00 00 00 00 01 00 00 40 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 NA NA 00 00 0e 00 00 00 00 00 LDN 0x0a (Consumer IR) idx 30 60 61 70 f0 val 00 03 10 0b 06 def 00 03 10 0b 06 Environment controller (0x0a35) Register dump: idx 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0e 0f 11 12 13 14 16 17 19 1a 1c 1d 1e 1f 20 21 22 24 25 27 28 29 2a 2b 2c 2d 2e 2f 30 31 32 33 34 35 38 39 3a 3b 40 41 42 43 44 45 46 47 50 51 52 53 54 55 56 57 58 59 5a 5b 5c 5d 5e 5f 68 69 6a 6b 6c 6d 6e 70 71 72 73 74 75 76 78 79 7a 7b 7c 7d 7e 80 81 82 83 84 85 86 87 88 89 8a 8b 8c 8d 8e 8f 90 91 98 99 9c 9d 9e 9f a0 a1 a2 a3 a4 a5 a6 b4 b5 b6 b7 b8 b9 val 19 90 df 09 00 00 00 00 00 c0 64 00 00 ff 00 ff ff a0 c0 80 00 ff 00 ff ff 00 00 32 60 98 5c 77 f0 8e 26 80 80 00 80 80 97 20 05 a8 e3 6c 9c 20 72 8a 6c ff 83 ff 64 ff 72 e7 1d 00 00 7f 7f 7f 40 63 00 90 00 00 12 e0 00 00 00 27 27 4f 64 18 00 0f 7f 7f 7f 80 00 00 0f 7f 7f 7f 80 00 00 0f 00 00 00 00 ff ff ff ff 02 30 01 02 01 78 e0 c6 00 00 40 00 00 d2 00 00 7f 7f 7f 80 00 00 0f 44 4c d7 82 3e 0d def 18 00 00 00 00 00 00 00 00 c0 44 00 00 MM MM NA NA 00 40 00 00 MM MM NA NA 00 00 MM MM MM MM MM MM MM MM MM MM MM MM MM MM NA NA NA NA NA NA NA NA NA NA NA NA NA NA NA NA NA NA 00 00 7f 7f 7f 40 00 00 90 00 00 12 60 00 00 00 7f 7f 7f 00 00 7f 0f 7f 7f 7f 00 00 7f 0f 7f 7f 7f 00 00 7f 0f MM MM MM MM NA NA NA NA 00 00 00 00 00 00 00 MM 00 00 00 00 00 NA 00 NA 7f 7f 7f 00 00 7f 0f NA NA NA NA NA NA BRAM (0x0000)
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
I'll look into the Amberlake FSB integration. The naming is confusing since it's not an Amberlake at all ... but that's just Intel I assume.
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. […]
I'll remove them.