[coreboot-gerrit] Change in ...coreboot[master]: soc/intel/cannonlake: Support different SPD read type for each slot

[Philip Chen (Code Review)]

Philip Chen 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 15:

(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;
> The variable is static, so it will have a definite memory address. […]

OK, SGTM.

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 int […]

Done

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. […]

Done