HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45554 )
Change subject: mb/getac/p470/acpi: Convert 'battery.asl' to ASL 2.0 syntax ......................................................................
mb/getac/p470/acpi: Convert 'battery.asl' to ASL 2.0 syntax
Change-Id: Ifcc8bf4022838056bf1fff853eb2027af684064e Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/mainboard/getac/p470/acpi/battery.asl 1 file changed, 81 insertions(+), 77 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/45554/1
diff --git a/src/mainboard/getac/p470/acpi/battery.asl b/src/mainboard/getac/p470/acpi/battery.asl index 7924edf..33b19ae 100644 --- a/src/mainboard/getac/p470/acpi/battery.asl +++ b/src/mainboard/getac/p470/acpi/battery.asl @@ -1,6 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */
-Scope(_SB) { +Scope(_SB) {
Name(NIMH, "NiMH") Name(LION, "Lion") @@ -10,7 +10,7 @@ Name(_HID, EisaId("PNP0C0A")) Name(_UID, 1)
- Name(_PCL, Package(){ _SB }) + Name(_PCL, Package(){ _SB })
Name(PBST, Package() { 0x00, 0x04b0, 0x0bb8, 0x03e8 })
@@ -18,7 +18,7 @@ Method(_STA, 0) { Sleep(120) - If(_SB.PCI0.LPCB.EC0.BAT) { + If (^^PCI0.LPCB.EC0.BAT) { Return(0x1f) } Else { Return(0x0f) @@ -45,28 +45,28 @@ })
// Is battery there? - Store(_STA(), Local0) - And(Local0, 0x10, Local0) - If(LNot(Local0)) { + Local0 = _STA () + Local0 &= 0x10 + If (!Local0) { Return (PBIF) }
- Store(_SB.PCI0.LPCB.EC0.BDC0, Index(PBIF, 1)) - Store(_SB.PCI0.LPCB.EC0.BDV0, Index(PBIF, 4)) + PBIF [1] = ^^PCI0.LPCB.EC0.BDC0 + PBIF [4] = ^^PCI0.LPCB.EC0.BDV0
- Store(_SB.PCI0.LPCB.EC0.BFC0, Local0) - Store(Local0, Index(PBIF, 2)) + Local0 = ^^PCI0.LPCB.EC0.BFC0 + PBIF [2] = Local0
- Divide(Local0, 10, , Local2) - Store(Local2, Index(PBIF, 5)) + Local2 = Local0 / 10 + PBIF [5] = Local2
- Divide(Local0, 20, , Local2) - Store(Local2, Index(PBIF, 6)) + Local2 = Local0 / 20 + PBIF [6] = Local2
- If(_SB.PCI0.LPCB.EC0.BTYP) { - Store(NIMH, Index(PBIF, 11)) + If (^^PCI0.LPCB.EC0.BTYP) { + PBIF [11] = NIMH } Else { - Store(LION, Index(PBIF, 11)) + PBIF [11] = LION }
Return(PBIF) @@ -75,33 +75,35 @@ /* Battery Status */ Method(_BST, 0) { - If(_SB.PCI0.LPCB.EC0.BAT) { - Store(_SB.PCI0.LPCB.EC0.BPV0, Index(PBST, 3)) + If (^^PCI0.LPCB.EC0.BAT) { + PBST [3] = ^^PCI0.LPCB.EC0.BPV0
- Multiply(_SB.PCI0.LPCB.EC0.BRC0, 100, Local3) - Divide(Local3, _SB.PCI0.LPCB.EC0.BFC0, Local3, Local0) - Multiply(_SB.PCI0.LPCB.EC0.BFC0, Local0, Local3) - Divide(Local3, 0x64, Local3, Local0) - Increment(Local0) - Store(Local0, Index(PBST, 2)) + Local3 = ^^PCI0.LPCB.EC0.BRC0 * 0x64 + Local0 = Local3 / ^^PCI0.LPCB.EC0.BFC0 + Local3 -= (Local0 * ^^PCI0.LPCB.EC0.BFC0) + Local3 = ^^PCI0.LPCB.EC0.BFC0 * Local0 + Local0 = Local3 / 0x64 + Local3 -= (Local0 * 0x64) + Local0++ + PBST [0x02] = Local0
- Store (_SB.PCI0.LPCB.EC0.BRC0, Local3) - Store (_SB.PCI0.LPCB.EC0.BPR0, Local0) - And (Not (Local0), 0xFFFF, Local0) - Store (Local0, Index(PBST,1)) + Local3 = ^^PCI0.LPCB.EC0.BRC0 + Local0 = ^^PCI0.LPCB.EC0.BPR0 + Local0 = (~Local0 & 0xFFFF) + PBST [1] = Local0
// AC Power connected? - If(_SB.PCI0.LPCB.EC0.ADP) { - If(_SB.PCI0.LPCB.EC0.CHRG) { - Store(2, Index(PBST, 0)) + If (^^PCI0.LPCB.EC0.ADP) { + If (^^PCI0.LPCB.EC0.CHRG) { + PBST [0] = 2 } Else { - Store(0, Index(PBST, 0)) + PBST [0] = 0 } } Else { - If(LLess(Local3, 25)) { - Store(5, Index(PBST, 0)) + If (Local3 < 25) { + PBST [0] = 5 } Else { - Store(1, Index(PBST, 0)) + PBST [0] = 0 } } } @@ -115,7 +117,7 @@ Name(_HID, EisaId("PNP0C0A")) Name(_UID, 1)
- Name(_PCL, Package(){ _SB }) + Name(_PCL, Package(){ _SB })
Name(PBST, Package() { 0x00, 0x04b0, 0x0bb8, 0x03e8 })
@@ -123,7 +125,7 @@ Method(_STA, 0) { Sleep(120) - If(_SB.PCI0.LPCB.EC0.BAT2) { + If (^^PCI0.LPCB.EC0.BAT2) { Return(0x1f) } Else { Return(0x0f) @@ -150,28 +152,28 @@ })
// Is battery there? - Store(_STA(), Local0) - And(Local0, 0x10, Local0) - If(LNot(Local0)) { + Local0 = _STA () + Local0 &= 0x10 + If (!Local0) { Return (PBIF) }
- Store(_SB.PCI0.LPCB.EC0.BDC2, Index(PBIF, 1)) - Store(_SB.PCI0.LPCB.EC0.BDV2, Index(PBIF, 4)) + PBIF [1] = ^^PCI0.LPCB.EC0.BDC2 + PBIF [0x04] = ^^PCI0.LPCB.EC0.BDV2
- Store(_SB.PCI0.LPCB.EC0.BFC2, Local0) - Store(Local0, Index(PBIF, 2)) + Local0 = ^^PCI0.LPCB.EC0.BFC2 + PBIF [2] = Local0
- Divide(Local0, 10, , Local2) - Store(Local2, Index(PBIF, 5)) + Local2 = Local0 / 10 + PBIF [5] = Local2
- Divide(Local0, 20, , Local2) - Store(Local2, Index(PBIF, 6)) + Local2 = Local0 / 20 + PBIF [6] = Local2
- If(_SB.PCI0.LPCB.EC0.BTY2) { - Store(NIMH, Index(PBIF, 11)) + If (^^PCI0.LPCB.EC0.BTY2) { + PBIF [11] = NIMH } Else { - Store(LION, Index(PBIF, 11)) + PBIF [11] = LION }
Return(PBIF) @@ -180,33 +182,35 @@ /* Battery Status */ Method(_BST, 0) { - If(_SB.PCI0.LPCB.EC0.BAT2) { - Store(_SB.PCI0.LPCB.EC0.BPV2, Index(PBST, 3)) + If (^^PCI0.LPCB.EC0.BAT2) { + PBST [0x03] = ^^PCI0.LPCB.EC0.BPV2
- Multiply(_SB.PCI0.LPCB.EC0.BRC2, 100, Local3) - Divide(Local3, _SB.PCI0.LPCB.EC0.BFC2, Local3, Local0) - Multiply(_SB.PCI0.LPCB.EC0.BFC2, Local0, Local3) - Divide(Local3, 0x64, Local3, Local0) - Increment(Local0) - Store(Local0, Index(PBST, 2)) + Local3 = ^^PCI0.LPCB.EC0.BRC2 * 100 + Local0 = Local3 / ^^PCI0.LPCB.EC0.BRC2 + Local3 -= (Local0 * ^^PCI0.LPCB.EC0.BRC2) + Local3 = ^^PCI0.LPCB.EC0.BFC2 * Local0 + Local0 = Local3 / 0x64 + Local3 -= (Local0 * 0x64) + Local0++ + PBST [2] = Local0
- Store (_SB.PCI0.LPCB.EC0.BRC2, Local3) - Store (_SB.PCI0.LPCB.EC0.BPR2, Local0) - And (Not (Local0), 0xFFFF, Local0) - Store (Local0, Index(PBST,1)) + Local3 = ^^PCI0.LPCB.EC0.BRC2 + Local0 = ^^PCI0.LPCB.EC0.BPR2 + Local0 = (~Local0 & 0xFFFF) + PBST [One] = Local0
// AC Power connected? - If(_SB.PCI0.LPCB.EC0.ADP) { - If(_SB.PCI0.LPCB.EC0.CRG2) { - Store(2, Index(PBST, 0)) + If (^^PCI0.LPCB.EC0.ADP) { + If (^^PCI0.LPCB.EC0.CRG2) { + PBST [0] = 2 } Else { - Store(0, Index(PBST, 0)) + PBST [0] = 0 } } Else { - If(LLess(Local3, 25)) { - Store(5, Index(PBST, 0)) + If (Local3 < 25) { + PBST [0] = 5 } Else { - Store(1, Index(PBST, 0)) + PBST [0] = 1 } } } @@ -223,19 +227,19 @@ Name (ACST, 0x00) Method (_PSR, 0) { - If(ACFG) { - Store(ACST, Local0) + If (ACFG) { + Local0 = ACST } Else { - Store(_SB.PCI0.LPCB.EC0.ADP, Local0) - Store(Local0, ACST) - Store(1, ACFG) + Local0 = ^^PCI0.LPCB.EC0.ADP + ACST = Local0 + ACFG = 1 } Sleep(120) Return (Local0) }
Name(_PCL, Package(){ - _SB, + _SB, BAT0, BAT1 })
Hello build bot (Jenkins), Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45554
to look at the new patch set (#2).
Change subject: mb/getac/p470/acpi: Convert 'battery.asl' to ASL 2.0 syntax ......................................................................
mb/getac/p470/acpi: Convert 'battery.asl' to ASL 2.0 syntax
Changes the binary using 'BUILD_TIMELESS=1'. Probably because of optimization.
Change-Id: Ifcc8bf4022838056bf1fff853eb2027af684064e Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/mainboard/getac/p470/acpi/battery.asl 1 file changed, 81 insertions(+), 77 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/45554/2
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45554 )
Change subject: mb/getac/p470/acpi: Convert 'battery.asl' to ASL 2.0 syntax ......................................................................
Patch Set 2:
(3 comments)
pretty sure all of those ^^PCI0. are incorrect,
https://review.coreboot.org/c/coreboot/+/45554/2/src/mainboard/getac/p470/ac... File src/mainboard/getac/p470/acpi/battery.asl:
https://review.coreboot.org/c/coreboot/+/45554/2/src/mainboard/getac/p470/ac... PS2, Line 3: Scope(_SB) { _SB is technically more correct. the \ indicates root of the hierarchy
https://review.coreboot.org/c/coreboot/+/45554/2/src/mainboard/getac/p470/ac... PS2, Line 21: If (^^PCI0.LPCB.EC0.BAT) { I don't think that's right, the current scope here is `_SB.BAT0`, so going up 2 is `\PCI0.LPCB.EC0.BAT`, which isn't correct; I think the correct relative path here is `^PCI0.LPCB.EC0.BAT`
https://review.coreboot.org/c/coreboot/+/45554/2/src/mainboard/getac/p470/ac... PS2, Line 128: If (^^PCI0.LPCB.EC0.BAT2) { same problem as above
Hello build bot (Jenkins), Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45554
to look at the new patch set (#3).
Change subject: mb/getac/p470/acpi: Convert 'battery.asl' to ASL 2.0 syntax ......................................................................
mb/getac/p470/acpi: Convert 'battery.asl' to ASL 2.0 syntax
Changes the binary using 'BUILD_TIMELESS=1'. Probably because of optimization. Generated "dsdt.dsl" are same.
Change-Id: Ifcc8bf4022838056bf1fff853eb2027af684064e Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/mainboard/getac/p470/acpi/battery.asl 1 file changed, 79 insertions(+), 75 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/45554/3
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45554 )
Change subject: mb/getac/p470/acpi: Convert 'battery.asl' to ASL 2.0 syntax ......................................................................
Patch Set 2:
(4 comments)
Thx
https://review.coreboot.org/c/coreboot/+/45554/2/src/mainboard/getac/p470/ac... File src/mainboard/getac/p470/acpi/battery.asl:
https://review.coreboot.org/c/coreboot/+/45554/2/src/mainboard/getac/p470/ac... PS2, Line 3: Scope(_SB) {
_SB is technically more correct. […]
Ack
(the compile will change it to "_SB" in generated "dsdt.dsl" file)
https://review.coreboot.org/c/coreboot/+/45554/2/src/mainboard/getac/p470/ac... PS2, Line 21: If (^^PCI0.LPCB.EC0.BAT) {
I don't think that's right, the current scope here is `_SB.BAT0`, so going up 2 is `\PCI0.LPCB.EC0. […]
I'll replace it with "_SB."
(compiler in generated dsdt.dsl file, will change it to "^^PCI0.LPCB.EC0.BAT")
https://review.coreboot.org/c/coreboot/+/45554/2/src/mainboard/getac/p470/ac... PS2, Line 106: 0 Oops!
https://review.coreboot.org/c/coreboot/+/45554/2/src/mainboard/getac/p470/ac... PS2, Line 128: If (^^PCI0.LPCB.EC0.BAT2) {
same problem as above
Ack
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45554 )
Change subject: mb/getac/p470/acpi: Convert 'battery.asl' to ASL 2.0 syntax ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45554/3/src/mainboard/getac/p470/ac... File src/mainboard/getac/p470/acpi/battery.asl:
https://review.coreboot.org/c/coreboot/+/45554/3/src/mainboard/getac/p470/ac... PS3, Line 13: _SB I'll change also this (compiler will change it to "_SB" in dsdt.dsl file)
Hello build bot (Jenkins), Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45554
to look at the new patch set (#4).
Change subject: mb/getac/p470/acpi: Convert 'battery.asl' to ASL 2.0 syntax ......................................................................
mb/getac/p470/acpi: Convert 'battery.asl' to ASL 2.0 syntax
Changes the binary using 'BUILD_TIMELESS=1'. Probably because of optimization. Generated "dsdt.dsl" are same.
Change-Id: Ifcc8bf4022838056bf1fff853eb2027af684064e Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/mainboard/getac/p470/acpi/battery.asl 1 file changed, 78 insertions(+), 74 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/45554/4
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45554 )
Change subject: mb/getac/p470/acpi: Convert 'battery.asl' to ASL 2.0 syntax ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45554/4/src/mainboard/getac/p470/ac... File src/mainboard/getac/p470/acpi/battery.asl:
https://review.coreboot.org/c/coreboot/+/45554/4/src/mainboard/getac/p470/ac... PS4, Line 187: Local3 remainder is not needed as we write on it "Multiply(_SB.PCI0.LPCB.EC0.BFC2, Local0, Local3)"
Hello build bot (Jenkins), Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45554
to look at the new patch set (#5).
Change subject: mb/getac/p470/acpi: Convert 'battery.asl' to ASL 2.0 syntax ......................................................................
mb/getac/p470/acpi: Convert 'battery.asl' to ASL 2.0 syntax
Changes the binary using 'BUILD_TIMELESS=1'. Probably because of optimization. Generated "dsdt.dsl" are same.
Change-Id: Ifcc8bf4022838056bf1fff853eb2027af684064e Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/mainboard/getac/p470/acpi/battery.asl 1 file changed, 74 insertions(+), 74 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/45554/5
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45554 )
Change subject: mb/getac/p470/acpi: Convert 'battery.asl' to ASL 2.0 syntax ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45554/4/src/mainboard/getac/p470/ac... File src/mainboard/getac/p470/acpi/battery.asl:
https://review.coreboot.org/c/coreboot/+/45554/4/src/mainboard/getac/p470/ac... PS4, Line 187: Local3
remainder is not needed as we write on it "Multiply(_SB.PCI0.LPCB.EC0. […]
Done
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45554 )
Change subject: mb/getac/p470/acpi: Convert 'battery.asl' to ASL 2.0 syntax ......................................................................
Patch Set 5:
(5 comments)
https://review.coreboot.org/c/coreboot/+/45554/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45554/5//COMMIT_MSG@9 PS5, Line 9: Changes the binary using 'BUILD_TIMELESS=1'. Probably because of optimization. : Generated "dsdt.dsl" are same. I would say:
IASL optimizes the code differently, which changes the binary. However, the generated `build/dsdt.dsl` remains identical.
https://review.coreboot.org/c/coreboot/+/45554/5/src/mainboard/getac/p470/ac... File src/mainboard/getac/p470/acpi/battery.asl:
https://review.coreboot.org/c/coreboot/+/45554/5/src/mainboard/getac/p470/ac... PS5, Line 81: 0x64 hmmmmm, 100?
https://review.coreboot.org/c/coreboot/+/45554/5/src/mainboard/getac/p470/ac... PS5, Line 84: 0x64 hmmmmm
https://review.coreboot.org/c/coreboot/+/45554/5/src/mainboard/getac/p470/ac... PS5, Line 189: 0x64 hmmmmm
https://review.coreboot.org/c/coreboot/+/45554/5/src/mainboard/getac/p470/ac... PS5, Line 196: One 1
Hello build bot (Jenkins), Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45554
to look at the new patch set (#6).
Change subject: mb/getac/p470/acpi: Convert 'battery.asl' to ASL 2.0 syntax ......................................................................
mb/getac/p470/acpi: Convert 'battery.asl' to ASL 2.0 syntax
IASL optimizes the code differently, which changes the binary. However, the generated `build/dsdt.dsl` remains identical.
Change-Id: Ifcc8bf4022838056bf1fff853eb2027af684064e Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/mainboard/getac/p470/acpi/battery.asl 1 file changed, 81 insertions(+), 81 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/45554/6
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45554 )
Change subject: mb/getac/p470/acpi: Convert 'battery.asl' to ASL 2.0 syntax ......................................................................
Patch Set 5:
(5 comments)
https://review.coreboot.org/c/coreboot/+/45554/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45554/5//COMMIT_MSG@9 PS5, Line 9: Changes the binary using 'BUILD_TIMELESS=1'. Probably because of optimization. : Generated "dsdt.dsl" are same.
I would say: […]
Done
https://review.coreboot.org/c/coreboot/+/45554/5/src/mainboard/getac/p470/ac... File src/mainboard/getac/p470/acpi/battery.asl:
https://review.coreboot.org/c/coreboot/+/45554/5/src/mainboard/getac/p470/ac... PS5, Line 81: 0x64
hmmmmm, 100?
Done
https://review.coreboot.org/c/coreboot/+/45554/5/src/mainboard/getac/p470/ac... PS5, Line 84: 0x64
hmmmmm
Done
https://review.coreboot.org/c/coreboot/+/45554/5/src/mainboard/getac/p470/ac... PS5, Line 189: 0x64
hmmmmm
Done
https://review.coreboot.org/c/coreboot/+/45554/5/src/mainboard/getac/p470/ac... PS5, Line 196: One
1
Done
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45554 )
Change subject: mb/getac/p470/acpi: Convert 'battery.asl' to ASL 2.0 syntax ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45554/6/src/mainboard/getac/p470/ac... File src/mainboard/getac/p470/acpi/battery.asl:
https://review.coreboot.org/c/coreboot/+/45554/6/src/mainboard/getac/p470/ac... PS6, Line 160: 0x04 4
Hello build bot (Jenkins), Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45554
to look at the new patch set (#7).
Change subject: mb/getac/p470/acpi: Convert 'battery.asl' to ASL 2.0 syntax ......................................................................
mb/getac/p470/acpi: Convert 'battery.asl' to ASL 2.0 syntax
IASL optimizes the code differently, which changes the binary. However, the generated `build/dsdt.dsl` remains identical.
Change-Id: Ifcc8bf4022838056bf1fff853eb2027af684064e Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/mainboard/getac/p470/acpi/battery.asl 1 file changed, 81 insertions(+), 81 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/45554/7
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45554 )
Change subject: mb/getac/p470/acpi: Convert 'battery.asl' to ASL 2.0 syntax ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45554/6/src/mainboard/getac/p470/ac... File src/mainboard/getac/p470/acpi/battery.asl:
https://review.coreboot.org/c/coreboot/+/45554/6/src/mainboard/getac/p470/ac... PS6, Line 160: 0x04
4
Done Thank you.
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45554 )
Change subject: mb/getac/p470/acpi: Convert 'battery.asl' to ASL 2.0 syntax ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45554/6/src/mainboard/getac/p470/ac... File src/mainboard/getac/p470/acpi/battery.asl:
https://review.coreboot.org/c/coreboot/+/45554/6/src/mainboard/getac/p470/ac... PS6, Line 160: 0x04
Done […]
Done
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45554 )
Change subject: mb/getac/p470/acpi: Convert 'battery.asl' to ASL 2.0 syntax ......................................................................
Patch Set 7: Code-Review+1
I think this looks correct.
I think this overall ASL 2 conversion is a fantastic idea! But it's also amenable to automation... If I take a quick look in the coreboot source tree, I see: `git grep -c Store -- *.asl | wc -l` 327 .asl files that could use conversion to the new syntax.
Food for thought 😊
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45554 )
Change subject: mb/getac/p470/acpi: Convert 'battery.asl' to ASL 2.0 syntax ......................................................................
Patch Set 7:
Patch Set 7: Code-Review+1
I think this looks correct.
I think this overall ASL 2 conversion is a fantastic idea! But it's also amenable to automation... If I take a quick look in the coreboot source tree, I see: `git grep -c Store -- *.asl | wc -l` 327 .asl files that could use conversion to the new syntax.
Food for thought 😊
Honestly I have no idea how to make an automation. 'iasl -ca' cmd will not work.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45554 )
Change subject: mb/getac/p470/acpi: Convert 'battery.asl' to ASL 2.0 syntax ......................................................................
Patch Set 7:
Patch Set 7:
Patch Set 7: Code-Review+1
I think this looks correct.
I think this overall ASL 2 conversion is a fantastic idea! But it's also amenable to automation... If I take a quick look in the coreboot source tree, I see: `git grep -c Store -- *.asl | wc -l` 327 .asl files that could use conversion to the new syntax.
Food for thought 😊
Honestly I have no idea how to make an automation. 'iasl -ca' cmd will not work.
Things like "do not use 0x64 when calculating percentages" won't be caught by automation
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45554 )
Change subject: mb/getac/p470/acpi: Convert 'battery.asl' to ASL 2.0 syntax ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45554/7/src/mainboard/getac/p470/ac... File src/mainboard/getac/p470/acpi/battery.asl:
https://review.coreboot.org/c/coreboot/+/45554/7/src/mainboard/getac/p470/ac... PS7, Line 86: 0x02 2
Hello build bot (Jenkins), Patrick Georgi, Tim Wawrzynczak,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45554
to look at the new patch set (#8).
Change subject: mb/getac/p470/acpi: Convert 'battery.asl' to ASL 2.0 syntax ......................................................................
mb/getac/p470/acpi: Convert 'battery.asl' to ASL 2.0 syntax
IASL optimizes the code differently, which changes the binary. However, the generated `build/dsdt.dsl` remains identical.
Change-Id: Ifcc8bf4022838056bf1fff853eb2027af684064e Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/mainboard/getac/p470/acpi/battery.asl 1 file changed, 81 insertions(+), 81 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/45554/8
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45554 )
Change subject: mb/getac/p470/acpi: Convert 'battery.asl' to ASL 2.0 syntax ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45554/7/src/mainboard/getac/p470/ac... File src/mainboard/getac/p470/acpi/battery.asl:
https://review.coreboot.org/c/coreboot/+/45554/7/src/mainboard/getac/p470/ac... PS7, Line 86: 0x02
2
Done
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45554 )
Change subject: mb/getac/p470/acpi: Convert 'battery.asl' to ASL 2.0 syntax ......................................................................
Patch Set 8:
Patch Set 7:
Patch Set 7:
Patch Set 7: Code-Review+1
I think this looks correct.
I think this overall ASL 2 conversion is a fantastic idea! But it's also amenable to automation... If I take a quick look in the coreboot source tree, I see: `git grep -c Store -- *.asl | wc -l` 327 .asl files that could use conversion to the new syntax.
Food for thought 😊
Honestly I have no idea how to make an automation. 'iasl -ca' cmd will not work.
Things like "do not use 0x64 when calculating percentages" won't be caught by automation
Nope, but to be fair, it was already 100, so any automation wouldn't change that.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45554 )
Change subject: mb/getac/p470/acpi: Convert 'battery.asl' to ASL 2.0 syntax ......................................................................
Patch Set 9: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/45554 )
Change subject: mb/getac/p470/acpi: Convert 'battery.asl' to ASL 2.0 syntax ......................................................................
mb/getac/p470/acpi: Convert 'battery.asl' to ASL 2.0 syntax
IASL optimizes the code differently, which changes the binary. However, the generated `build/dsdt.dsl` remains identical.
Change-Id: Ifcc8bf4022838056bf1fff853eb2027af684064e Signed-off-by: Elyes HAOUAS ehaouas@noos.fr Reviewed-on: https://review.coreboot.org/c/coreboot/+/45554 Reviewed-by: Angel Pons th3fanbus@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/getac/p470/acpi/battery.asl 1 file changed, 81 insertions(+), 81 deletions(-)
Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved
diff --git a/src/mainboard/getac/p470/acpi/battery.asl b/src/mainboard/getac/p470/acpi/battery.asl index d7b642a..4eca3f0 100644 --- a/src/mainboard/getac/p470/acpi/battery.asl +++ b/src/mainboard/getac/p470/acpi/battery.asl @@ -18,10 +18,10 @@ Method(_STA, 0) { Sleep(120) - If(_SB.PCI0.LPCB.EC0.BAT) { - Return(0x1f) + If (_SB.PCI0.LPCB.EC0.BAT) { + Return (0x1f) } Else { - Return(0x0f) + Return (0x0f) } }
@@ -45,67 +45,67 @@ })
// Is battery there? - Store(_STA(), Local0) - And(Local0, 0x10, Local0) - If(LNot(Local0)) { + Local0 = _STA () + Local0 &= 0x10 + If (!Local0) { Return (PBIF) }
- Store(_SB.PCI0.LPCB.EC0.BDC0, Index(PBIF, 1)) - Store(_SB.PCI0.LPCB.EC0.BDV0, Index(PBIF, 4)) + PBIF [1] = _SB.PCI0.LPCB.EC0.BDC0 + PBIF [4] = _SB.PCI0.LPCB.EC0.BDV0
- Store(_SB.PCI0.LPCB.EC0.BFC0, Local0) - Store(Local0, Index(PBIF, 2)) + Local0 = _SB.PCI0.LPCB.EC0.BFC0 + PBIF [2] = Local0
- Divide(Local0, 10, , Local2) - Store(Local2, Index(PBIF, 5)) + Local2 = Local0 / 10 + PBIF [5] = Local2
- Divide(Local0, 20, , Local2) - Store(Local2, Index(PBIF, 6)) + Local2 = Local0 / 20 + PBIF [6] = Local2
- If(_SB.PCI0.LPCB.EC0.BTYP) { - Store(NIMH, Index(PBIF, 11)) + If (_SB.PCI0.LPCB.EC0.BTYP) { + PBIF [11] = NIMH } Else { - Store(LION, Index(PBIF, 11)) + PBIF [11] = LION }
- Return(PBIF) + Return (PBIF) }
/* Battery Status */ Method(_BST, 0) { - If(_SB.PCI0.LPCB.EC0.BAT) { - Store(_SB.PCI0.LPCB.EC0.BPV0, Index(PBST, 3)) + If (_SB.PCI0.LPCB.EC0.BAT) { + PBST [3] = _SB.PCI0.LPCB.EC0.BPV0
- Multiply(_SB.PCI0.LPCB.EC0.BRC0, 100, Local3) - Divide(Local3, _SB.PCI0.LPCB.EC0.BFC0, , Local0) - Multiply(_SB.PCI0.LPCB.EC0.BFC0, Local0, Local3) - Divide(Local3, 0x64, , Local0) - Increment(Local0) - Store(Local0, Index(PBST, 2)) + Local3 = _SB.PCI0.LPCB.EC0.BRC0 * 100 + Local0 = Local3 / _SB.PCI0.LPCB.EC0.BFC0 + Local3 = _SB.PCI0.LPCB.EC0.BFC0 * Local0 + Local0 = Local3 / 100 + Local0++ + PBST [2] = Local0
- Store (_SB.PCI0.LPCB.EC0.BRC0, Local3) - Store (_SB.PCI0.LPCB.EC0.BPR0, Local0) - And (Not (Local0), 0xFFFF, Local0) - Store (Local0, Index(PBST,1)) + Local3 = _SB.PCI0.LPCB.EC0.BRC0 + Local0 = _SB.PCI0.LPCB.EC0.BPR0 + Local0 = ~Local0 & 0xFFFF + PBST [1] = Local0
// AC Power connected? - If(_SB.PCI0.LPCB.EC0.ADP) { - If(_SB.PCI0.LPCB.EC0.CHRG) { - Store(2, Index(PBST, 0)) + If (_SB.PCI0.LPCB.EC0.ADP) { + If (_SB.PCI0.LPCB.EC0.CHRG) { + PBST [0] = 2 } Else { - Store(0, Index(PBST, 0)) + PBST [0] = 0 } } Else { - If(LLess(Local3, 25)) { - Store(5, Index(PBST, 0)) + If (Local3 < 25) { + PBST [0] = 5 } Else { - Store(1, Index(PBST, 0)) + PBST [0] = 1 } } } - Return(PBST) + Return (PBST) } }
@@ -123,10 +123,10 @@ Method(_STA, 0) { Sleep(120) - If(_SB.PCI0.LPCB.EC0.BAT2) { - Return(0x1f) + If (_SB.PCI0.LPCB.EC0.BAT2) { + Return (0x1f) } Else { - Return(0x0f) + Return (0x0f) } }
@@ -150,67 +150,67 @@ })
// Is battery there? - Store(_STA(), Local0) - And(Local0, 0x10, Local0) - If(LNot(Local0)) { + Local0 = _STA () + Local0 &= 0x10 + If (!Local0) { Return (PBIF) }
- Store(_SB.PCI0.LPCB.EC0.BDC2, Index(PBIF, 1)) - Store(_SB.PCI0.LPCB.EC0.BDV2, Index(PBIF, 4)) + PBIF [1] = _SB.PCI0.LPCB.EC0.BDC2 + PBIF [4] = _SB.PCI0.LPCB.EC0.BDV2
- Store(_SB.PCI0.LPCB.EC0.BFC2, Local0) - Store(Local0, Index(PBIF, 2)) + Local0 = _SB.PCI0.LPCB.EC0.BFC2 + PBIF [2] = Local0
- Divide(Local0, 10, , Local2) - Store(Local2, Index(PBIF, 5)) + Local2 = Local0 / 10 + PBIF [5] = Local2
- Divide(Local0, 20, , Local2) - Store(Local2, Index(PBIF, 6)) + Local2 = Local0 / 20 + PBIF [6] = Local2
- If(_SB.PCI0.LPCB.EC0.BTY2) { - Store(NIMH, Index(PBIF, 11)) + If (_SB.PCI0.LPCB.EC0.BTY2) { + PBIF [11] = NIMH } Else { - Store(LION, Index(PBIF, 11)) + PBIF [11] = LION }
- Return(PBIF) + Return (PBIF) }
/* Battery Status */ Method(_BST, 0) { - If(_SB.PCI0.LPCB.EC0.BAT2) { - Store(_SB.PCI0.LPCB.EC0.BPV2, Index(PBST, 3)) + If (_SB.PCI0.LPCB.EC0.BAT2) { + PBST [3] = _SB.PCI0.LPCB.EC0.BPV2
- Multiply(_SB.PCI0.LPCB.EC0.BRC2, 100, Local3) - Divide(Local3, _SB.PCI0.LPCB.EC0.BFC2, , Local0) - Multiply(_SB.PCI0.LPCB.EC0.BFC2, Local0, Local3) - Divide(Local3, 0x64, , Local0) - Increment(Local0) - Store(Local0, Index(PBST, 2)) + Local3 = _SB.PCI0.LPCB.EC0.BRC2 * 100 + Local0 = Local3 / _SB.PCI0.LPCB.EC0.BRC2 + Local3 = _SB.PCI0.LPCB.EC0.BFC2 * Local0 + Local0 = Local3 / 100 + Local0++ + PBST [2] = Local0
- Store (_SB.PCI0.LPCB.EC0.BRC2, Local3) - Store (_SB.PCI0.LPCB.EC0.BPR2, Local0) - And (Not (Local0), 0xFFFF, Local0) - Store (Local0, Index(PBST,1)) + Local3 = _SB.PCI0.LPCB.EC0.BRC2 + Local0 = _SB.PCI0.LPCB.EC0.BPR2 + Local0 = ~Local0 & 0xFFFF + PBST [1] = Local0
// AC Power connected? - If(_SB.PCI0.LPCB.EC0.ADP) { - If(_SB.PCI0.LPCB.EC0.CRG2) { - Store(2, Index(PBST, 0)) + If (_SB.PCI0.LPCB.EC0.ADP) { + If (_SB.PCI0.LPCB.EC0.CRG2) { + PBST [0] = 2 } Else { - Store(0, Index(PBST, 0)) + PBST [0] = 0 } } Else { - If(LLess(Local3, 25)) { - Store(5, Index(PBST, 0)) + If (Local3 < 25) { + PBST [0] = 5 } Else { - Store(1, Index(PBST, 0)) + PBST [0] = 1 } } } - Return(PBST) + Return (PBST) } }
@@ -223,12 +223,12 @@ Name (ACST, 0x00) Method (_PSR, 0) { - If(ACFG) { - Store(ACST, Local0) + If (ACFG) { + Local0 = ACST } Else { - Store(_SB.PCI0.LPCB.EC0.ADP, Local0) - Store(Local0, ACST) - Store(1, ACFG) + Local0 = _SB.PCI0.LPCB.EC0.ADP + ACST = Local0 + ACFG = 1 } Sleep(120) Return (Local0)
-
Angel Pons (Code Review) -
HAOUAS Elyes (Code Review)
-
Patrick Georgi (Code Review)
-
Tim Wawrzynczak (Code Review)