Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40259 )
Change subject: src/soc/intel/tigerlake: Add ACPI LPIT table ......................................................................
Patch Set 1:
(9 comments)
https://review.coreboot.org/c/coreboot/+/40259/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40259/1//COMMIT_MSG@7 PS1, Line 7: src/ Remove.
https://review.coreboot.org/c/coreboot/+/40259/1//COMMIT_MSG@10 PS1, Line 10: in LPIT table. This provides OSPM with s0ix entry and exit Is that defined in the ACPI spec? Why is it not done in the operating system?
https://review.coreboot.org/c/coreboot/+/40259/1//COMMIT_MSG@12 PS1, Line 12: s0ix S0ix
https://review.coreboot.org/c/coreboot/+/40259/1//COMMIT_MSG@12 PS1, Line 12: methods (MS0X) for s0ix entry and exit, when they exist Please add a dot/period at the end of sentences.
https://review.coreboot.org/c/coreboot/+/40259/1//COMMIT_MSG@9 PS1, Line 9: This patch adds support for Low Power S0 idle Device Specific Method : in LPIT table. This provides OSPM with s0ix entry and exit : hooks. These LPIT S0ix entry and exit hooks call platform specific : methods (MS0X) for s0ix entry and exit, when they exist Please re-flow for 72/75 charcaters per line.
https://review.coreboot.org/c/coreboot/+/40259/1//COMMIT_MSG@13 PS1, Line 13: Is this just boiler plate or is it actually doing something. Shouldn’t an ACPI generator be implemented, once such methods should be created?
https://review.coreboot.org/c/coreboot/+/40259/1//COMMIT_MSG@16 PS1, Line 16: BUILD to check if is successful build is enough.
https://review.coreboot.org/c/coreboot/+/40259/1/src/soc/intel/tigerlake/acp... File src/soc/intel/tigerlake/acpi/lpit.asl:
https://review.coreboot.org/c/coreboot/+/40259/1/src/soc/intel/tigerlake/acp... PS1, Line 54: s0ix S0ix
https://review.coreboot.org/c/coreboot/+/40259/1/src/soc/intel/tigerlake/acp... PS1, Line 64: s0ix S0ix