Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32734 )
Change subject: mb/supermicro/x11ssh: Add Supermicro X11SSH-TF ......................................................................
Patch Set 77:
(16 comments)
https://review.coreboot.org/c/coreboot/+/32734/68//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/32734/68//COMMIT_MSG@12 PS68, Line 12: SeaBios
SeaBIOS
Done
https://review.coreboot.org/c/coreboot/+/32734/68//COMMIT_MSG@20 PS68, Line 20: Tianocore
TianoCore
Done
https://review.coreboot.org/c/coreboot/+/32734/68/Documentation/mainboard/su... File Documentation/mainboard/supermicro/x11ssh-tf.md:
https://review.coreboot.org/c/coreboot/+/32734/68/Documentation/mainboard/su... PS68, Line 15: Flashing
flashing
Done
https://review.coreboot.org/c/coreboot/+/32734/68/Documentation/mainboard/su... PS68, Line 40: SuperIO
Super I/O
Done
https://review.coreboot.org/c/coreboot/+/32734/74/Documentation/mainboard/su... File Documentation/mainboard/supermicro/x11ssh-tf.md:
https://review.coreboot.org/c/coreboot/+/32734/74/Documentation/mainboard/su... PS74, Line 37: 10GB
Nit: That's probably 10 Gigabit (not GigaByte), so 10Gb
Done
https://review.coreboot.org/c/coreboot/+/32734/30/src/mainboard/supermicro/x... File src/mainboard/supermicro/x11ssh/Kconfig:
https://review.coreboot.org/c/coreboot/+/32734/30/src/mainboard/supermicro/x... PS30, Line 34: default "src/mainboard/$(CONFIG_MAINBOARD_DIR)/vboot-ro-rwab.fmd" if VBOOT
Since the partition scheme is designed for RO/AB, I think it makes more sense to check for VBOOT_SLO […]
Done
https://review.coreboot.org/c/coreboot/+/32734/30/src/mainboard/supermicro/x... PS30, Line 35: default "src/mainboard/$(CONFIG_MAINBOARD_DIR)/board.fmd"
Check here for VBOOT
Done
https://review.coreboot.org/c/coreboot/+/32734/30/src/mainboard/supermicro/x... PS30, Line 39: default 0x0032d000 if VBOOT
Not necessary, since the CBFS size is defined in vboot-ro-rwab. […]
Done
https://review.coreboot.org/c/coreboot/+/32734/35/src/mainboard/supermicro/x... File src/mainboard/supermicro/x11ssh/Kconfig:
https://review.coreboot.org/c/coreboot/+/32734/35/src/mainboard/supermicro/x... PS35, Line 39: default "src/mainboard/$(CONFIG_MAINBOARD_DIR)/board.fmd" if VBOOT
remove and let coreboot autogenerate it
Done
https://review.coreboot.org/c/coreboot/+/32734/35/src/mainboard/supermicro/x... PS35, Line 39: default "src/mainboard/$(CONFIG_MAINBOARD_DIR)/board.fmd" if VBOOT
remove and let coreboot autogenerate it
Done
https://review.coreboot.org/c/coreboot/+/32734/35/src/mainboard/supermicro/x... PS35, Line 73: config CONFIG_USE_BLOBS
The user can still deselect it with this way. It only can't be changed if it's enabled by `select .. […]
Done
https://review.coreboot.org/c/coreboot/+/32734/35/src/mainboard/supermicro/x... PS35, Line 77: ADD_FSP_BINARIES
user selectable on all boards
Done
https://review.coreboot.org/c/coreboot/+/32734/35/src/mainboard/supermicro/x... PS35, Line 81: config FSP_USE_REPO
same here
Done
https://review.coreboot.org/c/coreboot/+/32734/68/src/mainboard/supermicro/x... File src/mainboard/supermicro/x11ssh/Kconfig:
https://review.coreboot.org/c/coreboot/+/32734/68/src/mainboard/supermicro/x... PS68, Line 1: config BOARD_SUPERMICRO_BASEBOARD_X11SSH
config DIMM_SPD_SIZE -> 512
Done
https://review.coreboot.org/c/coreboot/+/32734/35/src/mainboard/supermicro/x... File src/mainboard/supermicro/x11ssh/board.fmd:
https://review.coreboot.org/c/coreboot/+/32734/35/src/mainboard/supermicro/x... PS35, Line 1: FLASH 16M {
will be autogenerated
Done
https://review.coreboot.org/c/coreboot/+/32734/35/src/mainboard/supermicro/x... File src/mainboard/supermicro/x11ssh/bootblock.c:
https://review.coreboot.org/c/coreboot/+/32734/35/src/mainboard/supermicro/x... PS35, Line 40: aspeed_enable_serial(SERIAL_DEV, CONFIG_TTYS0_BASE);
if (CONFIG(CONSOLE_SERIAL))
Done
-
Felix Singer (Code Review)