Nico Huber 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 10:
(3 comments)
https://review.coreboot.org/c/coreboot/+/33102/10/src/ec/apple/acpi/ac_01.as... File src/ec/apple/acpi/ac_01.asl:
PS10: ac_01/60 and lid_01/60 could easily be merged. All you'd have to do is to define the offset(s) before inclusion. e.g.
#define HPAC_OFFSET 0x01 #include <ec/apple/acpi/ac.asl>
https://review.coreboot.org/c/coreboot/+/33102/10/src/ec/apple/acpi/ec.asl File src/ec/apple/acpi/ec.asl:
https://review.coreboot.org/c/coreboot/+/33102/10/src/ec/apple/acpi/ec.asl@5... PS10, Line 50: return (Package () { 0x23, 0x04 }) This is the same as in lid_60 so I guess the _PRW method there is not needed. Also noticed: GPE 0x23 is "reserved" in the older ICHs. But I guess it doesn't hurt to keep it here.
https://review.coreboot.org/c/coreboot/+/33102/10/src/ec/apple/acpi/lid_01.a... File src/ec/apple/acpi/lid_01.asl:
https://review.coreboot.org/c/coreboot/+/33102/10/src/ec/apple/acpi/lid_01.a... PS10, Line 39: Return (Package() { 0x1d, 0x03 }) This would need `gpi13_routing = 2` in the devicetree, I guess.