Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40259 )
Change subject: soc/intel/common: Add _DSM methods for LPIT table ......................................................................
Patch Set 2:
(10 comments)
https://review.coreboot.org/c/coreboot/+/40259/2/src/soc/intel/common/acpi/l... File src/soc/intel/common/acpi/lpit.asl:
PS2: I realize you're just copying this to common/, but the cleanups I've suggested below would be appreciated!
https://review.coreboot.org/c/coreboot/+/40259/2/src/soc/intel/common/acpi/l... PS2, Line 21: scope Scope
https://review.coreboot.org/c/coreboot/+/40259/2/src/soc/intel/common/acpi/l... PS2, Line 23: Device(LPID) { Please put the brace on the next line, and begin indenting there. All three Names below should line up with the Method(_DSM, 4) line.
https://review.coreboot.org/c/coreboot/+/40259/2/src/soc/intel/common/acpi/l... PS2, Line 28: Method(_DSM, 4) { Same thing here, brace on the next line please.
https://review.coreboot.org/c/coreboot/+/40259/2/src/soc/intel/common/acpi/l... PS2, Line 33: Zero Maybe some #defines for the enumerated values that are valid for Arg2 would be nice.
#define LPID_DSM_ARG2_ENUM_FUNCTIONS 0 #define LPID_DSM_ARG2_GET_DEVICE_CONSTRAINTS 1 ... etc.
https://review.coreboot.org/c/coreboot/+/40259/2/src/soc/intel/common/acpi/l... PS2, Line 35: 0x60} This can go on the previous line.
https://review.coreboot.org/c/coreboot/+/40259/2/src/soc/intel/common/acpi/l... PS2, Line 43: 0, Ones, Ones, Ones, Ones} This can go on the previous line.
https://review.coreboot.org/c/coreboot/+/40259/2/src/soc/intel/common/acpi/l... PS2, Line 51: 0x0} This can go on the previous line.
https://review.coreboot.org/c/coreboot/+/40259/2/src/soc/intel/common/acpi/l... PS2, Line 108: Return(Buffer(One) {0x00}) Blank line before Return.
https://review.coreboot.org/c/coreboot/+/40259/2/src/soc/intel/common/acpi/l... PS2, Line 110: device Device