Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34497 )
Change subject: mb/google/drallion: Add new mainboard ......................................................................
Patch Set 4:
(7 comments)
https://review.coreboot.org/c/coreboot/+/34497/4/src/mainboard/google/dralli... File src/mainboard/google/drallion/Kconfig:
https://review.coreboot.org/c/coreboot/+/34497/4/src/mainboard/google/dralli... PS4, Line 7: if BOARD_GOOGLE_DRALLION not needed? (throughout)
https://review.coreboot.org/c/coreboot/+/34497/4/src/mainboard/google/dralli... PS4, Line 23: select SYSTEM_TYPE_LAPTOP if BOARD_GOOGLE_DRALLION : select SYSTEM_TYPE_CONVERTIBLE if BOARD_GOOGLE_DRALLION Need to pick one here to avoid confusion.
https://review.coreboot.org/c/coreboot/+/34497/4/src/mainboard/google/dralli... PS4, Line 27: select MAINBOARD_USES_IFD_GBE_REGION if BOARD_GOOGLE_DRALLION can remove
https://review.coreboot.org/c/coreboot/+/34497/4/src/mainboard/google/dralli... File src/mainboard/google/drallion/ramstage.c:
https://review.coreboot.org/c/coreboot/+/34497/4/src/mainboard/google/dralli... PS4, Line 79: /* Disable unused pads for devices with board ID > 2 */ : if (board_id() > 2) : gpio_configure_pads(gpio_unused, ARRAY_SIZE(gpio_unused)); The board id check is probably not needed.
https://review.coreboot.org/c/coreboot/+/34497/4/src/mainboard/google/dralli... File src/mainboard/google/drallion/romstage.c:
https://review.coreboot.org/c/coreboot/+/34497/4/src/mainboard/google/dralli... PS4, Line 62: cannonlake_memcfg_init cometlake?
https://review.coreboot.org/c/coreboot/+/34497/4/src/mainboard/google/dralli... File src/mainboard/google/drallion/variants/drallion/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/34497/4/src/mainboard/google/dralli... PS4, Line 1: cannonlake cometlake
(I'm ignoring the rest of this file as I don't have block diagram here)
https://review.coreboot.org/c/coreboot/+/34497/4/src/mainboard/google/dralli... File src/mainboard/google/drallion/variants/drallion/include/variant/variant.h:
https://review.coreboot.org/c/coreboot/+/34497/4/src/mainboard/google/dralli... PS4, Line 19: /* Need to update for Drallion with right SKU IDs : Arcada is SKU ID 2 and 4 */ : #define VARIANT_SKU_ID 2 : #define VARIANT_SKU_NAME "sku2" : #define VARIANT_SKU_ID_SIGNED_EC 4 : #define VARIANT_SKU_NAME_SIGNED_EC "sku4" This needs to be defined still, but probably at least remove the "Arcada" string.