Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32708 )
Change subject: mb/gigabyte/ga-b75m-d3{h,v}: Switch to variant setup ......................................................................
Patch Set 14:
(2 comments)
https://review.coreboot.org/#/c/32708/14/src/mainboard/gigabyte/ga-b75m-d3h/... File src/mainboard/gigabyte/ga-b75m-d3h/Kconfig:
https://review.coreboot.org/#/c/32708/14/src/mainboard/gigabyte/ga-b75m-d3h/... PS14, Line 43: config ROMSTAGE_C : string : default "romstage.c"
Why is this needed? mainbaord/romstage.c is included from arch/x86/Makefile. […]
This looks familiar... It's the GPIO_C thing on asus/p5qc, but renamed to ROMSTAGE_C. There's several differences between gpio.c and romstage.c that make this Kconfig value unnecessary for romstage.c:
gpio.c files are usually meant to be used for a single board variant, so on variants there isn't a mainboard-level gpio.c in most cases. The mainboard Makefile.inc needs to explicitly include them (other platforms might use something different).
romstage.c files, however, are more generic. If anything variant-specific is needed, it can be moved into variant/*/romstage.c, so that the mainboard-level romstage.c can still be the same for all the variants. Conditional code that depends on Kconfig symbols could also be used. The mainboard-level romstage.c is automatically built if it exists, but the variant-specific romstage.c files need to be explicitly included (not everyone needs variants).
In the asus/p5qc case, two variants used the same gpio.c file, placed at the mainboard level (which is unusual). When p5ql_pro, which uses a different gpio.c, was added into the mix, something had to be done. Duplicating the shared gpio.c isn't an option, so the silly GPIO_C Kconfig symbol was the least evil kludge I could think of. (Yes, I wrote such an ugly thing)
TL;DR in this case, and for now, no variant romstage.c is needed. The ugly GPIO_C-like kludge in asus/p5qc isn't needed here either.
https://review.coreboot.org/#/c/32708/14/src/mainboard/gigabyte/ga-b75m-d3h/... File src/mainboard/gigabyte/ga-b75m-d3h/Makefile.inc:
https://review.coreboot.org/#/c/32708/14/src/mainboard/gigabyte/ga-b75m-d3h/... PS14, Line 19: romstage-y += $(CONFIG_ROMSTAGE_C) This, plus the strip call above, aren't needed. See the comments on Kconfig.