Attention is currently required from: Kevin Keijzer, Angel Pons, Arthur Heymans, Nicholas Chin, Fabian Groffen, Elyes Haouas.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/73097 )
Change subject: mb/asrock/b75m-itx: Add Sandy/Ivy Bridge board B75M-ITX ......................................................................
Patch Set 5:
(13 comments)
Patchset:
PS5: Thanks! Mostly some style comments.
File src/mainboard/asrock/b75m-itx/Kconfig:
https://review.coreboot.org/c/coreboot/+/73097/comment/091e5ea2_34803ea0 PS5, Line 20: select MAINBOARD_HAS_LIBGFXINIT Please put it before NORTHBRIDGE_INTEL_SANDYBRIDGE to keep the options sorted.
File src/mainboard/asrock/b75m-itx/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/73097/comment/4a7fed2c_7f53d538 PS5, Line 6: ramstage-$(CONFIG_MAINBOARD_USE_LIBGFXINIT) += gma-mainboard.ads : bootblock-y += early_init.c : romstage-y += early_init.c Please order by the stages. bootblock, romstage, ramstage
File src/mainboard/asrock/b75m-itx/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/73097/comment/f139d427_163f6bdd PS5, Line 4: register "gfx.use_spread_spectrum_clock" = "0" : register "gpu_cpu_backlight" = "0x00000000" : register "gpu_dp_b_hotplug" = "4" : register "gpu_dp_c_hotplug" = "4" : register "gpu_dp_d_hotplug" = "4" : register "gpu_panel_port_select" = "PANEL_PORT_LVDS" : register "gpu_panel_power_backlight_off_delay" = "0" : register "gpu_panel_power_backlight_on_delay" = "0" : register "gpu_panel_power_cycle_delay" = "4" : register "gpu_panel_power_down_delay" = "0" : register "gpu_panel_power_up_delay" = "0" : register "gpu_pch_backlight" = "0x00000000 The zeroed options can be removed. Unset options are zero by default.
https://review.coreboot.org/c/coreboot/+/73097/comment/05d44373_ac5f4ec4 PS5, Line 47: device pci 16.1 off # Management Engine Interface 2 : end Please move the end-keyword in the lines before if the device scope doesn't contain anything.
File src/mainboard/asrock/b75m-itx/dsdt.asl:
https://review.coreboot.org/c/coreboot/+/73097/comment/34741798_bca125b3 PS5, Line 10: // OEM revision Remove comment
https://review.coreboot.org/c/coreboot/+/73097/comment/79fe1c9f_5072f855 PS5, Line 17: /* global NVS and variables. */ Remove comment
https://review.coreboot.org/c/coreboot/+/73097/comment/d75e4cf1_79b2dee4 PS5, Line 21: Scope (_SB) { : Device (PCI0) Simplify to `Device (_SB.PCI0) {`
File src/mainboard/asrock/b75m-itx/early_init.c:
https://review.coreboot.org/c/coreboot/+/73097/comment/390a7711_ce705aa3 PS5, Line 12: void bootblock_mainboard_early_init(void) : { : /* no COM1 header installed, do not enable UART */ : } Can be removed. There is a weak function which is called instead.
File src/mainboard/asrock/b75m-itx/gpio.c:
https://review.coreboot.org/c/coreboot/+/73097/comment/3f2274c1_8f48e63d PS5, Line 157: }, Remove one tab
https://review.coreboot.org/c/coreboot/+/73097/comment/aaa0e1a4_14a8bf99 PS5, Line 163: }, Remove one tab
https://review.coreboot.org/c/coreboot/+/73097/comment/b94ad2ff_fff4a8e1 PS5, Line 169: }, Remove one tab
File src/mainboard/asrock/b75m-itx/hda_verb.c:
https://review.coreboot.org/c/coreboot/+/73097/comment/1820d3a1_a3c13780 PS5, Line 1: /* SPDX-License-Identifier: GPL-2.0-only */ Some files are already licensed under GPL-2.0-or-later. Would you mind licensing the others this way as well?
-
Felix Singer (Code Review)