Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32708 )
Change subject: mb/gigabyte/ga-b75m-d3{h,v}: Switch to a variant setup ......................................................................
Patch Set 4:
(7 comments)
https://review.coreboot.org/#/c/32708/4/src/mainboard/gigabyte/ga-panther-po... File src/mainboard/gigabyte/ga-panther-point/Kconfig:
https://review.coreboot.org/#/c/32708/4/src/mainboard/gigabyte/ga-panther-po... PS4, Line 18: select ARCH_X86 : select BOARD_ROMSIZE_KB_8192 : select HAVE_ACPI_RESUME : select HAVE_ACPI_TABLES : select INTEL_INT15 : select NORTHBRIDGE_INTEL_SANDYBRIDGE : select SERIRQ_CONTINUOUS_MODE : select SOUTHBRIDGE_INTEL_BD82X6X : select USE_NATIVE_RAMINIT : select SUPERIO_ITE_IT8728F : select MAINBOARD_HAS_LIBGFXINIT : select INTEL_GMA_HAVE_VBT : select HAVE_OPTION_TABLE : select HAVE_CMOS_DEFAULT : Doesn't look like the original selection and is causing the build failure.
https://review.coreboot.org/#/c/32708/4/src/mainboard/gigabyte/ga-panther-po... PS4, Line 65: onfig VGA_BIOS_ID : string : default "8086,0162" : : config VGA_BIOS_FILE : string : default "pci8086,0162.rom" : The depends on the CPU put into the socket, and I guess we can't predict that.
https://review.coreboot.org/#/c/32708/4/src/mainboard/gigabyte/ga-panther-po... File src/mainboard/gigabyte/ga-panther-point/dsdt.asl:
PS4: Changes here seem unrelated to the variant setup?
https://review.coreboot.org/#/c/32708/4/src/mainboard/gigabyte/ga-panther-po... File src/mainboard/gigabyte/ga-panther-point/variants/ga-b75m-d3h/include/variant/acpi/superio.asl:
PS4: Still the same file for both boards. And...
https://review.coreboot.org/#/c/32708/4/src/mainboard/gigabyte/ga-panther-po... PS4, Line 19: #define SIO_ENABLE_PS2M // Enable PS/2 Mouse ...it seems these definitions are meaningless for the selected chip drivers. Maybe stub the whole file out in a follow-up commit.
https://review.coreboot.org/#/c/32708/4/src/mainboard/gigabyte/ga-panther-po... File src/mainboard/gigabyte/ga-panther-point/variants/ga-b75m-d3h/include/variant/hda_verb.h:
https://review.coreboot.org/#/c/32708/4/src/mainboard/gigabyte/ga-panther-po... PS4, Line 3: Belongs below the license header.
https://review.coreboot.org/#/c/32708/4/src/mainboard/gigabyte/ga-panther-po... File src/mainboard/gigabyte/ga-panther-point/variants/ga-b75m-d3h/include/variant/mainboard_init.h:
PS4: No code in .h files, please. You can add it as `ga-b75m-d3h/mainboard.c` and hook it up in the `Makefile.inc`.
NB. These registers should already be set by southbridge code.