Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37069 )
Change subject: trogdor: add support for Bubs variant ......................................................................
Patch Set 1:
(4 comments)
https://review.coreboot.org/c/coreboot/+/37069/1/src/mainboard/google/trogdo... File src/mainboard/google/trogdor/Kconfig:
https://review.coreboot.org/c/coreboot/+/37069/1/src/mainboard/google/trogdo... PS1, Line 11: select EC_GOOGLE_CHROMEEC if BOARD_GOOGLE_TROGDOR These should all be
if !BOARD_GOOGLE_BUBS
instead. Other boards will follow Trogdor, not Bubs. (At least until we make a Chromebox, then this becomes more complicated.)
https://review.coreboot.org/c/coreboot/+/37069/1/src/mainboard/google/trogdo... PS1, Line 21: select EC_GOOGLE_CHROMEEC_SWITCHES if BOARD_GOOGLE_TROGDOR I would assume that you get the linker issues Doug mentioned (for things like get_recovery_mode_switch()) when you disable this for Bubs. You'll need to also add a
select VBOOT_NO_BOARD_SUPPORT if BOARD_GOOGLE_BUBS
https://review.coreboot.org/c/coreboot/+/37069/1/src/mainboard/google/trogdo... PS1, Line 23: select GBB_FLAG_DISABLE_EC_SOFTWARE_SYNC if BOARD_GOOGLE_BUBS You shouldn't need this since the depthcharge Kconfig already disables software sync.
https://review.coreboot.org/c/coreboot/+/37069/1/src/mainboard/google/trogdo... File src/mainboard/google/trogdor/reset.c:
https://review.coreboot.org/c/coreboot/+/37069/1/src/mainboard/google/trogdo... PS1, Line 24: BOARD_GOOGLE_TROGDOR
I would probably go the opposite and say "not bubs". […]
In this case, we can instead add
select MISSING_BOARD_RESET if BOARD_GOOGLE_BUBS
to the Kconfig, and wrap the Makefile lines compiling this file in an
ifneq($(CONFIG_BOARD_GOOGLE_BUBS),y)
that should be a bit cleaner.