Attention is currently required from: Cliff Huang, Furquan Shaikh, Tim Wawrzynczak, Michael Niewöhner, Angel Pons, Patrick Rudolph, Karthik Ramasubramanian. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56004 )
Change subject: soc/intel/common/block/acpi: Move pep.asl to acpigen ......................................................................
Patch Set 1:
(5 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/56004/comment/82a24b75_6ad1cc20 PS1, Line 13: TEST=select this on brya, dump SSDT: If true, I'd explicitly mention that the resulting SSDT is equivalent to the DSDT
File src/soc/intel/common/block/acpi/pep.c:
https://review.coreboot.org/c/coreboot/+/56004/comment/c9e69baf_5c3c43f5 PS1, Line 25: 0x63 How about:
1 << 6 | 1 << 5 | 1 << 1 | 1 << 0
https://review.coreboot.org/c/coreboot/+/56004/comment/1bed5dba_a79de377 PS1, Line 33: static void lpi_get_constraints(void *unused)
please, no spaces at the start of a line
Please fix.
https://review.coreboot.org/c/coreboot/+/56004/comment/f3b13529_df6e17d0 PS1, Line 40: "\NULL" I vaguely recall seeing some ASL that used the host bridge device here. Although I don't think it would make much of a difference (constraints are disabled), IMHO it feels slightly "less wrong" to me, whatever it means.
https://review.coreboot.org/c/coreboot/+/56004/comment/8723fcdc_308eb9cb PS1, Line 37: acpigen_emit_byte(RETURN_OP); : acpigen_write_package(1); : acpigen_write_package(3); : acpigen_emit_namestring("\NULL"); /* non-existent */ : acpigen_write_integer(0); /* disabled, no constraints */ : acpigen_write_package(2); : acpigen_write_integer(0); /* revision */ : acpigen_write_package(2); : acpigen_write_integer(0xff); /* apply to all LPIT states */ : acpigen_write_integer(0); /* minimum D-state */ : acpigen_write_package_end(); /* inner 3 */ : acpigen_write_package_end(); /* inner 2 */ : acpigen_write_package_end(); /* inner 1 */ : acpigen_write_package_end(); /* outer */ How about using scopes to show what closes what?
acpigen_emit_byte(RETURN_OP); acpigen_write_package(1); { acpigen_write_package(3); { acpigen_emit_namestring("\NULL"); /* non-existent */ acpigen_write_integer(0); /* disabled, no constraints */ acpigen_write_package(2); { acpigen_write_integer(0); /* revision */ acpigen_write_package(2); { acpigen_write_integer(0xff); /* apply to all LPIT states */ acpigen_write_integer(0); /* minimum D-state */ } acpigen_write_package_end(); } acpigen_write_package_end(); } acpigen_write_package_end(); } acpigen_write_package_end();