Change in coreboot[master]: soc/intel/cannonlake: Make EC S0ix notification optional in LPIT

[Tim Wawrzynczak (Code Review)]

Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34178 )

Change subject: soc/intel/cannonlake: Make EC S0ix notification optional in LPIT
......................................................................

soc/intel/cannonlake: Make EC S0ix notification optional in LPIT

Only call the _SB.PCI0.LPCB.EC0.S0IX method if it exists.

Change-Id: Idf465f8ad7cb016f3ad3d9710b46e35f66f8939b
Signed-off-by: Tim Wawrzynczak 
---
M src/soc/intel/cannonlake/acpi/lpit.asl
1 file changed, 17 insertions(+), 8 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/78/34178/1

diff --git a/src/soc/intel/cannonlake/acpi/lpit.asl b/src/soc/intel/cannonlake/acpi/lpit.asl
index 93bce26..74d4fe6 100644
--- a/src/soc/intel/cannonlake/acpi/lpit.asl
+++ b/src/soc/intel/cannonlake/acpi/lpit.asl
@@ -15,6 +15,7 @@
  */

External(_SB.MS0X, MethodObj)
+External(_SB.PCI0.LPCB.EC0.S0IX, MethodObj)

scope(_SB)
 {
@@ -34,7 +35,7 @@
    			)
    		}
    		/*
-			 * Function 1.
+			 * Function 1 - Get Device Constraints
    		 */
    		If(Arg2 == 1) {
    			Return(Package(5) {
@@ -42,7 +43,7 @@
    			)
    		}
    		/*
-			 * Function 2.
+			 * Function 2 - Get Crash Dump Device
    		 */
    		If(Arg2 == 2) {
    			Return(Buffer(One) {
@@ -50,30 +51,38 @@
    			)
    		}
    		/*
-			 * Function 3.
+			 * Function 3 - Display Off Notification
    		 */
    		If(Arg2 == 3) {
    		}
    		/*
-			 * Function 4.
+			 * Function 4 - Display On Notification
    		 */
    		If(Arg2 == 4) {
    		}
    		/*
-			 * Function 5.
+			 * Function 5 - Low Power S0 Entry Notification
    		 */
    		If(Arg2 == 5) {
-				_SB.PCI0.LPCB.EC0.S0IX(1)
+				/* Inform the EC */
+				If (CondRefOf (_SB.PCI0.LPCB.EC0.S0IX)) {
+					_SB.PCI0.LPCB.EC0.S0IX(1)
+				}
+
    			/* provide board level s0ix hook */
    			If (CondRefOf (_SB.MS0X)) {
    				_SB.MS0X(1)
    			}
    		}
    		/*
-			 * Function 6.
+			 * Function 6 - Low Power S0 Exit Notification
    		 */
    		If(Arg2 == 6) {
-				_SB.PCI0.LPCB.EC0.S0IX(0)
+				/* Inform the EC */
+				If (CondRefOf (_SB.PCI0.LPCB.EC0.S0IX)) {
+					_SB.PCI0.LPCB.EC0.S0IX(0)
+				}
+
    			/* provide board level s0ix hook */
    			If (CondRefOf (_SB.MS0X)) {
    				_SB.MS0X(0)

[Tim Wawrzynczak (Code Review)]

Hello Patrick Rudolph, Subrata Banik, Duncan Laurie, Shelley Chen, build bot (Jenkins), Furquan Shaikh,

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

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

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

Change subject: soc/intel/cannonlake: Make EC S0ix notification optional in LPIT
......................................................................

soc/intel/cannonlake: Make EC S0ix notification optional in LPIT

Only call the _SB.PCI0.LPCB.EC0.S0IX method if it exists.

Change-Id: Idf465f8ad7cb016f3ad3d9710b46e35f66f8939b
Signed-off-by: Tim Wawrzynczak 
---
M src/soc/intel/cannonlake/acpi/lpit.asl
1 file changed, 17 insertions(+), 8 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/78/34178/3

[Paul Fagerburg (Code Review)]

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

Change subject: soc/intel/cannonlake: Make EC S0ix notification optional in LPIT
......................................................................

Patch Set 3: Code-Review+1

[Furquan Shaikh (Code Review)]

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

Change subject: soc/intel/cannonlake: Make EC S0ix notification optional in LPIT
......................................................................

Patch Set 3: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/34178/3/src/soc/intel/cannonlake/ac... 
File src/soc/intel/cannonlake/acpi/lpit.asl:

PS3: 
Not for this change, but I think we should eventually move this file to soc/intel/common since there is nothing cannonlake specific in it.

[Shaunak Saha (Code Review)]

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

Change subject: soc/intel/cannonlake: Make EC S0ix notification optional in LPIT
......................................................................

Patch Set 3: Code-Review+1
> Patch Set 3: Code-Review+2
>
> (1 comment)

[Duncan Laurie (Code Review)]

Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34178 )

Change subject: soc/intel/cannonlake: Make EC S0ix notification optional in LPIT
......................................................................

Patch Set 3: Code-Review+2

[Subrata Banik (Code Review)]

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

Change subject: soc/intel/cannonlake: Make EC S0ix notification optional in LPIT
......................................................................

Patch Set 3: Code-Review+2

[Martin Roth (Code Review)]

Martin Roth has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34178 )

Change subject: soc/intel/cannonlake: Make EC S0ix notification optional in LPIT
......................................................................

soc/intel/cannonlake: Make EC S0ix notification optional in LPIT

Only call the _SB.PCI0.LPCB.EC0.S0IX method if it exists.

Change-Id: Idf465f8ad7cb016f3ad3d9710b46e35f66f8939b
Signed-off-by: Tim Wawrzynczak 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/34178
Tested-by: build bot (Jenkins) 
Reviewed-by: Paul Fagerburg 
Reviewed-by: Furquan Shaikh 
Reviewed-by: Shaunak Saha 
Reviewed-by: Duncan Laurie 
Reviewed-by: Subrata Banik 
---
M src/soc/intel/cannonlake/acpi/lpit.asl
1 file changed, 17 insertions(+), 8 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Duncan Laurie: Looks good to me, approved
  Furquan Shaikh: Looks good to me, approved
  Shaunak Saha: Looks good to me, but someone else must approve
  Subrata Banik: Looks good to me, approved
  Paul Fagerburg: Looks good to me, but someone else must approve

diff --git a/src/soc/intel/cannonlake/acpi/lpit.asl b/src/soc/intel/cannonlake/acpi/lpit.asl
index 93bce26..74d4fe6 100644
--- a/src/soc/intel/cannonlake/acpi/lpit.asl
+++ b/src/soc/intel/cannonlake/acpi/lpit.asl
@@ -15,6 +15,7 @@
  */

External(_SB.MS0X, MethodObj)
+External(_SB.PCI0.LPCB.EC0.S0IX, MethodObj)

scope(_SB)
 {
@@ -34,7 +35,7 @@
    			)
    		}
    		/*
-			 * Function 1.
+			 * Function 1 - Get Device Constraints
    		 */
    		If(Arg2 == 1) {
    			Return(Package(5) {
@@ -42,7 +43,7 @@
    			)
    		}
    		/*
-			 * Function 2.
+			 * Function 2 - Get Crash Dump Device
    		 */
    		If(Arg2 == 2) {
    			Return(Buffer(One) {
@@ -50,30 +51,38 @@
    			)
    		}
    		/*
-			 * Function 3.
+			 * Function 3 - Display Off Notification
    		 */
    		If(Arg2 == 3) {
    		}
    		/*
-			 * Function 4.
+			 * Function 4 - Display On Notification
    		 */
    		If(Arg2 == 4) {
    		}
    		/*
-			 * Function 5.
+			 * Function 5 - Low Power S0 Entry Notification
    		 */
    		If(Arg2 == 5) {
-				_SB.PCI0.LPCB.EC0.S0IX(1)
+				/* Inform the EC */
+				If (CondRefOf (_SB.PCI0.LPCB.EC0.S0IX)) {
+					_SB.PCI0.LPCB.EC0.S0IX(1)
+				}
+
    			/* provide board level s0ix hook */
    			If (CondRefOf (_SB.MS0X)) {
    				_SB.MS0X(1)
    			}
    		}
    		/*
-			 * Function 6.
+			 * Function 6 - Low Power S0 Exit Notification
    		 */
    		If(Arg2 == 6) {
-				_SB.PCI0.LPCB.EC0.S0IX(0)
+				/* Inform the EC */
+				If (CondRefOf (_SB.PCI0.LPCB.EC0.S0IX)) {
+					_SB.PCI0.LPCB.EC0.S0IX(0)
+				}
+
    			/* provide board level s0ix hook */
    			If (CondRefOf (_SB.MS0X)) {
    				_SB.MS0X(0)