Edward O'Callaghan 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:
(4 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. […]
The trouble is the weak symbol smbios_system_sku is general and not necessarily a Google thing. However I think I have a solution however I will push the prior of that because I want to unblock our factory image.
https://review.coreboot.org/c/coreboot/+/39018/3/src/ec/google/chromeec/ec_b... PS3, Line 50: sku_str
Also in decimal form (%u) the value value could be […]
Thanks for picking this up, silly copy-paste error.
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?
Done
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 ""; : }
Nevermind about the firmware configuration comment here... […]
Done