Sumeet R Pawnikar 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 17:
(6 comments)
https://review.coreboot.org/c/coreboot/+/39346/17/src/mainboard/intel/glkrvp... File src/mainboard/intel/glkrvp/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/39346/17/src/mainboard/intel/glkrvp... PS17, Line 58: 7.5W setting gives a run-time 6W actual
to what extent does this still hold?
This was based on glkrvp reference open board during early enablement.
https://review.coreboot.org/c/coreboot/+/39346/17/src/soc/intel/apollolake/c... File src/soc/intel/apollolake/chip.h:
https://review.coreboot.org/c/coreboot/+/39346/17/src/soc/intel/apollolake/c... PS17, Line 115: mW
just W
Done
https://review.coreboot.org/c/coreboot/+/39346/17/src/soc/intel/apollolake/c... PS17, Line 117: mW
W
Done
https://review.coreboot.org/c/coreboot/+/39346/17/src/soc/intel/apollolake/c... File src/soc/intel/apollolake/chip.c:
https://review.coreboot.org/c/coreboot/+/39346/17/src/soc/intel/apollolake/c... PS17, Line 322: / 1000
really?
This is not required any more.
https://review.coreboot.org/c/coreboot/+/39346/17/src/soc/intel/apollolake/c... PS17, Line 342: %d
does this need to be %02d?
Ack
https://review.coreboot.org/c/coreboot/+/39346/17/src/soc/intel/common/block... File src/soc/intel/common/block/power_limit/power_limit.c:
https://review.coreboot.org/c/coreboot/+/39346/17/src/soc/intel/common/block... PS17, Line 206: msr
'msr' isnt' used after this. […]
This is not required. Thanks for finding this.