Evgeny Zinoviev has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33102
Change subject: ec/apple: ACPI code for Apple MacBooks ......................................................................
ec/apple: ACPI code for Apple MacBooks
Move ACPI code for Apple MacBooks to a separate directory to avoid it's duplication in mainboards.
AC and LID implementation files are named by EC register that's used in them. Older generations (macbook2,1) use 0x01 while newer generations like 2011-2012 Airs use 0x60. Battery registers seem to be the same.
Tested on MacBook Air 5,2.
Change-Id: I3d4585aac8e3ebbfed6ce4d4e39fbc33ac983069 Signed-off-by: Evgeny Zinoviev me@ch1p.io --- A src/ec/apple/acpi/ac_01.asl A src/ec/apple/acpi/ac_60.asl A src/ec/apple/acpi/battery.asl A src/ec/apple/acpi/ec.asl A src/ec/apple/acpi/lid_01.asl A src/ec/apple/acpi/lid_60.asl 6 files changed, 405 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/33102/1
diff --git a/src/ec/apple/acpi/ac_01.asl b/src/ec/apple/acpi/ac_01.asl new file mode 100644 index 0000000..86966ae8 --- /dev/null +++ b/src/ec/apple/acpi/ac_01.asl @@ -0,0 +1,42 @@ + + * This file is part of the coreboot project. + * + * Copyright (c) 2019 Evgeny Zinoviev me@ch1p.io + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; version 2 of + * the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +Scope(_SB.PCI0.LPCB.EC) +{ + Field(ERAM, ByteAcc, NoLock, Preserve) + { + Offset(0x01), + 1, + HPAC, 1, /* AC status */ + } + + Device(AC) + { + Name(_HID, "ACPI0003") + Name(_UID, 0x00) + Name(_PCL, Package() { _SB } ) + + Method(_PSR, 0, NotSerialized) + { + return(HPAC) + } + + Method(_STA, 0, NotSerialized) + { + Return(0x0f) + } + } +} diff --git a/src/ec/apple/acpi/ac_60.asl b/src/ec/apple/acpi/ac_60.asl new file mode 100644 index 0000000..76adea1 --- /dev/null +++ b/src/ec/apple/acpi/ac_60.asl @@ -0,0 +1,42 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (c) 2019 Evgeny Zinoviev me@ch1p.io + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; version 2 of + * the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +Scope(_SB.PCI0.LPCB.EC) +{ + Field(ERAM, ByteAcc, NoLock, Preserve) + { + Offset(0x60), + , 1, + HPAC, 1, /* AC status */ + } + + Device(AC) + { + Name(_HID, "ACPI0003") + Name(_UID, 0x00) + Name(_PCL, Package() { _SB } ) + + Method(_PSR, 0, NotSerialized) + { + return(HPAC) + } + + Method(_STA, 0, NotSerialized) + { + Return(0x0f) + } + } +} diff --git a/src/ec/apple/acpi/battery.asl b/src/ec/apple/acpi/battery.asl new file mode 100644 index 0000000..f1706be --- /dev/null +++ b/src/ec/apple/acpi/battery.asl @@ -0,0 +1,163 @@ +/* + * This file is part of the coreboot project. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +Field(ERAM, ByteAcc, NoLock, Preserve) +{ + Offset(0x20), + SPTR, 8, + SSTS, 8, + SADR, 8, + SCMD, 8, + SBFR, 256, +} + +Field(ERAM, ByteAcc, Lock, Preserve) +{ + Offset(0x24), + SBDW, 16, +} + +Method(SBPC, 0, NotSerialized) +{ + Store(1000, Local0) + While(Local0) + { + If(LEqual(SPTR, 0x00)) + { + Return() + } + + Sleep(1) + Decrement(Local0) + } +} + +Method(SBRW, 2, NotSerialized) +{ + Acquire(ECLK, 0xFFFF) + Store(ShiftLeft(Arg0, 0x01), SADR) + Store(Arg1, SCMD) + Store(0x09, SPTR) + SBPC() + Store(SBDW, Local0) + Release(ECLK) + Return(Local0) +} + +Method(SBRB, 2, NotSerialized) +{ + Acquire(ECLK, 0xFFFF) + Store(ShiftLeft(Arg0, 0x01), SADR) + Store(Arg1, SCMD) + Store(0x0B, SPTR) + SBPC() + Store(SBFR, Local0) + Release(ECLK) + Return(Local0) +} + +Device(BAT0) +{ + Name(_HID, EisaId("PNP0C0A")) + Name(_UID, 0x00) + Name(_PCL, Package() { _SB }) + + Name(BATS, Package() + { + 0x00, // 0: PowerUnit: Report in mWh + 0xFFFFFFFF, // 1: Design cap + 0xFFFFFFFF, // 2: Last full charge cap + 0x01, // 3: Battery Technology + 10800, // 4: Design Voltage(mV) + 0x00, // 5: Warning design capacity + 200, // 6: Low design capacity + 10, // 7: granularity1 + 10, // 8: granularity2 + "", // 9: Model number + "", // A: Serial number + "", // B: Battery Type + "" // C: OEM information + }) + + Name(BATI, Package() + { + 0, // Battery State + // Bit 0 - discharge + // Bit 1 - charge + // Bit 2 - critical state + 0, // Battery present Rate + 0, // Battery remaining capacity + 0 // Battery present voltage + }) + + Method(_BIF, 0, NotSerialized) + { + Multiply(^^SBRW(0x0B, 0x18), 10, Index(BATS, 0x01)) + Multiply(^^SBRW(0x0B, 0x10), 10, Index(BATS, 0x02)) + Store(^^SBRW(0x0B, 0x19), Index(BATS, 0x04)) + Store(^^SBRB(0x0B, 0x21), Index(BATS, 0x09)) + Store(^^SBRB(0x0B, 0x22), Index(BATS, 0x0B)) + Store(^^SBRB(0x0B, 0x20), Index(BATS, 0x0C)) + + Return(BATS) + } + + Method(_STA, 0, NotSerialized) + { + If(And(^^SBRW(0x0A, 0x01), 0x01)) { + Return(0x1f) + } else { + Return(0x0f) + } + } + + Method(_BST, 0, NotSerialized) + { + /* Check for battery presence. */ + If(LNot(And(^^SBRW(0x0A, 0x01), 0x01))) { + Return(Package(4) { + 0, + 0xFFFFFFFF, + 0xFFFFFFFF, + 0xFFFFFFFF + }) + } + Store(^^SBRW(0x0B, 0x09), Local1) + Store(Local1, Index(BATI, 0x03)) + Store(^^SBRW(0x0B, 0x0A), Local0) + /* Sign-extend Local0. */ + If(And(Local0, 0x8000)) + { + Not(Local0, Local0) + And(Increment(Local0), 0xFFFF, Local0) + } + + Multiply(Local0, Local1, Local0) + Divide(Local0, 1000, , Index(BATI, 1)) + Multiply(^^SBRW(0x0B, 0x0F), 10, Index(BATI, 2)) + If(HPAC) + { + If(LNot(And(^^SBRW(0x0B, 0x16), 0x40))) { + Store(2, Index(BATI, 0)) + } Else { + Store(0, Index(BATI, 0)) + } + } + Else + { + Store(0x01, Index(BATI, 0)) + } + + Return(BATI) + } +} diff --git a/src/ec/apple/acpi/ec.asl b/src/ec/apple/acpi/ec.asl new file mode 100644 index 0000000..982011c --- /dev/null +++ b/src/ec/apple/acpi/ec.asl @@ -0,0 +1,56 @@ +/* + * This file is part of the coreboot project. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +Device(EC) +{ + Name(_HID, EISAID("PNP0C09")) + Name(_UID, 0) + + Name(_GPE, 0x17) + Mutex(ECLK, 0) + + OperationRegion(ERAM, EmbeddedControl, 0x00, 0x100) + + /* LID status change. */ + Method(_Q20, 0, NotSerialized) + { + Notify(LID, 0x80) + } + + /* AC status change. */ + Method(_Q21, 0, NotSerialized) + { + Notify(AC, 0x80) + } + + Method(_CRS, 0) + { + Name(ECMD, ResourceTemplate() + { + IO(Decode16, 0x62, 0x62, 1, 1) + IO(Decode16, 0x66, 0x66, 1, 1) + }) + Return(ECMD) + } + + Method(_PRW, 0, NotSerialized) + { + return (Package () { 0x23, 0x04 }) + } + + Method(_INI, 0, NotSerialized) + { + } + +#include "battery.asl" +} diff --git a/src/ec/apple/acpi/lid_01.asl b/src/ec/apple/acpi/lid_01.asl new file mode 100644 index 0000000..88ad045 --- /dev/null +++ b/src/ec/apple/acpi/lid_01.asl @@ -0,0 +1,51 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (c) 2019 Evgeny Zinoviev me@ch1p.io + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; version 2 of + * the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +Scope(_SB.PCI0.LPCB.EC) +{ + Field(ERAM, ByteAcc, NoLock, Preserve) + { + Offset(0x01), + LIDS, 1, /* Lid status */ + + Offset(0x02), + WKLD, 1, /* Lid wake */ + } + + Device(LID) + { + Name(_HID, "PNP0C0D") + + Method(_LID, 0, NotSerialized) + { + return(LIDS) + } + + Method(_PRW, 0, NotSerialized) + { + Return (Package() { 0x1d, 0x03 }) + } + + Method(_PSW, 1, NotSerialized) + { + if (Arg0) { + Store(1, WKLD) + } else { + Store(0, WKLD) + } + } + } +} diff --git a/src/ec/apple/acpi/lid_60.asl b/src/ec/apple/acpi/lid_60.asl new file mode 100644 index 0000000..e0836b6 --- /dev/null +++ b/src/ec/apple/acpi/lid_60.asl @@ -0,0 +1,51 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (c) 2019 Evgeny Zinoviev me@ch1p.io + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; version 2 of + * the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +Scope(_SB.PCI0.LPCB.EC) +{ + Field(ERAM, ByteAcc, NoLock, Preserve) + { + Offset(0x60), + LIDS, 1, /* Lid status */ + + Offset(0x68), + WKLD, 1, /* Lid wake */ + } + + Device(LID) + { + Name(_HID, "PNP0C0D") + + Method(_LID, 0, NotSerialized) + { + return(LIDS) + } + + Method(_PRW, 0, NotSerialized) + { + Return (Package() { 0x23, 0x04 }) + } + + Method(_PSW, 1, NotSerialized) + { + if (Arg0) { + Store(1, WKLD) + } else { + Store(0, WKLD) + } + } + } +}
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33102
to look at the new patch set (#2).
Change subject: ec/apple: ACPI code for Apple MacBooks ......................................................................
ec/apple: ACPI code for Apple MacBooks
Move ACPI code for Apple MacBooks to a separate directory to avoid it's duplication in mainboards.
AC and LID implementation files are named by EC register that's used in them. Older generations (macbook2,1) use 0x01 while newer generations like 2011-2012 Airs use 0x60. Battery registers seem to be the same.
Tested on MacBook Air 5,2.
Change-Id: I3d4585aac8e3ebbfed6ce4d4e39fbc33ac983069 Signed-off-by: Evgeny Zinoviev me@ch1p.io --- A src/ec/apple/acpi/ac_01.asl A src/ec/apple/acpi/ac_60.asl A src/ec/apple/acpi/battery.asl A src/ec/apple/acpi/ec.asl A src/ec/apple/acpi/lid_01.asl A src/ec/apple/acpi/lid_60.asl 6 files changed, 405 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/33102/2
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33102 )
Change subject: ec/apple: ACPI code for Apple MacBooks ......................................................................
Patch Set 2:
(2 comments)
Can you use the new acpi syntax to make it more readable?
https://review.coreboot.org/#/c/33102/2/src/ec/apple/acpi/battery.asl File src/ec/apple/acpi/battery.asl:
https://review.coreboot.org/#/c/33102/2/src/ec/apple/acpi/battery.asl@3 PS2, Line 3: * missing copyright
https://review.coreboot.org/#/c/33102/2/src/ec/apple/acpi/battery.asl@17 PS2, Line 17: SPTR, 8, to many tabs
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33102
to look at the new patch set (#3).
Change subject: ec/apple: ACPI code for Apple MacBooks ......................................................................
ec/apple: ACPI code for Apple MacBooks
Move ACPI code for Apple MacBooks to a separate directory to avoid it's duplication in mainboards.
AC and lid implementation files are named by EC register that's used in them. Older generations (macbook2,1) use 0x01 while newer generations like 2011-2012 Airs use 0x60. Battery registers seem to be the same.
Tested on MacBook Air 5,2.
Change-Id: I3d4585aac8e3ebbfed6ce4d4e39fbc33ac983069 Signed-off-by: Evgeny Zinoviev me@ch1p.io --- A src/ec/apple/acpi/ac_01.asl A src/ec/apple/acpi/ac_60.asl A src/ec/apple/acpi/battery.asl A src/ec/apple/acpi/ec.asl A src/ec/apple/acpi/lid_01.asl A src/ec/apple/acpi/lid_60.asl 6 files changed, 405 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/33102/3
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33102
to look at the new patch set (#4).
Change subject: ec/apple: ACPI code for Apple MacBooks ......................................................................
ec/apple: ACPI code for Apple MacBooks
Move ACPI code for Apple MacBooks to a separate directory to avoid it's duplication in mainboards.
AC and lid implementation files are named by EC register that's used in them. Older generations (macbook2,1) use 0x01 while newer generations like 2011-2012 Airs use 0x60. Battery registers seem to be the same.
Old code is rewritten with new ASL syntax.
Tested on MacBook Air 5,2.
Change-Id: I3d4585aac8e3ebbfed6ce4d4e39fbc33ac983069 Signed-off-by: Evgeny Zinoviev me@ch1p.io --- A src/ec/apple/acpi/ac_01.asl A src/ec/apple/acpi/ac_60.asl A src/ec/apple/acpi/battery.asl A src/ec/apple/acpi/ec.asl A src/ec/apple/acpi/lid_01.asl A src/ec/apple/acpi/lid_60.asl 6 files changed, 412 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/33102/4
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33102 )
Change subject: ec/apple: ACPI code for Apple MacBooks ......................................................................
Patch Set 4:
(3 comments)
https://review.coreboot.org/#/c/33102/4/src/ec/apple/acpi/battery.asl File src/ec/apple/acpi/battery.asl:
https://review.coreboot.org/#/c/33102/4/src/ec/apple/acpi/battery.asl@109 PS4, Line 109: trailing whitespace
https://review.coreboot.org/#/c/33102/4/src/ec/apple/acpi/battery.asl@138 PS4, Line 138: trailing whitespace
https://review.coreboot.org/#/c/33102/4/src/ec/apple/acpi/battery.asl@142 PS4, Line 142: trailing whitespace
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33102
to look at the new patch set (#5).
Change subject: ec/apple: ACPI code for Apple MacBooks ......................................................................
ec/apple: ACPI code for Apple MacBooks
Move ACPI code for Apple MacBooks to a separate directory to avoid it's duplication in mainboards.
AC and lid implementation files are named by EC register that's used in them. Older generations (macbook2,1) use 0x01 while newer generations like 2011-2012 Airs use 0x60. Battery registers seem to be the same.
Old code is rewritten with new ASL syntax.
Tested on MacBook Air 5,2.
Change-Id: I3d4585aac8e3ebbfed6ce4d4e39fbc33ac983069 Signed-off-by: Evgeny Zinoviev me@ch1p.io --- A src/ec/apple/acpi/ac_01.asl A src/ec/apple/acpi/ac_60.asl A src/ec/apple/acpi/battery.asl A src/ec/apple/acpi/ec.asl A src/ec/apple/acpi/lid_01.asl A src/ec/apple/acpi/lid_60.asl 6 files changed, 409 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/33102/5
Evgeny Zinoviev has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33102 )
Change subject: ec/apple: ACPI code for Apple MacBooks ......................................................................
Patch Set 5:
(3 comments)
Rewritten with new syntax, tested again on MBA 5,1.
https://review.coreboot.org/#/c/33102/2/src/ec/apple/acpi/battery.asl File src/ec/apple/acpi/battery.asl:
https://review.coreboot.org/#/c/33102/2/src/ec/apple/acpi/battery.asl@3 PS2, Line 3: *
missing copyright
Done
https://review.coreboot.org/#/c/33102/2/src/ec/apple/acpi/battery.asl@17 PS2, Line 17: SPTR, 8,
to many tabs
Done
https://review.coreboot.org/#/c/33102/5/src/ec/apple/acpi/battery.asl File src/ec/apple/acpi/battery.asl:
https://review.coreboot.org/#/c/33102/5/src/ec/apple/acpi/battery.asl@147 PS5, Line 147: Local0++ & 0xFFFF I have doubts about this, is this correct? It was `And(Increment(Local0), 0xFFFF, Local0)`
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33102
to look at the new patch set (#6).
Change subject: ec/apple: ACPI code for Apple MacBooks ......................................................................
ec/apple: ACPI code for Apple MacBooks
- Moved ACPI code for Apple MacBooks to a separate directory to avoid it's duplication in mainboards - Added AC and lid implementations for newer generations - Old code is rewritten with new ASL syntax
AC and lid implementation files are named after EC register that's used in them. Older generations (macbook2,1) use 0x01 while newer generations like 2011-2012 Airs use 0x60. Battery registers seem to be the same.
Tested on MacBook Air 5,2.
Change-Id: I3d4585aac8e3ebbfed6ce4d4e39fbc33ac983069 Signed-off-by: Evgeny Zinoviev me@ch1p.io --- A src/ec/apple/acpi/ac_01.asl A src/ec/apple/acpi/ac_60.asl A src/ec/apple/acpi/battery.asl A src/ec/apple/acpi/ec.asl A src/ec/apple/acpi/lid_01.asl A src/ec/apple/acpi/lid_60.asl 6 files changed, 409 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/33102/6
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33102
to look at the new patch set (#7).
Change subject: ec/apple: ACPI code for Apple MacBooks ......................................................................
ec/apple: ACPI code for Apple MacBooks
- Moved ACPI code for Apple MacBooks to a separate directory to avoid it's duplication in mainboards - Added AC and lid implementations for newer generations - Old code is rewritten with new ASL syntax
AC and lid implementation files are named after EC register that's used in them. Older generations (macbook2,1) use 0x01 while newer generations like 2011-2012 Airs use 0x60. Battery registers seem to be the same.
Tested on MacBook Air 5,2.
Change-Id: I3d4585aac8e3ebbfed6ce4d4e39fbc33ac983069 Signed-off-by: Evgeny Zinoviev me@ch1p.io --- A src/ec/apple/acpi/ac_01.asl A src/ec/apple/acpi/ac_60.asl A src/ec/apple/acpi/battery.asl A src/ec/apple/acpi/ec.asl A src/ec/apple/acpi/lid_01.asl A src/ec/apple/acpi/lid_60.asl 6 files changed, 409 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/33102/7
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33102 )
Change subject: ec/apple: ACPI code for Apple MacBooks ......................................................................
Patch Set 8:
(3 comments)
https://review.coreboot.org/#/c/33102/8//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/33102/8//COMMIT_MSG@7 PS8, Line 7: ec/apple: ACPI code for Apple MacBooks Please make it a statement by adding a verb (in imperative mood).
Add ACPI code for Apple MacBooks
https://review.coreboot.org/#/c/33102/8//COMMIT_MSG@9 PS8, Line 9: Moved Move
https://review.coreboot.org/#/c/33102/8//COMMIT_MSG@11 PS8, Line 11: Added Add
Hello Patrick Rudolph, Felix Held, Angel Pons, build bot (Jenkins), Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33102
to look at the new patch set (#10).
Change subject: ec/apple: Add ACPI code for Apple MacBooks ......................................................................
ec/apple: Add ACPI code for Apple MacBooks
- Move ACPI code for Apple MacBooks to a separate directory to avoid it's duplication in mainboards - Add AC and lid implementations for newer generations - Old code is rewritten with new ASL syntax
AC and lid implementation files are named after EC register that's used in them. Older generations (macbook2,1) use 0x01 while newer generations like 2011-2012 Airs use 0x60. Battery registers seem to be the same.
Tested on MBA 5,2 and MBP 10,1.
Change-Id: I3d4585aac8e3ebbfed6ce4d4e39fbc33ac983069 Signed-off-by: Evgeny Zinoviev me@ch1p.io --- A src/ec/apple/acpi/ac_01.asl A src/ec/apple/acpi/ac_60.asl A src/ec/apple/acpi/battery.asl A src/ec/apple/acpi/ec.asl A src/ec/apple/acpi/lid_01.asl A src/ec/apple/acpi/lid_60.asl 6 files changed, 409 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/33102/10
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 10:
(3 comments)
https://review.coreboot.org/#/c/33102/8//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/33102/8//COMMIT_MSG@7 PS8, Line 7: ec/apple: ACPI code for Apple MacBooks
Please make it a statement by adding a verb (in imperative mood). […]
Done
https://review.coreboot.org/#/c/33102/8//COMMIT_MSG@9 PS8, Line 9: Moved
Move
Done
https://review.coreboot.org/#/c/33102/8//COMMIT_MSG@11 PS8, Line 11: Added
Add
Done
Felix Held 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: Code-Review+1
(1 comment)
https://review.coreboot.org/#/c/33102/10/src/ec/apple/acpi/ec.asl File src/ec/apple/acpi/ec.asl:
https://review.coreboot.org/#/c/33102/10/src/ec/apple/acpi/ec.asl@42 PS10, Line 42: IO FixedIO instead of IO could be used for these two
Patrick Rudolph 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: Code-Review+1
(3 comments)
Can you autodetect the EC version at runtime? Some have revision registers in the EC space.
https://review.coreboot.org/#/c/33102/10/src/ec/apple/acpi/ac_01.asl File src/ec/apple/acpi/ac_01.asl:
https://review.coreboot.org/#/c/33102/10/src/ec/apple/acpi/ac_01.asl@34 PS10, Line 34: return(HPAC) Return
https://review.coreboot.org/#/c/33102/10/src/ec/apple/acpi/ac_60.asl File src/ec/apple/acpi/ac_60.asl:
https://review.coreboot.org/#/c/33102/10/src/ec/apple/acpi/ac_60.asl@34 PS10, Line 34: return(HPAC) Return
https://review.coreboot.org/#/c/33102/10/src/ec/apple/acpi/lid_01.asl File src/ec/apple/acpi/lid_01.asl:
https://review.coreboot.org/#/c/33102/10/src/ec/apple/acpi/lid_01.asl@34 PS10, Line 34: return(LIDS) Return
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.
Hello Patrick Rudolph, Felix Held, Angel Pons, build bot (Jenkins), Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33102
to look at the new patch set (#11).
Change subject: ec/apple: ACPI code for Apple MacBooks ......................................................................
ec/apple: ACPI code for Apple MacBooks
- Moved ACPI code for Apple MacBooks to a separate directory to avoid it's duplication in mainboards - Added AC and lid implementations for newer generations - Old code is rewritten with new ASL syntax
AC and lid implementation files are named after EC register that's used in them. Older generations (macbook2,1) use 0x01 while newer generations like 2011-2012 Airs use 0x60. Battery registers seem to be the same.
Tested on MacBook Air 5,2.
Change-Id: I3d4585aac8e3ebbfed6ce4d4e39fbc33ac983069 Signed-off-by: Evgeny Zinoviev me@ch1p.io --- A src/ec/apple/acpi/ac_01.asl A src/ec/apple/acpi/ac_60.asl A src/ec/apple/acpi/battery.asl A src/ec/apple/acpi/ec.asl A src/ec/apple/acpi/lid_01.asl A src/ec/apple/acpi/lid_60.asl 6 files changed, 409 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/33102/11
Hello Patrick Rudolph, Felix Held, Angel Pons, build bot (Jenkins), Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33102
to look at the new patch set (#13).
Change subject: ec/apple: Add ACPI code for Apple MacBooks ......................................................................
ec/apple: Add ACPI code for Apple MacBooks
- Move ACPI code for Apple MacBooks to a separate directory to avoid it's duplication in mainboards - Add AC and lid implementations for newer generations - Old code is rewritten with new ASL syntax
AC and lid implementation files are named after EC register that's used in them. Older generations (macbook2,1) use 0x01 while newer generations like 2011-2012 Airs use 0x60. Battery registers seem to be the same.
Tested on MBA 5,2 and MBP 10,1.
Change-Id: I3d4585aac8e3ebbfed6ce4d4e39fbc33ac983069 Signed-off-by: Evgeny Zinoviev me@ch1p.io --- A src/ec/apple/acpi/ac_01.asl A src/ec/apple/acpi/ac_60.asl A src/ec/apple/acpi/battery.asl A src/ec/apple/acpi/ec.asl A src/ec/apple/acpi/lid_01.asl A src/ec/apple/acpi/lid_60.asl 6 files changed, 409 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/33102/13
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 13:
(8 comments)
https://review.coreboot.org/c/coreboot/+/33102/13//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/33102/13//COMMIT_MSG@10 PS13, Line 10: it's Please use possesive "its"
Also, please indent the line with two spaces
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 […]
Still unresolved
https://review.coreboot.org/c/coreboot/+/33102/10/src/ec/apple/acpi/ac_01.as... PS10, Line 34: return(HPAC)
Return
Still unresolved
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)
Return
Still unresolved
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
FixedIO instead of IO could be used for these two
Still unresolved
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 […]
Still unresolved
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)
Return
Still unresolved
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.
Still unresolved
Hello Patrick Rudolph, Felix Held, Angel Pons, build bot (Jenkins), Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33102
to look at the new patch set (#14).
Change subject: ec/apple: ACPI code for Apple MacBooks ......................................................................
ec/apple: ACPI code for Apple MacBooks
- Moved ACPI code for Apple MacBooks to a separate directory to avoid it's duplication in mainboards - Added AC and lid implementations for newer generations - Old code is rewritten with new ASL syntax
AC and lid implementation files are named after EC register that's used in them. Older generations (macbook2,1) use 0x01 while newer generations like 2011-2012 Airs use 0x60. Battery registers seem to be the same.
Tested on MacBook Air 5,2.
Change-Id: I3d4585aac8e3ebbfed6ce4d4e39fbc33ac983069 Signed-off-by: Evgeny Zinoviev me@ch1p.io --- A src/ec/apple/acpi/ac_01.asl A src/ec/apple/acpi/ac_60.asl A src/ec/apple/acpi/battery.asl A src/ec/apple/acpi/ec.asl A src/ec/apple/acpi/lid_01.asl A src/ec/apple/acpi/lid_60.asl 6 files changed, 409 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/33102/14
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33102 )
Change subject: ec/apple: ACPI code for Apple MacBooks ......................................................................
Patch Set 14:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33102/14//COMMIT_MSG Commit Message:
PS14: Looks like the commit message regressed compared to Patchset 13
Evgeny Zinoviev has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33102 )
Change subject: ec/apple: ACPI code for Apple MacBooks ......................................................................
Patch Set 14:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33102/14//COMMIT_MSG Commit Message:
PS14:
Looks like the commit message regressed compared to Patchset 13
Yeah... I have copies on all macbooks and they are not very good synchronized with each other. Going to fix all this on holidays.
Hello Patrick Rudolph, Felix Held, Angel Pons, build bot (Jenkins), Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33102
to look at the new patch set (#15).
Change subject: ec/apple: Add ACPI code for Apple MacBooks ......................................................................
ec/apple: Add ACPI code for Apple MacBooks
- Move ACPI code for Apple MacBooks to a separate directory to avoid its duplication in mainboards - Add AC and lid implementations for newer generations - Old code is rewritten with new ASL syntax
AC and lid implementation files are named after EC register that's used in them. Older generations (macbook2,1) use 0x01 while newer generations like 2011-2012 Airs use 0x60. Battery registers seem to be the same.
Tested on MBA 5,2, MBP 8,1 and MBP 10,1.
Change-Id: I3d4585aac8e3ebbfed6ce4d4e39fbc33ac983069 Signed-off-by: Evgeny Zinoviev me@ch1p.io --- A src/ec/apple/acpi/ac_01.asl A src/ec/apple/acpi/ac_60.asl A src/ec/apple/acpi/battery.asl A src/ec/apple/acpi/ec.asl A src/ec/apple/acpi/lid_01.asl A src/ec/apple/acpi/lid_60.asl 6 files changed, 409 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/33102/15
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
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 16:
(1 comment)
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. […]
Already done, but will upload together with all affected boards.
Hello Patrick Rudolph, Felix Held, Angel Pons, build bot (Jenkins), Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33102
to look at the new patch set (#17).
Change subject: ec/apple: Add ACPI code for Apple MacBooks ......................................................................
ec/apple: Add ACPI code for Apple MacBooks
- Move ACPI code for Apple MacBooks to a separate directory to avoid its duplication in mainboards - Add AC and lid implementations for newer generations - Rewrite old code using the new ASL syntax
Tested on MBA 5,2, MBP 8,1 and MBP 10,1.
Change-Id: I3d4585aac8e3ebbfed6ce4d4e39fbc33ac983069 Signed-off-by: Evgeny Zinoviev me@ch1p.io --- A src/ec/apple/acpi/ac.asl A src/ec/apple/acpi/battery.asl A src/ec/apple/acpi/ec.asl A src/ec/apple/acpi/lid.asl 4 files changed, 320 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/33102/17
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
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:
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 also I don't have an old macbook 2,1 atm...
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 17: Code-Review+1
(11 comments)
https://review.coreboot.org/c/coreboot/+/33102/13//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/33102/13//COMMIT_MSG@10 PS13, Line 10: it's
Please use possesive "its" […]
Done
https://review.coreboot.org/c/coreboot/+/33102/16//COMMIT_MSG Commit Message:
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: […]
Done
https://review.coreboot.org/c/coreboot/+/33102/17/src/ec/apple/acpi/ac.asl File src/ec/apple/acpi/ac.asl:
https://review.coreboot.org/c/coreboot/+/33102/17/src/ec/apple/acpi/ac.asl@1... PS17, Line 16: Just to be safe:
#ifndef HPAC_OFFSET #error (i don't recall the syntax) #endif
https://review.coreboot.org/c/coreboot/+/33102/17/src/ec/apple/acpi/ac.asl@2... PS17, Line 22: , 1, If this is LIDS, I'd say using different macro names isn't worth the hassle.
Also, I don't know why the original code is split over so many files, it makes keeping some things in sync harder
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
Already done, but will upload together with all affected boards.
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,
Yes
Ack
https://review.coreboot.org/c/coreboot/+/33102/5/src/ec/apple/acpi/battery.a... File src/ec/apple/acpi/battery.asl:
https://review.coreboot.org/c/coreboot/+/33102/5/src/ec/apple/acpi/battery.a... PS5, Line 147: Local0++ & 0xFFFF
I have doubts about this, is this correct? It was `And(Increment(Local0), 0xFFFF, Local0)`
It's basically doing the two's complement of Local0. This means that the addition must be performed before the AND
https://review.coreboot.org/c/coreboot/+/33102/17/src/ec/apple/acpi/battery.... File src/ec/apple/acpi/battery.asl:
https://review.coreboot.org/c/coreboot/+/33102/17/src/ec/apple/acpi/battery.... PS17, Line 80: 0xFFFFFFFF, // 1: Design cap These two aren't indented with tabs
https://review.coreboot.org/c/coreboot/+/33102/17/src/ec/apple/acpi/battery.... PS17, Line 132: What's with these spaces?
https://review.coreboot.org/c/coreboot/+/33102/17/src/ec/apple/acpi/ec.asl File src/ec/apple/acpi/ec.asl:
https://review.coreboot.org/c/coreboot/+/33102/17/src/ec/apple/acpi/ec.asl@4... PS17, Line 43: 0x62 Why was this changed to 0x62 ?
https://review.coreboot.org/c/coreboot/+/33102/17/src/ec/apple/acpi/lid.asl File src/ec/apple/acpi/lid.asl:
https://review.coreboot.org/c/coreboot/+/33102/17/src/ec/apple/acpi/lid.asl@... PS17, Line 16: Just to be safe:
#ifndef LIDS_OFFSET #error (i don't recall the syntax) #endif #ifndef WKLD_OFFSET #error (i don't recall the syntax) #endif
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:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33102/17/src/ec/apple/acpi/ec.asl File src/ec/apple/acpi/ec.asl:
https://review.coreboot.org/c/coreboot/+/33102/17/src/ec/apple/acpi/ec.asl@4... PS17, Line 43: 0x62
Why was this changed to 0x62 ?
Oh, my mistake. Thanks.
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 17:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33102/17/src/ec/apple/acpi/ec.asl File src/ec/apple/acpi/ec.asl:
https://review.coreboot.org/c/coreboot/+/33102/17/src/ec/apple/acpi/ec.asl@4... PS17, Line 43: 0x62
Oh, my mistake. Thanks.
Could this have been why FixedIO caused so many errors?
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:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33102/17/src/ec/apple/acpi/ec.asl File src/ec/apple/acpi/ec.asl:
https://review.coreboot.org/c/coreboot/+/33102/17/src/ec/apple/acpi/ec.asl@4... PS17, Line 43: 0x62
Could this have been why FixedIO caused so many errors?
I'm quite sure there were correct values with FixedIO. But I can test again no problem.
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 17:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33102/17/src/ec/apple/acpi/ac.asl File src/ec/apple/acpi/ac.asl:
https://review.coreboot.org/c/coreboot/+/33102/17/src/ec/apple/acpi/ac.asl@1... PS17, Line 16:
Just to be safe: […]
Not 100% sure how iasl's message would look like, but it should result in a compile error anyway, right? if we'd literally feed `Offset(HPAC_OFFSET)` to the compiler, that's not even ASL syntax.
Felix Held 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:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33102/17/src/ec/apple/acpi/ec.asl File src/ec/apple/acpi/ec.asl:
https://review.coreboot.org/c/coreboot/+/33102/17/src/ec/apple/acpi/ec.asl@4... PS17, Line 43: 0x62
I'm quite sure there were correct values with FixedIO. But I can test again no problem.
yep, this should be 0x66. the ec has its io ports at 0x62 and 0x66; one is index and the other one data
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 17:
(2 comments)
https://review.coreboot.org/c/coreboot/+/33102/17/src/ec/apple/acpi/ac.asl File src/ec/apple/acpi/ac.asl:
https://review.coreboot.org/c/coreboot/+/33102/17/src/ec/apple/acpi/ac.asl@1... PS17, Line 16:
Not 100% sure how iasl's message would look like, but it should […]
Yup, it would be enough I guess.
https://review.coreboot.org/c/coreboot/+/33102/17/src/ec/apple/acpi/lid.asl File src/ec/apple/acpi/lid.asl:
https://review.coreboot.org/c/coreboot/+/33102/17/src/ec/apple/acpi/lid.asl@... PS17, Line 16:
Just to be safe: […]
Not necessary
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 17:
(4 comments)
https://review.coreboot.org/c/coreboot/+/33102/17/src/ec/apple/acpi/ac.asl File src/ec/apple/acpi/ac.asl:
PS17: We use SPDX license headers now
https://review.coreboot.org/c/coreboot/+/33102/17/src/ec/apple/acpi/ac.asl@2... PS17, Line 22: , 1,
If this is LIDS, I'd say using different macro names isn't worth the hassle. […]
Bleh, not a big deal
https://review.coreboot.org/c/coreboot/+/33102/17/src/ec/apple/acpi/battery.... File src/ec/apple/acpi/battery.asl:
https://review.coreboot.org/c/coreboot/+/33102/17/src/ec/apple/acpi/battery.... PS17, Line 80: 0xFFFFFFFF, // 1: Design cap
These two aren't indented with tabs
Poke
https://review.coreboot.org/c/coreboot/+/33102/17/src/ec/apple/acpi/ec.asl File src/ec/apple/acpi/ec.asl:
https://review.coreboot.org/c/coreboot/+/33102/17/src/ec/apple/acpi/ec.asl@4... PS17, Line 43: 0x62
yep, this should be 0x66. […]
Still not changed?
Attention is currently required from: Evgeny Zinoviev. Hello build bot (Jenkins), Patrick Georgi, Angel Pons, Patrick Rudolph, Felix Held,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33102
to look at the new patch set (#19).
Change subject: ec/apple: Add ACPI code for Apple MacBooks ......................................................................
ec/apple: Add ACPI code for Apple MacBooks
- Move ACPI code for Apple MacBooks to a separate directory to avoid its duplication in mainboards - Add AC and lid implementations for newer generations - Rewrite old code using the new ASL syntax
Tested on MBA 5,2, MBP 8,1 and MBP 10,1.
Change-Id: I3d4585aac8e3ebbfed6ce4d4e39fbc33ac983069 Signed-off-by: Evgeny Zinoviev me@ch1p.io --- A src/ec/apple/acpi/ac.asl A src/ec/apple/acpi/battery.asl A src/ec/apple/acpi/ec.asl A src/ec/apple/acpi/lid.asl 4 files changed, 320 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/33102/19
Attention is currently required from: Evgeny Zinoviev. Hello build bot (Jenkins), Patrick Georgi, Angel Pons, Patrick Rudolph, Felix Held,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33102
to look at the new patch set (#20).
Change subject: ec/apple: Add ACPI code for Apple MacBooks ......................................................................
ec/apple: Add ACPI code for Apple MacBooks
- Move ACPI code for Apple MacBooks to a separate directory to avoid its duplication in mainboards - Add AC and lid implementations for newer generations - Rewrite old code using the new ASL syntax
Tested on MBA 5,2, MBP 8,1 and MBP 10,1.
Change-Id: I3d4585aac8e3ebbfed6ce4d4e39fbc33ac983069 Signed-off-by: Evgeny Zinoviev me@ch1p.io --- A src/ec/apple/acpi/ac.asl A src/ec/apple/acpi/battery.asl A src/ec/apple/acpi/ec.asl A src/ec/apple/acpi/lid.asl 4 files changed, 266 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/33102/20
Attention is currently required from: Evgeny Zinoviev. Hello build bot (Jenkins), Patrick Georgi, Angel Pons, Patrick Rudolph, Felix Held,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33102
to look at the new patch set (#21).
Change subject: ec/apple: Add ACPI code for Apple MacBooks ......................................................................
ec/apple: Add ACPI code for Apple MacBooks
- Move ACPI code for Apple MacBooks to a separate directory to avoid its duplication in mainboards - Add AC and lid implementations for newer generations - Rewrite old code using the new ASL syntax
Tested on MBA 5,2, MBP 8,1 and MBP 10,1.
Change-Id: I3d4585aac8e3ebbfed6ce4d4e39fbc33ac983069 Signed-off-by: Evgeny Zinoviev me@ch1p.io --- A src/ec/apple/acpi/ac.asl A src/ec/apple/acpi/battery.asl A src/ec/apple/acpi/ec.asl A src/ec/apple/acpi/lid.asl 4 files changed, 266 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/33102/21
Attention is currently required from: Evgeny Zinoviev, Felix Held. 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 21:
(4 comments)
File src/ec/apple/acpi/ac.asl:
PS17:
We use SPDX license headers now
Done
File src/ec/apple/acpi/battery.asl:
https://review.coreboot.org/c/coreboot/+/33102/comment/9405825a_8969be11 PS17, Line 80: 0xFFFFFFFF, // 1: Design cap
Poke
Done
https://review.coreboot.org/c/coreboot/+/33102/comment/450efb27_00449267 PS17, Line 132:
What's with these spaces?
Done
File src/ec/apple/acpi/ec.asl:
https://review.coreboot.org/c/coreboot/+/33102/comment/872b328f_5215c00e PS17, Line 43: 0x62
Still not changed?
Done
Attention is currently required from: Evgeny Zinoviev, Felix Held. 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 21:
(1 comment)
File src/ec/apple/acpi/battery.asl:
https://review.coreboot.org/c/coreboot/+/33102/comment/9111a995_f19d730b PS5, Line 147: Local0++ & 0xFFFF
It's basically doing the two's complement of Local0. […]
You can rewrite this as:
Local0 = ~Local0 Local0++ Local0 &= 0xFFFF
Attention is currently required from: Angel Pons, Felix Held. 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 21:
(1 comment)
File src/ec/apple/acpi/ec.asl:
https://review.coreboot.org/c/coreboot/+/33102/comment/23d41dc1_f2cd4600 PS17, Line 43: 0x62
Still not changed?
Hey, I was going to post all those dones!
Attention is currently required from: Angel Pons, Felix Held. Hello build bot (Jenkins), Patrick Georgi, Angel Pons, Patrick Rudolph, Felix Held,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33102
to look at the new patch set (#22).
Change subject: ec/apple: Add ACPI code for Apple MacBooks ......................................................................
ec/apple: Add ACPI code for Apple MacBooks
- Move ACPI code for Apple MacBooks to a separate directory to avoid its duplication in mainboards - Add AC and lid implementations for newer generations - Rewrite old code using the new ASL syntax
Tested on MBA 5,2, MBP 8,1 and MBP 10,1.
Change-Id: I3d4585aac8e3ebbfed6ce4d4e39fbc33ac983069 Signed-off-by: Evgeny Zinoviev me@ch1p.io --- A src/ec/apple/acpi/ac.asl A src/ec/apple/acpi/battery.asl A src/ec/apple/acpi/ec.asl A src/ec/apple/acpi/lid.asl 4 files changed, 267 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/33102/22
Attention is currently required from: Angel Pons, Felix Held. 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 22:
(2 comments)
File src/ec/apple/acpi/battery.asl:
https://review.coreboot.org/c/coreboot/+/33102/comment/236c7644_15558a58 PS5, Line 147: Local0++ & 0xFFFF
You can rewrite this as: […]
Done 😊
File src/ec/apple/acpi/ec.asl:
https://review.coreboot.org/c/coreboot/+/33102/comment/7f0c0803_3815b2af PS10, Line 42: IO
It stops working with FixedIO. […]
Done
Attention is currently required from: Evgeny Zinoviev, Felix Held. 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 22: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/33102 )
Change subject: ec/apple: Add ACPI code for Apple MacBooks ......................................................................
ec/apple: Add ACPI code for Apple MacBooks
- Move ACPI code for Apple MacBooks to a separate directory to avoid its duplication in mainboards - Add AC and lid implementations for newer generations - Rewrite old code using the new ASL syntax
Tested on MBA 5,2, MBP 8,1 and MBP 10,1.
Change-Id: I3d4585aac8e3ebbfed6ce4d4e39fbc33ac983069 Signed-off-by: Evgeny Zinoviev me@ch1p.io Reviewed-on: https://review.coreboot.org/c/coreboot/+/33102 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Angel Pons th3fanbus@gmail.com --- A src/ec/apple/acpi/ac.asl A src/ec/apple/acpi/battery.asl A src/ec/apple/acpi/ec.asl A src/ec/apple/acpi/lid.asl 4 files changed, 267 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved
diff --git a/src/ec/apple/acpi/ac.asl b/src/ec/apple/acpi/ac.asl new file mode 100644 index 0000000..0103229 --- /dev/null +++ b/src/ec/apple/acpi/ac.asl @@ -0,0 +1,28 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +Scope(_SB.PCI0.LPCB.EC) +{ + Field(ERAM, ByteAcc, NoLock, Preserve) + { + Offset(HPAC_OFFSET), + , 1, + HPAC, 1, /* AC status */ + } + + Device(AC) + { + Name(_HID, "ACPI0003") + Name(_UID, 0x00) + Name(_PCL, Package() { _SB } ) + + Method(_PSR, 0, NotSerialized) + { + Return(HPAC) + } + + Method(_STA, 0, NotSerialized) + { + Return(0x0f) + } + } +} diff --git a/src/ec/apple/acpi/battery.asl b/src/ec/apple/acpi/battery.asl new file mode 100644 index 0000000..291451f --- /dev/null +++ b/src/ec/apple/acpi/battery.asl @@ -0,0 +1,153 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +Field(ERAM, ByteAcc, NoLock, Preserve) +{ + Offset(0x20), + SPTR, 8, + SSTS, 8, + SADR, 8, + SCMD, 8, + SBFR, 256, +} + +Field(ERAM, ByteAcc, Lock, Preserve) +{ + Offset(0x24), + SBDW, 16, +} + +Method(SBPC, 0, NotSerialized) +{ + Local0 = 1000 + While (Local0) + { + If (SPTR == 0x00) + { + Return() + } + + Sleep(1) + Local0-- + } +} + +Method(SBRW, 2, NotSerialized) +{ + Acquire(ECLK, 0xFFFF) + SADR = ShiftLeft(Arg0, 0x01) + SCMD = Arg1 + SPTR = 0x09 + SBPC() + Local0 = SBDW + Release(ECLK) + Return(Local0) +} + +Method(SBRB, 2, NotSerialized) +{ + Acquire(ECLK, 0xFFFF) + SADR = ShiftLeft(Arg0, 0x01) + SCMD = Arg1 + SPTR = 0x0B + SBPC() + Local0 = SBFR + Release(ECLK) + Return(Local0) +} + +Device(BAT0) +{ + Name(_HID, EisaId("PNP0C0A")) + Name(_UID, 0x00) + Name(_PCL, Package() { _SB }) + + Name(BATS, Package() + { + 0x00, // 0: PowerUnit: Report in mWh + 0xFFFFFFFF, // 1: Design cap + 0xFFFFFFFF, // 2: Last full charge cap + 0x01, // 3: Battery Technology + 10800, // 4: Design Voltage(mV) + 0x00, // 5: Warning design capacity + 200, // 6: Low design capacity + 10, // 7: granularity1 + 10, // 8: granularity2 + "", // 9: Model number + "", // A: Serial number + "", // B: Battery Type + "" // C: OEM information + }) + + Name(BATI, Package() + { + 0, // Battery State + // Bit 0 - discharge + // Bit 1 - charge + // Bit 2 - critical state + 0, // Battery present Rate + 0, // Battery remaining capacity + 0 // Battery present voltage + }) + + Method(_BIF, 0, NotSerialized) + { + Index(BATS, 0x01) = ^^SBRW(0x0B, 0x18) * 10 + Index(BATS, 0x02) = ^^SBRW(0x0B, 0x10) * 10 + + Index(BATS, 0x04) = ^^SBRW(0x0B, 0x19) + Index(BATS, 0x09) = ^^SBRB(0x0B, 0x21) + Index(BATS, 0x0B) = ^^SBRB(0x0B, 0x22) + Index(BATS, 0x0C) = ^^SBRB(0x0B, 0x20) + + Return(BATS) + } + + Method(_STA, 0, NotSerialized) + { + If (^^SBRW(0x0A, 0x01) & 0x01) { + Return(0x1f) + } else { + Return(0x0f) + } + } + + Method(_BST, 0, NotSerialized) + { + /* Check for battery presence. */ + If (!(^^SBRW(0x0A, 0x01) & 0x01)) { + Return(Package(4) { + 0, + 0xFFFFFFFF, + 0xFFFFFFFF, + 0xFFFFFFFF + }) + } + + Local1 = ^^SBRW(0x0B, 0x09) + Index(BATI, 0x03) = Local1 + Local0 = ^^SBRW(0x0B, 0x0A) + + /* Sign-extend Local0. */ + If (Local0 & 0x8000) + { + Local0 = ~Local0 + Local0++ + Local0 &= 0xFFFF + } + + Local0 *= Local1 + Index(BATI, 1) = Local0 / 1000 + Index(BATI, 2) = ^^SBRW(0x0B, 0x0F) * 10 + If (HPAC) { + If (!(^^SBRW(0x0B, 0x16) & 0x40)) { + Index(BATI, 0) = 2 + } Else { + Index(BATI, 0) = 0 + } + } Else { + Index(BATI, 0) = 1 + } + + Return(BATI) + } +} diff --git a/src/ec/apple/acpi/ec.asl b/src/ec/apple/acpi/ec.asl new file mode 100644 index 0000000..3adb7f6 --- /dev/null +++ b/src/ec/apple/acpi/ec.asl @@ -0,0 +1,45 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +Device(EC) +{ + Name(_HID, EISAID("PNP0C09")) + Name(_UID, 0) + + Name(_GPE, 0x17) + Mutex(ECLK, 0) + + OperationRegion(ERAM, EmbeddedControl, 0x00, 0x100) + + /* LID status change. */ + Method(_Q20, 0, NotSerialized) + { + Notify(LID, 0x80) + } + + /* AC status change. */ + Method(_Q21, 0, NotSerialized) + { + Notify(AC, 0x80) + } + + Method(_CRS, 0) + { + Name(ECMD, ResourceTemplate() + { + IO(Decode16, 0x62, 0x62, 1, 1) + IO(Decode16, 0x66, 0x66, 1, 1) + }) + Return(ECMD) + } + + Method(_PRW, 0, NotSerialized) + { + Return(Package () { 0x23, 0x04 }) + } + + Method(_INI, 0, NotSerialized) + { + } + +#include "battery.asl" +} diff --git a/src/ec/apple/acpi/lid.asl b/src/ec/apple/acpi/lid.asl new file mode 100644 index 0000000..ec91d8f --- /dev/null +++ b/src/ec/apple/acpi/lid.asl @@ -0,0 +1,41 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +Scope(_SB.PCI0.LPCB.EC) +{ + Field(ERAM, ByteAcc, NoLock, Preserve) + { + Offset(LIDS_OFFSET), + LIDS, 1, /* Lid status */ + + Offset(WKLD_OFFSET), + WKLD, 1, /* Lid wake */ + } + + Device(LID) + { + Name(_HID, "PNP0C0D") + + Method(_LID, 0, NotSerialized) + { + Return(LIDS) + } + + Method(_PRW, 0, NotSerialized) + { +#if LIDS_OFFSET == 0x01 + Return(Package() { 0x1d, 0x03 }) +#else + Return(Package() { 0x23, 0x04 }) +#endif + } + + Method(_PSW, 1, NotSerialized) + { + if (Arg0) { + WKLD = 1 + } else { + WKLD = 0 + } + } + } +}