Evgeny Zinoviev 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 17:
(14 comments)
Patch Set 10: Code-Review+1
(3 comments)
Can you autodetect the EC version at runtime? Some have revision registers in the EC space.
I don't know such registers and I don't have old MacBook 2,2 at the moment.
https://review.coreboot.org/c/coreboot/+/33102/14//COMMIT_MSG Commit Message:
PS14:
Yeah... I have copies on all macbooks and they are not very good synchronized with each other. […]
Done
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
Done
https://review.coreboot.org/c/coreboot/+/33102/10/src/ec/apple/acpi/ac_01.as... File src/ec/apple/acpi/ac_01.asl:
PS10:
Still unresolved
Done
https://review.coreboot.org/c/coreboot/+/33102/10/src/ec/apple/acpi/ac_01.as... PS10, Line 34: return(HPAC)
Still unresolved
Done
https://review.coreboot.org/c/coreboot/+/33102/10/src/ec/apple/acpi/ac_60.as... File src/ec/apple/acpi/ac_60.asl:
https://review.coreboot.org/c/coreboot/+/33102/10/src/ec/apple/acpi/ac_60.as... PS10, Line 34: return(HPAC)
Still unresolved
Done
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?
Yes
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@4... PS10, Line 42: IO
Still unresolved
It stops working with FixedIO. Linux throws multiple EmbeddedController ACPI-related errors, something with AE_TIME and boot takes 5 min instead of 5 sec.
https://review.coreboot.org/c/coreboot/+/33102/10/src/ec/apple/acpi/ec.asl@5... PS10, Line 50: return (Package () { 0x23, 0x04 })
Still unresolved
Done
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 34: return(LIDS)
Still unresolved
Done
https://review.coreboot.org/c/coreboot/+/33102/10/src/ec/apple/acpi/lid_01.a... PS10, Line 39: Return (Package() { 0x1d, 0x03 })
Still unresolved
Done
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
Done
https://review.coreboot.org/c/coreboot/+/33102/16/src/ec/apple/acpi/lid_01.a... PS16, Line 24: 0x02
The first difference
Done
https://review.coreboot.org/c/coreboot/+/33102/16/src/ec/apple/acpi/lid_01.a... PS16, Line 39: { 0x1d, 0x03 }
The second difference
Done
https://review.coreboot.org/c/coreboot/+/33102/16/src/ec/apple/acpi/lid_01.a... PS16, Line 39: { 0x1d, 0x03 }
The second difference
Done