Christoph Pomaska has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34873 )
Change subject: mb/supermicro: Add Supermicro X9SCL+-F ......................................................................
Patch Set 2:
(10 comments)
https://review.coreboot.org/c/coreboot/+/34873/1/src/mainboard/supermicro/x9... File src/mainboard/supermicro/x9scl-f/Kconfig:
https://review.coreboot.org/c/coreboot/+/34873/1/src/mainboard/supermicro/x9... PS1, Line 51: : config VGA_BIOS_FILE : string : default "3rdparty/blobs/mainboard/$(MAINBOARDDIR)/vgabios.bin"
I don't think the VBIOS can be committed to the blobs repo. […]
Done
https://review.coreboot.org/c/coreboot/+/34873/1/src/mainboard/supermicro/x9... File src/mainboard/supermicro/x9scl-f/acpi_tables.c:
https://review.coreboot.org/c/coreboot/+/34873/1/src/mainboard/supermicro/x9... PS1, Line 24: /* Disable USB ports in S3 by default */ : gnvs->s3u0 = 0; : gnvs->s3u1 = 0; : : /* Disable USB ports in S5 by default */ : gnvs->s5u0 = 0; : gnvs->s5u1 = 0;
I don't think there's code handling these values
Done
https://review.coreboot.org/c/coreboot/+/34873/1/src/mainboard/supermicro/x9... PS1, Line 32: // the lid is open by default. : gnvs->lids = 1;
I don't think so
Done
https://review.coreboot.org/c/coreboot/+/34873/1/src/mainboard/supermicro/x9... File src/mainboard/supermicro/x9scl-f/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/34873/1/src/mainboard/supermicro/x9... PS1, Line 4: : register "gfx.use_spread_spectrum_clock" = "0" : register "gpu_cpu_backlight" = "0x00000000" : register "gpu_dp_b_hotplug" = "0" : register "gpu_dp_c_hotplug" = "0" : register "gpu_dp_d_hotplug" = "0" : register "gpu_panel_port_select" = "0" : register "gpu_panel_power_backlight_off_delay" = "0" : register "gpu_panel_power_backlight_on_delay" = "0" : register "gpu_panel_power_cycle_delay" = "0" : register "gpu_panel_power_down_delay" = "0" : register "gpu_panel_power_up_delay" = "0" : register "gpu_pch_backlight" = "0x00000000"
These values that are zero can be dropped
Done
https://review.coreboot.org/c/coreboot/+/34873/1/src/mainboard/supermicro/x9... PS1, Line 26: end
Could move the lone "end" to the previous line (applies to all similar cases)
Done
https://review.coreboot.org/c/coreboot/+/34873/1/src/mainboard/supermicro/x9... PS1, Line 31: d
Right after the "device domain 0x0 on" line, add this line: […]
Done
https://review.coreboot.org/c/coreboot/+/34873/1/src/mainboard/supermicro/x9... PS1, Line 34: r
Can be dropped
Done
https://review.coreboot.org/c/coreboot/+/34873/1/src/mainboard/supermicro/x9... PS1, Line 39: r
Can be dropped
Done
https://review.coreboot.org/c/coreboot/+/34873/1/src/mainboard/supermicro/x9... PS1, Line 56: subsystemid 0x15d9 0x0624
Can drop these when inheriting the subsystemid
Done
https://review.coreboot.org/c/coreboot/+/34873/1/src/mainboard/supermicro/x9... File src/mainboard/supermicro/x9scl-f/dsdt.asl:
https://review.coreboot.org/c/coreboot/+/34873/1/src/mainboard/supermicro/x9... PS1, Line 16: : #define BRIGHTNESS_UP _SB.PCI0.GFX0.INCB : #define BRIGHTNESS_DOWN _SB.PCI0.GFX0.DECB
I think these can be dropped
Done