Change in coreboot[master]: mb/google/beltino: Move Super I/O init to bootblock

[Angel Pons (Code Review)]

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43104 )

Change subject: mb/google/beltino: Move Super I/O init to bootblock
......................................................................

mb/google/beltino: Move Super I/O init to bootblock

Also remove an unneeded `pch_enable_lpc` function call.

Change-Id: I83158a655670d4e6cd91f6bf3332d1b6f9f655d1
Signed-off-by: Angel Pons 
---
M src/mainboard/google/beltino/Makefile.inc
A src/mainboard/google/beltino/bootblock.c
M src/mainboard/google/beltino/romstage.c
3 files changed, 22 insertions(+), 13 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/04/43104/1

diff --git a/src/mainboard/google/beltino/Makefile.inc b/src/mainboard/google/beltino/Makefile.inc
index ee8b8e5..8c5d6c5 100644
--- a/src/mainboard/google/beltino/Makefile.inc
+++ b/src/mainboard/google/beltino/Makefile.inc
@@ -1,5 +1,7 @@
 ## SPDX-License-Identifier: GPL-2.0-only

+bootblock-y += bootblock.c
+
 romstage-$(CONFIG_CHROMEOS) += chromeos.c
 ramstage-$(CONFIG_CHROMEOS) += chromeos.c
 verstage-$(CONFIG_VBOOT_SEPARATE_VERSTAGE) += chromeos.c
@@ -8,7 +10,8 @@
 smm-y += smihandler.c variants/$(VARIANT_DIR)/led.c

romstage-y += variants/$(VARIANT_DIR)/gpio.c
-romstage-y += variants/$(VARIANT_DIR)/led.c
+
+bootblock-y += variants/$(VARIANT_DIR)/led.c

ramstage-$(CONFIG_MAINBOARD_USE_LIBGFXINIT) += gma-mainboard.ads

diff --git a/src/mainboard/google/beltino/bootblock.c b/src/mainboard/google/beltino/bootblock.c
new file mode 100644
index 0000000..c52059b
--- /dev/null
+++ b/src/mainboard/google/beltino/bootblock.c
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include <stdint.h>
+#include <southbridge/intel/lynxpoint/pch.h>
+#include <superio/ite/common/ite.h>
+#include <superio/ite/it8772f/it8772f.h>
+#include "onboard.h"
+
+void mainboard_config_superio(void)
+{
+	/* Early SuperIO setup */
+	ite_kill_watchdog(IT8772F_GPIO_DEV);
+	it8772f_ac_resume_southbridge(IT8772F_SUPERIO_DEV);
+	ite_enable_serial(IT8772F_SERIAL_DEV, CONFIG_TTYS0_BASE);
+
+	/* Turn on Power LED */
+	set_power_led(LED_ON);
+}
diff --git a/src/mainboard/google/beltino/romstage.c b/src/mainboard/google/beltino/romstage.c
index 5d9c37c..6f95831 100644
--- a/src/mainboard/google/beltino/romstage.c
+++ b/src/mainboard/google/beltino/romstage.c
@@ -7,9 +7,6 @@
 #include <northbridge/intel/haswell/raminit.h>
 #include <southbridge/intel/lynxpoint/lp_gpio.h>
 #include <southbridge/intel/lynxpoint/pch.h>
-#include <superio/ite/common/ite.h>
-#include <superio/ite/it8772f/it8772f.h>
-#include "onboard.h"

void mainboard_config_rcba(void)
 {
@@ -107,15 +104,6 @@
    	.pei_data = &pei_data,
    };

-	/* Early SuperIO setup */
-	ite_kill_watchdog(IT8772F_GPIO_DEV);
-	it8772f_ac_resume_southbridge(IT8772F_SUPERIO_DEV);
-	pch_enable_lpc();
-	ite_enable_serial(IT8772F_SERIAL_DEV, CONFIG_TTYS0_BASE);
-
-    /* Turn on Power LED */
-	set_power_led(LED_ON);
-
    /* Call into the real romstage main with this board's attributes. */
    romstage_common(&romstage_params);
 }

[Tristan Corrick (Code Review)]

Tristan Corrick has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43104 )

Change subject: mb/google/beltino: Move Super I/O init to bootblock
......................................................................

Patch Set 3: Code-Review+1

[Matt DeVillier (Code Review)]

Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43104 )

Change subject: mb/google/beltino: Move Super I/O init to bootblock
......................................................................

Patch Set 3:

is this just a "we can do it earlier, so might as well" type change? There's no serial output on retail devices, so really it's just turning on the LED and fan a few ms earlier

[Angel Pons (Code Review)]

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

Change subject: mb/google/beltino: Move Super I/O init to bootblock
......................................................................

Patch Set 3:
> Patch Set 3:
>
> is this just a "we can do it earlier, so might as well" type change? There's no serial output on retail devices, so really it's just turning on the LED and fan a few ms earlier

It's to have `mainboard_romstage_entry` only fill PEI data and call common romstage, so that I can move `mainboard_romstage_entry` to the northbridge code.

[Michael Niewöhner (Code Review)]

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43104 )

Change subject: mb/google/beltino: Move Super I/O init to bootblock
......................................................................

Patch Set 3: Code-Review+1

[Michael Niewöhner (Code Review)]

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43104 )

Change subject: mb/google/beltino: Move Super I/O init to bootblock
......................................................................

Patch Set 3: Code-Review+2

[Nico Huber (Code Review)]

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

Change subject: mb/google/beltino: Move Super I/O init to bootblock
......................................................................

Patch Set 3: Code-Review+2

[Matt DeVillier (Code Review)]

Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43104 )

Change subject: mb/google/beltino: Move Super I/O init to bootblock
......................................................................

Patch Set 3: Code-Review+2

built/booted google/zako, nothing appears to have broken

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43104 )

Change subject: mb/google/beltino: Move Super I/O init to bootblock
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/43104/3/src/mainboard/google/beltin... 
File src/mainboard/google/beltino/bootblock.c:

https://review.coreboot.org/c/coreboot/+/43104/3/src/mainboard/google/beltin... 
PS3, Line 3: <stdint.h>
not used

[Angel Pons (Code Review)]

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

Change subject: mb/google/beltino: Move Super I/O init to bootblock
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/43104/3/src/mainboard/google/beltin... 
File src/mainboard/google/beltino/bootblock.c:

https://review.coreboot.org/c/coreboot/+/43104/3/src/mainboard/google/beltin... 
PS3, Line 3: <stdint.h>
> not used

will handle on a follow-up, this commit has been blocking a patch train for too long.

[Angel Pons (Code Review)]

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

Change subject: mb/google/beltino: Move Super I/O init to bootblock
......................................................................

Patch Set 3:
> Patch Set 3: Code-Review+2
>
> built/booted google/zako, nothing appears to have broken

Thanks for testing!

[Angel Pons (Code Review)]

Angel Pons has submitted this change. ( https://review.coreboot.org/c/coreboot/+/43104 )

Change subject: mb/google/beltino: Move Super I/O init to bootblock
......................................................................

mb/google/beltino: Move Super I/O init to bootblock

Also remove an unneeded `pch_enable_lpc` function call.

Change-Id: I83158a655670d4e6cd91f6bf3332d1b6f9f655d1
Signed-off-by: Angel Pons 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/43104
Tested-by: build bot (Jenkins) 
Reviewed-by: Tristan Corrick 
Reviewed-by: Michael Niewöhner
Reviewed-by: Nico Huber 
Reviewed-by: Matt DeVillier 
---
M src/mainboard/google/beltino/Makefile.inc
A src/mainboard/google/beltino/bootblock.c
M src/mainboard/google/beltino/romstage.c
3 files changed, 22 insertions(+), 13 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Nico Huber: Looks good to me, approved
  Matt DeVillier: Looks good to me, approved
  Tristan Corrick: Looks good to me, but someone else must approve
  Michael Niewöhner: Looks good to me, approved

diff --git a/src/mainboard/google/beltino/Makefile.inc b/src/mainboard/google/beltino/Makefile.inc
index ee8b8e5..8c5d6c5 100644
--- a/src/mainboard/google/beltino/Makefile.inc
+++ b/src/mainboard/google/beltino/Makefile.inc
@@ -1,5 +1,7 @@
 ## SPDX-License-Identifier: GPL-2.0-only

+bootblock-y += bootblock.c
+
 romstage-$(CONFIG_CHROMEOS) += chromeos.c
 ramstage-$(CONFIG_CHROMEOS) += chromeos.c
 verstage-$(CONFIG_VBOOT_SEPARATE_VERSTAGE) += chromeos.c
@@ -8,7 +10,8 @@
 smm-y += smihandler.c variants/$(VARIANT_DIR)/led.c

romstage-y += variants/$(VARIANT_DIR)/gpio.c
-romstage-y += variants/$(VARIANT_DIR)/led.c
+
+bootblock-y += variants/$(VARIANT_DIR)/led.c

ramstage-$(CONFIG_MAINBOARD_USE_LIBGFXINIT) += gma-mainboard.ads

diff --git a/src/mainboard/google/beltino/bootblock.c b/src/mainboard/google/beltino/bootblock.c
new file mode 100644
index 0000000..c52059b
--- /dev/null
+++ b/src/mainboard/google/beltino/bootblock.c
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include <stdint.h>
+#include <southbridge/intel/lynxpoint/pch.h>
+#include <superio/ite/common/ite.h>
+#include <superio/ite/it8772f/it8772f.h>
+#include "onboard.h"
+
+void mainboard_config_superio(void)
+{
+	/* Early SuperIO setup */
+	ite_kill_watchdog(IT8772F_GPIO_DEV);
+	it8772f_ac_resume_southbridge(IT8772F_SUPERIO_DEV);
+	ite_enable_serial(IT8772F_SERIAL_DEV, CONFIG_TTYS0_BASE);
+
+	/* Turn on Power LED */
+	set_power_led(LED_ON);
+}
diff --git a/src/mainboard/google/beltino/romstage.c b/src/mainboard/google/beltino/romstage.c
index 5d9c37c..6f95831 100644
--- a/src/mainboard/google/beltino/romstage.c
+++ b/src/mainboard/google/beltino/romstage.c
@@ -7,9 +7,6 @@
 #include <northbridge/intel/haswell/raminit.h>
 #include <southbridge/intel/lynxpoint/lp_gpio.h>
 #include <southbridge/intel/lynxpoint/pch.h>
-#include <superio/ite/common/ite.h>
-#include <superio/ite/it8772f/it8772f.h>
-#include "onboard.h"

void mainboard_config_rcba(void)
 {
@@ -107,15 +104,6 @@
    	.pei_data = &pei_data,
    };

-	/* Early SuperIO setup */
-	ite_kill_watchdog(IT8772F_GPIO_DEV);
-	it8772f_ac_resume_southbridge(IT8772F_SUPERIO_DEV);
-	pch_enable_lpc();
-	ite_enable_serial(IT8772F_SERIAL_DEV, CONFIG_TTYS0_BASE);
-
-    /* Turn on Power LED */
-	set_power_led(LED_ON);
-
    /* Call into the real romstage main with this board's attributes. */
    romstage_common(&romstage_params);
 }

[Angel Pons (Code Review)]

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

Change subject: mb/google/beltino: Move Super I/O init to bootblock
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/43104/3/src/mainboard/google/beltin... 
File src/mainboard/google/beltino/bootblock.c:

https://review.coreboot.org/c/coreboot/+/43104/3/src/mainboard/google/beltin... 
PS3, Line 3: <stdint.h>
> will handle on a follow-up, this commit has been blocking a patch train for too long.

CB:43366