Change in ...coreboot[master]: mb/google/sarien: Disable POWER_OFF_ON_CR50_UPDATE

[Keith Short (Code Review)]

Keith Short has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32436

Change subject: mb/google/sarien: Disable POWER_OFF_ON_CR50_UPDATE
......................................................................

mb/google/sarien: Disable POWER_OFF_ON_CR50_UPDATE

Disable the POWER_OFF_ON_CR50_UPDATE option on sarien/arcada.  This is
needed so that platform properly boots after doing a Cr50 firmware
update when running on battery.

BUG=b:126632503
BRANCH=none
TEST=Build coreboot on sarien/arcada.
TEST=Perform Cr50 firmware update on Sarien, confirm the platform boots
normally after sending TURN_UPDATE_ON to the Cr50.

Change-Id: I0b687285eb95070eaffb68611a7d98eb8434ce2c
Signed-off-by: Keith Short 
---
M src/mainboard/google/sarien/Kconfig
1 file changed, 4 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/32436/1

diff --git a/src/mainboard/google/sarien/Kconfig b/src/mainboard/google/sarien/Kconfig
index 9d658fa..a2f8adc 100644
--- a/src/mainboard/google/sarien/Kconfig
+++ b/src/mainboard/google/sarien/Kconfig
@@ -59,6 +59,10 @@
    int
    default 82  # GPE0_DW2_18 (GPP_D18)

+config POWER_OFF_ON_CR50_UPDATE
+	bool
+	default n
+
 config GBB_HWID
    string
    depends on CHROMEOS

[Duncan Laurie (Code Review)]

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

Change subject: mb/google/sarien: Disable POWER_OFF_ON_CR50_UPDATE
......................................................................

Patch Set 1: Code-Review+2

[Furquan Shaikh (Code Review)]

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

Change subject: mb/google/sarien: Disable POWER_OFF_ON_CR50_UPDATE
......................................................................

Patch Set 1: Code-Review+2

[Duncan Laurie (Code Review)]

Duncan Laurie has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32436 )

Change subject: mb/google/sarien: Disable POWER_OFF_ON_CR50_UPDATE
......................................................................

mb/google/sarien: Disable POWER_OFF_ON_CR50_UPDATE

Disable the POWER_OFF_ON_CR50_UPDATE option on sarien/arcada.  This is
needed so that platform properly boots after doing a Cr50 firmware
update when running on battery.

BUG=b:126632503
BRANCH=none
TEST=Build coreboot on sarien/arcada.
TEST=Perform Cr50 firmware update on Sarien, confirm the platform boots
normally after sending TURN_UPDATE_ON to the Cr50.

Change-Id: I0b687285eb95070eaffb68611a7d98eb8434ce2c
Signed-off-by: Keith Short 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/32436
Tested-by: build bot (Jenkins) 
Reviewed-by: Duncan Laurie 
Reviewed-by: Furquan Shaikh 
---
M src/mainboard/google/sarien/Kconfig
1 file changed, 4 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Duncan Laurie: Looks good to me, approved
  Furquan Shaikh: Looks good to me, approved

diff --git a/src/mainboard/google/sarien/Kconfig b/src/mainboard/google/sarien/Kconfig
index f1046ec..1e6df57 100644
--- a/src/mainboard/google/sarien/Kconfig
+++ b/src/mainboard/google/sarien/Kconfig
@@ -59,6 +59,10 @@
    int
    default 82  # GPE0_DW2_18 (GPP_D18)

+config POWER_OFF_ON_CR50_UPDATE
+	bool
+	default n
+
 config GBB_HWID
    string
    depends on CHROMEOS

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32436 )

Change subject: mb/google/sarien: Disable POWER_OFF_ON_CR50_UPDATE
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/#/c/32436/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/32436/2//COMMIT_MSG@11 
PS2, Line 11: update when running on battery.
I'm a bit confused about this commit message. It states why it is
desirable to turn it off, but not why it isn't required any more.
So was it enabled by accident before?

[Duncan Laurie (Code Review)]

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

Change subject: mb/google/sarien: Disable POWER_OFF_ON_CR50_UPDATE
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/#/c/32436/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/32436/2//COMMIT_MSG@11 
PS2, Line 11: update when running on battery.
> I'm a bit confused about this commit message. It states why it is […]

It was enabled globally before.  This power architecture on this platform behaves differently than our previous x86 devices (with different behavior on battery and AC) and we need some special handling to ensure it powers up again after an EC reset.