Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34157 )
Change subject: soc/intel/cannonlake: Add device Ids for CFL Refresh support ......................................................................
Patch Set 8:
(12 comments)
https://review.coreboot.org/c/coreboot/+/34157/8//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34157/8//COMMIT_MSG@7 PS8, Line 7: soc/intel/cannonlake: Add device Ids for CFL Refresh support Some of the IDs date back to mid 2018, are you sure it's all CFL "Refresh"?
https://review.coreboot.org/c/coreboot/+/34157/8/src/include/device/pci_ids.... File src/include/device/pci_ids.h:
https://review.coreboot.org/c/coreboot/+/34157/8/src/include/device/pci_ids.... PS8, Line 3052: #define PCI_DEVICE_ID_INTEL_CFL_S_GT2x8_DT 0x3e98 : #define PCI_DEVICE_ID_INTEL_CFL_S_GT2x8_WS 0x3e9a These can be 6+2 too
https://review.coreboot.org/c/coreboot/+/34157/8/src/include/device/pci_ids.... PS8, Line 3117: #define PCI_DEVICE_ID_INTEL_CFL_ID_Sx8_WS 0x3e31 Please realign the whole block (i.e. add another tab to the other lines)
https://review.coreboot.org/c/coreboot/+/34157/8/src/include/device/pci_ids.... PS8, Line 3219: #define PCH_CNP_H_MOBILE_C246 0xa309 This is not mobile, is it?
But anyway, the whole block seems to be redundant and unused, I'll remove it in a sec.
https://review.coreboot.org/c/coreboot/+/34157/8/src/soc/intel/cannonlake/bo... File src/soc/intel/cannonlake/bootblock/report_platform.c:
https://review.coreboot.org/c/coreboot/+/34157/8/src/soc/intel/cannonlake/bo... PS8, Line 109: 2 This is the same 2 as in GT2, why specify it two times?
https://review.coreboot.org/c/coreboot/+/34157/8/src/soc/intel/cannonlake/bo... PS8, Line 111: 2 same here
https://review.coreboot.org/c/coreboot/+/34157/8/src/soc/intel/cannonlake/bo... PS8, Line 112: 2 and here
https://review.coreboot.org/c/coreboot/+/34157/8/src/soc/intel/common/block/... File src/soc/intel/common/block/cpu/mp_init.c:
https://review.coreboot.org/c/coreboot/+/34157/8/src/soc/intel/common/block/... PS8, Line 4: 2019 This should reflect the year when some creative addition was made, not just anytime the file was touched.
https://review.coreboot.org/c/coreboot/+/34157/8/src/soc/intel/common/block/... File src/soc/intel/common/block/graphics/graphics.c:
https://review.coreboot.org/c/coreboot/+/34157/8/src/soc/intel/common/block/... PS8, Line 4: 2019 This should reflect the year when some creative addition was made, not just anytime the file was touched.
https://review.coreboot.org/c/coreboot/+/34157/8/src/soc/intel/common/block/... File src/soc/intel/common/block/include/intelblocks/mp_init.h:
https://review.coreboot.org/c/coreboot/+/34157/8/src/soc/intel/common/block/... PS8, Line 4: 2019 This should reflect the year when some creative addition was made, not just anytime the file was touched.
https://review.coreboot.org/c/coreboot/+/34157/8/src/soc/intel/common/block/... File src/soc/intel/common/block/lpc/lpc.c:
https://review.coreboot.org/c/coreboot/+/34157/8/src/soc/intel/common/block/... PS8, Line 4: 2019 This should reflect the year when some creative addition was made, not just anytime the file was touched.
https://review.coreboot.org/c/coreboot/+/34157/8/src/soc/intel/common/block/... File src/soc/intel/common/block/systemagent/systemagent.c:
https://review.coreboot.org/c/coreboot/+/34157/8/src/soc/intel/common/block/... PS8, Line 4: 2019 This should reflect the year when some creative addition was made, not just anytime the file was touched.