Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32531 )
Change subject: Add support for the 51nb X210 ......................................................................
Patch Set 24: Code-Review+1
(4 comments)
https://review.coreboot.org/c/coreboot/+/32531/24/Documentation/mainboard/51... File Documentation/mainboard/51nb/x210.md:
https://review.coreboot.org/c/coreboot/+/32531/24/Documentation/mainboard/51... PS24, Line 1: # 51NB X210 file needs to be referenced by index.md
https://review.coreboot.org/c/coreboot/+/32531/24/Documentation/mainboard/51... PS24, Line 19: is located on the upper side of the motherboard, below the keyboard can you provide a picture? Note: pictures should not be bigger than 800px and use 70% compression to keep size small.
https://review.coreboot.org/c/coreboot/+/32531/24/Documentation/mainboard/51... PS24, Line 31: Use libgfxinit once Kaby Lake is supported. isn't that supported by now?
https://review.coreboot.org/c/coreboot/+/32531/24/src/ec/51nb/npce985la0dx/n... File src/ec/51nb/npce985la0dx/npce985la0dx.c:
https://review.coreboot.org/c/coreboot/+/32531/24/src/ec/51nb/npce985la0dx/n... PS24, Line 19: { NULL, 0x05 }, { NULL, 0x06 }, { NULL, 0x11 } what are those LDNs used for? Do you need to set additional registers in the superio space?