Attention is currently required from: Sean Rhodes, Andy Pont, Paul Menzel, Felix Held. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58343 )
Change subject: ec/starlabs: Add standardised ITE EC support ......................................................................
Patch Set 39:
(2 comments)
File src/ec/starlabs/merlin/Kconfig:
https://review.coreboot.org/c/coreboot/+/58343/comment/4ba9da5f_419f4acf PS37, Line 22: bool "Adjustable Keyboard Backlight"
Mainboard dependent. […]
If it's not meant to be user-configurable, the prompt (text after `bool`) should be removed.
File src/ec/starlabs/merlin/ec.c:
https://review.coreboot.org/c/coreboot/+/58343/comment/f8546608_e23f5d74 PS19, Line 134: #if CONFIG(EC_STARLABS_FAN) : switch (get_uint_option("fan_mode", 0)) { : case FAN_AGGRESSIVE: : ec_write(ECRAM_FAN_MODE, FAN_AGGRESSIVE); : break; : case FAN_QUIET: : ec_write(ECRAM_FAN_MODE, FAN_QUIET); : break; : default: : ec_write(ECRAM_FAN_MODE, FAN_NORMAL); : break; : } : #endif
Just realised why I didn't before - APL/GLK/GLK-R don't have ECRAM_FAN_MODE defined so won't build w […]
Ah, I see. Then, you should do the following:
#include <assert.h> #include <stdint.h>
#define ECRAM_FAN_MODE dead_code_t(uint8_t)
The `dead_code_t` macro turns compile-time errors (e.g. "undeclared identifier") into link-time errors ("unresolved symbol" about an undefined function). The compiler can optimize out uses of this macro if the code is unreachable, e.g. if it's in an if-block whose condition is known to be false at compile time.
So, using this allows using regular C checks while still getting build-time errors for invalid configs (e.g. `EC_STARLABS_FAN` selected on an APL/GLK variant).
-
Angel Pons (Code Review)