Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/22932 )
Change subject: mb/lenovo/x200: Use lower C-states on battery ......................................................................
Patch Set 15:
(2 comments)
It would be better if this information was passed via devicetree and then checked if both CPU and SB support the C-state. For now this may be good enough?
https://review.coreboot.org/c/coreboot/+/22932/15/src/cpu/intel/speedstep/ac... File src/cpu/intel/speedstep/acpi.c:
https://review.coreboot.org/c/coreboot/+/22932/15/src/cpu/intel/speedstep/ac... PS15, Line 159: acpigen_emit_namestring("PWRS")
Does PWRS exist on all callers?
PWRS is in the GNVS of i82810{g,i,j}x which are all the platforms using this code.
https://review.coreboot.org/c/coreboot/+/22932/15/src/cpu/intel/speedstep/ac... PS15, Line 163: num_bat_cstates
Always true?
Can be 0, just like num_cstates can be 0.
-
Arthur Heymans (Code Review)