Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38346 )
Change subject: mainboard: add Supermicro X9SCL/X9SCM ......................................................................
Patch Set 2:
(11 comments)
https://review.coreboot.org/c/coreboot/+/38346/2/src/mainboard/supermicro/x9... File src/mainboard/supermicro/x9scl/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/38346/2/src/mainboard/supermicro/x9... PS2, Line 2: 0x0 0
https://review.coreboot.org/c/coreboot/+/38346/2/src/mainboard/supermicro/x9... PS2, Line 11: end You should "pick up" these lone `end` (put them on the previous line)
https://review.coreboot.org/c/coreboot/+/38346/2/src/mainboard/supermicro/x9... PS2, Line 16: 0x0 0
https://review.coreboot.org/c/coreboot/+/38346/2/src/mainboard/supermicro/x9... PS2, Line 19: end Same here, but also make sure the various `end` are aligned. It just takes an extra space after any `on`:
device pci 00.0 on end # Host bridge device pci 01.0 on end # PEG device pci 01.1 on end # PEG device pci 06.0 on end # PEG device pci 02.0 off end # iGPU
https://review.coreboot.org/c/coreboot/+/38346/2/src/mainboard/supermicro/x9... PS2, Line 30: register "docking_supported" = "0" Can be dropped
https://review.coreboot.org/c/coreboot/+/38346/2/src/mainboard/supermicro/x9... PS2, Line 35: register "pcie_hotplug_map" = "{ 0, 0, 0, 0, 0, 0, 0, 0 }" Can be dropped
https://review.coreboot.org/c/coreboot/+/38346/2/src/mainboard/supermicro/x9... PS2, Line 41: off Why is this device off?
https://review.coreboot.org/c/coreboot/+/38346/2/src/mainboard/supermicro/x9... File src/mainboard/supermicro/x9scl/dsdt.asl:
https://review.coreboot.org/c/coreboot/+/38346/2/src/mainboard/supermicro/x9... PS2, Line 33: Scope (_SB) : { : Device (PCI0) You can collapse this:
Device (_SB.PCI0)
https://review.coreboot.org/c/coreboot/+/38346/2/src/mainboard/supermicro/x9... File src/mainboard/supermicro/x9scl/early_init.c:
https://review.coreboot.org/c/coreboot/+/38346/2/src/mainboard/supermicro/x9... PS2, Line 45: /* 0:1d.0? */ As written on the devicetree comments, EHCI 1 is 1d.0 and EHCI2 is 1a.0
https://review.coreboot.org/c/coreboot/+/38346/2/src/mainboard/supermicro/x9... PS2, Line 63: static const u16 superio_initvals[] = { Why, just why?
https://review.coreboot.org/c/coreboot/+/38346/2/src/mainboard/supermicro/x9... File src/mainboard/supermicro/x9scl/superio.h:
PS2: I'm pretty sure you can use things in src/superio/ instead.