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 skus ......................................................................
Patch Set 6:
(4 comments)
https://review.coreboot.org/#/c/31262/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/31262/6//COMMIT_MSG@7 PS6, Line 7: skus memory. I don't think SoC cares about the concept of SKUs. It is mostly the mainboard.
https://review.coreboot.org/#/c/31262/6//COMMIT_MSG@8 PS6, Line 8: It would be good to add some comment indicating that SPD data passed to FSP differs based on the single channel flag.
https://review.coreboot.org/#/c/31262/6/src/soc/intel/cannonlake/cnl_memcfg_... File src/soc/intel/cannonlake/cnl_memcfg_init.c:
https://review.coreboot.org/#/c/31262/6/src/soc/intel/cannonlake/cnl_memcfg_... PS6, Line 59: ) Typically you would use { } for both if and else blocks or none.
https://review.coreboot.org/#/c/31262/6/src/soc/intel/cannonlake/include/soc... File src/soc/intel/cannonlake/include/soc/cnl_memcfg_init.h:
https://review.coreboot.org/#/c/31262/6/src/soc/intel/cannonlake/include/soc... PS6, Line 108: current sku memory configuration