Shaunak Saha has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32350 )
Change subject: src/arch/x86:Add support for low power idle table ......................................................................
Patch Set 4:
(14 comments)
https://review.coreboot.org/c/coreboot/+/32350/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/32350/4//COMMIT_MSG@7 PS4, Line 7: src/arch/x86:Add
- `src/` not needed. […]
Ack
https://review.coreboot.org/c/coreboot/+/32350/4//COMMIT_MSG@7 PS4, Line 7: low power idle table
Use LPIT here, and the full name in the commit message body.
Ack
https://review.coreboot.org/c/coreboot/+/32350/4//COMMIT_MSG@9 PS4, Line 9: residency.Residencies
Space after .
Ack
https://review.coreboot.org/c/coreboot/+/32350/4//COMMIT_MSG@13 PS4, Line 13: CPU PKG C10 (Read via FFH interface) : Platform Controller Hub (PCH) SLP_S0 (Read via memory mapped interface)
Please add blank lines around, and format it as a list.
Ack
https://review.coreboot.org/c/coreboot/+/32350/4//COMMIT_MSG@17 PS4, Line 17: /sys/devices/system/cpu/cpuidle/low_power_idle_cpu_residency_us : /sys/devices/system/cpu/cpuidle/low_power_idle_system_residency_us
Please indent with four spaces, and add blank lines around the block.
Ack
https://review.coreboot.org/c/coreboot/+/32350/4//COMMIT_MSG@21 PS4, Line 21: The “low_power_idle_system_residency_us” attribute shows SLP_S0 residency, or
Please add a blank line above to separate paragraphs visually.
Ack
https://review.coreboot.org/c/coreboot/+/32350/4//COMMIT_MSG@26 PS4, Line 26: Example:
Please add a blank line above.
Ack
https://review.coreboot.org/c/coreboot/+/32350/4//COMMIT_MSG@28 PS4, Line 28: thet
that
Ack
https://review.coreboot.org/c/coreboot/+/32350/4//COMMIT_MSG@31 PS4, Line 31: elected
selected
Ack
https://review.coreboot.org/c/coreboot/+/32350/4//COMMIT_MSG@31 PS4, Line 31: thet
that
Ack
https://review.coreboot.org/c/coreboot/+/32350/4/src/arch/x86/acpi.c File src/arch/x86/acpi.c:
https://review.coreboot.org/c/coreboot/+/32350/4/src/arch/x86/acpi.c@1244 PS4, Line 1244: * implementation specific.e.e.g, scenerios:
no need for SoC comments in arch/x86.
Ack
https://review.coreboot.org/c/coreboot/+/32350/4/src/arch/x86/acpi.c@1253 PS4, Line 1253: __weak void soc_residency_counter(struct acpi_lpit_native *lpit_native)
no need for weak, code will be removed if not CONFIG(ACPI_LPIT)
Ack
https://review.coreboot.org/c/coreboot/+/32350/4/src/arch/x86/acpi.c@1275 PS4, Line 1275: header->asl_compiler_revision = 0;
header->asl_compiler_revision = asl_revision;
Ack
https://review.coreboot.org/c/coreboot/+/32350/4/src/arch/x86/acpi.c@1278 PS4, Line 1278: lpit->lpit_native.header.type = 0;
already cleared by memset
Ack
-
Shaunak Saha (Code Review)