Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33102 )
Change subject: ec/apple: Add ACPI code for Apple MacBooks ......................................................................
Patch Set 16: Code-Review+1
(6 comments)
https://review.coreboot.org/c/coreboot/+/33102/16//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/33102/16//COMMIT_MSG@10 PS16, Line 10: its duplication in mainboards nit: add two spaces at the beginning of the line to align the text
https://review.coreboot.org/c/coreboot/+/33102/16//COMMIT_MSG@12 PS16, Line 12: Old code is rewritten with new ASL syntax Use the same tense as the other statements:
Rewrite old code using the new ASL syntax
https://review.coreboot.org/c/coreboot/+/33102/16/src/ec/apple/acpi/ac_01.as... File src/ec/apple/acpi/ac_01.asl:
https://review.coreboot.org/c/coreboot/+/33102/16/src/ec/apple/acpi/ac_01.as... PS16, Line 21: 0x01 The AC files only differ in this number. What about using preprocessor to define these values?
https://review.coreboot.org/c/coreboot/+/33102/16/src/ec/apple/acpi/ac_60.as... File src/ec/apple/acpi/ac_60.asl:
https://review.coreboot.org/c/coreboot/+/33102/16/src/ec/apple/acpi/ac_60.as... PS16, Line 22: , 1, Is this the bit where LIDS resides?
https://review.coreboot.org/c/coreboot/+/33102/16/src/ec/apple/acpi/lid_01.a... File src/ec/apple/acpi/lid_01.asl:
https://review.coreboot.org/c/coreboot/+/33102/16/src/ec/apple/acpi/lid_01.a... PS16, Line 24: 0x02 The first difference
https://review.coreboot.org/c/coreboot/+/33102/16/src/ec/apple/acpi/lid_01.a... PS16, Line 39: { 0x1d, 0x03 } The second difference