Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45635 )
Change subject: vendorcode/google: add CHROMEOS_DRAM_PART_NUMBER_IN_CBI Kconfig option ......................................................................
Patch Set 21: Code-Review+1
(3 comments)
https://review.coreboot.org/c/coreboot/+/45635/21/src/vendorcode/google/chro... File src/vendorcode/google/chromeos/dram_part_num_override.c:
https://review.coreboot.org/c/coreboot/+/45635/21/src/vendorcode/google/chro... PS21, Line 11: UNINITIALIZED nit: I think the names can be a little more descriptive. You can probably use the same ones as done by hatch: https://review.coreboot.org/cgit/coreboot.git/tree/src/mainboard/google/hatc...
static enum { PART_NUM_NOT_READ, PART_NUM_AVAILABLE, PART_NUM_NOT_IN_CBI, } part_num_state = PART_NUM_NOT_READ;
https://review.coreboot.org/c/coreboot/+/45635/21/src/vendorcode/google/chro... PS21, Line 15: int Use the enum type?
static enum { ... } ec_checked;
https://review.coreboot.org/c/coreboot/+/45635/21/src/vendorcode/google/chro... PS21, Line 26: printk Is it intentional that this error will be printed every time mainboard_get_dram_part_num() is called? Probably okay to do it just the first time?