Attention is currently required from: Felix Singer, Hung-Te Lin, Yidi Lin, Yu-Ping Wu.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79063?usp=email )
Change subject: google/*: Clean up Kconfg board selection for Google MTK boards ......................................................................
Patch Set 2:
(4 comments)
File src/mainboard/google/corsola/Kconfig:
https://review.coreboot.org/c/coreboot/+/79063/comment/5df5bf87_7ff2e60a : PS1, Line 22: select DRIVER_ANALOGIX_ANX7625 if BOARD_GOOGLE_CHINCHOU
I prefer Yu-Ping's idea in the first comment. […]
Oh okay, I just kinda assumed here from the existing patterns that there was some expectation that most boards of the same subvariant would use the same bridge. If that's not true, then I agree it would be best to not depend on the subvariant options at all for this. (I hope I can still assume that at least Staryu is the variant for all the MIPI boards, though? Or is that also just coincidence?)
I'm also not really convinced about the "everything that's not ANX is Parade", though... don't you think that risks boards that get added in the future being accidentally treated as "Parade" because someone just forgot to add an option. If we expect the two to be roughly even and there's no preset assumption that "most" boards are Parade, I think it would be best to just have an explicit list for each instead. I wrote it that way now, let me know what you think.
File src/mainboard/google/geralt/Kconfig:
https://review.coreboot.org/c/coreboot/+/79063/comment/ba2f6152_d223e7cd : PS1, Line 5: def_bool BOARD_GOOGLE_GERALT
And `BOARD_GOOGLE_CIRI` (added by a recent patch CB:78954).
Done
https://review.coreboot.org/c/coreboot/+/79063/comment/6f7cb01c_6fd85507 : PS1, Line 62: SDCARD_INIT
Also modify this for consistency (need to rebase).
Done
File src/mainboard/google/geralt/Kconfig:
https://review.coreboot.org/c/coreboot/+/79063/comment/ed58048c_41e07162 : PS2, Line 74: choice I removed the `choice` here because a choice always needs to have a prompt, and these kinds of hardware description Kconfigs should not show up in menuconfig. I think it's fine to just have two separate `bool` options, but if you think it is important that they're mutually exclusive I could also add a `depends on !THE_OTHER_ONE` to each (or maybe a static assertion to the code... for two it's not a big difference, but when you want to select exactly one out of more than two then `_Static_assert(CONFIG(A) + CONFIG(B) + CONFIG(C) == 1)` is usually the shortest way to check that).