Change in coreboot[master]: soc/intel/common: Have weak implementation for do_global_reset()

[Subrata Banik (Code Review)]

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

Change subject: soc/intel/common: Have weak implementation for do_global_reset()
......................................................................

soc/intel/common: Have weak implementation for do_global_reset()

Not all IA SoC implement do_global_reset() function locally hence have a
weak implementation for do_global_reset() inside common reset.c in order
to call global_reset() function from common code.

Signed-off-by: Subrata Banik 
Change-Id: I934b41affed7bb146f53ff6a4654fdbc6626101b
---
M src/soc/intel/common/reset.c
1 file changed, 5 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/47017/1

diff --git a/src/soc/intel/common/reset.c b/src/soc/intel/common/reset.c
index c6c394b..0eb532b 100644
--- a/src/soc/intel/common/reset.c
+++ b/src/soc/intel/common/reset.c
@@ -8,6 +8,11 @@

#include "reset.h"

+void __weak do_global_reset(void)
+{
+	/* Default empty implementation. */
+}
+
 void global_reset(void)
 {
    printk(BIOS_INFO, "%s() called!\n", __func__);

[Angel Pons (Code Review)]

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

Change subject: soc/intel/common: Have weak implementation for do_global_reset()
......................................................................

Patch Set 1:

(1 comment)

I don't like the idea of a reset function that doesn't reset...

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

https://review.coreboot.org/c/coreboot/+/47017/1//COMMIT_MSG@9 
PS1, Line 9: Not all IA SoC
Which ones do not implement do_global_reset()? Why not implement it on the SoCs that do not have it instead?

[Tim Wawrzynczak (Code Review)]

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

Change subject: soc/intel/common: Have weak implementation for do_global_reset()
......................................................................

Patch Set 1:

(1 comment)

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

https://review.coreboot.org/c/coreboot/+/47017/1//COMMIT_MSG@9 
PS1, Line 9: Not all IA SoC
> Which ones do not implement do_global_reset()? Why not implement it on the SoCs that do not have it  […]

I agree with Angel here, a weak no-op reset function is hazardous...

Change in coreboot[master]: soc/intel/common: Create common Intel FSP reset code block

[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/+/47017

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

Change subject: soc/intel/common: Create common Intel FSP reset code block
......................................................................

soc/intel/common: Create common Intel FSP reset code block

Create SOC_INTEL_COMMON_FSP_RESET Kconfig to have IA common code block to
handle platform reset request raised by FSP. The FSP will use the FSP EAS
v2.0 section 12.2.2 (OEM Status Code) to indicate that a reset is require.

Signed-off-by: Subrata Banik 
Change-Id: I934b41affed7bb146f53ff6a4654fdbc6626101b
---
M src/drivers/intel/fsp2_0/Kconfig
M src/soc/intel/common/Makefile.inc
A src/soc/intel/common/fsp_reset.c
3 files changed, 26 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/47017/2

Change in coreboot[master]: soc/intel/common: Create common Intel FSP reset code block

[Subrata Banik (Code Review)]

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

Change subject: soc/intel/common: Create common Intel FSP reset code block
......................................................................

Patch Set 2:

(1 comment)

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

https://review.coreboot.org/c/coreboot/+/47017/1//COMMIT_MSG@9 
PS1, Line 9: Not all IA SoC
> I agree with Angel here, a weak no-op reset function is hazardous...

Ack

Change in coreboot[master]: soc/intel/common: Create common Intel FSP reset code block

[Angel Pons (Code Review)]

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

Change subject: soc/intel/common: Create common Intel FSP reset code block
......................................................................

Patch Set 2: Code-Review+1

(2 comments)

This approach looks much better. Thanks!

https://review.coreboot.org/c/coreboot/+/47017/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/47017/2//COMMIT_MSG@9 
PS2, Line 9: to
Please wrap at 72 characters

https://review.coreboot.org/c/coreboot/+/47017/2/src/drivers/intel/fsp2_0/Kc... 
File src/drivers/intel/fsp2_0/Kconfig:

https://review.coreboot.org/c/coreboot/+/47017/2/src/drivers/intel/fsp2_0/Kc... 
PS2, Line 229: FSP_STATUS_GLOBAL_RESET
depends on SOC_INTEL_COMMON_FSP_RESET

Change in coreboot[master]: soc/intel/common: Create common Intel FSP reset code block

[Subrata Banik (Code Review)]

Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Furquan Shaikh, Tim Wawrzynczak, Angel Pons, Andrey Petrov, Patrick Rudolph,

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

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

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

Change subject: soc/intel/common: Create common Intel FSP reset code block
......................................................................

soc/intel/common: Create common Intel FSP reset code block

Create SOC_INTEL_COMMON_FSP_RESET Kconfig to have IA common code block
to handle platform reset request raised by FSP. The FSP will use the
FSP EAS v2.0 section 12.2.2 (OEM Status Code) to indicate that a reset
is require.

Signed-off-by: Subrata Banik 
Change-Id: I934b41affed7bb146f53ff6a4654fdbc6626101b
---
M src/drivers/intel/fsp2_0/Kconfig
M src/soc/intel/common/Makefile.inc
A src/soc/intel/common/fsp_reset.c
3 files changed, 27 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/47017/3

Change in coreboot[master]: soc/intel/common: Create common Intel FSP reset code block

[Subrata Banik (Code Review)]

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

Change subject: soc/intel/common: Create common Intel FSP reset code block
......................................................................

Patch Set 3:

(2 comments)
> Patch Set 2: Code-Review+1
>
> (2 comments)
>
> This approach looks much better. Thanks!

Thanks Angel for your suggestion

https://review.coreboot.org/c/coreboot/+/47017/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/47017/2//COMMIT_MSG@9 
PS2, Line 9: to
> Please wrap at 72 characters

Ack

https://review.coreboot.org/c/coreboot/+/47017/2/src/drivers/intel/fsp2_0/Kc... 
File src/drivers/intel/fsp2_0/Kconfig:

https://review.coreboot.org/c/coreboot/+/47017/2/src/drivers/intel/fsp2_0/Kc... 
PS2, Line 229: FSP_STATUS_GLOBAL_RESET
> depends on SOC_INTEL_COMMON_FSP_RESET

Ack

Change in coreboot[master]: soc/intel/common: Create common Intel FSP reset code block

[Subrata Banik (Code Review)]

Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Furquan Shaikh, Tim Wawrzynczak, Angel Pons, Andrey Petrov, Patrick Rudolph,

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

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

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

Change subject: soc/intel/common: Create common Intel FSP reset code block
......................................................................

soc/intel/common: Create common Intel FSP reset code block

Create SOC_INTEL_COMMON_FSP_RESET Kconfig to have IA common code block
to handle platform reset request raised by FSP. The FSP will use the
FSP EAS v2.0 section 12.2.2 (OEM Status Code) to indicate that a reset
is require.

Make FSP_STATUS_GLOBAL_RESET depends on SOC_INTEL_COMMON_FSP_RESET.

Signed-off-by: Subrata Banik 
Change-Id: I934b41affed7bb146f53ff6a4654fdbc6626101b
---
M src/drivers/intel/fsp2_0/Kconfig
M src/soc/intel/common/Makefile.inc
A src/soc/intel/common/fsp_reset.c
3 files changed, 27 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/47017/4

Change in coreboot[master]: soc/intel/common: Create common Intel FSP reset code block

[Subrata Banik (Code Review)]

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

Change subject: soc/intel/common: Create common Intel FSP reset code block
......................................................................

Patch Set 4:

Angel, can you please take a relook whenever you have time

Change in coreboot[master]: soc/intel/common: Create common Intel FSP reset code block

[Angel Pons (Code Review)]

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

Change subject: soc/intel/common: Create common Intel FSP reset code block
......................................................................

Patch Set 4: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/47017/4//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/47017/4//COMMIT_MSG@12 
PS4, Line 12: require
nit: require*d*

Change in coreboot[master]: soc/intel/common: Create common Intel FSP reset code block

[Subrata Banik (Code Review)]

Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Furquan Shaikh, Tim Wawrzynczak, Angel Pons, Andrey Petrov, Patrick Rudolph,

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

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

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

Change subject: soc/intel/common: Create common Intel FSP reset code block
......................................................................

soc/intel/common: Create common Intel FSP reset code block

Create SOC_INTEL_COMMON_FSP_RESET Kconfig to have IA common code block
to handle platform reset request raised by FSP. The FSP will use the
FSP EAS v2.0 section 12.2.2 (OEM Status Code) to indicate that a reset
is required.

Make FSP_STATUS_GLOBAL_RESET depends on SOC_INTEL_COMMON_FSP_RESET.

Signed-off-by: Subrata Banik 
Change-Id: I934b41affed7bb146f53ff6a4654fdbc6626101b
---
M src/drivers/intel/fsp2_0/Kconfig
M src/soc/intel/common/Makefile.inc
A src/soc/intel/common/fsp_reset.c
3 files changed, 27 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/47017/5

Change in coreboot[master]: soc/intel/common: Create common Intel FSP reset code block

[Subrata Banik (Code Review)]

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

Change subject: soc/intel/common: Create common Intel FSP reset code block
......................................................................

Patch Set 5:

(1 comment)

https://review.coreboot.org/c/coreboot/+/47017/4//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/47017/4//COMMIT_MSG@12 
PS4, Line 12: require
> nit: require*d*

Ack

Change in coreboot[master]: soc/intel/common: Create common Intel FSP reset code block

[Patrick Georgi (Code Review)]

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

Change subject: soc/intel/common: Create common Intel FSP reset code block
......................................................................

soc/intel/common: Create common Intel FSP reset code block

Create SOC_INTEL_COMMON_FSP_RESET Kconfig to have IA common code block
to handle platform reset request raised by FSP. The FSP will use the
FSP EAS v2.0 section 12.2.2 (OEM Status Code) to indicate that a reset
is required.

Make FSP_STATUS_GLOBAL_RESET depends on SOC_INTEL_COMMON_FSP_RESET.

Signed-off-by: Subrata Banik 
Change-Id: I934b41affed7bb146f53ff6a4654fdbc6626101b
Reviewed-on: https://review.coreboot.org/c/coreboot/+/47017
Tested-by: build bot (Jenkins) 
Reviewed-by: Angel Pons 
---
M src/drivers/intel/fsp2_0/Kconfig
M src/soc/intel/common/Makefile.inc
A src/soc/intel/common/fsp_reset.c
3 files changed, 27 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Angel Pons: Looks good to me, approved

diff --git a/src/drivers/intel/fsp2_0/Kconfig b/src/drivers/intel/fsp2_0/Kconfig
index ad7afd8..f12ff6e 100644
--- a/src/drivers/intel/fsp2_0/Kconfig
+++ b/src/drivers/intel/fsp2_0/Kconfig
@@ -228,6 +228,7 @@

config FSP_STATUS_GLOBAL_RESET
    hex
+	depends on SOC_INTEL_COMMON_FSP_RESET
    default 0x40000003 if FSP_STATUS_GLOBAL_RESET_REQUIRED_3
    default 0x40000004 if FSP_STATUS_GLOBAL_RESET_REQUIRED_4
    default 0x40000005 if FSP_STATUS_GLOBAL_RESET_REQUIRED_5
@@ -240,6 +241,13 @@
      reset type from SoC Kconfig based on available Kconfig options
      FSP_STATUS_GLOBAL_RESET_REQUIRED_X. Default is unsupported.

+config SOC_INTEL_COMMON_FSP_RESET
+	bool
+	help
+	  Common code block to handle platform reset request raised by FSP. The FSP
+	  will use the FSP EAS v2.0 section 12.2.2 (OEM Status Code) to indicate that
+	  a reset is required.
+
 if FSP_PEIM_TO_PEIM_INTERFACE
 source "src/drivers/intel/fsp2_0/ppi/Kconfig"
 endif
diff --git a/src/soc/intel/common/Makefile.inc b/src/soc/intel/common/Makefile.inc
index 22d350c..9993bce 100644
--- a/src/soc/intel/common/Makefile.inc
+++ b/src/soc/intel/common/Makefile.inc
@@ -27,6 +27,9 @@
 ramstage-$(CONFIG_TPM_CR50) += tpm_tis.c
 postcar-$(CONFIG_TPM_CR50) += tpm_tis.c

+romstage-$(CONFIG_SOC_INTEL_COMMON_FSP_RESET) += fsp_reset.c
+ramstage-$(CONFIG_SOC_INTEL_COMMON_FSP_RESET) += fsp_reset.c
+
 ifeq ($(CONFIG_MMA),y)
 MMA_BLOBS_PATH = $(call strip_quotes,$(CONFIG_MMA_BLOBS_PATH))
 MMA_TEST_NAMES = $(notdir $(wildcard $(MMA_BLOBS_PATH)/tests/*))
diff --git a/src/soc/intel/common/fsp_reset.c b/src/soc/intel/common/fsp_reset.c
new file mode 100644
index 0000000..e89fe4c
--- /dev/null
+++ b/src/soc/intel/common/fsp_reset.c
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include <console/console.h>
+#include <fsp/util.h>
+#include <soc/intel/common/reset.h>
+
+void chipset_handle_reset(uint32_t status)
+{
+	if (status == CONFIG_FSP_STATUS_GLOBAL_RESET) {
+		printk(BIOS_DEBUG, "GLOBAL RESET!\n");
+		global_reset();
+	}
+
+	printk(BIOS_ERR, "unhandled reset type %x\n", status);
+	die("unknown reset type");
+}