Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32531 )
Change subject: Add support for the 51nb X210 ......................................................................
Patch Set 33:
(8 comments)
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 12: select MAINBOARD_USES_FSP2_0
Not needed, since this is selected by the SOC
Done
https://review.coreboot.org/c/coreboot/+/32531/28/src/mainboard/51nb/x210/Kc... PS28, Line 16: config MAINBOARD_VENDOR : string : default "51NB"
Not needed, since it is set by `src/mainboard/51nb/Kconfig`
Done
https://review.coreboot.org/c/coreboot/+/32531/28/src/mainboard/51nb/x210/Kc... PS28, Line 36: config DEVICETREE : string : default "devicetree.cb"
Not needed, since this is the default name
Done
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.
Done
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 i […]
Done
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 y […]
Done
https://review.coreboot.org/c/coreboot/+/32531/28/src/mainboard/51nb/x210/ro... File src/mainboard/51nb/x210/romstage.c:
https://review.coreboot.org/c/coreboot/+/32531/28/src/mainboard/51nb/x210/ro... PS28, Line 28: /* Rcomp resistor */
Not strictly required as the function is static and called in a way that it can't be NULL
Done
https://review.coreboot.org/c/coreboot/+/32531/28/src/mainboard/51nb/x210/ro... PS28, Line 36: /* Rcomp target */
Not strictly required as the function is static and called in a way that it can't be NULL
Done