Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39018 )
Change subject: ec/google/chromeec: Introduce SKU_ID helpers ......................................................................
Patch Set 3:
(3 comments)
https://review.coreboot.org/c/coreboot/+/39018/3/src/ec/google/chromeec/ec_b... File src/ec/google/chromeec/ec_boardid.c:
https://review.coreboot.org/c/coreboot/+/39018/3/src/ec/google/chromeec/ec_b... PS3, Line 48: google_chromeec_smbios_system_sku What do you think about adding a Kconfig EC_GOOGLE_CHROMEEC_SKUID which pulls in a file ec_skuid.c that provides implementation of google_chromeec_get_board_sku() as well as smbios_system_sku(). Then, we don't need mainboards to provide smbios_system_sku() just to call google_chromeec_smbios_system_sku(). And this Kconfig can be selected by most recent mainboards.
https://review.coreboot.org/c/coreboot/+/39018/3/src/ec/google/chromeec/ec_b... PS3, Line 50: 255 If SKU can be as big as 0x7FFFFFF why is the string only allowing upto 255?
https://review.coreboot.org/c/coreboot/+/39018/3/src/ec/google/chromeec/ec_b... PS3, Line 53: if ((sku_id == SKU_UNKNOWN) || : (sku_id > CONFIG_EC_GOOGLE_CHROMEEC_SKU_ID_MAX)) { : printk(BIOS_ERR, "%s: Unexpected SKU ID %u\n", : __func__, sku_id); : return ""; : } I am wondering.. is it bad if we just skip this check completely? i.e. just pass whatever we get from google_chromeec_get_board_sku() as sku%u in SMBIOS? Then we don't need another Kconfig for this.