Change in ...coreboot[master]: mainboard/google/sarien: set diagnostic LEDs on boot failure

[Keith Short (Code Review)]

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

Change subject: mainboard/google/sarien: set diagnostic LEDs on boot failure
......................................................................

mainboard/google/sarien: set diagnostic LEDs on boot failure

On Wilco devices, if any of the coreboot stages fails with a fatal
error, set the diagnostic LEDs with the Wilco EC.

BUG=b:124401932
BRANCH=sarien
TEST=build coreboot for sarien and arcada platforms
TEST=Remove DIMM module, confirm diagnostic LED pattern for memory
failure (2 amber, 4 white).
TEST=Forced a fatal error in both bootblock and verstage to confirm
diagnostic LEDs during these stages. This works on cold-boots only. Bug
b:132622888 tracks the mailbox failures on warm boots.

Change-Id: If865ab8203f89e499130f4677fec166b40d80174
Signed-off-by: Keith Short 
---
M src/mainboard/google/sarien/chromeos.c
1 file changed, 42 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/32777/1

diff --git a/src/mainboard/google/sarien/chromeos.c b/src/mainboard/google/sarien/chromeos.c
index fafc469..d6b34f9 100644
--- a/src/mainboard/google/sarien/chromeos.c
+++ b/src/mainboard/google/sarien/chromeos.c
@@ -15,22 +15,41 @@

#include <arch/acpi.h>
 #include <boot/coreboot_tables.h>
+#include <console/console.h>
 #include <gpio.h>
 #include <soc/gpio.h>
 #include <variant/gpio.h>
 #include <vendorcode/google/chromeos/chromeos.h>
 #include <security/tpm/tss.h>
 #include <device/device.h>
+#include <ec/google/wilco/commands.h>
 #include <intelblocks/pmclib.h>
 #include <soc/pmc.h>
 #include <soc/pci_devs.h>

+struct err_code_entry {
+	uint8_t post_code;
+	enum ec_err_code ec_err;
+};
+
 enum rec_mode_state {
    REC_MODE_UNINITIALIZED,
    REC_MODE_NOT_REQUESTED,
    REC_MODE_REQUESTED,
 };

+static uint8_t sarien_post_code;
+
+/*
+ * Any post codes not listed in the sarien_err_codes[] use this default.
+ */
+static const enum ec_err_code sarien_default_ec_err = DLED_ROM;
+static const struct err_code_entry sarien_err_codes[] = {
+	{ .post_code = POST_RAM_FAILURE, .ec_err = DLED_MEMORY, },
+	{ .post_code = POST_VIDEO_FAILURE, .ec_err = DLED_PANEL, },
+};
+
+
 void fill_lb_gpios(struct lb_gpios *gpios)
 {
    struct lb_gpio chromeos_gpios[] = {
@@ -125,3 +144,26 @@
    pmc_set_afterg3(PCH_DEV_PMC, MAINBOARD_POWER_STATE_ON);
 #endif
 }
+
+void mainboard_post(uint8_t value)
+{
+	sarien_post_code = value;
+}
+
+void die_notify(void)
+{
+	size_t i;
+	enum ec_err_code ec_err = sarien_default_ec_err;
+
+	for (i = 0; i < ARRAY_SIZE(sarien_err_codes); i++) {
+		if (sarien_err_codes[i].post_code == sarien_post_code) {
+			ec_err = sarien_err_codes[i].ec_err;
+			break;
+		}
+	}
+
+	printk(BIOS_EMERG, "Fatal error: post_code 0x%02x, EC err 0x%02x\n",
+		sarien_post_code, ec_err);
+
+	wilco_ec_err_code(ec_err);
+}

[Keith Short (Code Review)]

Hello Duncan Laurie, build bot (Jenkins),

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

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

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

Change subject: mainboard/google/sarien: set diagnostic LEDs on boot failure
......................................................................

mainboard/google/sarien: set diagnostic LEDs on boot failure

On Wilco devices, if any of the coreboot stages fails with a fatal
error, set the diagnostic LEDs with the Wilco EC.

BUG=b:124401932
BRANCH=sarien
TEST=build coreboot for sarien and arcada platforms
TEST=Remove DIMM module, confirm diagnostic LED pattern for memory
failure (2 amber, 4 white).
TEST=Forced a fatal error in both bootblock and verstage to confirm
diagnostic LEDs during these stages. This works on cold-boots only. Bug
b:132622888 tracks the mailbox failures on warm boots.

Change-Id: If865ab8203f89e499130f4677fec166b40d80174
Signed-off-by: Keith Short 
---
M src/mainboard/google/sarien/chromeos.c
1 file changed, 42 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/32777/5

[Duncan Laurie (Code Review)]

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

Change subject: mainboard/google/sarien: set diagnostic LEDs on boot failure
......................................................................

Patch Set 5:

You might move some of this to the EC so it can be reused easier.  It is a bit odd since the mainboard is the one that can intercept the post codes, but that could be passed to the EC.

[Jett Rink (Code Review)]

Jett Rink has uploaded a new patch set (#6) to the change originally created by Keith Short. ( https://review.coreboot.org/c/coreboot/+/32777 )

Change subject: mainboard/google/sarien: set diagnostic LEDs on boot failure
......................................................................

mainboard/google/sarien: set diagnostic LEDs on boot failure

On Wilco devices, if any of the coreboot stages fails with a fatal
error, set the diagnostic LEDs with the Wilco EC.

BUG=b:124401932
BRANCH=sarien
TEST=build coreboot for sarien and arcada platforms
TEST=Remove DIMM module, confirm diagnostic LED pattern for memory
failure (2 amber, 4 white).
TEST=Forced a fatal error in both bootblock and verstage to confirm
diagnostic LEDs during these stages. This works on cold-boots only. Bug
b:132622888 tracks the mailbox failures on warm boots.

Change-Id: If865ab8203f89e499130f4677fec166b40d80174
Signed-off-by: Keith Short 
---
M src/ec/google/wilco/commands.c
M src/ec/google/wilco/commands.h
M src/mainboard/google/sarien/chromeos.c
3 files changed, 50 insertions(+), 6 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/32777/6

[Jett Rink (Code Review)]

Jett Rink has uploaded a new patch set (#9) to the change originally created by Keith Short. ( https://review.coreboot.org/c/coreboot/+/32777 )

Change subject: mainboard/google/sarien: set diagnostic LEDs on boot failure
......................................................................

mainboard/google/sarien: set diagnostic LEDs on boot failure

On Wilco devices, if any of the coreboot stages fails with a fatal
error, set the diagnostic LEDs with the Wilco EC.

BUG=b:124401932
BRANCH=sarien
TEST=build coreboot for sarien and arcada platforms
TEST=Remove DIMM module, confirm diagnostic LED pattern for memory
failure (2 amber, 4 white).
TEST=Forced a fatal error in both bootblock and verstage to confirm
diagnostic LEDs during these stages. This works on cold-boots only. Bug
b:132622888 tracks the mailbox failures on warm boots.

Change-Id: If865ab8203f89e499130f4677fec166b40d80174
Signed-off-by: Keith Short 
---
M src/ec/google/wilco/commands.c
M src/ec/google/wilco/commands.h
M src/mainboard/google/sarien/chromeos.c
3 files changed, 50 insertions(+), 6 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/32777/9

Change in ...coreboot[master]: ec/google/wilco: set diagnostic LEDs on boot failure

[Duncan Laurie (Code Review)]

Duncan Laurie has uploaded a new patch set (#13) to the change originally created by Keith Short. ( https://review.coreboot.org/c/coreboot/+/32777 )

Change subject: ec/google/wilco: set diagnostic LEDs on boot failure
......................................................................

ec/google/wilco: set diagnostic LEDs on boot failure

On Wilco devices, if any of the coreboot stages fails with a fatal
error, set the diagnostic LEDs with the Wilco EC.  The last saved
post code is used to determine the error code sent to the EC.

BUG=b:124401932
BRANCH=sarien
TEST=build coreboot for sarien and arcada platforms
TEST=Remove DIMM module, confirm diagnostic LED pattern for memory
failure (2 amber, 4 white).
TEST=Forced a fatal error in both bootblock and verstage to confirm
diagnostic LEDs during these stages. This works on cold-boots only. Bug
b:132622888 tracks the mailbox failures on warm boots.

Change-Id: If865ab8203f89e499130f4677fec166b40d80174
Signed-off-by: Keith Short 
Signed-off-by: Duncan Laurie 
---
M src/ec/google/wilco/commands.c
M src/ec/google/wilco/commands.h
2 files changed, 52 insertions(+), 11 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/32777/13

Change in ...coreboot[master]: ec/google/wilco: set diagnostic LEDs on boot failure

[Duncan Laurie (Code Review)]

Duncan Laurie has uploaded a new patch set (#14) to the change originally created by Keith Short. ( https://review.coreboot.org/c/coreboot/+/32777 )

Change subject: ec/google/wilco: set diagnostic LEDs on boot failure
......................................................................

ec/google/wilco: set diagnostic LEDs on boot failure

On Wilco devices, if any of the coreboot stages fails with a fatal
error, set the diagnostic LEDs with the Wilco EC.  The last saved
post code is used to determine the error code sent to the EC.

BUG=b:124401932
BRANCH=sarien
TEST=build coreboot for sarien and arcada platforms
TEST=Remove DIMM module, confirm diagnostic LED pattern for memory
failure (2 amber, 4 white).
TEST=Forced a fatal error in both bootblock and verstage to confirm
diagnostic LEDs during these stages. This works on cold-boots only. Bug
b:132622888 tracks the mailbox failures on warm boots.

Change-Id: If865ab8203f89e499130f4677fec166b40d80174
Signed-off-by: Keith Short 
Signed-off-by: Duncan Laurie 
---
M src/ec/google/wilco/commands.c
M src/ec/google/wilco/commands.h
2 files changed, 46 insertions(+), 11 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/32777/14

Change in ...coreboot[master]: ec/google/wilco: set diagnostic LEDs on boot failure

[Jett Rink (Code Review)]

Jett Rink has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32777 )

Change subject: ec/google/wilco: set diagnostic LEDs on boot failure
......................................................................

Patch Set 14: Code-Review+1

(1 comment)

https://review.coreboot.org/#/c/32777/14/src/ec/google/wilco/commands.c 
File src/ec/google/wilco/commands.c:

https://review.coreboot.org/#/c/32777/14/src/ec/google/wilco/commands.c@208 
PS14, Line 208: die_notify
makes sense. I thought die_notify needed to stay in mainboard for some reason. This is better.

Change in ...coreboot[master]: ec/google/wilco: set diagnostic LEDs on boot failure

[Duncan Laurie (Code Review)]

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

Change subject: ec/google/wilco: set diagnostic LEDs on boot failure
......................................................................

Patch Set 14: Code-Review+2

Going to +2 since this isn't really my change, and I want to submit it to unblock testing.

Change in ...coreboot[master]: ec/google/wilco: set diagnostic LEDs on boot failure

[Furquan Shaikh (Code Review)]

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

Change subject: ec/google/wilco: set diagnostic LEDs on boot failure
......................................................................

Patch Set 14: Code-Review+2

Change in ...coreboot[master]: ec/google/wilco: set diagnostic LEDs on boot failure

[Duncan Laurie (Code Review)]

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

Change subject: ec/google/wilco: set diagnostic LEDs on boot failure
......................................................................

ec/google/wilco: set diagnostic LEDs on boot failure

On Wilco devices, if any of the coreboot stages fails with a fatal
error, set the diagnostic LEDs with the Wilco EC.  The last saved
post code is used to determine the error code sent to the EC.

BUG=b:124401932
BRANCH=sarien
TEST=build coreboot for sarien and arcada platforms
TEST=Remove DIMM module, confirm diagnostic LED pattern for memory
failure (2 amber, 4 white).
TEST=Forced a fatal error in both bootblock and verstage to confirm
diagnostic LEDs during these stages. This works on cold-boots only. Bug
b:132622888 tracks the mailbox failures on warm boots.

Change-Id: If865ab8203f89e499130f4677fec166b40d80174
Signed-off-by: Keith Short 
Signed-off-by: Duncan Laurie 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/32777
Tested-by: build bot (Jenkins) 
Reviewed-by: Jett Rink 
Reviewed-by: Duncan Laurie 
Reviewed-by: Furquan Shaikh 
---
M src/ec/google/wilco/commands.c
M src/ec/google/wilco/commands.h
2 files changed, 46 insertions(+), 11 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Duncan Laurie: Looks good to me, approved
  Furquan Shaikh: Looks good to me, approved
  Jett Rink: Looks good to me, but someone else must approve

diff --git a/src/ec/google/wilco/commands.c b/src/ec/google/wilco/commands.c
index a97a28e..9d4170f 100644
--- a/src/ec/google/wilco/commands.c
+++ b/src/ec/google/wilco/commands.c
@@ -182,8 +182,45 @@
    return !!ec_read(EC_RAM_SIGNED_FW);
 }

-int wilco_ec_err_code(enum ec_err_code err_code)
+struct err_code_entry {
+	uint8_t post_code;
+	enum ec_err_code ec_err;
+};
+
+/*
+ * Any post codes not listed in the post_code_err_map[] use default.
+ */
+static const enum ec_err_code default_ec_err = DLED_ROM;
+static const struct err_code_entry post_code_err_map[] = {
+	{ .post_code = POST_RAM_FAILURE, .ec_err = DLED_MEMORY, },
+	{ .post_code = POST_VIDEO_FAILURE, .ec_err = DLED_PANEL, },
+};
+
+/* Records the most recent post code during boot */
+static uint8_t wilco_ec_saved_post_code;
+
+void wilco_ec_save_post_code(uint8_t post_code)
 {
-	return wilco_ec_mailbox(WILCO_EC_MSG_DEFAULT, KB_ERR_CODE,
-				&err_code, 1, NULL, 0);
+	wilco_ec_saved_post_code = post_code;
+}
+
+/* Send error code to the EC based on last saved post code */
+void die_notify(void)
+{
+	size_t i;
+	enum ec_err_code err_code = default_ec_err;
+
+	for (i = 0; i < ARRAY_SIZE(post_code_err_map); i++) {
+		if (post_code_err_map[i].post_code ==
+		    wilco_ec_saved_post_code) {
+			err_code = post_code_err_map[i].ec_err;
+			break;
+		}
+	}
+
+	printk(BIOS_EMERG, "Fatal error: post_code 0x%02x, EC err 0x%02x\n",
+	       wilco_ec_saved_post_code, err_code);
+
+	wilco_ec_mailbox(WILCO_EC_MSG_DEFAULT, KB_ERR_CODE,
+			 &err_code, 1, NULL, 0);
 }
diff --git a/src/ec/google/wilco/commands.h b/src/ec/google/wilco/commands.h
index fafb7fd..9b7f2e5 100644
--- a/src/ec/google/wilco/commands.h
+++ b/src/ec/google/wilco/commands.h
@@ -319,16 +319,14 @@
 int wilco_ec_signed_fw(void);

/**
- * wilco_ec_err_code
+ * wilco_ec_save_post_code
  *
- * Send an error code to the EC to indicate a failed boot.  The EC flashes the
- * platform LED amber and white to provide user indication of the failure type.
+ * Save this post code as the most recent progress step.  If the boot fails
+ * and calls die_notify() this post code will be used to send an error code
+ * to the EC indicating the failure.
  *
- * @err_code:		Error code to send to the EC
- *
- * Returns 0 if EC command was successful
- * Returns -1 if EC command failed
+ * @post_code: Post code to save
  */
-int wilco_ec_err_code(enum ec_err_code err_code);
+void wilco_ec_save_post_code(uint8_t post_code);

#endif /* EC_GOOGLE_WILCO_COMMANDS_H */