Change in coreboot[master]: soc/intel: Fix GRXS function to get GPIO number proper

[Subrata Banik (Code Review)]

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

Change subject: soc/intel: Fix GRXS function to get GPIO number proper
......................................................................

soc/intel: Fix GRXS function to get GPIO number proper

This patch ensures that GRXS perform PAD_CFG0_RX_STATE
mask first and then right shift PAD_CFG0_RX_STATE_BIT to
get correct GPIO number.

Change-Id: I96611936f70f79e9dc5ee9414ec68cef00d0d13a
Signed-off-by: Subrata Banik 
---
M src/soc/intel/cannonlake/acpi/gpio_op.asl
M src/soc/intel/icelake/acpi/gpio.asl
M src/soc/intel/jasperlake/acpi/gpio_op.asl
M src/soc/intel/skylake/acpi/gpio.asl
M src/soc/intel/tigerlake/acpi/gpio_op.asl
5 files changed, 5 insertions(+), 5 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/45738/1

diff --git a/src/soc/intel/cannonlake/acpi/gpio_op.asl b/src/soc/intel/cannonlake/acpi/gpio_op.asl
index 7f2a40c..98a4f24 100644
--- a/src/soc/intel/cannonlake/acpi/gpio_op.asl
+++ b/src/soc/intel/cannonlake/acpi/gpio_op.asl
@@ -11,7 +11,7 @@
    {
    	VAL0, 32
    }
-	Local0 = GPIORXSTATE_MASK & (VAL0 >> GPIORXSTATE_SHIFT)
+	Local0 = (GPIORXSTATE_MASK & VAL0) >> GPIORXSTATE_SHIFT

Return (Local0)
 }
diff --git a/src/soc/intel/icelake/acpi/gpio.asl b/src/soc/intel/icelake/acpi/gpio.asl
index f0a6fa0..71acb75 100644
--- a/src/soc/intel/icelake/acpi/gpio.asl
+++ b/src/soc/intel/icelake/acpi/gpio.asl
@@ -114,7 +114,7 @@
    {
    	VAL0, 32
    }
-	Local0 = GPIORXSTATE_MASK & (VAL0 >> GPIORXSTATE_SHIFT)
+	Local0 = (GPIORXSTATE_MASK & VAL0) >> GPIORXSTATE_SHIFT

Return (Local0)
 }
diff --git a/src/soc/intel/jasperlake/acpi/gpio_op.asl b/src/soc/intel/jasperlake/acpi/gpio_op.asl
index 9b9dc44..ff12da4 100644
--- a/src/soc/intel/jasperlake/acpi/gpio_op.asl
+++ b/src/soc/intel/jasperlake/acpi/gpio_op.asl
@@ -11,7 +11,7 @@
    {
    	VAL0, 32
    }
-	Local0 = PAD_CFG0_RX_STATE & (VAL0 >> PAD_CFG0_RX_STATE_BIT)
+	Local0 = (PAD_CFG0_RX_STATE & VAL0) >> PAD_CFG0_RX_STATE_BIT

Return (Local0)
 }
diff --git a/src/soc/intel/skylake/acpi/gpio.asl b/src/soc/intel/skylake/acpi/gpio.asl
index de6ff42..f83069f 100644
--- a/src/soc/intel/skylake/acpi/gpio.asl
+++ b/src/soc/intel/skylake/acpi/gpio.asl
@@ -119,7 +119,7 @@
    {
    	VAL0, 32
    }
-	Local0 = GPIORXSTATE_MASK & (VAL0 >> PAD_CFG0_RX_STATE_BIT)
+	Local0 = (GPIORXSTATE_MASK & VAL0) >> PAD_CFG0_RX_STATE_BIT

Return (Local0)
 }
diff --git a/src/soc/intel/tigerlake/acpi/gpio_op.asl b/src/soc/intel/tigerlake/acpi/gpio_op.asl
index 9b9dc44..ff12da4 100644
--- a/src/soc/intel/tigerlake/acpi/gpio_op.asl
+++ b/src/soc/intel/tigerlake/acpi/gpio_op.asl
@@ -11,7 +11,7 @@
    {
    	VAL0, 32
    }
-	Local0 = PAD_CFG0_RX_STATE & (VAL0 >> PAD_CFG0_RX_STATE_BIT)
+	Local0 = (PAD_CFG0_RX_STATE & VAL0) >> PAD_CFG0_RX_STATE_BIT

Return (Local0)
 }

[Tim Wawrzynczak (Code Review)]

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45738 )

Change subject: soc/intel: Fix GRXS function to get GPIO number proper
......................................................................

Patch Set 1: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/45738/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/45738/1//COMMIT_MSG@9 
PS1, Line 9: This patch ensures that GRXS perform PAD_CFG0_RX_STATE
           : mask first and then right shift PAD_CFG0_RX_STATE_BIT to
           : get correct GPIO number.
72 chars wide

[Subrata Banik (Code Review)]

Hello 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/+/45738

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

Change subject: soc/intel: Fix GRXS function to get GPIO number proper
......................................................................

soc/intel: Fix GRXS function to get GPIO number proper

This patch ensures that GRXS perform PAD_CFG0_RX_STATE mask first
and then right shift PAD_CFG0_RX_STATE_BIT to get correct GPIO number.

Change-Id: I96611936f70f79e9dc5ee9414ec68cef00d0d13a
Signed-off-by: Subrata Banik 
---
M src/soc/intel/cannonlake/acpi/gpio_op.asl
M src/soc/intel/icelake/acpi/gpio.asl
M src/soc/intel/jasperlake/acpi/gpio_op.asl
M src/soc/intel/skylake/acpi/gpio.asl
M src/soc/intel/tigerlake/acpi/gpio_op.asl
5 files changed, 5 insertions(+), 5 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/45738/2

[Subrata Banik (Code Review)]

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

Change subject: soc/intel: Fix GRXS function to get GPIO number proper
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/45738/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/45738/1//COMMIT_MSG@9 
PS1, Line 9: This patch ensures that GRXS perform PAD_CFG0_RX_STATE
           : mask first and then right shift PAD_CFG0_RX_STATE_BIT to
           : get correct GPIO number.
> 72 chars wide

Ack

[Furquan Shaikh (Code Review)]

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

Change subject: soc/intel: Fix GRXS function to get GPIO number proper
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/45738/2/src/soc/intel/cannonlake/ac... 
File src/soc/intel/cannonlake/acpi/gpio_op.asl:

https://review.coreboot.org/c/coreboot/+/45738/2/src/soc/intel/cannonlake/ac... 
PS2, Line 14:  (GPIORXSTATE_MASK & VAL0) >> GPIORXSTATE_SHIFT
No, this is not correct. The bug was only in TGL and JSL which are using the common code macro PAD_CFG0_RX_STATE.

GPIORXSTATE_MASK is defined differently and so what we had before here is correct.

[Tim Wawrzynczak (Code Review)]

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45738 )

Change subject: soc/intel: Fix GRXS function to get GPIO number proper
......................................................................

Patch Set 2: -Code-Review

... and that's why this needs to get fixed 😄

Change in coreboot[master]: soc/intel/{jsl,tgl}: Fix GRXS function to get GPIO number proper

[Subrata Banik (Code Review)]

Hello 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/+/45738

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

Change subject: soc/intel/{jsl,tgl}: Fix GRXS function to get GPIO number proper
......................................................................

soc/intel/{jsl,tgl}: Fix GRXS function to get GPIO number proper

This patch ensures that GRXS perform PAD_CFG0_RX_STATE mask first
and then right shift PAD_CFG0_RX_STATE_BIT to get correct GPIO number.

Change-Id: I96611936f70f79e9dc5ee9414ec68cef00d0d13a
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, 2 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/45738/3

Change in coreboot[master]: soc/intel/{jsl,tgl}: Fix GRXS function to get GPIO number proper

[Furquan Shaikh (Code Review)]

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

Change subject: soc/intel/{jsl,tgl}: Fix GRXS function to get GPIO number proper
......................................................................

Patch Set 3: Code-Review+2

Change in coreboot[master]: soc/intel/{jsl,tgl}: Fix GRXS function to get GPIO number proper

[Subrata Banik (Code Review)]

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

Change subject: soc/intel/{jsl,tgl}: Fix GRXS function to get GPIO number proper
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/45738/2/src/soc/intel/cannonlake/ac... 
File src/soc/intel/cannonlake/acpi/gpio_op.asl:

https://review.coreboot.org/c/coreboot/+/45738/2/src/soc/intel/cannonlake/ac... 
PS2, Line 14:  (GPIORXSTATE_MASK & VAL0) >> GPIORXSTATE_SHIFT
> No, this is not correct. […]

looks like i got carried away 🙏

Change in coreboot[master]: soc/intel/{jsl,tgl}: Fix GRXS function to get GPIO number proper

[Angel Pons (Code Review)]

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

Change subject: soc/intel/{jsl,tgl}: Fix GRXS function to get GPIO number proper
......................................................................

Patch Set 3: Code-Review+2

Change in coreboot[master]: soc/intel/{jsl,tgl}: Fix GRXS function to get GPIO number proper

[Subrata Banik (Code Review)]

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

Change subject: soc/intel/{jsl,tgl}: Fix GRXS function to get GPIO number proper
......................................................................

soc/intel/{jsl,tgl}: Fix GRXS function to get GPIO number proper

This patch ensures that GRXS perform PAD_CFG0_RX_STATE mask first
and then right shift PAD_CFG0_RX_STATE_BIT to get correct GPIO number.

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

Approvals:
  build bot (Jenkins): Verified
  Furquan Shaikh: 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 9b9dc44..ff12da4 100644
--- a/src/soc/intel/jasperlake/acpi/gpio_op.asl
+++ b/src/soc/intel/jasperlake/acpi/gpio_op.asl
@@ -11,7 +11,7 @@
    {
    	VAL0, 32
    }
-	Local0 = PAD_CFG0_RX_STATE & (VAL0 >> PAD_CFG0_RX_STATE_BIT)
+	Local0 = (PAD_CFG0_RX_STATE & VAL0) >> PAD_CFG0_RX_STATE_BIT

Return (Local0)
 }
diff --git a/src/soc/intel/tigerlake/acpi/gpio_op.asl b/src/soc/intel/tigerlake/acpi/gpio_op.asl
index 9b9dc44..ff12da4 100644
--- a/src/soc/intel/tigerlake/acpi/gpio_op.asl
+++ b/src/soc/intel/tigerlake/acpi/gpio_op.asl
@@ -11,7 +11,7 @@
    {
    	VAL0, 32
    }
-	Local0 = PAD_CFG0_RX_STATE & (VAL0 >> PAD_CFG0_RX_STATE_BIT)
+	Local0 = (PAD_CFG0_RX_STATE & VAL0) >> PAD_CFG0_RX_STATE_BIT

Return (Local0)
 }