Patrick Rudolph 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:
(7 comments)
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.
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)
https://review.coreboot.org/c/coreboot/+/32350/4/src/arch/x86/acpi.c@1255 PS4, Line 1255: lpit_native->residency_counter.space_id = 0x7F; why is this "the" default for all platforms?
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;
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
https://review.coreboot.org/c/coreboot/+/32350/4/src/arch/x86/include/arch/a... File src/arch/x86/include/arch/acpi.h:
https://review.coreboot.org/c/coreboot/+/32350/4/src/arch/x86/include/arch/a... PS4, Line 263: typedef struct acpi_lpi_state_flags { not endian safe
https://review.coreboot.org/c/coreboot/+/32350/4/src/arch/x86/include/arch/a... PS4, Line 309: } __packed acpi_table_lpit; doesn't follow existing naming scheme