Change in coreboot[master]: soc/intel/tigerlake: Add Asl support for GPIO

[Shaunak Saha (Code Review)]

Shaunak Saha has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38442 )

Change subject: soc/intel/tigerlake: Add Asl support for GPIO
......................................................................

soc/intel/tigerlake: Add Asl support for GPIO

This patch adds asl methods like GRXS, GTXS, STXS
and CTXS which are used to get,set and clear gpio
values. We followed ASL 2.0 syntax here.

UG=b:144680462
BRANCH=none
TEST=Build and boot tigerlake rvp board

Change-Id: I17e75ff2a7cb67e94669059a1ed9d73a720ebcb1
Signed-off-by: Shaunak Saha 
---
M src/soc/intel/tigerlake/acpi/gpio.asl
1 file changed, 45 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/38442/1

diff --git a/src/soc/intel/tigerlake/acpi/gpio.asl b/src/soc/intel/tigerlake/acpi/gpio.asl
index 5471ea7..c3b16a8 100644
--- a/src/soc/intel/tigerlake/acpi/gpio.asl
+++ b/src/soc/intel/tigerlake/acpi/gpio.asl
@@ -163,7 +163,51 @@
    {
    	VAL0, 32
    }
-	And (GPIORXSTATE_MASK, ShiftRight (VAL0, GPIORXSTATE_SHIFT), Local0)
+	Local0 = GPIORXSTATE_MASK & ( VAL0 >> GPIORXSTATE_SHIFT )

Return (Local0)
 }
+
+/*
+ * Get GPIO Tx Value
+ * Arg0 - GPIO Number
+ */
+Method (GTXS, 1, Serialized)
+{
+	OperationRegion (PREG, SystemMemory, GADD (Arg0), 4)
+	Field (PREG, AnyAcc, NoLock, Preserve)
+	{
+		VAL0, 32
+	}
+	Local0 = GPIOTXSTATE_MASK & VAL0
+
+	Return (Local0)
+}
+
+/*
+ * Set GPIO Tx Value
+ * Arg0 - GPIO Number
+ */
+Method (STXS, 1, Serialized)
+{
+	OperationRegion (PREG, SystemMemory, GADD (Arg0), 4)
+	Field (PREG, AnyAcc, NoLock, Preserve)
+	{
+		VAL0, 32
+	}
+	VAL0 |= GPIOTXSTATE_MASK
+}
+
+/*
+ * Clear GPIO Tx Value
+ * Arg0 - GPIO Number
+ */
+Method (CTXS, 1, Serialized)
+{
+	OperationRegion (PREG, SystemMemory, GADD (Arg0), 4)
+	Field (PREG, AnyAcc, NoLock, Preserve)
+	{
+		VAL0, 32
+	}
+	VAL0 &= ~GPIOTXSTATE_MASK
+}

[Nick Vaccaro (Code Review)]

Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38442 )

Change subject: soc/intel/tigerlake: Add Asl support for GPIO
......................................................................

Patch Set 1: Code-Review+1

(2 comments)

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

https://review.coreboot.org/c/coreboot/+/38442/1/src/soc/intel/tigerlake/acp... 
PS1, Line 166:  
remove space

https://review.coreboot.org/c/coreboot/+/38442/1/src/soc/intel/tigerlake/acp... 
PS1, Line 166:  
remove space

[Shaunak Saha (Code Review)]

Shaunak Saha has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38442 )

Change subject: soc/intel/tigerlake: Add Asl support for GPIO
......................................................................

Patch Set 1:

(2 comments)

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

https://review.coreboot.org/c/coreboot/+/38442/1/src/soc/intel/tigerlake/acp... 
PS1, Line 166:
> remove space

Ack

https://review.coreboot.org/c/coreboot/+/38442/1/src/soc/intel/tigerlake/acp... 
PS1, Line 166:
> remove space

Ack

[Shaunak Saha (Code Review)]

Shaunak Saha has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38442 )

Change subject: soc/intel/tigerlake: Add Asl support for GPIO
......................................................................

Patch Set 2:

This change is ready for review.

[Shaunak Saha (Code Review)]

Shaunak Saha has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38442 )

Change subject: soc/intel/tigerlake: Add Asl support for GPIO
......................................................................

Patch Set 2:

Furquan,Nick. I was trying to test the methods here. I tried this method:
1. copy acpiexec binary inside the target.
2. run the dsdt file through acpiexec: "acpiexec /sys/firmware/acpi/tables/DSDT"
3. I can see the asl methods when i ran methods command in acpiexec.
4. For the method GADD it worked fine and gave me the value of DW0. Command is:
- Evaluate _SB.PCI0.GADD 0(for gpio 0). it gave the proper address 00000000FD6E0700
5. but when i tried to evaluate  GRXS,GTXS i always gets 0 for any gpio number i give.

So my question is for those methods GRXS, GTXS we should not be geting the gpio values from asl directly. Those needs to be called from acpigen only?

[Furquan Shaikh (Code Review)]

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

Change subject: soc/intel/tigerlake: Add Asl support for GPIO
......................................................................

Patch Set 2:
> but when i tried to evaluate  GRXS,GTXS i always gets 0 for any gpio number i give.

Did it match the GPIO Tx Rx condition on the physical line as well?
> So my question is for those methods GRXS, GTXS we should not be geting the gpio values from asl directly. Those needs to be called from acpigen only?

Do you mean if GRXS and GTXS are called from within acpigen only? Currently, that is the case because of the use cases for these methods. But, it there is a new use case requiring the method calls in .asl, that will be required.

[Shaunak Saha (Code Review)]

Hello Srinidhi N Kaushik, Raj Astekar, Patrick Rudolph, Nick Vaccaro, Wonkyu Kim, Caveh Jalali, build bot (Jenkins), Furquan Shaikh,

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

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

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

Change subject: soc/intel/tigerlake: Add Asl support for GPIO
......................................................................

soc/intel/tigerlake: Add Asl support for GPIO

This patch adds asl methods like GRXS, GTXS, STXS
and CTXS which are used to get,set and clear gpio
values. We followed ASL 2.0 syntax here.

BUG=b:144680462
BRANCH=none
TEST=Build and boot tigerlake rvp board

Change-Id: I17e75ff2a7cb67e94669059a1ed9d73a720ebcb1
Signed-off-by: Shaunak Saha 
---
M src/soc/intel/tigerlake/acpi/gpio.asl
1 file changed, 45 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/38442/4

[Shaunak Saha (Code Review)]

Hello Srinidhi N Kaushik, Raj Astekar, Patrick Rudolph, Nick Vaccaro, Wonkyu Kim, Caveh Jalali, build bot (Jenkins), Furquan Shaikh,

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

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

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

Change subject: soc/intel/tigerlake: Add Asl support for GPIO
......................................................................

soc/intel/tigerlake: Add Asl support for GPIO

This patch adds asl methods like GRXS, GTXS, STXS
and CTXS which are used to get,set and clear gpio
values. We followed ASL 2.0 syntax here.

UG=b:144680462
BRANCH=none
TEST=Build and boot tigerlake rvp board

Change-Id: I17e75ff2a7cb67e94669059a1ed9d73a720ebcb1
Signed-off-by: Shaunak Saha 
---
M src/soc/intel/tigerlake/acpi/gpio.asl
1 file changed, 45 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/38442/5

[Shaunak Saha (Code Review)]

Hello Srinidhi N Kaushik, Raj Astekar, Patrick Rudolph, Nick Vaccaro, Wonkyu Kim, Caveh Jalali, build bot (Jenkins), Furquan Shaikh,

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

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

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

Change subject: soc/intel/tigerlake: Add Asl support for GPIO
......................................................................

soc/intel/tigerlake: Add Asl support for GPIO

This patch adds asl methods like GRXS, GTXS, STXS
and CTXS which are used to get,set and clear gpio
values. We followed ASL 2.0 syntax here.

BUG=b:144680462
BRANCH=none
TEST=Build and boot tigerlake rvp board

Change-Id: I17e75ff2a7cb67e94669059a1ed9d73a720ebcb1
Signed-off-by: Shaunak Saha 
---
M src/soc/intel/tigerlake/acpi/gpio.asl
1 file changed, 45 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/38442/6

[Shaunak Saha (Code Review)]

Hello Srinidhi N Kaushik, Raj Astekar, Patrick Rudolph, Nick Vaccaro, Wonkyu Kim, Caveh Jalali, build bot (Jenkins), Furquan Shaikh,

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

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

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

Change subject: soc/intel/tigerlake: Add Asl support for GPIO
......................................................................

soc/intel/tigerlake: Add Asl support for GPIO

This patch adds asl methods like GRXS, GTXS, STXS
and CTXS which are used to get,set and clear gpio
values. We followed ASL 2.0 syntax here.

BUG=b:144680462
BRANCH=none
TEST=Build and boot tigerlake rvp board

Change-Id: I17e75ff2a7cb67e94669059a1ed9d73a720ebcb1
Signed-off-by: Shaunak Saha 
---
M src/soc/intel/tigerlake/acpi/gpio.asl
1 file changed, 45 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/38442/9

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38442 )

Change subject: soc/intel/tigerlake: Add Asl support for GPIO
......................................................................

Patch Set 9:

(5 comments)

https://review.coreboot.org/c/coreboot/+/38442/9//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/38442/9//COMMIT_MSG@7 
PS9, Line 7: Asl support
… ASL code …

https://review.coreboot.org/c/coreboot/+/38442/9//COMMIT_MSG@9 
PS9, Line 9: asl
ASL

https://review.coreboot.org/c/coreboot/+/38442/9//COMMIT_MSG@10 
PS9, Line 10: get,set
Space after ,.

https://review.coreboot.org/c/coreboot/+/38442/9//COMMIT_MSG@11 
PS9, Line 11: followed
use

https://review.coreboot.org/c/coreboot/+/38442/9//COMMIT_MSG@11 
PS9, Line 11: values. We followed ASL 2.0 syntax here.
Line length is 75 characters. Please re-flow.

[Shaunak Saha (Code Review)]

Shaunak Saha has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38442 )

Change subject: soc/intel/tigerlake: Add Asl support for GPIO
......................................................................

Patch Set 10:

(5 comments)

https://review.coreboot.org/c/coreboot/+/38442/9//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/38442/9//COMMIT_MSG@7 
PS9, Line 7: Asl support
> … ASL code …

Ack

https://review.coreboot.org/c/coreboot/+/38442/9//COMMIT_MSG@9 
PS9, Line 9: asl
> ASL

Ack

https://review.coreboot.org/c/coreboot/+/38442/9//COMMIT_MSG@10 
PS9, Line 10: get,set
> Space after ,.

Ack

https://review.coreboot.org/c/coreboot/+/38442/9//COMMIT_MSG@11 
PS9, Line 11: followed
> use

Ack

https://review.coreboot.org/c/coreboot/+/38442/9//COMMIT_MSG@11 
PS9, Line 11: values. We followed ASL 2.0 syntax here.
> Line length is 75 characters. Please re-flow.

Ack

Change in coreboot[master]: soc/intel/tigerlake: Add ASL code for GPIO

[Shaunak Saha (Code Review)]

Hello Srinidhi N Kaushik, Raj Astekar, Patrick Rudolph, Nick Vaccaro, Wonkyu Kim, Caveh Jalali, build bot (Jenkins), Furquan Shaikh,

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

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

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

Change subject: soc/intel/tigerlake: Add ASL code for GPIO
......................................................................

soc/intel/tigerlake: Add ASL code for GPIO

This patch adds ASL methods like GRXS, GTXS, STXS
and CTXS which are used to get, set and clear gpio
values. We use ASL 2.0 syntax here for gpio.asl.

BUG=b:144680462
BRANCH=none
TEST=Build and boot tigerlake rvp board

Change-Id: I17e75ff2a7cb67e94669059a1ed9d73a720ebcb1
Signed-off-by: Shaunak Saha 
---
M src/soc/intel/tigerlake/acpi/gpio.asl
1 file changed, 45 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/38442/11

Change in coreboot[master]: soc/intel/tigerlake: Add ASL code for GPIO

[Shaunak Saha (Code Review)]

Hello Srinidhi N Kaushik, Raj Astekar, Patrick Rudolph, Nick Vaccaro, Wonkyu Kim, caveh jalali, build bot (Jenkins), Furquan Shaikh,

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

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

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

Change subject: soc/intel/tigerlake: Add ASL code for GPIO
......................................................................

soc/intel/tigerlake: Add ASL code for GPIO

iThis patch adds ASL methods like GRXS, GTXS, STXS and CTXS
which are used to get, set and clear gpio values. We use
ASL 2.0 syntax here for gpio.asl.

BUG=b:144680462
BRANCH=none
TEST=Build and boot tigerlake rvp board

Change-Id: I17e75ff2a7cb67e94669059a1ed9d73a720ebcb1
Signed-off-by: Shaunak Saha 
---
M src/soc/intel/tigerlake/acpi/gpio.asl
1 file changed, 45 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/38442/12

Change in coreboot[master]: soc/intel/tigerlake: Add ASL code for GPIO

[Shaunak Saha (Code Review)]

Shaunak Saha has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38442 )

Change subject: soc/intel/tigerlake: Add ASL code for GPIO
......................................................................

Patch Set 12: Code-Review+1

Change in coreboot[master]: soc/intel/tigerlake: Add ASL code for GPIO

[Wonkyu Kim (Code Review)]

Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38442 )

Change subject: soc/intel/tigerlake: Add ASL code for GPIO
......................................................................

Patch Set 12: Code-Review+1

Change in coreboot[master]: soc/intel/tigerlake: Add ASL code for GPIO

[Subrata Banik (Code Review)]

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

Change subject: soc/intel/tigerlake: Add ASL code for GPIO
......................................................................

Patch Set 12:

(2 comments)

https://review.coreboot.org/c/coreboot/+/38442/12//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/38442/12//COMMIT_MSG@7 
PS12, Line 7: Add ASL code for GPIO
Add GPIO helper function

https://review.coreboot.org/c/coreboot/+/38442/12//COMMIT_MSG@9 
PS12, Line 9: iThis 
This

Change in coreboot[master]: soc/intel/tigerlake: Add ASL code for GPIO

[Shaunak Saha (Code Review)]

Shaunak Saha has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38442 )

Change subject: soc/intel/tigerlake: Add ASL code for GPIO
......................................................................

Patch Set 12:

(2 comments)

https://review.coreboot.org/c/coreboot/+/38442/12//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/38442/12//COMMIT_MSG@7 
PS12, Line 7: Add ASL code for GPIO
> Add GPIO helper function

Ack

https://review.coreboot.org/c/coreboot/+/38442/12//COMMIT_MSG@9 
PS12, Line 9: iThis
> This

Ack

Change in coreboot[master]: soc/intel/tigerlake: Add GPIO helper function

[Shaunak Saha (Code Review)]

Hello Srinidhi N Kaushik, Raj Astekar, Patrick Rudolph, Subrata Banik, Nick Vaccaro, Aamir Bohra, Wonkyu Kim, Maulik V Vaghela, caveh jalali, build bot (Jenkins), Furquan Shaikh,

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

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

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

Change subject: soc/intel/tigerlake: Add GPIO helper function
......................................................................

soc/intel/tigerlake: Add GPIO helper function

This patch adds ASL methods like GRXS, GTXS, STXS and CTXS
which are used to get, set and clear gpio values. We use
ASL 2.0 syntax here for gpio.asl.

BUG=b:144680462
BRANCH=none
TEST=Build and boot tigerlake rvp board

Change-Id: I17e75ff2a7cb67e94669059a1ed9d73a720ebcb1
Signed-off-by: Shaunak Saha 
---
M src/soc/intel/tigerlake/acpi/gpio.asl
1 file changed, 45 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/38442/13

Change in coreboot[master]: soc/intel/tigerlake: Add GPIO helper function

[Subrata Banik (Code Review)]

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

Change subject: soc/intel/tigerlake: Add GPIO helper function
......................................................................

Patch Set 13: Code-Review+2

Change in coreboot[master]: soc/intel/tigerlake: Add GPIO helper function

[Nick Vaccaro (Code Review)]

Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38442 )

Change subject: soc/intel/tigerlake: Add GPIO helper function
......................................................................

Patch Set 13: Code-Review+2

Change in coreboot[master]: soc/intel/tigerlake: Add GPIO helper function

[Wonkyu Kim (Code Review)]

Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38442 )

Change subject: soc/intel/tigerlake: Add GPIO helper function
......................................................................

Patch Set 13: Code-Review+2

Change in coreboot[master]: soc/intel/tigerlake: Add GPIO helper function

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/38442 )

Change subject: soc/intel/tigerlake: Add GPIO helper function
......................................................................

soc/intel/tigerlake: Add GPIO helper function

This patch adds ASL methods like GRXS, GTXS, STXS and CTXS
which are used to get, set and clear gpio values. We use
ASL 2.0 syntax here for gpio.asl.

BUG=b:144680462
BRANCH=none
TEST=Build and boot tigerlake rvp board

Change-Id: I17e75ff2a7cb67e94669059a1ed9d73a720ebcb1
Signed-off-by: Shaunak Saha 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/38442
Reviewed-by: Subrata Banik 
Reviewed-by: Nick Vaccaro 
Reviewed-by: Wonkyu Kim 
Tested-by: build bot (Jenkins) 
---
M src/soc/intel/tigerlake/acpi/gpio.asl
1 file changed, 45 insertions(+), 1 deletion(-)

Approvals:
  build bot (Jenkins): Verified
  Subrata Banik: Looks good to me, approved
  Nick Vaccaro: Looks good to me, approved
  Wonkyu Kim: Looks good to me, approved

diff --git a/src/soc/intel/tigerlake/acpi/gpio.asl b/src/soc/intel/tigerlake/acpi/gpio.asl
index dc76a38..f6cccfb 100644
--- a/src/soc/intel/tigerlake/acpi/gpio.asl
+++ b/src/soc/intel/tigerlake/acpi/gpio.asl
@@ -163,7 +163,51 @@
    {
    	VAL0, 32
    }
-	And (GPIORXSTATE_MASK, ShiftRight (VAL0, GPIORXSTATE_SHIFT), Local0)
+	Local0 = GPIORXSTATE_MASK & (VAL0 >> GPIORXSTATE_SHIFT)

Return (Local0)
 }
+
+/*
+ * Get GPIO Tx Value
+ * Arg0 - GPIO Number
+ */
+Method (GTXS, 1, Serialized)
+{
+	OperationRegion (PREG, SystemMemory, GADD (Arg0), 4)
+	Field (PREG, AnyAcc, NoLock, Preserve)
+	{
+		VAL0, 32
+	}
+	Local0 = GPIOTXSTATE_MASK & VAL0
+
+	Return (Local0)
+}
+
+/*
+ * Set GPIO Tx Value
+ * Arg0 - GPIO Number
+ */
+Method (STXS, 1, Serialized)
+{
+	OperationRegion (PREG, SystemMemory, GADD (Arg0), 4)
+	Field (PREG, AnyAcc, NoLock, Preserve)
+	{
+		VAL0, 32
+	}
+	VAL0 |= GPIOTXSTATE_MASK
+}
+
+/*
+ * Clear GPIO Tx Value
+ * Arg0 - GPIO Number
+ */
+Method (CTXS, 1, Serialized)
+{
+	OperationRegion (PREG, SystemMemory, GADD (Arg0), 4)
+	Field (PREG, AnyAcc, NoLock, Preserve)
+	{
+		VAL0, 32
+	}
+	VAL0 &= ~GPIOTXSTATE_MASK
+}

Change in coreboot[master]: soc/intel/tigerlake: Add GPIO helper function

[9elements QA (Code Review)]

9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38442 )

Change subject: soc/intel/tigerlake: Add GPIO helper function
......................................................................

Patch Set 15:

Automatic boot test returned (PASS/FAIL/TOTAL): 3/0/3
Emulation targets:
EMULATION_QEMU_X86_Q35 using payload TianoCore : SUCCESS : https://lava.9esec.io/r/269
EMULATION_QEMU_X86_Q35 using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/268
EMULATION_QEMU_X86_I440FX using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/267

Please note: This test is under development and might not be accurate at all!