Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32531 )
Change subject: Add support for the 51nb X210 ......................................................................
Patch Set 28:
(3 comments)
A few nits...
https://review.coreboot.org/c/coreboot/+/32531/28/src/mainboard/51nb/x210/Kc... File src/mainboard/51nb/x210/Kconfig:
https://review.coreboot.org/c/coreboot/+/32531/28/src/mainboard/51nb/x210/Kc... PS28, Line 56: config CPU_MICROCODE_CBFS_LEN : hex : default 0x18000 : : config CPU_MICROCODE_CBFS_LOC : hex : default 0xFFE115A0 only needed if using FSP-T, which may be discouraged if not tested.
https://review.coreboot.org/c/coreboot/+/32531/28/src/mainboard/51nb/x210/bo... File src/mainboard/51nb/x210/board.fmd:
https://review.coreboot.org/c/coreboot/+/32531/28/src/mainboard/51nb/x210/bo... PS28, Line 12: CONSOLE@0x60200 0x20000 Afaik there is no alignment requirements on this region, but for simplicity of erasing this region it might be better to place it above the FMAP region to make it aligned to common erase blocks like 64K?
https://review.coreboot.org/c/coreboot/+/32531/28/src/mainboard/51nb/x210/bo... PS28, Line 8: EC@0x0 0x10000 : RW_MRC_CACHE@0x10000 0x10000 : SMMSTORE@0x20000 0x40000 : FMAP@0x60000 0x200 : CONSOLE@0x60200 0x20000 : COREBOOT(CBFS)@0x80200 0x57FE00 You can skip specifying offsets of most of the regions and also the size of the COREBOOT region if you want.