Shaunak Saha has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32350 )
Change subject: Add support for low power idle table ......................................................................
Patch Set 2:
(4 comments)
https://review.coreboot.org/#/c/32350/2/src/arch/x86/acpi.c File src/arch/x86/acpi.c:
https://review.coreboot.org/#/c/32350/2/src/arch/x86/acpi.c@1108 PS2, Line 1108: #if CONFIG(ACPI_LPIT)
no need for preprocessor
Ack
https://review.coreboot.org/#/c/32350/2/src/arch/x86/acpi.c@1112 PS2, Line 1112: unsigned long current = (unsigned long)lpit + sizeof(acpi_table_lpit);
can be removed
Ack
https://review.coreboot.org/#/c/32350/2/src/arch/x86/acpi.c@1151 PS2, Line 1151: /* (Re)calculate length and checksum. */
why? you did not increase current?
Ack
https://review.coreboot.org/#/c/32350/2/src/arch/x86/acpi.c@1343 PS2, Line 1343: #if CONFIG(ACPI_LPIT)
use if(CONFIG(ACPI_LPIT)). The linker will drop unused code.
Ack