caveh jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39346 )
Change subject: soc/intel/tigerlake: Add processor power limits control support ......................................................................
Patch Set 4:
(5 comments)
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/cpu... File src/soc/intel/tigerlake/cpu.c:
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/cpu... PS4, Line 114: >
=
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/cpu... PS4, Line 115: 28 so, 28s is the default if out of range? 1) let's add a constant for this 2) should this be the nearest supported value instead of some default? how do you guarantee that power_limit_1_time has an entry in the table? 3) is this the same "max_time" SKU default queried a few lines later?
please reconcile differences with src/soc/intel/cannonlake/cpu.c
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/cpu... PS4, Line 148: extra indent?
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/cpu... PS4, Line 198: extra space
https://review.coreboot.org/c/coreboot/+/39346/4/src/soc/intel/tigerlake/cpu... PS4, Line 209: = be consistent about spaces around '=' in these messages and the format of these strings in general.