Attention is currently required from: Sean Rhodes, Andy Pont, Paul Menzel. 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 26: Code-Review+1
(2 comments)
File src/ec/starlabs/merlin/ec.c:
https://review.coreboot.org/c/coreboot/+/58343/comment/978c06c9_a66a86d5 PS26, Line 68: 4 uh, the initializer has 5 elements
https://review.coreboot.org/c/coreboot/+/58343/comment/29fe6c7b_afd6fd43 PS26, Line 77: get_uint_option("kbl_timeout", SEC_30) If CMOS is somehow corrupted but the checksums are still valid, this can return a value that goes past the array length. I'd add a helper function that sanitizes and encodes the CMOS value:
static uint8_t get_ec_value_from_option(const char *name, unsigned int fallback, const uint8_t *lut, size_t lut_size) { unsigned int index = get_uint_option(name, fallback); if (index >= lut_size) index = fallback;
return lut[index]; }
This would be used as follows:
const uint8_t kbl_timeout[4] = { SEC_30, MIN_1, MIN_3, MIN_5, NEVER };
ec_write(ECRAM_KBL_TIMEOUT, get_ec_value_from_option("kbl_timeout", 0, kbl_timeout, ARRAY_SIZE(kbl_timeout)));
Feel free to adapt this.
Also, I remember I said in a comment that it would be good to use enum values for the fallback value of `get_uint_option()`, but this was before I noticed that the defined values are for the EC registers. Just use 0 instead.