Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32513 )
Change subject: soc/intel/cannonlake: Support different SPD read type for each slot ......................................................................
Patch Set 14:
(3 comments)
https://review.coreboot.org/#/c/32513/13/src/soc/intel/cannonlake/cnl_memcfg... File src/soc/intel/cannonlake/cnl_memcfg_init.c:
https://review.coreboot.org/#/c/32513/13/src/soc/intel/cannonlake/cnl_memcfg... PS13, Line 57: static size_t last_set_spd_data_len; You should explicitly initialize the variable to 0.
https://review.coreboot.org/#/c/32513/13/src/soc/intel/cannonlake/cnl_memcfg... PS13, Line 59: assert(spd_data_ptr && spd_data_len); Please explicitly test spd_data_len != 0 instead of relying on C language behavior to convert an integer from zero/non-zero to false/true.
https://review.coreboot.org/#/c/32513/13/src/soc/intel/cannonlake/cnl_memcfg... PS13, Line 61: if (last_set_spd_data_len && last_set_spd_data_len != spd_data_len) Please explicitly test last_set_spd_data_len != 0. Also, this could fail if last_set_spd_data_len is uninitialized and you happen to get something non-zero in that memory.