Attention is currently required from: Tarun Tuli, Subrata Banik, Eric Lai.
Sumeet R Pawnikar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74380 )
Change subject: soc/intel/meteoerlake: set power limits dynamically ......................................................................
Patch Set 2:
(5 comments)
File src/soc/intel/meteorlake/chip.h:
https://review.coreboot.org/c/coreboot/+/74380/comment/bc9300ee_37180bbf PS2, Line 33: static const struct
wondering if u have more consumers of this data structure beyond systemagent. […]
As of now it's only for systemagent.c file.
File src/soc/intel/meteorlake/systemagent.c:
https://review.coreboot.org/c/coreboot/+/74380/comment/7fce6765_bdec0d61 PS2, Line 152: soc_systemagent_init
i would have refactored this code as below […]
Done
https://review.coreboot.org/c/coreboot/+/74380/comment/cafa50fc_7346e749 PS2, Line 168: 0xFFFF
shouldn't we skip the below operations if ID is 0xFFFF? what is the point of running into the `for` […]
Ack
https://review.coreboot.org/c/coreboot/+/74380/comment/7fdd7a99_3879fdde PS2, Line 183: if (i == ARRAY_SIZE(cpuid_to_mtl)) {
can u use a bool to know if config_tdp is successful ?
Let me modify the code accordingly and submit next version.
https://review.coreboot.org/c/coreboot/+/74380/comment/514145f8_732618fb PS2, Line 188: printk(BIOS_DEBUG, "Configured power limits for SA ID: 0x%4x\n", sa_pci_id);
please move this line into line#178 to avoid a else case here.
Done
-
Sumeet R Pawnikar (Code Review)