Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31262 )
Change subject: soc/intel/cannonlake: Add field to identify single channel memory ......................................................................
Patch Set 10:
(4 comments)
https://review.coreboot.org/#/c/31262/10/src/soc/intel/cannonlake/cnl_memcfg... File src/soc/intel/cannonlake/cnl_memcfg_init.c:
https://review.coreboot.org/#/c/31262/10/src/soc/intel/cannonlake/cnl_memcfg... PS10, Line 19: #include <soc/gpio.h> Why is this required?
https://review.coreboot.org/#/c/31262/10/src/soc/intel/cannonlake/cnl_memcfg... PS10, Line 53: cnl_cfg maybe cnl_mb_cfg?
https://review.coreboot.org/#/c/31262/10/src/soc/intel/cannonlake/cnl_memcfg... PS10, Line 61: MemorySpdPtr00 Default value of this field is 0. Should be okay to just:
if (cnl_cfg->pop_channel[0]) mem_cfg->MemorySpdPtr00 = spd_data_ptr; if (cnl_cfg->pop_channel[1]) mem_cfg->MemorySpdPtr10 = spd_data_ptr;
https://review.coreboot.org/#/c/31262/10/src/soc/intel/cannonlake/include/so... File src/soc/intel/cannonlake/include/soc/cnl_memcfg_init.h:
https://review.coreboot.org/#/c/31262/10/src/soc/intel/cannonlake/include/so... PS10, Line 108: Flags to indicate which channels are populated Actually, by doing this, it will break all boards unless you update configuration for each CNL/WHL board to set these fields.