Change in coreboot[master]: nb/intel/ironlake/acpi/hostbridge.asl: Use ASL 2.0 syntax

[Angel Pons (Code Review)]

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43687 )

Change subject: nb/intel/ironlake/acpi/hostbridge.asl: Use ASL 2.0 syntax
......................................................................

nb/intel/ironlake/acpi/hostbridge.asl: Use ASL 2.0 syntax

This brings Ironlake closer to Sandy Bridge.

Tested with BUILD_TIMELESS=1, Packard Bell MS2290 does not change.

Change-Id: Idde75e7295f642f8add34168bffe5851ea02fbc6
Signed-off-by: Angel Pons 
---
M src/northbridge/intel/ironlake/acpi/hostbridge.asl
1 file changed, 28 insertions(+), 30 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/43687/1

diff --git a/src/northbridge/intel/ironlake/acpi/hostbridge.asl b/src/northbridge/intel/ironlake/acpi/hostbridge.asl
index 6a59fb5..324fb35 100644
--- a/src/northbridge/intel/ironlake/acpi/hostbridge.asl
+++ b/src/northbridge/intel/ironlake/acpi/hostbridge.asl
@@ -1,6 +1,5 @@
 /* SPDX-License-Identifier: GPL-2.0-only */

-
 Name(_HID,EISAID("PNP0A08"))	// PCIe
 Name(_CID,EISAID("PNP0A03"))	// PCI

@@ -92,17 +91,16 @@
    External (_SB.CP00._PSS)
    Method (PSSS, 1, NotSerialized)
    {
-		Store (One, Local0) /* Start at P1 */
-		Store (SizeOf (_SB.CP00._PSS), Local1)
+		Local0 = 1 /* Start at P1 */
+		Local1 = SizeOf (_SB.CP00._PSS)

-		While (LLess (Local0, Local1)) {
+		While (Local0 < Local1) {
    		/* Store _PSS entry Control value to Local2 */
-			ShiftRight (DeRefOf (Index (DeRefOf (Index
-			      (_SB.CP00._PSS, Local0)), 4)), 8, Local2)
-			If (LEqual (Local2, Arg0)) {
-				Return (Subtract (Local0, 1))
+			Local2 = DeRefOf (Index (DeRefOf (Index (_SB.CP00._PSS, Local0)), 4)) >> 8
+			If (Local2 == Arg0) {
+				Return (Local0 - 1)
    		}
-			Increment (Local0)
+			Local0++
    	}

Return (0)
@@ -122,23 +120,23 @@
    	Store ("Set TDP Down", Debug)

/* Set CTC */
-		Store (CTCD, CTCS)
+		CTCS = CTCD

/* Set TAR */
-		Store (TARD, TARS)
+		TARS = TARD

/* Set PPC limit and notify OS */
-		Store (PSSS (TARD), PPCM)
+		PPCM = PSSS (TARD)
    	PPCN ()

/* Set PL2 to 1.25 * PL1 */
-		Divide (Multiply (CTDD, 125), 100, , PL2V)
+		PL2V = (CTDD * 125) / 100

/* Set PL1 */
-		Store (CTDD, PL1V)
+		PL1V = CTDD

/* Store the new TDP Down setting */
-		Store (CTCD, CTCC)
+		CTCC = CTCD

Release (CTCM)
    	Return (1)
@@ -158,23 +156,23 @@
    	Store ("Set TDP Nominal", Debug)

/* Set PL1 */
-		Store (CTDN, PL1V)
+		PL1V = CTDN

/* Set PL2 to 1.25 * PL1 */
-		Divide (Multiply (CTDN, 125), 100, , PL2V)
+		PL2V = (CTDN * 125) / 100

/* Set PPC limit and notify OS */
-		Store (PSSS (TARN), PPCM)
+		PPCM = PSSS (TARN)
    	PPCN ()

/* Set TAR */
-		Store (TARN, TARS)
+		TARS = TARN

/* Set CTC */
-		Store (CTCN, CTCS)
+		CTCS = CTCN

/* Store the new TDP Nominal setting */
-		Store (CTCN, CTCC)
+		CTCC = CTCN

Release (CTCM)
    	Return (1)
@@ -305,20 +303,20 @@

// Fix up PCI memory region
    // Start with Top of Lower Usable DRAM
-	Store (^MCHC.TLUD, Local0)
-	ShiftRight (Local0, 4, Local0)
-	Store (^MCHC.TUUD, Local1)
+	Local0 = ^MCHC.TLUD
+	Local0 >>= 4
+	Local1 = ^MCHC.TUUD

// Check if ME base is equal
-	If (LEqual (Local0, Local1)) {
+	If (Local0 == Local1) {
    	// Use Top Of Memory instead
-		Store (^MCHC.TOM, Local0)
-		ShiftRight (Local0, 6, Local0)
+		Local0 = ^MCHC.TOM
+		Local0 >>= 6
    }

-	ShiftLeft (Local0, 20, Local0)
-	Store (Local0, PMIN)
-	Add(Subtract(PMAX, PMIN), 1, PLEN)
+	Local0 <<= 20
+	PMIN = Local0
+	PLEN = PMAX - PMIN + 1

Return (MCRS)
 }

[Patrick Rudolph (Code Review)]

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43687 )

Change subject: nb/intel/ironlake/acpi/hostbridge.asl: Use ASL 2.0 syntax
......................................................................

Patch Set 1: Code-Review+2

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43687 )

Change subject: nb/intel/ironlake/acpi/hostbridge.asl: Use ASL 2.0 syntax
......................................................................

Patch Set 1:

(4 comments)

https://review.coreboot.org/c/coreboot/+/43687/1/src/northbridge/intel/ironl... 
File src/northbridge/intel/ironlake/acpi/hostbridge.asl:

https://review.coreboot.org/c/coreboot/+/43687/1/src/northbridge/intel/ironl... 
PS1, Line 112: If (Acquire (CTCM, 100)) {
             : 			Return (0)
             : 		}
             : 		If (LEqual (CTCD, CTCC)) {
             : 			Release (CTC
old

https://review.coreboot.org/c/coreboot/+/43687/1/src/northbridge/intel/ironl... 
PS1, Line 120: Store (
old syntax

https://review.coreboot.org/c/coreboot/+/43687/1/src/northbridge/intel/ironl... 
PS1, Line 148: If (Acquire (CTCM, 100)) {
old syntax

https://review.coreboot.org/c/coreboot/+/43687/1/src/northbridge/intel/ironl... 
PS1, Line 151: If (LEqual (CTCN, CTCC)
old syntax

[Patrick Georgi (Code Review)]

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43687 )

Change subject: nb/intel/ironlake/acpi/hostbridge.asl: Use ASL 2.0 syntax
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/43687/1/src/northbridge/intel/ironl... 
File src/northbridge/intel/ironlake/acpi/hostbridge.asl:

https://review.coreboot.org/c/coreboot/+/43687/1/src/northbridge/intel/ironl... 
PS1, Line 112: If (Acquire (CTCM, 100)) {
             : 			Return (0)
             : 		}
             : 		If (LEqual (CTCD, CTCC)) {
             : 			Release (CTC
> old

Are these in ASL1 syntax for sandybridge as well? if so, I'd propose keeping them here (to minimize the diffs) and have a follow up that touches both. Same for the other instances.

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43687 )

Change subject: nb/intel/ironlake/acpi/hostbridge.asl: Use ASL 2.0 syntax
......................................................................

Patch Set 1:

(2 comments)

https://review.coreboot.org/c/coreboot/+/43687/1/src/northbridge/intel/ironl... 
File src/northbridge/intel/ironlake/acpi/hostbridge.asl:

https://review.coreboot.org/c/coreboot/+/43687/1/src/northbridge/intel/ironl... 
PS1, Line 112: If (Acquire (CTCM, 100)) {
             : 			Return (0)
             : 		}
             : 		If (LEqual (CTCD, CTCC)) {
             : 			Release (CTC
> Are these in ASL1 syntax for sandybridge as well? if so, I'd propose keeping them here (to minimize  […]

Yes, I'll take care of that. Note that `Acquire` isn't old syntax.

https://review.coreboot.org/c/coreboot/+/43687/1/src/northbridge/intel/ironl... 
PS1, Line 148: If (Acquire (CTCM, 100)) {
> old syntax

No

[Angel Pons (Code Review)]

Hello build bot (Jenkins), Nico Huber, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/43687

to look at the new patch set (#2).

Change subject: nb/intel/ironlake/acpi/hostbridge.asl: Use ASL 2.0 syntax
......................................................................

nb/intel/ironlake/acpi/hostbridge.asl: Use ASL 2.0 syntax

This brings Ironlake closer to Sandy Bridge.

Tested with BUILD_TIMELESS=1, Packard Bell MS2290 does not change.

Change-Id: Idde75e7295f642f8add34168bffe5851ea02fbc6
Signed-off-by: Angel Pons 
---
M src/northbridge/intel/ironlake/acpi/hostbridge.asl
1 file changed, 32 insertions(+), 34 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/43687/2

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43687 )

Change subject: nb/intel/ironlake/acpi/hostbridge.asl: Use ASL 2.0 syntax
......................................................................

Patch Set 2:

(3 comments)

https://review.coreboot.org/c/coreboot/+/43687/1/src/northbridge/intel/ironl... 
File src/northbridge/intel/ironlake/acpi/hostbridge.asl:

https://review.coreboot.org/c/coreboot/+/43687/1/src/northbridge/intel/ironl... 
PS1, Line 112: If (Acquire (CTCM, 100)) {
             : 			Return (0)
             : 		}
             : 		If (LEqual (CTCD, CTCC)) {
             : 			Release (CTC
> Yes, I'll take care of that. Note that `Acquire` isn't old syntax.

Done

https://review.coreboot.org/c/coreboot/+/43687/1/src/northbridge/intel/ironl... 
PS1, Line 120: Store (
> old syntax

Done

https://review.coreboot.org/c/coreboot/+/43687/1/src/northbridge/intel/ironl... 
PS1, Line 151: If (LEqual (CTCN, CTCC)
> old syntax

Done

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43687 )

Change subject: nb/intel/ironlake/acpi/hostbridge.asl: Use ASL 2.0 syntax
......................................................................

Patch Set 2: Code-Review+2

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43687 )

Change subject: nb/intel/ironlake/acpi/hostbridge.asl: Use ASL 2.0 syntax
......................................................................

Patch Set 2: Code-Review+2

[Angel Pons (Code Review)]

Angel Pons has submitted this change. ( https://review.coreboot.org/c/coreboot/+/43687 )

Change subject: nb/intel/ironlake/acpi/hostbridge.asl: Use ASL 2.0 syntax
......................................................................

nb/intel/ironlake/acpi/hostbridge.asl: Use ASL 2.0 syntax

This brings Ironlake closer to Sandy Bridge.

Tested with BUILD_TIMELESS=1, Packard Bell MS2290 does not change.

Change-Id: Idde75e7295f642f8add34168bffe5851ea02fbc6
Signed-off-by: Angel Pons 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/43687
Reviewed-by: Arthur Heymans 
Reviewed-by: HAOUAS Elyes 
Tested-by: build bot (Jenkins) 
---
M src/northbridge/intel/ironlake/acpi/hostbridge.asl
1 file changed, 32 insertions(+), 34 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  HAOUAS Elyes: Looks good to me, approved
  Arthur Heymans: Looks good to me, approved

diff --git a/src/northbridge/intel/ironlake/acpi/hostbridge.asl b/src/northbridge/intel/ironlake/acpi/hostbridge.asl
index 6a59fb5..d6a2cf8 100644
--- a/src/northbridge/intel/ironlake/acpi/hostbridge.asl
+++ b/src/northbridge/intel/ironlake/acpi/hostbridge.asl
@@ -1,6 +1,5 @@
 /* SPDX-License-Identifier: GPL-2.0-only */

-
 Name(_HID,EISAID("PNP0A08"))	// PCIe
 Name(_CID,EISAID("PNP0A03"))	// PCI

@@ -92,17 +91,16 @@
    External (_SB.CP00._PSS)
    Method (PSSS, 1, NotSerialized)
    {
-		Store (One, Local0) /* Start at P1 */
-		Store (SizeOf (_SB.CP00._PSS), Local1)
+		Local0 = 1 /* Start at P1 */
+		Local1 = SizeOf (_SB.CP00._PSS)

-		While (LLess (Local0, Local1)) {
+		While (Local0 < Local1) {
    		/* Store _PSS entry Control value to Local2 */
-			ShiftRight (DeRefOf (Index (DeRefOf (Index
-			      (_SB.CP00._PSS, Local0)), 4)), 8, Local2)
-			If (LEqual (Local2, Arg0)) {
-				Return (Subtract (Local0, 1))
+			Local2 = DeRefOf (Index (DeRefOf (Index (_SB.CP00._PSS, Local0)), 4)) >> 8
+			If (Local2 == Arg0) {
+				Return (Local0 - 1)
    		}
-			Increment (Local0)
+			Local0++
    	}

Return (0)
@@ -114,31 +112,31 @@
    	If (Acquire (CTCM, 100)) {
    		Return (0)
    	}
-		If (LEqual (CTCD, CTCC)) {
+		If (CTCD == CTCC) {
    		Release (CTCM)
    		Return (0)
    	}

-		Store ("Set TDP Down", Debug)
+		Debug = "Set TDP Down"

/* Set CTC */
-		Store (CTCD, CTCS)
+		CTCS = CTCD

/* Set TAR */
-		Store (TARD, TARS)
+		TARS = TARD

/* Set PPC limit and notify OS */
-		Store (PSSS (TARD), PPCM)
+		PPCM = PSSS (TARD)
    	PPCN ()

/* Set PL2 to 1.25 * PL1 */
-		Divide (Multiply (CTDD, 125), 100, , PL2V)
+		PL2V = (CTDD * 125) / 100

/* Set PL1 */
-		Store (CTDD, PL1V)
+		PL1V = CTDD

/* Store the new TDP Down setting */
-		Store (CTCD, CTCC)
+		CTCC = CTCD

Release (CTCM)
    	Return (1)
@@ -150,31 +148,31 @@
    	If (Acquire (CTCM, 100)) {
    		Return (0)
    	}
-		If (LEqual (CTCN, CTCC)) {
+		If (CTCN == CTCC) {
    		Release (CTCM)
    		Return (0)
    	}

-		Store ("Set TDP Nominal", Debug)
+		Debug = "Set TDP Nominal"

/* Set PL1 */
-		Store (CTDN, PL1V)
+		PL1V = CTDN

/* Set PL2 to 1.25 * PL1 */
-		Divide (Multiply (CTDN, 125), 100, , PL2V)
+		PL2V = (CTDN * 125) / 100

/* Set PPC limit and notify OS */
-		Store (PSSS (TARN), PPCM)
+		PPCM = PSSS (TARN)
    	PPCN ()

/* Set TAR */
-		Store (TARN, TARS)
+		TARS = TARN

/* Set CTC */
-		Store (CTCN, CTCS)
+		CTCS = CTCN

/* Store the new TDP Nominal setting */
-		Store (CTCN, CTCC)
+		CTCC = CTCN

Release (CTCM)
    	Return (1)
@@ -305,20 +303,20 @@

// Fix up PCI memory region
    // Start with Top of Lower Usable DRAM
-	Store (^MCHC.TLUD, Local0)
-	ShiftRight (Local0, 4, Local0)
-	Store (^MCHC.TUUD, Local1)
+	Local0 = ^MCHC.TLUD
+	Local0 >>= 4
+	Local1 = ^MCHC.TUUD

// Check if ME base is equal
-	If (LEqual (Local0, Local1)) {
+	If (Local0 == Local1) {
    	// Use Top Of Memory instead
-		Store (^MCHC.TOM, Local0)
-		ShiftRight (Local0, 6, Local0)
+		Local0 = ^MCHC.TOM
+		Local0 >>= 6
    }

-	ShiftLeft (Local0, 20, Local0)
-	Store (Local0, PMIN)
-	Add(Subtract(PMAX, PMIN), 1, PLEN)
+	Local0 <<= 20
+	PMIN = Local0
+	PLEN = PMAX - PMIN + 1

Return (MCRS)
 }