Furquan Shaikh has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41480 )
Change subject: soc/intel/skylake: Update systemagent.asl to ASL2.0 ......................................................................
soc/intel/skylake: Update systemagent.asl to ASL2.0
This change updates systemagent.asl to use ASL2.0 syntax. This increases the readability of the ASL code.
Change-Id: If8d8dd50af9a79d30f54e98f7f2fe7ce49188763 Signed-off-by: Furquan Shaikh furquan@google.com --- M src/soc/intel/skylake/acpi/systemagent.asl 1 file changed, 21 insertions(+), 21 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/41480/1
diff --git a/src/soc/intel/skylake/acpi/systemagent.asl b/src/soc/intel/skylake/acpi/systemagent.asl index 6ea782b..5ff6224 100644 --- a/src/soc/intel/skylake/acpi/systemagent.asl +++ b/src/soc/intel/skylake/acpi/systemagent.asl @@ -188,30 +188,30 @@ * Fix up PCI memory region * Start with Top of Lower Usable DRAM */ - Store (_SB.PCI0.MCHC.TLUD, Local0) - Store (_SB.PCI0.MCHC.MEBA, Local1) + Local0 = _SB.PCI0.MCHC.TLUD + Local1 = _SB.PCI0.MCHC.MEBA
/* Check if ME base is equal */ - If (LEqual (Local0, Local1)) { + If (Local0 == Local1) { /* Use Top Of Memory instead */ - Store (_SB.PCI0.MCHC.TOM, Local0) + Local0 = _SB.PCI0.MCHC.TOM }
Store (Local0, PMIN) - Add (Subtract (PMAX, PMIN), 1, PLEN) + PLEN = (PMAX - PMIN) + 1
/* Patch PM02 range based on Memory Size */ - If (LEqual (A4GS, 0)) { + If (A4GS == 0) { CreateQwordField (MCRS, PM02._LEN, MSEN) - Store (0, MSEN) + MSEN = 0 } Else { CreateQwordField (MCRS, PM02._MIN, MMIN) CreateQwordField (MCRS, PM02._MAX, MMAX) CreateQwordField (MCRS, PM02._LEN, MLEN) /* Set 64bit MMIO resource base and length */ - Store (A4GS, MLEN) - Store (A4GB, MMIN) - Subtract (Add (MMIN, MLEN), 1, MMAX) + MLEN = A4GS + MMIN = A4GB + MMAX = (MMIN + MLEN) - 1 }
Return (MCRS) @@ -220,35 +220,35 @@ /* Get MCH BAR */ Method (GMHB, 0, Serialized) { - ShiftLeft (_SB.PCI0.MCHC.MHBR, 15, Local0) + Local0 = _SB.PCI0.MCHC.MHBR << 15 Return (Local0) }
/* Get EP BAR */ Method (GEPB, 0, Serialized) { - ShiftLeft (_SB.PCI0.MCHC.EPBR, 12, Local0) + Local0 = _SB.PCI0.MCHC.EPBR << 12 Return (Local0) }
/* Get PCIe BAR */ Method (GPCB, 0, Serialized) { - ShiftLeft (_SB.PCI0.MCHC.PXBR, 26, Local0) + Local0 = _SB.PCI0.MCHC.PXBR << 26 Return (Local0) }
/* Get PCIe Length */ Method (GPCL, 0, Serialized) { - ShiftRight (0x10000000, _SB.PCI0.MCHC.PXSZ, Local0) + Local0 = 0x10000000 << _SB.PCI0.MCHC.PXSZ Return (Local0) }
/* Get DMI BAR */ Method (GDMB, 0, Serialized) { - ShiftLeft (_SB.PCI0.MCHC.DIBR, 12, Local0) + Local0 = _SB.PCI0.MCHC.DIBR << 12 Return (Local0) }
@@ -296,22 +296,22 @@ })
CreateDwordField (BUF0, MCHB._BAS, MBR0) - Store (_SB.PCI0.GMHB (), MBR0) + MBR0 = _SB.PCI0.GMHB ()
CreateDwordField (BUF0, DMIB._BAS, DBR0) - Store (_SB.PCI0.GDMB (), DBR0) + DBR0 = _SB.PCI0.GDMB ()
CreateDwordField (BUF0, EGPB._BAS, EBR0) - Store (_SB.PCI0.GEPB (), EBR0) + EBR0 = _SB.PCI0.GEPB ()
CreateDwordField (BUF0, PCIX._BAS, XBR0) - Store (_SB.PCI0.GPCB (), XBR0) + XBR0 = _SB.PCI0.GPCB ()
CreateDwordField (BUF0, PCIX._LEN, XSZ0) - Store (_SB.PCI0.GPCL (), XSZ0) + XSZ0 = _SB.PCI0.GPCL ()
CreateDwordField (BUF0, FIOH._BAS, FBR0) - Subtract(0x100000000, CONFIG_ROM_SIZE, FBR0) + FBR0 = 0x100000000 - CONFIG_ROM_SIZE
Return (BUF0) }
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41480 )
Change subject: soc/intel/skylake: Update systemagent.asl to ASL2.0 ......................................................................
Patch Set 1: Code-Review+2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41480 )
Change subject: soc/intel/skylake: Update systemagent.asl to ASL2.0 ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41480/1/src/soc/intel/skylake/acpi/... File src/soc/intel/skylake/acpi/systemagent.asl:
https://review.coreboot.org/c/coreboot/+/41480/1/src/soc/intel/skylake/acpi/... PS1, Line 244: << Um, this is not ShiftRight...
Hello build bot (Jenkins), Angel Pons, Subrata Banik, Aaron Durbin, Patrick Rudolph, HAOUAS Elyes,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41480
to look at the new patch set (#2).
Change subject: soc/intel/skylake: Update systemagent.asl to ASL2.0 ......................................................................
soc/intel/skylake: Update systemagent.asl to ASL2.0
This change updates systemagent.asl to use ASL2.0 syntax. This increases the readability of the ASL code.
Change-Id: If8d8dd50af9a79d30f54e98f7f2fe7ce49188763 Signed-off-by: Furquan Shaikh furquan@google.com --- M src/soc/intel/skylake/acpi/systemagent.asl 1 file changed, 21 insertions(+), 21 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/41480/2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41480 )
Change subject: soc/intel/skylake: Update systemagent.asl to ASL2.0 ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41480/1/src/soc/intel/skylake/acpi/... File src/soc/intel/skylake/acpi/systemagent.asl:
https://review.coreboot.org/c/coreboot/+/41480/1/src/soc/intel/skylake/acpi/... PS1, Line 244: <<
Um, this is not ShiftRight...
Good catch. Fixed it.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41480 )
Change subject: soc/intel/skylake: Update systemagent.asl to ASL2.0 ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41480/1/src/soc/intel/skylake/acpi/... File src/soc/intel/skylake/acpi/systemagent.asl:
https://review.coreboot.org/c/coreboot/+/41480/1/src/soc/intel/skylake/acpi/... PS1, Line 244: <<
Good catch. Fixed it.
Done
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41480 )
Change subject: soc/intel/skylake: Update systemagent.asl to ASL2.0 ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41480/1/src/soc/intel/skylake/acpi/... File src/soc/intel/skylake/acpi/systemagent.asl:
https://review.coreboot.org/c/coreboot/+/41480/1/src/soc/intel/skylake/acpi/... PS1, Line 244: <<
Done
If you ever want to verify code refactoring in coreboot, you can use BUILD_TIMELESS=1 to check if the resulting coreboot binary has changed or not. I've used that kind of stuff to make sure that my refactoring on sandybridge raminit wouldn't break things 😄
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41480 )
Change subject: soc/intel/skylake: Update systemagent.asl to ASL2.0 ......................................................................
Patch Set 2: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41480 )
Change subject: soc/intel/skylake: Update systemagent.asl to ASL2.0 ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41480/1/src/soc/intel/skylake/acpi/... File src/soc/intel/skylake/acpi/systemagent.asl:
https://review.coreboot.org/c/coreboot/+/41480/1/src/soc/intel/skylake/acpi/... PS1, Line 244: <<
If you ever want to verify code refactoring in coreboot, you can use BUILD_TIMELESS=1 to check if th […]
Ah. Thanks for the pointer. That is very helpful.
Hello build bot (Jenkins), Angel Pons, Subrata Banik, Aaron Durbin, Patrick Rudolph, HAOUAS Elyes,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/41480
to look at the new patch set (#4).
Change subject: soc/intel/skylake: Update systemagent.asl to ASL2.0 ......................................................................
soc/intel/skylake: Update systemagent.asl to ASL2.0
This change updates systemagent.asl to use ASL2.0 syntax. This increases the readability of the ASL code.
TEST=Verified using --timeless option to abuild that the resulting coreboot.rom is same as without the ASL2.0 syntax changes for soraka.
Change-Id: If8d8dd50af9a79d30f54e98f7f2fe7ce49188763 Signed-off-by: Furquan Shaikh furquan@google.com --- M src/soc/intel/skylake/acpi/systemagent.asl 1 file changed, 21 insertions(+), 21 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/41480/4
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41480 )
Change subject: soc/intel/skylake: Update systemagent.asl to ASL2.0 ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41480/1/src/soc/intel/skylake/acpi/... File src/soc/intel/skylake/acpi/systemagent.asl:
https://review.coreboot.org/c/coreboot/+/41480/1/src/soc/intel/skylake/acpi/... PS1, Line 244: <<
Ah. Thanks for the pointer. That is very helpful.
Performed test using --timeless and updated commit message.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41480 )
Change subject: soc/intel/skylake: Update systemagent.asl to ASL2.0 ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41480/1/src/soc/intel/skylake/acpi/... File src/soc/intel/skylake/acpi/systemagent.asl:
https://review.coreboot.org/c/coreboot/+/41480/1/src/soc/intel/skylake/acpi/... PS1, Line 244: <<
Performed test using --timeless and updated commit message.
Perfect!
Furquan Shaikh has submitted this change. ( https://review.coreboot.org/c/coreboot/+/41480 )
Change subject: soc/intel/skylake: Update systemagent.asl to ASL2.0 ......................................................................
soc/intel/skylake: Update systemagent.asl to ASL2.0
This change updates systemagent.asl to use ASL2.0 syntax. This increases the readability of the ASL code.
TEST=Verified using --timeless option to abuild that the resulting coreboot.rom is same as without the ASL2.0 syntax changes for soraka.
Change-Id: If8d8dd50af9a79d30f54e98f7f2fe7ce49188763 Signed-off-by: Furquan Shaikh furquan@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/41480 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Angel Pons th3fanbus@gmail.com --- M src/soc/intel/skylake/acpi/systemagent.asl 1 file changed, 21 insertions(+), 21 deletions(-)
Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved
diff --git a/src/soc/intel/skylake/acpi/systemagent.asl b/src/soc/intel/skylake/acpi/systemagent.asl index 6ea782b..b2e691d 100644 --- a/src/soc/intel/skylake/acpi/systemagent.asl +++ b/src/soc/intel/skylake/acpi/systemagent.asl @@ -188,30 +188,30 @@ * Fix up PCI memory region * Start with Top of Lower Usable DRAM */ - Store (_SB.PCI0.MCHC.TLUD, Local0) - Store (_SB.PCI0.MCHC.MEBA, Local1) + Local0 = _SB.PCI0.MCHC.TLUD + Local1 = _SB.PCI0.MCHC.MEBA
/* Check if ME base is equal */ - If (LEqual (Local0, Local1)) { + If (Local0 == Local1) { /* Use Top Of Memory instead */ - Store (_SB.PCI0.MCHC.TOM, Local0) + Local0 = _SB.PCI0.MCHC.TOM }
Store (Local0, PMIN) - Add (Subtract (PMAX, PMIN), 1, PLEN) + PLEN = (PMAX - PMIN) + 1
/* Patch PM02 range based on Memory Size */ - If (LEqual (A4GS, 0)) { + If (A4GS == 0) { CreateQwordField (MCRS, PM02._LEN, MSEN) - Store (0, MSEN) + MSEN = 0 } Else { CreateQwordField (MCRS, PM02._MIN, MMIN) CreateQwordField (MCRS, PM02._MAX, MMAX) CreateQwordField (MCRS, PM02._LEN, MLEN) /* Set 64bit MMIO resource base and length */ - Store (A4GS, MLEN) - Store (A4GB, MMIN) - Subtract (Add (MMIN, MLEN), 1, MMAX) + MLEN = A4GS + MMIN = A4GB + MMAX = (MMIN + MLEN) - 1 }
Return (MCRS) @@ -220,35 +220,35 @@ /* Get MCH BAR */ Method (GMHB, 0, Serialized) { - ShiftLeft (_SB.PCI0.MCHC.MHBR, 15, Local0) + Local0 = _SB.PCI0.MCHC.MHBR << 15 Return (Local0) }
/* Get EP BAR */ Method (GEPB, 0, Serialized) { - ShiftLeft (_SB.PCI0.MCHC.EPBR, 12, Local0) + Local0 = _SB.PCI0.MCHC.EPBR << 12 Return (Local0) }
/* Get PCIe BAR */ Method (GPCB, 0, Serialized) { - ShiftLeft (_SB.PCI0.MCHC.PXBR, 26, Local0) + Local0 = _SB.PCI0.MCHC.PXBR << 26 Return (Local0) }
/* Get PCIe Length */ Method (GPCL, 0, Serialized) { - ShiftRight (0x10000000, _SB.PCI0.MCHC.PXSZ, Local0) + Local0 = 0x10000000 >> _SB.PCI0.MCHC.PXSZ Return (Local0) }
/* Get DMI BAR */ Method (GDMB, 0, Serialized) { - ShiftLeft (_SB.PCI0.MCHC.DIBR, 12, Local0) + Local0 = _SB.PCI0.MCHC.DIBR << 12 Return (Local0) }
@@ -296,22 +296,22 @@ })
CreateDwordField (BUF0, MCHB._BAS, MBR0) - Store (_SB.PCI0.GMHB (), MBR0) + MBR0 = _SB.PCI0.GMHB ()
CreateDwordField (BUF0, DMIB._BAS, DBR0) - Store (_SB.PCI0.GDMB (), DBR0) + DBR0 = _SB.PCI0.GDMB ()
CreateDwordField (BUF0, EGPB._BAS, EBR0) - Store (_SB.PCI0.GEPB (), EBR0) + EBR0 = _SB.PCI0.GEPB ()
CreateDwordField (BUF0, PCIX._BAS, XBR0) - Store (_SB.PCI0.GPCB (), XBR0) + XBR0 = _SB.PCI0.GPCB ()
CreateDwordField (BUF0, PCIX._LEN, XSZ0) - Store (_SB.PCI0.GPCL (), XSZ0) + XSZ0 = _SB.PCI0.GPCL ()
CreateDwordField (BUF0, FIOH._BAS, FBR0) - Subtract(0x100000000, CONFIG_ROM_SIZE, FBR0) + FBR0 = 0x100000000 - CONFIG_ROM_SIZE
Return (BUF0) }
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41480 )
Change subject: soc/intel/skylake: Update systemagent.asl to ASL2.0 ......................................................................
Patch Set 5:
Automatic boot test returned (PASS/FAIL/TOTAL): 4/0/4 Emulation targets: "QEMU x86 q35/ich9" using payload TianoCore : SUCCESS : https://lava.9esec.io/r/3602 "QEMU x86 q35/ich9" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/3601 "QEMU x86 i440fx/piix4" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/3600 "QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/3599
Please note: This test is under development and might not be accurate at all!
-
9elements QA (Code Review) -
Angel Pons (Code Review)
-
Furquan Shaikh (Code Review)
-
HAOUAS Elyes (Code Review)