Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32734 )
Change subject: mb/supermicro/x11ssh: Add Supermicro X11SSH-TF ......................................................................
Patch Set 71:
(7 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 payload : * LinuxBoot payload
What version?
master
https://review.coreboot.org/c/coreboot/+/32734/68//COMMIT_MSG@25 PS68, Line 25: Please apply those patches as well for good user experience: : : I456be647b159f7a2ea7d94986a24424e56dcc8c4 : I22c6885eae6fd7c778ac37b18f95b8775e9064e3 : Ica0c20255f661dd61edc3a7d15646b7447c4658e
This should be a comment, and not in the commit message in my opinion.
How do you add comments to the commit message?
https://review.coreboot.org/c/coreboot/+/32734/68/src/mainboard/supermicro/x... File src/mainboard/supermicro/x11ssh/variants/tf/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/32734/68/src/mainboard/supermicro/x... PS68, Line 28: register "SkipExtGfxScan" = "1"
I think it would be better to set Aspeed VGA as primary: […]
Works without FSP knowing about that device as it only claims 16MiB BAR.
https://review.coreboot.org/c/coreboot/+/32734/68/src/mainboard/supermicro/x... PS68, Line 146: 0x50
May be better to use the macro VR_CFG_AMP() […]
Done
https://review.coreboot.org/c/coreboot/+/32734/68/src/mainboard/supermicro/x... PS68, Line 209: device pci 01.0 on end # PCI Slot
Is this M.2 Interface or PEGx8 slot[1]? Please add more information to the comment. […]
unused
https://review.coreboot.org/c/coreboot/+/32734/68/src/mainboard/supermicro/x... PS68, Line 210: device pci 01.1 on
register "Peg1MaxLinkWidth" = "Peg1_x8" if PEGx8 slot […]
PEGx8 slot, No need to add additional register values. SRCCLKREQ isn't used.
https://review.coreboot.org/c/coreboot/+/32734/68/src/mainboard/supermicro/x... PS68, Line 211: 4X
X4 or X8???
X8 slot with X4 data width
-
Patrick Rudolph (Code Review)