Change in ...coreboot[master]: mb/google/sarien: Send post code to the EC in ROM stage

[EricR Lai (Code Review)]

EricR Lai has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33030

Change subject: mb/google/sarien: Send post code to the EC in ROM stage
......................................................................

mb/google/sarien: Send post code to the EC in ROM stage

Use the mainboard post code hook to inform the wilco EC driver of the
ROM stage.

BUG=b:124401932,b:133466714,b:133600566
BRANCH=sarien
TEST=Remove DIMM module, confirm diagnostic LED pattern for memory
failure (2 amber, 4 white).

Signed-off-by: Eric Lai 
Change-Id: Ic03827e148b8c05e6b97f73ad36964832942c09a
---
M src/mainboard/google/sarien/ramstage.c
M src/mainboard/google/sarien/romstage.c
2 files changed, 11 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/30/33030/1

diff --git a/src/mainboard/google/sarien/ramstage.c b/src/mainboard/google/sarien/ramstage.c
index e246419..804d35a 100644
--- a/src/mainboard/google/sarien/ramstage.c
+++ b/src/mainboard/google/sarien/ramstage.c
@@ -70,10 +70,12 @@
    cnl_configure_pads(gpio_table, num_gpios);
 }

+#if ENV_RAMSTAGE
 void mainboard_post(uint8_t value)
 {
    wilco_ec_save_post_code(value);
 }
+#endif

static void mainboard_enable(struct device *dev)
 {
diff --git a/src/mainboard/google/sarien/romstage.c b/src/mainboard/google/sarien/romstage.c
index 20eee7f..0b0c32a 100644
--- a/src/mainboard/google/sarien/romstage.c
+++ b/src/mainboard/google/sarien/romstage.c
@@ -16,6 +16,8 @@
 #include <ec/google/wilco/romstage.h>
 #include <soc/cnl_memcfg_init.h>
 #include <soc/romstage.h>
+#include <ec/google/wilco/commands.h>
+#include <console/console.h>

static const struct cnl_mb_cfg memcfg = {
    /* Access memory info through SMBUS. */
@@ -55,6 +57,13 @@
    .vref_ca_config = 2,
 };

+#if ENV_ROMSTAGE
+void mainboard_post(uint8_t value)
+{
+	wilco_ec_save_post_code(value);
+}
+#endif
+
 void mainboard_memory_init_params(FSPM_UPD *memupd)
 {
    wilco_ec_romstage_init();

[EricR Lai (Code Review)]

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33030 )

Change subject: mb/google/sarien: Send post code to the EC in ROM stage
......................................................................

Patch Set 1:

Please help review this.

[Duncan Laurie (Code Review)]

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

Change subject: mb/google/sarien: Send post code to the EC in ROM stage
......................................................................

Patch Set 1:

I guess this is because we deviated from Keith's original design that had this in chromeos.c where it gets included in all stages.

We can pull mainboard_post() into a separate file (ec.c?) and include it in all the appropriate stages and then it doesn't have to be duplicated or guarded.

[EricR Lai (Code Review)]

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33030 )

Change subject: mb/google/sarien: Send post code to the EC in ROM stage
......................................................................

Patch Set 1:
> Patch Set 1:
>
> I guess this is because we deviated from Keith's original design that had this in chromeos.c where it gets included in all stages.
>
> We can pull mainboard_post() into a separate file (ec.c?) and include it in all the appropriate stages and then it doesn't have to be duplicated or guarded.

I can try that and update the result here.

[Keith Short (Code Review)]

Keith Short has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33030 )

Change subject: mb/google/sarien: Send post code to the EC in ROM stage
......................................................................

Patch Set 1:
> Patch Set 1:
> > Patch Set 1:
> >
> > I guess this is because we deviated from Keith's original design that had this in chromeos.c where it gets included in all stages.
> >
> > We can pull mainboard_post() into a separate file (ec.c?) and include it in all the appropriate stages and then it doesn't have to be duplicated or guarded.
>
> I can try that and update the result here.

Is there a reason why mainboard_post() can't just be added back to chromeos.c instead of creating a new file.

[Duncan Laurie (Code Review)]

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

Change subject: mb/google/sarien: Send post code to the EC in ROM stage
......................................................................

Patch Set 1:
> Patch Set 1:
> > Patch Set 1:
> > > Patch Set 1:
> > >
> > > I guess this is because we deviated from Keith's original design that had this in chromeos.c where it gets included in all stages.
> > >
> > > We can pull mainboard_post() into a separate file (ec.c?) and include it in all the appropriate stages and then it doesn't have to be duplicated or guarded.
> >
> > I can try that and update the result here.
>
> Is there a reason why mainboard_post() can't just be added back to chromeos.c instead of creating a new file.

Only because this isn't really Chrome OS specific so it seems useful to send the post codes even if CONFIG_CHROMEOS was not enabled.

[EricR Lai (Code Review)]

EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33030 )

Change subject: mb/google/sarien: Send post code to the EC in ROM stage
......................................................................

Patch Set 1:
> Patch Set 1:
> > Patch Set 1:
> > > Patch Set 1:
> > >
> > > I guess this is because we deviated from Keith's original design that had this in chromeos.c where it gets included in all stages.
> > >
> > > We can pull mainboard_post() into a separate file (ec.c?) and include it in all the appropriate stages and then it doesn't have to be duplicated or guarded.
> >
> > I can try that and update the result here.
>
> Is there a reason why mainboard_post() can't just be added back to chromeos.c instead of creating a new file.

I wanna know as well. And I try add ec.c like https://review.coreboot.org/c/coreboot/+/33047. But this just a try dry code not verify.

[EricR Lai (Code Review)]

EricR Lai has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/33030 )

Change subject: mb/google/sarien: Send post code to the EC in ROM stage
......................................................................

Abandoned

change to https://review.coreboot.org/c/coreboot/+/33047