Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37457 )
Change subject: [RFC] asus/p2b-d: Fold into asus/p2b-ds as a variant ......................................................................
Patch Set 1:
(7 comments)
https://review.coreboot.org/c/coreboot/+/37457/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37457/1//COMMIT_MSG@6 PS1, Line 6: : [RFC] asus/p2b-d: Fold into asus/p2b-ds as a variant Only these two? I'd try to do the entire group of i440bx boards
https://review.coreboot.org/c/coreboot/+/37457/1//COMMIT_MSG@10 PS1, Line 10: devicetree Should use overridetree to handle these.
https://review.coreboot.org/c/coreboot/+/37457/1//COMMIT_MSG@10 PS1, Line 10: mptable, irq table, As these files are nicely formatted, I'd try to preserve them. Even if similar, a lot is already gained just with sharing the devicetree and romstage.c
https://review.coreboot.org/c/coreboot/+/37457/1//COMMIT_MSG@13 PS1, Line 13: Lines split and spaces added to pass lint. You don't need to pass lint. What's more: the less changes to the files while switching to a variants setup, the better.
https://review.coreboot.org/c/coreboot/+/37457/1//COMMIT_MSG@18 PS1, Line 18: Comments welcome I'd suggest using gigabyte/ga-h61m-s2pv as a reference. It's also using a variant setup, but does not have the concept of 'baseboard'
https://review.coreboot.org/c/coreboot/+/37457/1//COMMIT_MSG@18 PS1, Line 18: Build tested. Use `make BUILD_TIMELESS=1 -j$(nproc)` to build a coreboot image of the relevant boards before and after this change. Then, diff both. There should be no changes.
This is a great way to test changes: if the resulting binary does not change, then one can be sure that nothing got broken by accident :)
https://review.coreboot.org/c/coreboot/+/37457/1/src/mainboard/asus/p2b-ds/i... File src/mainboard/asus/p2b-ds/irq_tables.c:
https://review.coreboot.org/c/coreboot/+/37457/1/src/mainboard/asus/p2b-ds/i... PS1, Line 19: #if CONFIG(BOARD_ASUS_P2B_DS) : #include "p2b-ds/irq_tables.h" : #endif : #if CONFIG(BOARD_ASUS_P2B_D) : #include "p2b-d/irq_tables.h" : #endif I'd rather try to add a Makefile.inc so that $(CONFIG_VARIANT_DIR) can be used