Change in coreboot[master]: soc/intel/{jsl,tgl}: Refactor gpio_op.asl

[Subrata Banik (Code Review)]

Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45688 )

Change subject: soc/intel/{jsl,tgl}: Refactor gpio_op.asl
......................................................................

soc/intel/{jsl,tgl}: Refactor gpio_op.asl

Also align GPMO ASL function with TGL.

Change-Id: Ia40af2cba9867838a1f99141481a5e78cffa0111
Signed-off-by: Subrata Banik 
---
M src/soc/intel/jasperlake/acpi/gpio_op.asl
M src/soc/intel/tigerlake/acpi/gpio_op.asl
2 files changed, 14 insertions(+), 15 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/88/45688/1

diff --git a/src/soc/intel/jasperlake/acpi/gpio_op.asl b/src/soc/intel/jasperlake/acpi/gpio_op.asl
index 683686f..0da4696 100644
--- a/src/soc/intel/jasperlake/acpi/gpio_op.asl
+++ b/src/soc/intel/jasperlake/acpi/gpio_op.asl
@@ -43,7 +43,7 @@
    {
    	VAL0, 32
    }
-	VAL0 = PAD_CFG0_TX_STATE | VAL0
+	VAL0 |= PAD_CFG0_TX_STATE
 }

/*
@@ -57,7 +57,7 @@
    {
    	VAL0, 32
    }
-	VAL0 = ~PAD_CFG0_TX_STATE & VAL0
+	VAL0 &= ~PAD_CFG0_TX_STATE
 }

/*
@@ -76,9 +76,8 @@
    {
    	VAL0, 32
    }
-	Local0 = VAL0
-	Local0 = ~PAD_CFG0_MODE_MASK & Local0
-	Arg1 = (Arg1 <<= PAD_CFG0_MODE_SHIFT) & PAD_CFG0_MODE_MASK
+	Local0 = ~PAD_CFG0_MODE_MASK & VAL0
+	Arg1 = (Arg1 << PAD_CFG0_MODE_SHIFT) & PAD_CFG0_MODE_MASK
    VAL0 = Local0 | Arg1
 }

@@ -98,9 +97,9 @@
    }

If (Arg1 == 1) {
-		VAL0 = ~PAD_CFG0_TX_DISABLE & VAL0
+		VAL0 &= ~PAD_CFG0_TX_DISABLE
    } ElseIf (Arg1 == 0){
-		VAL0 = PAD_CFG0_TX_DISABLE | VAL0
+		VAL0 &= PAD_CFG0_TX_DISABLE
    }
 }

@@ -120,8 +119,8 @@
    }

If (Arg1 == 1) {
-		VAL0 = ~PAD_CFG0_RX_DISABLE & VAL0
+		VAL0 &= ~PAD_CFG0_RX_DISABLE
    } ElseIf (Arg1 == 0){
-		VAL0 = PAD_CFG0_RX_DISABLE | VAL0
+		VAL0 |= PAD_CFG0_RX_DISABL
    }
 }
diff --git a/src/soc/intel/tigerlake/acpi/gpio_op.asl b/src/soc/intel/tigerlake/acpi/gpio_op.asl
index f7332aa..0da4696 100644
--- a/src/soc/intel/tigerlake/acpi/gpio_op.asl
+++ b/src/soc/intel/tigerlake/acpi/gpio_op.asl
@@ -43,7 +43,7 @@
    {
    	VAL0, 32
    }
-	VAL0 = PAD_CFG0_TX_STATE | VAL0
+	VAL0 |= PAD_CFG0_TX_STATE
 }

/*
@@ -57,7 +57,7 @@
    {
    	VAL0, 32
    }
-	VAL0 = ~PAD_CFG0_TX_STATE & VAL0
+	VAL0 &= ~PAD_CFG0_TX_STATE
 }

/*
@@ -97,9 +97,9 @@
    }

If (Arg1 == 1) {
-		VAL0 = ~PAD_CFG0_TX_DISABLE & VAL0
+		VAL0 &= ~PAD_CFG0_TX_DISABLE
    } ElseIf (Arg1 == 0){
-		VAL0 = PAD_CFG0_TX_DISABLE | VAL0
+		VAL0 &= PAD_CFG0_TX_DISABLE
    }
 }

@@ -119,8 +119,8 @@
    }

If (Arg1 == 1) {
-		VAL0 = ~PAD_CFG0_RX_DISABLE & VAL0
+		VAL0 &= ~PAD_CFG0_RX_DISABLE
    } ElseIf (Arg1 == 0){
-		VAL0 = PAD_CFG0_RX_DISABLE | VAL0
+		VAL0 |= PAD_CFG0_RX_DISABL
    }
 }

[Angel Pons (Code Review)]

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

Change subject: soc/intel/{jsl,tgl}: Refactor gpio_op.asl
......................................................................

Patch Set 1: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/45688/1/src/soc/intel/tigerlake/acp... 
File src/soc/intel/tigerlake/acpi/gpio_op.asl:

https://review.coreboot.org/c/coreboot/+/45688/1/src/soc/intel/tigerlake/acp... 
PS1, Line 124: PAD_CFG0_RX_DISABL
missing an `E`

[Subrata Banik (Code Review)]

Hello Elyes HAOUAS, build bot (Jenkins), Furquan Shaikh, Tim Wawrzynczak, Angel Pons, Patrick Rudolph,

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

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

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

Change subject: soc/intel/{jsl,tgl}: Refactor gpio_op.asl
......................................................................

soc/intel/{jsl,tgl}: Refactor gpio_op.asl

Also align GPMO ASL function with TGL.

Change-Id: Ia40af2cba9867838a1f99141481a5e78cffa0111
Signed-off-by: Subrata Banik 
---
M src/soc/intel/jasperlake/acpi/gpio_op.asl
M src/soc/intel/tigerlake/acpi/gpio_op.asl
2 files changed, 14 insertions(+), 15 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/88/45688/2

[Subrata Banik (Code Review)]

Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45688 )

Change subject: soc/intel/{jsl,tgl}: Refactor gpio_op.asl
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/45688/1/src/soc/intel/tigerlake/acp... 
File src/soc/intel/tigerlake/acpi/gpio_op.asl:

https://review.coreboot.org/c/coreboot/+/45688/1/src/soc/intel/tigerlake/acp... 
PS1, Line 124: PAD_CFG0_RX_DISABL
> missing an `E`

Ack

[HAOUAS Elyes (Code Review)]

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

Change subject: soc/intel/{jsl,tgl}: Refactor gpio_op.asl
......................................................................

Patch Set 2: Code-Review+2

[Angel Pons (Code Review)]

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

Change subject: soc/intel/{jsl,tgl}: Refactor gpio_op.asl
......................................................................

Patch Set 2: Code-Review+2

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45688 )

Change subject: soc/intel/{jsl,tgl}: Refactor gpio_op.asl
......................................................................

Patch Set 2: Code-Review+2

[Subrata Banik (Code Review)]

Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/45688 )

Change subject: soc/intel/{jsl,tgl}: Refactor gpio_op.asl
......................................................................

soc/intel/{jsl,tgl}: Refactor gpio_op.asl

Also align GPMO ASL function with TGL.

Change-Id: Ia40af2cba9867838a1f99141481a5e78cffa0111
Signed-off-by: Subrata Banik 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/45688
Tested-by: build bot (Jenkins) 
Reviewed-by: HAOUAS Elyes 
Reviewed-by: Angel Pons 
Reviewed-by: Furquan Shaikh 
---
M src/soc/intel/jasperlake/acpi/gpio_op.asl
M src/soc/intel/tigerlake/acpi/gpio_op.asl
2 files changed, 14 insertions(+), 15 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Furquan Shaikh: Looks good to me, approved
  HAOUAS Elyes: Looks good to me, approved
  Angel Pons: Looks good to me, approved

diff --git a/src/soc/intel/jasperlake/acpi/gpio_op.asl b/src/soc/intel/jasperlake/acpi/gpio_op.asl
index 683686f..9b9dc44 100644
--- a/src/soc/intel/jasperlake/acpi/gpio_op.asl
+++ b/src/soc/intel/jasperlake/acpi/gpio_op.asl
@@ -43,7 +43,7 @@
    {
    	VAL0, 32
    }
-	VAL0 = PAD_CFG0_TX_STATE | VAL0
+	VAL0 |= PAD_CFG0_TX_STATE
 }

/*
@@ -57,7 +57,7 @@
    {
    	VAL0, 32
    }
-	VAL0 = ~PAD_CFG0_TX_STATE & VAL0
+	VAL0 &= ~PAD_CFG0_TX_STATE
 }

/*
@@ -76,9 +76,8 @@
    {
    	VAL0, 32
    }
-	Local0 = VAL0
-	Local0 = ~PAD_CFG0_MODE_MASK & Local0
-	Arg1 = (Arg1 <<= PAD_CFG0_MODE_SHIFT) & PAD_CFG0_MODE_MASK
+	Local0 = ~PAD_CFG0_MODE_MASK & VAL0
+	Arg1 = (Arg1 << PAD_CFG0_MODE_SHIFT) & PAD_CFG0_MODE_MASK
    VAL0 = Local0 | Arg1
 }

@@ -98,9 +97,9 @@
    }

If (Arg1 == 1) {
-		VAL0 = ~PAD_CFG0_TX_DISABLE & VAL0
+		VAL0 &= ~PAD_CFG0_TX_DISABLE
    } ElseIf (Arg1 == 0){
-		VAL0 = PAD_CFG0_TX_DISABLE | VAL0
+		VAL0 &= PAD_CFG0_TX_DISABLE
    }
 }

@@ -120,8 +119,8 @@
    }

If (Arg1 == 1) {
-		VAL0 = ~PAD_CFG0_RX_DISABLE & VAL0
+		VAL0 &= ~PAD_CFG0_RX_DISABLE
    } ElseIf (Arg1 == 0){
-		VAL0 = PAD_CFG0_RX_DISABLE | VAL0
+		VAL0 |= PAD_CFG0_RX_DISABLE
    }
 }
diff --git a/src/soc/intel/tigerlake/acpi/gpio_op.asl b/src/soc/intel/tigerlake/acpi/gpio_op.asl
index f7332aa..9b9dc44 100644
--- a/src/soc/intel/tigerlake/acpi/gpio_op.asl
+++ b/src/soc/intel/tigerlake/acpi/gpio_op.asl
@@ -43,7 +43,7 @@
    {
    	VAL0, 32
    }
-	VAL0 = PAD_CFG0_TX_STATE | VAL0
+	VAL0 |= PAD_CFG0_TX_STATE
 }

/*
@@ -57,7 +57,7 @@
    {
    	VAL0, 32
    }
-	VAL0 = ~PAD_CFG0_TX_STATE & VAL0
+	VAL0 &= ~PAD_CFG0_TX_STATE
 }

/*
@@ -97,9 +97,9 @@
    }

If (Arg1 == 1) {
-		VAL0 = ~PAD_CFG0_TX_DISABLE & VAL0
+		VAL0 &= ~PAD_CFG0_TX_DISABLE
    } ElseIf (Arg1 == 0){
-		VAL0 = PAD_CFG0_TX_DISABLE | VAL0
+		VAL0 &= PAD_CFG0_TX_DISABLE
    }
 }

@@ -119,8 +119,8 @@
    }

If (Arg1 == 1) {
-		VAL0 = ~PAD_CFG0_RX_DISABLE & VAL0
+		VAL0 &= ~PAD_CFG0_RX_DISABLE
    } ElseIf (Arg1 == 0){
-		VAL0 = PAD_CFG0_RX_DISABLE | VAL0
+		VAL0 |= PAD_CFG0_RX_DISABLE
    }
 }