Change in ...coreboot[master]: mb/lenovo/s230u: Rewrite trigger inversion ACPI code

[Martin Roth (Code Review)]

Martin Roth has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32753

Change subject: mb/lenovo/s230u: Rewrite trigger inversion ACPI code
......................................................................

mb/lenovo/s230u: Rewrite trigger inversion ACPI code

The GPIO invert registers are already defined in the PCH code, so
just use the 8-bit versions of the registers instead of creating
a new GPIO field for the single bits.

This allows us to get rid of the Field(GPIO...) code that's causing
problems with IASL version 20190509.

Signed-off-by: Martin Roth 
Change-Id: Iac5dfb71b3a2b5a25c05a403cf5f403c7acecaaf
---
M src/mainboard/lenovo/s230u/acpi/gpe.asl
1 file changed, 12 insertions(+), 11 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/53/32753/1

diff --git a/src/mainboard/lenovo/s230u/acpi/gpe.asl b/src/mainboard/lenovo/s230u/acpi/gpe.asl
index a69f562..41540a1 100644
--- a/src/mainboard/lenovo/s230u/acpi/gpe.asl
+++ b/src/mainboard/lenovo/s230u/acpi/gpe.asl
@@ -16,15 +16,6 @@

Scope (_GPE)
 {
-	Field(GPIO, ByteAcc, NoLock, Preserve)
-	{
-		Offset(0x2c),	// GPIO Invert
-		    ,   2,
-		GV02,   1,
-		    ,   1,
-		GV04,   1,
-	}
-
    Name (PDET, Zero)
    Method (PNOT, 2, Serialized) {
    	ShiftLeft (Arg0, Arg1, Local0)
@@ -39,10 +30,20 @@
    	}
    }

+	Method (TINV, 2, Serialized) {
+		ShiftLeft (One, Arg1, Local0)
+		If (LEqual (Arg0, Zero)) {
+			Not (Local0, Local0)
+			And (GIV0, local0, GIV0)
+		} Else {
+			Or (GIV0, local0, GIV0)
+		}
+	}
+
    /* Palm detect sensor 1 */
    Method (_L12, 0, NotSerialized) {
    	// Invert trigger
-		Store(GP02, GV02)
+		TINV(GP02, 2)

PNOT (GP02, 0)
    }
@@ -50,7 +51,7 @@
    /* Palm detect sensor 2 */
    Method (_L14, 0, NotSerialized) {
    	// Invert trigger
-		Store(GP04, GV04)
+		TINV(GP04, 4)

PNOT (GP04, 1)
    }

[Martin Roth (Code Review)]

Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32753 )

Change subject: mb/lenovo/s230u: Rewrite trigger inversion ACPI code
......................................................................

Patch Set 1:

This is just build tested.

[HAOUAS Elyes (Code Review)]

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

Change subject: mb/lenovo/s230u: Rewrite trigger inversion ACPI code
......................................................................

Patch Set 1: Code-Review+2

[Arthur Heymans (Code Review)]

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

Change subject: mb/lenovo/s230u: Rewrite trigger inversion ACPI code
......................................................................

Patch Set 2:

(1 comment)

I find it rather weird to have the hardware invert the value of the gpio input, instead of relying on software to deal with it...

https://review.coreboot.org/#/c/32753/2/src/mainboard/lenovo/s230u/acpi/gpe.... 
File src/mainboard/lenovo/s230u/acpi/gpe.asl:

https://review.coreboot.org/#/c/32753/2/src/mainboard/lenovo/s230u/acpi/gpe.... 
PS2, Line 46: (
space needed.

[Martin Roth (Code Review)]

Hello Alexander Couzens, Patrick Rudolph, HAOUAS Elyes, Tobias Diedrich, build bot (Jenkins),

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

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

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

Change subject: mb/lenovo/s230u: Rewrite trigger inversion ACPI code
......................................................................

mb/lenovo/s230u: Rewrite trigger inversion ACPI code

The GPIO invert registers are already defined in the PCH code, so
just use the 8-bit versions of the registers instead of creating
a new GPIO field for the single bits.

This allows us to get rid of the Field(GPIO...) code that's causing
problems with IASL version 20190509.

Signed-off-by: Martin Roth 
Change-Id: Iac5dfb71b3a2b5a25c05a403cf5f403c7acecaaf
---
M src/mainboard/lenovo/s230u/acpi/gpe.asl
1 file changed, 12 insertions(+), 11 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/53/32753/3

[Martin Roth (Code Review)]

Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32753 )

Change subject: mb/lenovo/s230u: Rewrite trigger inversion ACPI code
......................................................................

Patch Set 3:
> (1 comment)
>
> I find it rather weird to have the hardware invert the value of the
> gpio input, instead of relying on software to deal with it...

I'd have to go look at the datasheet, but I think this is just switching the level that it triggers on.  It reads the GPIO value and reverses the trigger accordingly.

[HAOUAS Elyes (Code Review)]

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

Change subject: mb/lenovo/s230u: Rewrite trigger inversion ACPI code
......................................................................

Patch Set 3: Code-Review+2

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32753 )

Change subject: mb/lenovo/s230u: Rewrite trigger inversion ACPI code
......................................................................

mb/lenovo/s230u: Rewrite trigger inversion ACPI code

The GPIO invert registers are already defined in the PCH code, so
just use the 8-bit versions of the registers instead of creating
a new GPIO field for the single bits.

This allows us to get rid of the Field(GPIO...) code that's causing
problems with IASL version 20190509.

Signed-off-by: Martin Roth 
Change-Id: Iac5dfb71b3a2b5a25c05a403cf5f403c7acecaaf
Reviewed-on: https://review.coreboot.org/c/coreboot/+/32753
Tested-by: build bot (Jenkins) 
Reviewed-by: HAOUAS Elyes 
---
M src/mainboard/lenovo/s230u/acpi/gpe.asl
1 file changed, 12 insertions(+), 11 deletions(-)

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

diff --git a/src/mainboard/lenovo/s230u/acpi/gpe.asl b/src/mainboard/lenovo/s230u/acpi/gpe.asl
index a69f562..e6f4153 100644
--- a/src/mainboard/lenovo/s230u/acpi/gpe.asl
+++ b/src/mainboard/lenovo/s230u/acpi/gpe.asl
@@ -16,15 +16,6 @@

Scope (_GPE)
 {
-	Field(GPIO, ByteAcc, NoLock, Preserve)
-	{
-		Offset(0x2c),	// GPIO Invert
-		    ,   2,
-		GV02,   1,
-		    ,   1,
-		GV04,   1,
-	}
-
    Name (PDET, Zero)
    Method (PNOT, 2, Serialized) {
    	ShiftLeft (Arg0, Arg1, Local0)
@@ -39,10 +30,20 @@
    	}
    }

+	Method (TINV, 2, Serialized) {
+		ShiftLeft (One, Arg1, Local0)
+		If (LEqual (Arg0, Zero)) {
+			Not (Local0, Local0)
+			And (GIV0, Local0, GIV0)
+		} Else {
+			Or (GIV0, Local0, GIV0)
+		}
+	}
+
    /* Palm detect sensor 1 */
    Method (_L12, 0, NotSerialized) {
    	// Invert trigger
-		Store(GP02, GV02)
+		TINV (GP02, 2)

PNOT (GP02, 0)
    }
@@ -50,7 +51,7 @@
    /* Palm detect sensor 2 */
    Method (_L14, 0, NotSerialized) {
    	// Invert trigger
-		Store(GP04, GV04)
+		TINV (GP04, 4)

PNOT (GP04, 1)
    }

[Tobias Diedrich (Code Review)]

Tobias Diedrich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32753 )

Change subject: mb/lenovo/s230u: Rewrite trigger inversion ACPI code
......................................................................

Patch Set 4: Code-Review+2

Seems to work fine on my S230U.