Change in coreboot[master]: mb/google/dedede: Fix board specific CSE reset

[Karthik Ramasubramanian (Code Review)]

Karthik Ramasubramanian has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44645 )

Change subject: mb/google/dedede: Fix board specific CSE reset
......................................................................

mb/google/dedede: Fix board specific CSE reset

As per the EDS, toggling SYS_RESET pin alone leads to warm reset. Set the
full reset bit in reset register before requesting EC to toggle SYS_RESET.
This will ensure that the cold reset is performed when CSE Lite jumps
from RO to RW.

BUG=b:162386991
TEST=Ensure that the cold reset is performed when CSE Lite jumps from RO
to RW. Please refer below the logs from Embedded Controller on receiving
the command to perform AP reset.
20-08-20 14:07:56.198 [14.593092 HC 0x125]
20-08-20 14:07:56.198 [14.593833 chipset_reset: 4]
20-08-20 14:07:56.230 [14.626565 power state 3 = S0, in 0x0039]
20-08-20 14:07:56.233 [14.627477 power state 8 = S0->S3, in 0x0039]
20-08-20 14:07:56.247 [14.643495 PD:S0->S3]
20-08-20 14:07:56.247 [14.644106 power state 2 = S3, in 0x0039]
20-08-20 14:07:56.254 [14.645666 power state 9 = S3->S5, in 0x0039]
20-08-20 14:07:56.278 [14.675107 PD:S3->S5]
20-08-20 14:07:56.279 [14.676098 VB hook_shutdown]
20-08-20 14:07:56.282 [14.676559 power state 1 = S5, in 0x0039]
20-08-20 14:07:57.310 [15.719640 power state 1 = S5, in 0x003f]
20-08-20 14:07:57.314 [15.720594 power state 6 = S5->S3, in 0x003f]
20-08-20 14:07:57.325 [15.735030 PD:S5->S3]
20-08-20 14:07:57.325 [15.735699 power state 2 = S3, in 0x003f]
20-08-20 14:07:57.332 [15.737500 power state 7 = S3->S0, in 0x003f]
20-08-20 14:07:57.346 [15.756216 PD:S3->S0]
20-08-20 14:07:57.347 [15.756650 power state 3 = S0, in 0x003f]

Change-Id: I46a390c71e380328cd7fe70214df09553b2db75c
Signed-off-by: Karthikeyan Ramasubramanian 
---
M src/mainboard/google/dedede/mainboard.c
1 file changed, 3 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/44645/1

diff --git a/src/mainboard/google/dedede/mainboard.c b/src/mainboard/google/dedede/mainboard.c
index 4695a9f..2725492 100644
--- a/src/mainboard/google/dedede/mainboard.c
+++ b/src/mainboard/google/dedede/mainboard.c
@@ -1,8 +1,10 @@
 /* SPDX-License-Identifier: GPL-2.0-or-later */

#include <acpi/acpi.h>
+#include <arch/io.h>
 #include <bootstate.h>
 #include <baseboard/variants.h>
+#include <cf9_reset.h>
 #include <device/device.h>
 #include <ec/ec.h>
 #include <ec/google/chromeec/ec.h>
@@ -13,6 +15,7 @@
 void cse_board_reset(void)
 {
    /* TODO: Check tpm firmware version before initiating AP reset. */
+	outb(FULL_RST | SYS_RST, RST_CNT);
    if (!google_chromeec_ap_reset())
    	halt();
 }

[Karthik Ramasubramanian (Code Review)]

Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44645 )

Change subject: mb/google/dedede: Fix board specific CSE reset
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44645/1/src/mainboard/google/dedede... 
File src/mainboard/google/dedede/mainboard.c:

https://review.coreboot.org/c/coreboot/+/44645/1/src/mainboard/google/dedede... 
PS1, Line 18: outb(FULL_RST | SYS_RST, RST_CNT);
I can not add inside google_chromeec_ap_reset since the EC command is architecture-independent and this Reset register is specific to x86 architecture.

[Furquan Shaikh (Code Review)]

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

Change subject: mb/google/dedede: Fix board specific CSE reset
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44645/1/src/mainboard/google/dedede... 
File src/mainboard/google/dedede/mainboard.c:

https://review.coreboot.org/c/coreboot/+/44645/1/src/mainboard/google/dedede... 
PS1, Line 18: outb(FULL_RST | SYS_RST, RST_CNT);
> I can not add inside google_chromeec_ap_reset since the EC command is architecture-independent and t […]

I wonder if this entire function should be moved to src/vendorcode/google/chromeos/ so that all mainboards needing it can take advantage of it. And have mainboard select a Kconfig that pulls in the required file that provides this.

Also, I think we should add a dcache_clean_all() before the outb() just like it is done here: https://source.chromium.org/chromiumos/chromiumos/codesearch/+/master:src/th...

[Karthik Ramasubramanian (Code Review)]

Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44645 )

Change subject: mb/google/dedede: Fix board specific CSE reset
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44645/1/src/mainboard/google/dedede... 
File src/mainboard/google/dedede/mainboard.c:

https://review.coreboot.org/c/coreboot/+/44645/1/src/mainboard/google/dedede... 
PS1, Line 18: outb(FULL_RST | SYS_RST, RST_CNT);
> I wonder if this entire function should be moved to src/vendorcode/google/chromeos/ so that all main […]

That is much cleaner so that we don't have to replicate it multiple times. I will move it and define a Kconfig so that it can be enabled in the required mainboards.

Change in coreboot[master]: mb/google/dedede: Enable CSE Board Reset Override

[Karthik Ramasubramanian (Code Review)]

Hello build bot (Jenkins), Furquan Shaikh, Tim Wawrzynczak, Sridhar Siricilla, Aamir Bohra,

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

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

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

Change subject: mb/google/dedede: Enable CSE Board Reset Override
......................................................................

mb/google/dedede: Enable CSE Board Reset Override

This will ensure that the cold reset is performed when CSE Lite jumps
from RO to RW.

BUG=b:162386991
TEST=Ensure that the cold reset is performed when CSE Lite jumps from RO
to RW. Please refer below the logs from Embedded Controller on receiving
the command to perform AP reset.
20-08-20 14:07:56.198 [14.593092 HC 0x125]
20-08-20 14:07:56.198 [14.593833 chipset_reset: 4]
20-08-20 14:07:56.230 [14.626565 power state 3 = S0, in 0x0039]
20-08-20 14:07:56.233 [14.627477 power state 8 = S0->S3, in 0x0039]
20-08-20 14:07:56.247 [14.643495 PD:S0->S3]
20-08-20 14:07:56.247 [14.644106 power state 2 = S3, in 0x0039]
20-08-20 14:07:56.254 [14.645666 power state 9 = S3->S5, in 0x0039]
20-08-20 14:07:56.278 [14.675107 PD:S3->S5]
20-08-20 14:07:56.279 [14.676098 VB hook_shutdown]
20-08-20 14:07:56.282 [14.676559 power state 1 = S5, in 0x0039]
20-08-20 14:07:57.310 [15.719640 power state 1 = S5, in 0x003f]
20-08-20 14:07:57.314 [15.720594 power state 6 = S5->S3, in 0x003f]
20-08-20 14:07:57.325 [15.735030 PD:S5->S3]
20-08-20 14:07:57.325 [15.735699 power state 2 = S3, in 0x003f]
20-08-20 14:07:57.332 [15.737500 power state 7 = S3->S0, in 0x003f]
20-08-20 14:07:57.346 [15.756216 PD:S3->S0]
20-08-20 14:07:57.347 [15.756650 power state 3 = S0, in 0x003f]

Change-Id: I46a390c71e380328cd7fe70214df09553b2db75c
Signed-off-by: Karthikeyan Ramasubramanian 
---
M src/mainboard/google/dedede/Kconfig
M src/mainboard/google/dedede/mainboard.c
2 files changed, 1 insertion(+), 10 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/44645/2

Change in coreboot[master]: mb/google/dedede: Enable CSE Board Reset Override

[Karthik Ramasubramanian (Code Review)]

Hello build bot (Jenkins), Furquan Shaikh, Tim Wawrzynczak, Sridhar Siricilla, Aamir Bohra,

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

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

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

Change subject: mb/google/dedede: Enable CSE Board Reset Override
......................................................................

mb/google/dedede: Enable CSE Board Reset Override

This will ensure that the cold reset is performed when CSE Lite jumps
from RO to RW.

BUG=b:162386991
TEST=Ensure that the cold reset is performed when CSE Lite jumps from RO
to RW. Please refer below the logs from Embedded Controller on receiving
the command to perform AP reset.
20-08-20 16:35:58.557 [14.558884 HC 0x125]
20-08-20 16:35:58.557 [14.559617 chipset_reset: 4]
20-08-20 16:35:58.590 [14.592352 power state 3 = S0, in 0x0039]
20-08-20 16:35:58.593 [14.593227 power state 8 = S0->S3, in 0x0039]
20-08-20 16:35:58.600 [14.603445 PD:S0->S3]
20-08-20 16:35:58.603 [14.604668 power state 2 = S3, in 0x0039]
20-08-20 16:35:58.606 [14.605532 power state 9 = S3->S5, in 0x0039]
20-08-20 16:35:58.632 [14.634336 PD:S3->S5]
20-08-20 16:35:58.632 [14.635260 VB hook_shutdown]
20-08-20 16:35:58.635 [14.635779 power state 1 = S5, in 0x0039]
20-08-20 16:35:59.611 [15.626004 power state 1 = S5, in 0x003f]
20-08-20 16:35:59.614 [15.626929 power state 6 = S5->S3, in 0x003f]
20-08-20 16:35:59.626 [15.641458 PD:S5->S3]
20-08-20 16:35:59.626 [15.642094 power state 2 = S3, in 0x003f]
20-08-20 16:35:59.633 [15.643879 power state 7 = S3->S0, in 0x003f]
20-08-20 16:35:59.648 [15.662842 PD:S3->S0]
20-08-20 16:35:59.648 [15.663278 power state 3 = S0, in 0x003f]

Change-Id: I46a390c71e380328cd7fe70214df09553b2db75c
Signed-off-by: Karthikeyan Ramasubramanian 
---
M src/mainboard/google/dedede/Kconfig
M src/mainboard/google/dedede/mainboard.c
2 files changed, 1 insertion(+), 10 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/44645/4

Change in coreboot[master]: mb/google/dedede: Enable CSE Board Reset Override

[Karthik Ramasubramanian (Code Review)]

Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44645 )

Change subject: mb/google/dedede: Enable CSE Board Reset Override
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44645/1/src/mainboard/google/dedede... 
File src/mainboard/google/dedede/mainboard.c:

https://review.coreboot.org/c/coreboot/+/44645/1/src/mainboard/google/dedede... 
PS1, Line 18: outb(FULL_RST | SYS_RST, RST_CNT);
> That is much cleaner so that we don't have to replicate it multiple times. […]

Addressed the comments in CB:44646

Change in coreboot[master]: mb/google/dedede: Enable CSE Board Reset Override

[Furquan Shaikh (Code Review)]

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

Change subject: mb/google/dedede: Enable CSE Board Reset Override
......................................................................

Patch Set 4: Code-Review+2

Change in coreboot[master]: mb/google/dedede: Enable CSE Board Reset Override

[Edward O'Callaghan (Code Review)]

Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44645 )

Change subject: mb/google/dedede: Enable CSE Board Reset Override
......................................................................

Patch Set 4: Code-Review+2

Change in coreboot[master]: mb/google/dedede: Enable CSE Board Reset Override

[Karthik Ramasubramanian (Code Review)]

Hello build bot (Jenkins), Furquan Shaikh, Tim Wawrzynczak, Edward O'Callaghan, Sridhar Siricilla, Aamir Bohra,

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

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

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

Change subject: mb/google/dedede: Enable CSE Board Reset Override
......................................................................

mb/google/dedede: Enable CSE Board Reset Override

This will ensure that the cold reset is performed when CSE Lite jumps
from RO to RW.

BUG=b:162386991
TEST=Ensure that Drawcia board boots to OS. Ensure that global reset is
triggered when cr50 is running firmware versions newer than 0.0.22. On
cr50 versions 0.0.22 or older, EC triggers cold reset of AP.
On board with cr50 firmware version 0.0.22:
===========================================
coreboot-coreboot-unknown.9999.6fcde00 Fri Aug 21 05:12:01 UTC 2020 ramstage starting (log level: 8)...
<snip>
cse_lite: RO version = 13.50.0.7130 (Status=0x0, Start=0x1000, End=0xeafff)
cse_lite: RW version = 13.50.0.7130 (Status=0x0, Start=0xeb000, End=0x26afff)
<snip>
cse_lite: Set Boot Partition Info Command (RW)
Probing TPM:  done!
Connected to device vid:did:rid of 1ae0:0028:00
Firmware version: B2-C:0 RO_B:0.0.11/4d655eab RW_A:0.0.22/cr50_v1.1.6138-b9f0b1d
Initialized TPM device CR50 revision 0

coreboot-coreboot-unknown.9999.6fcde00 Fri Aug 21 05:12:01 UTC 2020 bootblock starting (log level: 8)...
CPU: Genuine Intel(R) CPU 0000 @ 1.10GHz
<snip>

On board with cr50 firmware version 0.6.5:
==========================================
coreboot-coreboot-unknown.9999.6fcde00 Fri Aug 21 05:12:01 UTC 2020 ramstage starting (log level: 8)...
<snip>
cse_lite: RO version = 13.50.0.7130 (Status=0x0, Start=0x1000, End=0xeafff)
cse_lite: RW version = 13.50.0.7130 (Status=0x0, Start=0xeb000, End=0x26afff)
<snip>
cse_lite: Set Boot Partition Info Command (RW)
Probing TPM:  done!
Connected to device vid:did:rid of 1ae0:0028:00
Firmware version: B2-C:0 RO_B:0.0.11/4d655eab RW_A:0.6.5/cr50_v1.9308_B.889-3dad44541
Initialized TPM device CR50 revision 0
Cr50 firmware does not use SYS_RESET#, version: 0.6.5
HECI: Global Reset(Type:1) Command

coreboot-coreboot-unknown.9999.6fcde00 Fri Aug 21 05:12:01 UTC 2020 bootblock starting (log level: 8)...
CPU: Genuine Intel(R) CPU 0000 @ 1.10GHz
<snip>

Change-Id: I46a390c71e380328cd7fe70214df09553b2db75c
Signed-off-by: Karthikeyan Ramasubramanian 
---
M src/mainboard/google/dedede/Kconfig
M src/mainboard/google/dedede/mainboard.c
2 files changed, 1 insertion(+), 10 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/44645/5

Change in coreboot[master]: mb/google/dedede: Enable CSE Board Reset Override

[Edward O'Callaghan (Code Review)]

Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44645 )

Change subject: mb/google/dedede: Enable CSE Board Reset Override
......................................................................

Patch Set 5: Code-Review+2

Change in coreboot[master]: mb/google/dedede: Enable CSE Board Reset Override

[Karthik Ramasubramanian (Code Review)]

Hello build bot (Jenkins), Furquan Shaikh, Tim Wawrzynczak, Edward O'Callaghan, Sridhar Siricilla, Aamir Bohra,

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

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

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

Change subject: mb/google/dedede: Enable CSE Board Reset Override
......................................................................

mb/google/dedede: Enable CSE Board Reset Override

This will ensure that the cold reset is performed when CSE Lite jumps
from RO to RW.

BUG=b:162386991
TEST=Ensure that Drawcia board boots to OS. Ensure that global reset is
triggered when cr50 is running firmware versions newer than 0.0.22. On
cr50 versions 0.0.22 or older, EC triggers cold reset of AP.

Change-Id: I46a390c71e380328cd7fe70214df09553b2db75c
Signed-off-by: Karthikeyan Ramasubramanian 
---
M src/mainboard/google/dedede/Kconfig
M src/mainboard/google/dedede/mainboard.c
2 files changed, 1 insertion(+), 10 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/44645/7

Change in coreboot[master]: mb/google/dedede: Enable CSE Board Reset Override

[Furquan Shaikh (Code Review)]

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

Change subject: mb/google/dedede: Enable CSE Board Reset Override
......................................................................

Patch Set 7: Code-Review+2

Change in coreboot[master]: mb/google/dedede: Enable CSE Board Reset Override

[Edward O'Callaghan (Code Review)]

Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44645 )

Change subject: mb/google/dedede: Enable CSE Board Reset Override
......................................................................

Patch Set 8: Code-Review+2

Change in coreboot[master]: mb/google/dedede: Enable CSE Board Reset Override

[Edward O'Callaghan (Code Review)]

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44645 )

Change subject: mb/google/dedede: Enable CSE Board Reset Override
......................................................................

mb/google/dedede: Enable CSE Board Reset Override

This will ensure that the cold reset is performed when CSE Lite jumps
from RO to RW.

BUG=b:162386991
TEST=Ensure that Drawcia board boots to OS. Ensure that global reset is
triggered when cr50 is running firmware versions newer than 0.0.22. On
cr50 versions 0.0.22 or older, EC triggers cold reset of AP.

Change-Id: I46a390c71e380328cd7fe70214df09553b2db75c
Signed-off-by: Karthikeyan Ramasubramanian 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/44645
Tested-by: build bot (Jenkins) 
Reviewed-by: Edward O'Callaghan 
Reviewed-by: Furquan Shaikh 
---
M src/mainboard/google/dedede/Kconfig
M src/mainboard/google/dedede/mainboard.c
2 files changed, 1 insertion(+), 10 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Furquan Shaikh: Looks good to me, approved
  Edward O'Callaghan: Looks good to me, approved

diff --git a/src/mainboard/google/dedede/Kconfig b/src/mainboard/google/dedede/Kconfig
index d4c4d06..dd1125e 100644
--- a/src/mainboard/google/dedede/Kconfig
+++ b/src/mainboard/google/dedede/Kconfig
@@ -31,6 +31,7 @@
 config CHROMEOS
    bool
    default y
+	select CHROMEOS_CSE_BOARD_RESET_OVERRIDE
    select EC_GOOGLE_CHROMEEC_SWITCHES
    select GBB_FLAG_FORCE_DEV_SWITCH_ON
    select GBB_FLAG_FORCE_DEV_BOOT_USB
diff --git a/src/mainboard/google/dedede/mainboard.c b/src/mainboard/google/dedede/mainboard.c
index 4695a9f..cb84e1f 100644
--- a/src/mainboard/google/dedede/mainboard.c
+++ b/src/mainboard/google/dedede/mainboard.c
@@ -5,18 +5,8 @@
 #include <baseboard/variants.h>
 #include <device/device.h>
 #include <ec/ec.h>
-#include <ec/google/chromeec/ec.h>
-#include <halt.h>
-#include <intelblocks/cse.h>
 #include <vendorcode/google/chromeos/chromeos.h>

-void cse_board_reset(void)
-{
-	/* TODO: Check tpm firmware version before initiating AP reset. */
-	if (!google_chromeec_ap_reset())
-		halt();
-}
-
 __weak void variant_isst_override(void)
 {
    /*