Craig Hesling has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37459 )
Change subject: hatch: Fix FPMCU pwr/rst gpio handling ......................................................................
hatch: Fix FPMCU pwr/rst gpio handling
1. No gpio control in bootblock 2. Disable power and assert reset in ramstage gpio. 3. Power on and then deassert reset at the end of ramstage gpio. 4. Disable power and assert reset when entering S5.
On "reboot", the amount of time the power is disabled for is equivalent to the amount of time between #4 and #3, which is about 400ms on Kohaku.
Since #2 forces power off for FPMCU, S3 resume will still not work properly.
Change-Id: I2e3ff42715611d519677a4256bdd172ec98687f9 Signed-off-by: Craig Hesling hesling@chromium.org --- M src/mainboard/google/hatch/ramstage.c M src/mainboard/google/hatch/variants/baseboard/gpio.c M src/mainboard/google/hatch/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/hatch/variants/helios/Makefile.inc M src/mainboard/google/hatch/variants/helios/gpio.c A src/mainboard/google/hatch/variants/helios/ramstage.c M src/mainboard/google/hatch/variants/kindred/gpio.c M src/mainboard/google/hatch/variants/kohaku/Makefile.inc M src/mainboard/google/hatch/variants/kohaku/gpio.c A src/mainboard/google/hatch/variants/kohaku/ramstage.c 10 files changed, 92 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/37459/1
diff --git a/src/mainboard/google/hatch/ramstage.c b/src/mainboard/google/hatch/ramstage.c index e9f5062..e84aa18 100644 --- a/src/mainboard/google/hatch/ramstage.c +++ b/src/mainboard/google/hatch/ramstage.c @@ -31,6 +31,11 @@ /* Override dev tree settings per board */ }
+void __weak variant_ramstage_init(void) +{ + /* Default weak implementation */ +} + static void mainboard_init(struct device *dev) { mainboard_ec_init(); @@ -56,6 +61,8 @@ base_gpios, override_table, override_gpios); + + variant_ramstage_init(); }
struct chip_operations mainboard_ops = { diff --git a/src/mainboard/google/hatch/variants/baseboard/gpio.c b/src/mainboard/google/hatch/variants/baseboard/gpio.c index 598600b..976950f 100644 --- a/src/mainboard/google/hatch/variants/baseboard/gpio.c +++ b/src/mainboard/google/hatch/variants/baseboard/gpio.c @@ -38,7 +38,7 @@ /* A11 : PCH_SPI_FPMCU_CS_L */ PAD_CFG_NF(GPP_A11, NONE, DEEP, NF2), /* A12 : FPMCU_RST_ODL */ - PAD_CFG_GPO(GPP_A12, 1, DEEP), + PAD_CFG_GPO(GPP_A12, 0, DEEP), /* A13 : SUSWARN_L */ PAD_CFG_NF(GPP_A13, NONE, DEEP, NF1), /* A14 : ESPI_RST_L */ @@ -133,7 +133,7 @@ /* C10 : GPP_10 ==> GPP_C10_TP */ PAD_NC(GPP_C10, NONE), /* C11 : GPP_11 ==> EN_FP_RAILS */ - PAD_CFG_GPO(GPP_C11, 1, DEEP), + PAD_CFG_GPO(GPP_C11, 0, DEEP), /* C12 : GPP_C12 ==> NC */ PAD_NC(GPP_C12, NONE), /* C13 : EC_PCH_INT_L */ @@ -398,20 +398,19 @@ }
/* - * Default GPIO settings before entering sleep. Configure A12: FPMCU_RST_ODL - * as GPO before entering sleep. + * Default GPIO settings before entering sleep. */ static const struct pad_config default_sleep_gpio_table[] = { - PAD_CFG_GPO(GPP_A12, 1, DEEP), /* FPMCU_RST_ODL */ };
/* * GPIO settings before entering S5, which are same as * default_sleep_gpio_table but also, - * turn off EN_PP3300_WWAN. + * turn off EN_PP3300_WWAN and FPMCU. */ static const struct pad_config s5_sleep_gpio_table[] = { - PAD_CFG_GPO(GPP_A12, 1, DEEP), /* FPMCU_RST_ODL */ + PAD_CFG_GPO(GPP_A12, 0, DEEP), /* FPMCU_RST_ODL */ + PAD_CFG_GPO(GPP_C11, 0, DEEP), /* PCH_FP_PWR_EN */ PAD_CFG_GPO(GPP_A18, 0, DEEP), /* EN_PP3300_WWAN */ };
diff --git a/src/mainboard/google/hatch/variants/baseboard/include/baseboard/variants.h b/src/mainboard/google/hatch/variants/baseboard/include/baseboard/variants.h index 920e428..1542d9b 100644 --- a/src/mainboard/google/hatch/variants/baseboard/include/baseboard/variants.h +++ b/src/mainboard/google/hatch/variants/baseboard/include/baseboard/variants.h @@ -50,4 +50,7 @@ /* Modify devictree settings during ramstage. */ void variant_devtree_update(void);
+/* Perform variant specific initialization early on in ramstage. */ +void variant_ramstage_init(void); + #endif /* BASEBOARD_VARIANTS_H */ diff --git a/src/mainboard/google/hatch/variants/helios/Makefile.inc b/src/mainboard/google/hatch/variants/helios/Makefile.inc index fbd69c4..be074b7 100644 --- a/src/mainboard/google/hatch/variants/helios/Makefile.inc +++ b/src/mainboard/google/hatch/variants/helios/Makefile.inc @@ -17,4 +17,6 @@
romstage-y += memory.c bootblock-y += gpio.c + ramstage-y += gpio.c +ramstage-y += ramstage.c diff --git a/src/mainboard/google/hatch/variants/helios/gpio.c b/src/mainboard/google/hatch/variants/helios/gpio.c index 85eb3fc..ef87876 100644 --- a/src/mainboard/google/hatch/variants/helios/gpio.c +++ b/src/mainboard/google/hatch/variants/helios/gpio.c @@ -117,8 +117,6 @@ * needed in this table. */ static const struct pad_config early_gpio_table[] = { - /* A12 : FPMCU_RST_ODL */ - PAD_CFG_GPO(GPP_A12, 0, DEEP), /* B15 : H1_SLAVE_SPI_CS_L */ PAD_CFG_NF(GPP_B15, NONE, DEEP, NF1), /* B16 : H1_SLAVE_SPI_CLK */ @@ -127,8 +125,6 @@ PAD_CFG_NF(GPP_B17, NONE, DEEP, NF1), /* B18 : H1_SLAVE_SPI_MOSI_R */ PAD_CFG_NF(GPP_B18, NONE, DEEP, NF1), - /* C11 : GPP_C11 ==> EN_FP_RAILS */ - PAD_CFG_GPO(GPP_C11, 1, DEEP), /* C14 : BT_DISABLE_L */ PAD_CFG_GPO(GPP_C14, 0, DEEP), /* PCH_WP_OD */ @@ -149,15 +145,24 @@ return early_gpio_table; }
+static const struct pad_config s5_sleep_gpio_table[] = { + PAD_CFG_GPO(GPP_A12, 0, DEEP), /* FPMCU_RST_ODL */ + PAD_CFG_GPO(GPP_C11, 0, DEEP), /* PCH_FP_PWR_EN */ +}; + /* * GPIO settings before entering all sleep states */ static const struct pad_config sleep_gpio_table[] = { - PAD_CFG_GPO(GPP_A12, 1, DEEP), /* FPMCU_RST_ODL */ };
const struct pad_config *variant_sleep_gpio_table(u8 slp_typ, size_t *num) { + if (slp_typ == ACPI_S5) { + *num = ARRAY_SIZE(s5_sleep_gpio_table); + return s5_sleep_gpio_table; + } + *num = ARRAY_SIZE(sleep_gpio_table); return sleep_gpio_table; } diff --git a/src/mainboard/google/hatch/variants/helios/ramstage.c b/src/mainboard/google/hatch/variants/helios/ramstage.c new file mode 100644 index 0000000..8e735d1 --- /dev/null +++ b/src/mainboard/google/hatch/variants/helios/ramstage.c @@ -0,0 +1,26 @@ +/* + * This file is part of the coreboot project. + * + * Copyright 2019 Google LLC + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <delay.h> +#include <gpio.h> +#include <baseboard/variants.h> +#include <soc/gpio.h> + +void variant_ramstage_init(void) +{ + gpio_output(GPP_C11, 1); + mdelay(1); + gpio_output(GPP_A12, 1); +} diff --git a/src/mainboard/google/hatch/variants/kindred/gpio.c b/src/mainboard/google/hatch/variants/kindred/gpio.c index 827ee0b..9e2d818 100644 --- a/src/mainboard/google/hatch/variants/kindred/gpio.c +++ b/src/mainboard/google/hatch/variants/kindred/gpio.c @@ -154,8 +154,6 @@ * needed in this table. */ static const struct pad_config early_gpio_table[] = { - /* A12 : FPMCU_RST_ODL */ - PAD_CFG_GPO(GPP_A12, 0, DEEP), /* B15 : H1_SLAVE_SPI_CS_L */ PAD_CFG_NF(GPP_B15, NONE, DEEP, NF1), /* B16 : H1_SLAVE_SPI_CLK */ diff --git a/src/mainboard/google/hatch/variants/kohaku/Makefile.inc b/src/mainboard/google/hatch/variants/kohaku/Makefile.inc index 9cdff32..6bd2973 100644 --- a/src/mainboard/google/hatch/variants/kohaku/Makefile.inc +++ b/src/mainboard/google/hatch/variants/kohaku/Makefile.inc @@ -17,4 +17,6 @@ romstage-y += memory.c
bootblock-y += gpio.c + ramstage-y += gpio.c +ramstage-y += ramstage.c diff --git a/src/mainboard/google/hatch/variants/kohaku/gpio.c b/src/mainboard/google/hatch/variants/kohaku/gpio.c index 61d3375..a455141 100644 --- a/src/mainboard/google/hatch/variants/kohaku/gpio.c +++ b/src/mainboard/google/hatch/variants/kohaku/gpio.c @@ -97,8 +97,6 @@ * needed in this table. */ static const struct pad_config early_gpio_table[] = { - /* A12 : FPMCU_RST_ODL */ - PAD_CFG_GPO(GPP_A12, 0, DEEP), /* B15 : H1_SLAVE_SPI_CS_L */ PAD_CFG_NF(GPP_B15, NONE, DEEP, NF1), /* B16 : H1_SLAVE_SPI_CLK */ @@ -135,15 +133,24 @@ return early_gpio_table; }
+static const struct pad_config s5_sleep_gpio_table[] = { + PAD_CFG_GPO(GPP_A12, 0, DEEP), /* FPMCU_RST_ODL */ + PAD_CFG_GPO(GPP_C11, 0, DEEP), /* PCH_FP_PWR_EN */ +}; + /* * GPIO settings before entering all sleep states */ static const struct pad_config sleep_gpio_table[] = { - PAD_CFG_GPO(GPP_A12, 1, DEEP), /* FPMCU_RST_ODL */ };
const struct pad_config *variant_sleep_gpio_table(u8 slp_typ, size_t *num) { + if (slp_typ == ACPI_S5) { + *num = ARRAY_SIZE(s5_sleep_gpio_table); + return s5_sleep_gpio_table; + } + *num = ARRAY_SIZE(sleep_gpio_table); return sleep_gpio_table; } diff --git a/src/mainboard/google/hatch/variants/kohaku/ramstage.c b/src/mainboard/google/hatch/variants/kohaku/ramstage.c new file mode 100644 index 0000000..8e735d1 --- /dev/null +++ b/src/mainboard/google/hatch/variants/kohaku/ramstage.c @@ -0,0 +1,26 @@ +/* + * This file is part of the coreboot project. + * + * Copyright 2019 Google LLC + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <delay.h> +#include <gpio.h> +#include <baseboard/variants.h> +#include <soc/gpio.h> + +void variant_ramstage_init(void) +{ + gpio_output(GPP_C11, 1); + mdelay(1); + gpio_output(GPP_A12, 1); +}
Hello Tim Wawrzynczak, Shelley Chen, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37459
to look at the new patch set (#2).
Change subject: hatch: Fix FPMCU pwr/rst gpio handling ......................................................................
hatch: Fix FPMCU pwr/rst gpio handling
1. No gpio control in bootblock 2. Disable power and assert reset in ramstage gpio 3. Power on and then deassert reset at the end of ramstage gpio 4. Disable power and assert reset when entering S5
On "reboot", the amount of time the power is disabled for is equivalent to the amount of time between triggering #4 and wrapping around to #3, which is about 400ms on Kohaku.
Since #2 forces power off for FPMCU, S3 resume will still not work properly.
The control signals have been validated on a Kohaku in the following scenarios: 1. Cold startup 2. Issuing a "reboot" command 3. Issuing a "halt -p" and powering back on within 10 seconds 4. Issuing a "halt -p" and powering back on after 10 seconds 5. Entering and leaving S3 (does not work properly) 6. Entering and leaving S0iX
Change-Id: I2e3ff42715611d519677a4256bdd172ec98687f9 Signed-off-by: Craig Hesling hesling@chromium.org --- M src/mainboard/google/hatch/ramstage.c M src/mainboard/google/hatch/variants/baseboard/gpio.c M src/mainboard/google/hatch/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/hatch/variants/helios/Makefile.inc M src/mainboard/google/hatch/variants/helios/gpio.c A src/mainboard/google/hatch/variants/helios/ramstage.c M src/mainboard/google/hatch/variants/kindred/gpio.c M src/mainboard/google/hatch/variants/kohaku/Makefile.inc M src/mainboard/google/hatch/variants/kohaku/gpio.c A src/mainboard/google/hatch/variants/kohaku/ramstage.c 10 files changed, 92 insertions(+), 17 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/37459/2
Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37459 )
Change subject: hatch: Fix FPMCU pwr/rst gpio handling ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37459/2/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/helios/gpio.c:
https://review.coreboot.org/c/coreboot/+/37459/2/src/mainboard/google/hatch/... PS2, Line 161: if (slp_typ == ACPI_S5) { are you fixing s5 or s3? I thought that you were fixing entering/exiting s3.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37459 )
Change subject: hatch: Fix FPMCU pwr/rst gpio handling ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/37459/2/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/helios/ramstage.c:
https://review.coreboot.org/c/coreboot/+/37459/2/src/mainboard/google/hatch/... PS2, Line 22: { A comment here would be helpful
https://review.coreboot.org/c/coreboot/+/37459/2/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/kohaku/ramstage.c:
https://review.coreboot.org/c/coreboot/+/37459/2/src/mainboard/google/hatch/... PS2, Line 22: { Same here
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37459 )
Change subject: hatch: Fix FPMCU pwr/rst gpio handling ......................................................................
Patch Set 2:
(6 comments)
https://review.coreboot.org/c/coreboot/+/37459/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37459/2//COMMIT_MSG@20 PS2, Line 20: BUG=? BRANCH=hatch TEST=
https://review.coreboot.org/c/coreboot/+/37459/2/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/37459/2/src/mainboard/google/hatch/... PS2, Line 414: 1 Can you do a git blame on this to see why it was set to 1 in the first place? I am suspecting if there was some power leakage that was observed in S5?
https://review.coreboot.org/c/coreboot/+/37459/2/src/mainboard/google/hatch/... PS2, Line 424: *num = ARRAY_SIZE(default_sleep_gpio_table); : return default_sleep_gpio_table; This can just be: *num = 0; return NULL;
since default_sleep_gpio_table is just empty.
https://review.coreboot.org/c/coreboot/+/37459/2/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/helios/gpio.c:
https://review.coreboot.org/c/coreboot/+/37459/2/src/mainboard/google/hatch/... PS2, Line 161: if (slp_typ == ACPI_S5) {
are you fixing s5 or s3? I thought that you were fixing entering/exiting s3.
I believe Craig wants to disable the pads only when entering S5 and leave as is when going to S3.
https://review.coreboot.org/c/coreboot/+/37459/2/src/mainboard/google/hatch/... PS2, Line 166: *num = ARRAY_SIZE(sleep_gpio_table); : return sleep_gpio_table; Same comment as baseboard.
https://review.coreboot.org/c/coreboot/+/37459/2/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/kohaku/gpio.c:
https://review.coreboot.org/c/coreboot/+/37459/2/src/mainboard/google/hatch/... PS2, Line 154: *num = ARRAY_SIZE(sleep_gpio_table); : return sleep_gpio_table; same comment as baseboard.
Hello Tim Wawrzynczak, Shelley Chen, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37459
to look at the new patch set (#3).
Change subject: hatch: Fix FPMCU pwr/rst gpio handling ......................................................................
hatch: Fix FPMCU pwr/rst gpio handling
1. No gpio control in bootblock 2. Disable power and assert reset in ramstage gpio 3. Power on and then deassert reset at the end of ramstage gpio 4. Disable power and assert reset when entering S5
On "reboot", the amount of time the power is disabled for is equivalent to the amount of time between triggering #4 and wrapping around to #3, which is about 400ms on Kohaku.
Since #2 forces power off for FPMCU, S3 resume will still not work properly.
The control signals have been validated on a Kohaku in the following scenarios: 1. Cold startup 2. Issuing a "reboot" command 3. Issuing a "halt -p" and powering back on within 10 seconds 4. Issuing a "halt -p" and powering back on after 10 seconds 5. Entering and leaving S3 (does not work properly) 6. Entering and leaving S0iX
Change-Id: I2e3ff42715611d519677a4256bdd172ec98687f9 Signed-off-by: Craig Hesling hesling@chromium.org --- M src/mainboard/google/hatch/ramstage.c M src/mainboard/google/hatch/variants/baseboard/gpio.c M src/mainboard/google/hatch/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/hatch/variants/helios/Makefile.inc M src/mainboard/google/hatch/variants/helios/gpio.c A src/mainboard/google/hatch/variants/helios/ramstage.c M src/mainboard/google/hatch/variants/kindred/gpio.c M src/mainboard/google/hatch/variants/kohaku/Makefile.inc M src/mainboard/google/hatch/variants/kohaku/gpio.c A src/mainboard/google/hatch/variants/kohaku/ramstage.c 10 files changed, 103 insertions(+), 36 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/37459/3
Craig Hesling has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37459 )
Change subject: hatch: Fix FPMCU pwr/rst gpio handling ......................................................................
Patch Set 3:
(8 comments)
https://review.coreboot.org/c/coreboot/+/37459/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37459/2//COMMIT_MSG@20 PS2, Line 20:
BUG=? […]
Oh, I thought we were avoiding ChromeOS specific references. Is this not the case?
https://review.coreboot.org/c/coreboot/+/37459/2/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/37459/2/src/mainboard/google/hatch/... PS2, Line 414: 1
Can you do a git blame on this to see why it was set to 1 in the first place? I am suspecting if the […]
It looks like it points back to b/128686027, but it is unclear if this particular change fixed the bug. With my changes, I do not see the symptoms of this bug(not being able to export gpio control signals in user space).
https://review.coreboot.org/c/coreboot/+/37459/2/src/mainboard/google/hatch/... PS2, Line 424: *num = ARRAY_SIZE(default_sleep_gpio_table); : return default_sleep_gpio_table;
This can just be: […]
Gotcha.
https://review.coreboot.org/c/coreboot/+/37459/2/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/helios/gpio.c:
https://review.coreboot.org/c/coreboot/+/37459/2/src/mainboard/google/hatch/... PS2, Line 161: if (slp_typ == ACPI_S5) {
I believe Craig wants to disable the pads only when entering S5 and leave as is when going to S3.
Furquan is correct. This particular section simply ensures that power is cut immediately when user issues a shutdown. This sections does not break S3, setting the gpio base table breaks S3.
https://review.coreboot.org/c/coreboot/+/37459/2/src/mainboard/google/hatch/... PS2, Line 166: *num = ARRAY_SIZE(sleep_gpio_table); : return sleep_gpio_table;
Same comment as baseboard.
Done
https://review.coreboot.org/c/coreboot/+/37459/2/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/helios/ramstage.c:
https://review.coreboot.org/c/coreboot/+/37459/2/src/mainboard/google/hatch/... PS2, Line 22: {
A comment here would be helpful
Done
https://review.coreboot.org/c/coreboot/+/37459/2/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/kohaku/gpio.c:
https://review.coreboot.org/c/coreboot/+/37459/2/src/mainboard/google/hatch/... PS2, Line 154: *num = ARRAY_SIZE(sleep_gpio_table); : return sleep_gpio_table;
same comment as baseboard.
Done
https://review.coreboot.org/c/coreboot/+/37459/2/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/kohaku/ramstage.c:
https://review.coreboot.org/c/coreboot/+/37459/2/src/mainboard/google/hatch/... PS2, Line 22: {
Same here
Done
Craig Hesling has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37459 )
Change subject: hatch: Fix FPMCU pwr/rst gpio handling ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37459/2/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/37459/2/src/mainboard/google/hatch/... PS2, Line 414: 1
It looks like it points back to b/128686027, but it is unclear if this particular change fixed the b […]
Ahh, okay. I think I understand what https://review.coreboot.org/c/coreboot/+/32111 is doing now.
The concern is if user space configures the GPP_A12 to it's native3 function and the system enters s3 or s5. This could result in bad behavior.
Okay, I will ensure that this pin is set to output in either case.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37459 )
Change subject: hatch: Fix FPMCU pwr/rst gpio handling ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/37459/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37459/2//COMMIT_MSG@20 PS2, Line 20:
Oh, I thought we were avoiding ChromeOS specific references. […]
No, that is fine for coreboot reviews.
https://review.coreboot.org/c/coreboot/+/37459/2/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/37459/2/src/mainboard/google/hatch/... PS2, Line 414: 1
Ahh, okay. I think I understand what https://review.coreboot.org/c/coreboot/+/32111 is doing now. […]
Aah I remember now. Yes, as long as it is configured as GPO, it should be fine then. Thanks for checking Craig!
Hello Tim Wawrzynczak, Shelley Chen, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37459
to look at the new patch set (#4).
Change subject: hatch: Fix FPMCU pwr/rst gpio handling ......................................................................
hatch: Fix FPMCU pwr/rst gpio handling
1. No gpio control in bootblock 2. Disable power and assert reset in ramstage gpio 3. Power on and then deassert reset at the end of ramstage gpio 4. Disable power and assert reset when entering S5
On "reboot", the amount of time the power is disabled for is equivalent to the amount of time between triggering #4 and wrapping around to #3, which is about 400ms on Kohaku.
Since #2 forces power off for FPMCU, S3 resume will still not work properly.
The control signals have been validated on a Kohaku in the following scenarios: 1. Cold startup 2. Issuing a "reboot" command 3. Issuing a "halt -p" and powering back on within 10 seconds 4. Issuing a "halt -p" and powering back on after 10 seconds 5. Entering and leaving S3 (does not work properly) 6. Entering and leaving S0iX
Change-Id: I2e3ff42715611d519677a4256bdd172ec98687f9 Signed-off-by: Craig Hesling hesling@chromium.org --- M src/mainboard/google/hatch/ramstage.c M src/mainboard/google/hatch/variants/baseboard/gpio.c M src/mainboard/google/hatch/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/hatch/variants/helios/Makefile.inc M src/mainboard/google/hatch/variants/helios/gpio.c A src/mainboard/google/hatch/variants/helios/ramstage.c M src/mainboard/google/hatch/variants/kindred/gpio.c M src/mainboard/google/hatch/variants/kohaku/Makefile.inc M src/mainboard/google/hatch/variants/kohaku/gpio.c A src/mainboard/google/hatch/variants/kohaku/ramstage.c 10 files changed, 127 insertions(+), 22 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/37459/4
Hello Tim Wawrzynczak, Shelley Chen, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37459
to look at the new patch set (#5).
Change subject: hatch: Fix FPMCU pwr/rst gpio handling ......................................................................
hatch: Fix FPMCU pwr/rst gpio handling
1. No gpio control in bootblock 2. Disable power and assert reset in ramstage gpio 3. Power on and then deassert reset at the end of ramstage gpio 4. Disable power and assert reset when entering S5
On "reboot", the amount of time the power is disabled for is equivalent to the amount of time between triggering #4 and wrapping around to #3, which is about 400ms on Kohaku.
Since #2 forces power off for FPMCU, S3 resume will still not work properly.
The control signals have been validated on a Kohaku in the following scenarios: 1. Cold startup 2. Issuing a "reboot" command 3. Issuing a "halt -p" and powering back on within 10 seconds 4. Issuing a "halt -p" and powering back on after 10 seconds 5. Entering and leaving S3 (does not work properly) 6. Entering and leaving S0iX
BRANCH=hatch BUG=b/142751685 TEST=Verify all signals as mentioned above TEST=reboot flash_fp_mcu /opt/google/biod/fw/dartmonkey_v2.0.2417-af88cc91a.bin TEST=halt -p # power back on within 10 seconds flash_fp_mcu /opt/google/biod/fw/dartmonkey_v2.0.2417-af88cc91a.bin TEST=halt -p # power back on after 10 seconds flash_fp_mcu /opt/google/biod/fw/dartmonkey_v2.0.2417-af88cc91a.bin
Change-Id: I2e3ff42715611d519677a4256bdd172ec98687f9 Signed-off-by: Craig Hesling hesling@chromium.org --- M src/mainboard/google/hatch/ramstage.c M src/mainboard/google/hatch/variants/baseboard/gpio.c M src/mainboard/google/hatch/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/hatch/variants/helios/Makefile.inc M src/mainboard/google/hatch/variants/helios/gpio.c A src/mainboard/google/hatch/variants/helios/ramstage.c M src/mainboard/google/hatch/variants/kindred/gpio.c M src/mainboard/google/hatch/variants/kohaku/Makefile.inc M src/mainboard/google/hatch/variants/kohaku/gpio.c A src/mainboard/google/hatch/variants/kohaku/ramstage.c 10 files changed, 127 insertions(+), 22 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/37459/5
Hello Tim Wawrzynczak, Shelley Chen, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37459
to look at the new patch set (#7).
Change subject: hatch: Fix FPMCU pwr/rst gpio handling ......................................................................
hatch: Fix FPMCU pwr/rst gpio handling
1. No gpio control in bootblock 2. Disable power and assert reset in ramstage gpio 3. Power on and then deassert reset at the end of ramstage gpio 4. Disable power and assert reset when entering S5
On "reboot", the amount of time the power is disabled for is equivalent to the amount of time between triggering #4 and wrapping around to #3, which is about 400ms on Kohaku.
Since #2 forces power off for FPMCU, S3 resume will still not work properly.
Additionally, we must ensure that GPP_A12 is reconfigured as an output before going to any sleep state, since user space could have configured it to use its native3 function. See https://review.coreboot.org/c/coreboot/+/32111 for more detail.
The control signals have been validated on a Kohaku in the following scenarios: 1. Cold startup 2. Issuing a "reboot" command 3. Issuing a "halt -p" and powering back on within 10 seconds 4. Issuing a "halt -p" and powering back on after 10 seconds 5. Entering and leaving S3 (does not work properly) 6. Entering and leaving S0iX
BRANCH=hatch BUG=b/142751685 TEST=Verify all signals as mentioned above TEST=reboot flash_fp_mcu /opt/google/biod/fw/dartmonkey_v2.0.2417-af88cc91a.bin TEST=halt -p # power back on within 10 seconds flash_fp_mcu /opt/google/biod/fw/dartmonkey_v2.0.2417-af88cc91a.bin TEST=halt -p # power back on after 10 seconds flash_fp_mcu /opt/google/biod/fw/dartmonkey_v2.0.2417-af88cc91a.bin
Change-Id: I2e3ff42715611d519677a4256bdd172ec98687f9 Signed-off-by: Craig Hesling hesling@chromium.org --- M src/mainboard/google/hatch/ramstage.c M src/mainboard/google/hatch/variants/baseboard/gpio.c M src/mainboard/google/hatch/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/hatch/variants/helios/Makefile.inc M src/mainboard/google/hatch/variants/helios/gpio.c A src/mainboard/google/hatch/variants/helios/ramstage.c M src/mainboard/google/hatch/variants/kindred/gpio.c M src/mainboard/google/hatch/variants/kohaku/Makefile.inc M src/mainboard/google/hatch/variants/kohaku/gpio.c A src/mainboard/google/hatch/variants/kohaku/ramstage.c 10 files changed, 127 insertions(+), 22 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/37459/7
Craig Hesling has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37459 )
Change subject: hatch: Fix FPMCU pwr/rst gpio handling ......................................................................
Patch Set 7:
(2 comments)
Ready for rereview!
https://review.coreboot.org/c/coreboot/+/37459/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37459/2//COMMIT_MSG@20 PS2, Line 20:
No, that is fine for coreboot reviews.
Done
https://review.coreboot.org/c/coreboot/+/37459/2/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/37459/2/src/mainboard/google/hatch/... PS2, Line 414: 1
Aah I remember now. Yes, as long as it is configured as GPO, it should be fine then. […]
No worries. I should have noticed this to begin with.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37459 )
Change subject: hatch: Fix FPMCU pwr/rst gpio handling ......................................................................
Patch Set 7: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/37459/7/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/helios/ramstage.c:
https://review.coreboot.org/c/coreboot/+/37459/7/src/mainboard/google/hatch/... PS7, Line 27: Kohaku Helios :)
Craig Hesling has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37459 )
Change subject: hatch: Fix FPMCU pwr/rst gpio handling ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37459/7/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/helios/ramstage.c:
https://review.coreboot.org/c/coreboot/+/37459/7/src/mainboard/google/hatch/... PS7, Line 27: Kohaku
Helios :)
This timing value has only been measured on Kohaku, but increased to 1ms to cover other scenarios.
Craig Hesling has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37459 )
Change subject: hatch: Fix FPMCU pwr/rst gpio handling ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37459/2/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/helios/gpio.c:
https://review.coreboot.org/c/coreboot/+/37459/2/src/mainboard/google/hatch/... PS2, Line 161: if (slp_typ == ACPI_S5) {
Furquan is correct. […]
Shelley, does this answer your question?
Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37459 )
Change subject: hatch: Fix FPMCU pwr/rst gpio handling ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37459/2/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/helios/gpio.c:
https://review.coreboot.org/c/coreboot/+/37459/2/src/mainboard/google/hatch/... PS2, Line 161: if (slp_typ == ACPI_S5) {
Shelley, does this answer your question?
Done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37459 )
Change subject: hatch: Fix FPMCU pwr/rst gpio handling ......................................................................
Patch Set 7: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/37459/7/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/helios/ramstage.c:
https://review.coreboot.org/c/coreboot/+/37459/7/src/mainboard/google/hatch/... PS7, Line 27: Kohaku
This timing value has only been measured on Kohaku, but increased to 1ms to cover other scenarios.
I see.
Craig Hesling has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37459 )
Change subject: hatch: Fix FPMCU pwr/rst gpio handling ......................................................................
Patch Set 7: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/37459/7/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/helios/ramstage.c:
https://review.coreboot.org/c/coreboot/+/37459/7/src/mainboard/google/hatch/... PS7, Line 27: Kohaku
I see.
Ack
Shelley Chen has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37459 )
Change subject: hatch: Fix FPMCU pwr/rst gpio handling ......................................................................
hatch: Fix FPMCU pwr/rst gpio handling
1. No gpio control in bootblock 2. Disable power and assert reset in ramstage gpio 3. Power on and then deassert reset at the end of ramstage gpio 4. Disable power and assert reset when entering S5
On "reboot", the amount of time the power is disabled for is equivalent to the amount of time between triggering #4 and wrapping around to #3, which is about 400ms on Kohaku.
Since #2 forces power off for FPMCU, S3 resume will still not work properly.
Additionally, we must ensure that GPP_A12 is reconfigured as an output before going to any sleep state, since user space could have configured it to use its native3 function. See https://review.coreboot.org/c/coreboot/+/32111 for more detail.
The control signals have been validated on a Kohaku in the following scenarios: 1. Cold startup 2. Issuing a "reboot" command 3. Issuing a "halt -p" and powering back on within 10 seconds 4. Issuing a "halt -p" and powering back on after 10 seconds 5. Entering and leaving S3 (does not work properly) 6. Entering and leaving S0iX
BRANCH=hatch BUG=b/142751685 TEST=Verify all signals as mentioned above TEST=reboot flash_fp_mcu /opt/google/biod/fw/dartmonkey_v2.0.2417-af88cc91a.bin TEST=halt -p # power back on within 10 seconds flash_fp_mcu /opt/google/biod/fw/dartmonkey_v2.0.2417-af88cc91a.bin TEST=halt -p # power back on after 10 seconds flash_fp_mcu /opt/google/biod/fw/dartmonkey_v2.0.2417-af88cc91a.bin
Change-Id: I2e3ff42715611d519677a4256bdd172ec98687f9 Signed-off-by: Craig Hesling hesling@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/37459 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com --- M src/mainboard/google/hatch/ramstage.c M src/mainboard/google/hatch/variants/baseboard/gpio.c M src/mainboard/google/hatch/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/hatch/variants/helios/Makefile.inc M src/mainboard/google/hatch/variants/helios/gpio.c A src/mainboard/google/hatch/variants/helios/ramstage.c M src/mainboard/google/hatch/variants/kindred/gpio.c M src/mainboard/google/hatch/variants/kohaku/Makefile.inc M src/mainboard/google/hatch/variants/kohaku/gpio.c A src/mainboard/google/hatch/variants/kohaku/ramstage.c 10 files changed, 127 insertions(+), 22 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved Craig Hesling: Looks good to me, but someone else must approve
diff --git a/src/mainboard/google/hatch/ramstage.c b/src/mainboard/google/hatch/ramstage.c index e9f5062..e84aa18 100644 --- a/src/mainboard/google/hatch/ramstage.c +++ b/src/mainboard/google/hatch/ramstage.c @@ -31,6 +31,11 @@ /* Override dev tree settings per board */ }
+void __weak variant_ramstage_init(void) +{ + /* Default weak implementation */ +} + static void mainboard_init(struct device *dev) { mainboard_ec_init(); @@ -56,6 +61,8 @@ base_gpios, override_table, override_gpios); + + variant_ramstage_init(); }
struct chip_operations mainboard_ops = { diff --git a/src/mainboard/google/hatch/variants/baseboard/gpio.c b/src/mainboard/google/hatch/variants/baseboard/gpio.c index 598600b..94cb2e5 100644 --- a/src/mainboard/google/hatch/variants/baseboard/gpio.c +++ b/src/mainboard/google/hatch/variants/baseboard/gpio.c @@ -38,7 +38,7 @@ /* A11 : PCH_SPI_FPMCU_CS_L */ PAD_CFG_NF(GPP_A11, NONE, DEEP, NF2), /* A12 : FPMCU_RST_ODL */ - PAD_CFG_GPO(GPP_A12, 1, DEEP), + PAD_CFG_GPO(GPP_A12, 0, DEEP), /* A13 : SUSWARN_L */ PAD_CFG_NF(GPP_A13, NONE, DEEP, NF1), /* A14 : ESPI_RST_L */ @@ -133,7 +133,7 @@ /* C10 : GPP_10 ==> GPP_C10_TP */ PAD_NC(GPP_C10, NONE), /* C11 : GPP_11 ==> EN_FP_RAILS */ - PAD_CFG_GPO(GPP_C11, 1, DEEP), + PAD_CFG_GPO(GPP_C11, 0, DEEP), /* C12 : GPP_C12 ==> NC */ PAD_NC(GPP_C12, NONE), /* C13 : EC_PCH_INT_L */ @@ -398,8 +398,10 @@ }
/* - * Default GPIO settings before entering sleep. Configure A12: FPMCU_RST_ODL - * as GPO before entering sleep. + * Default GPIO settings before entering non-S5 sleep states. + * Configure A12: FPMCU_RST_ODL as GPO before entering sleep. + * This guarantees that A12's native3 function is disabled. + * See https://review.coreboot.org/c/coreboot/+/32111 . */ static const struct pad_config default_sleep_gpio_table[] = { PAD_CFG_GPO(GPP_A12, 1, DEEP), /* FPMCU_RST_ODL */ @@ -408,10 +410,11 @@ /* * GPIO settings before entering S5, which are same as * default_sleep_gpio_table but also, - * turn off EN_PP3300_WWAN. + * turn off EN_PP3300_WWAN and FPMCU. */ static const struct pad_config s5_sleep_gpio_table[] = { - PAD_CFG_GPO(GPP_A12, 1, DEEP), /* FPMCU_RST_ODL */ + PAD_CFG_GPO(GPP_A12, 0, DEEP), /* FPMCU_RST_ODL */ + PAD_CFG_GPO(GPP_C11, 0, DEEP), /* PCH_FP_PWR_EN */ PAD_CFG_GPO(GPP_A18, 0, DEEP), /* EN_PP3300_WWAN */ };
diff --git a/src/mainboard/google/hatch/variants/baseboard/include/baseboard/variants.h b/src/mainboard/google/hatch/variants/baseboard/include/baseboard/variants.h index 920e428..1542d9b 100644 --- a/src/mainboard/google/hatch/variants/baseboard/include/baseboard/variants.h +++ b/src/mainboard/google/hatch/variants/baseboard/include/baseboard/variants.h @@ -50,4 +50,7 @@ /* Modify devictree settings during ramstage. */ void variant_devtree_update(void);
+/* Perform variant specific initialization early on in ramstage. */ +void variant_ramstage_init(void); + #endif /* BASEBOARD_VARIANTS_H */ diff --git a/src/mainboard/google/hatch/variants/helios/Makefile.inc b/src/mainboard/google/hatch/variants/helios/Makefile.inc index fbd69c4..be074b7 100644 --- a/src/mainboard/google/hatch/variants/helios/Makefile.inc +++ b/src/mainboard/google/hatch/variants/helios/Makefile.inc @@ -17,4 +17,6 @@
romstage-y += memory.c bootblock-y += gpio.c + ramstage-y += gpio.c +ramstage-y += ramstage.c diff --git a/src/mainboard/google/hatch/variants/helios/gpio.c b/src/mainboard/google/hatch/variants/helios/gpio.c index 85eb3fc..456877c 100644 --- a/src/mainboard/google/hatch/variants/helios/gpio.c +++ b/src/mainboard/google/hatch/variants/helios/gpio.c @@ -117,8 +117,6 @@ * needed in this table. */ static const struct pad_config early_gpio_table[] = { - /* A12 : FPMCU_RST_ODL */ - PAD_CFG_GPO(GPP_A12, 0, DEEP), /* B15 : H1_SLAVE_SPI_CS_L */ PAD_CFG_NF(GPP_B15, NONE, DEEP, NF1), /* B16 : H1_SLAVE_SPI_CLK */ @@ -127,8 +125,6 @@ PAD_CFG_NF(GPP_B17, NONE, DEEP, NF1), /* B18 : H1_SLAVE_SPI_MOSI_R */ PAD_CFG_NF(GPP_B18, NONE, DEEP, NF1), - /* C11 : GPP_C11 ==> EN_FP_RAILS */ - PAD_CFG_GPO(GPP_C11, 1, DEEP), /* C14 : BT_DISABLE_L */ PAD_CFG_GPO(GPP_C14, 0, DEEP), /* PCH_WP_OD */ @@ -150,14 +146,30 @@ }
/* - * GPIO settings before entering all sleep states + * Default GPIO settings before entering non-S5 sleep states. + * Configure A12: FPMCU_RST_ODL as GPO before entering sleep. + * This guarantees that A12's native3 function is disabled. + * See https://review.coreboot.org/c/coreboot/+/32111 . */ -static const struct pad_config sleep_gpio_table[] = { +static const struct pad_config default_sleep_gpio_table[] = { PAD_CFG_GPO(GPP_A12, 1, DEEP), /* FPMCU_RST_ODL */ };
+/* + * GPIO settings before entering S5, which are same as + * default_sleep_gpio_table but also, turn off FPMCU. + */ +static const struct pad_config s5_sleep_gpio_table[] = { + PAD_CFG_GPO(GPP_A12, 0, DEEP), /* FPMCU_RST_ODL */ + PAD_CFG_GPO(GPP_C11, 0, DEEP), /* PCH_FP_PWR_EN */ +}; + const struct pad_config *variant_sleep_gpio_table(u8 slp_typ, size_t *num) { - *num = ARRAY_SIZE(sleep_gpio_table); - return sleep_gpio_table; + if (slp_typ == ACPI_S5) { + *num = ARRAY_SIZE(s5_sleep_gpio_table); + return s5_sleep_gpio_table; + } + *num = ARRAY_SIZE(default_sleep_gpio_table); + return default_sleep_gpio_table; } diff --git a/src/mainboard/google/hatch/variants/helios/ramstage.c b/src/mainboard/google/hatch/variants/helios/ramstage.c new file mode 100644 index 0000000..9b919fc --- /dev/null +++ b/src/mainboard/google/hatch/variants/helios/ramstage.c @@ -0,0 +1,32 @@ +/* + * This file is part of the coreboot project. + * + * Copyright 2019 Google LLC + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <delay.h> +#include <gpio.h> +#include <baseboard/variants.h> +#include <soc/gpio.h> + +void variant_ramstage_init(void) +{ + /* + * Enable power to FPMCU, wait for power rail to stabilize, + * and then deassert FPMCU reset. + * Waiting for the power rail to stabilize can take a while, + * a minimum of 400us on Kohaku. + */ + gpio_output(GPP_C11, 1); + mdelay(1); + gpio_output(GPP_A12, 1); +} diff --git a/src/mainboard/google/hatch/variants/kindred/gpio.c b/src/mainboard/google/hatch/variants/kindred/gpio.c index 827ee0b..9e2d818 100644 --- a/src/mainboard/google/hatch/variants/kindred/gpio.c +++ b/src/mainboard/google/hatch/variants/kindred/gpio.c @@ -154,8 +154,6 @@ * needed in this table. */ static const struct pad_config early_gpio_table[] = { - /* A12 : FPMCU_RST_ODL */ - PAD_CFG_GPO(GPP_A12, 0, DEEP), /* B15 : H1_SLAVE_SPI_CS_L */ PAD_CFG_NF(GPP_B15, NONE, DEEP, NF1), /* B16 : H1_SLAVE_SPI_CLK */ diff --git a/src/mainboard/google/hatch/variants/kohaku/Makefile.inc b/src/mainboard/google/hatch/variants/kohaku/Makefile.inc index 9cdff32..6bd2973 100644 --- a/src/mainboard/google/hatch/variants/kohaku/Makefile.inc +++ b/src/mainboard/google/hatch/variants/kohaku/Makefile.inc @@ -17,4 +17,6 @@ romstage-y += memory.c
bootblock-y += gpio.c + ramstage-y += gpio.c +ramstage-y += ramstage.c diff --git a/src/mainboard/google/hatch/variants/kohaku/gpio.c b/src/mainboard/google/hatch/variants/kohaku/gpio.c index 61d3375..f52cc27 100644 --- a/src/mainboard/google/hatch/variants/kohaku/gpio.c +++ b/src/mainboard/google/hatch/variants/kohaku/gpio.c @@ -97,8 +97,6 @@ * needed in this table. */ static const struct pad_config early_gpio_table[] = { - /* A12 : FPMCU_RST_ODL */ - PAD_CFG_GPO(GPP_A12, 0, DEEP), /* B15 : H1_SLAVE_SPI_CS_L */ PAD_CFG_NF(GPP_B15, NONE, DEEP, NF1), /* B16 : H1_SLAVE_SPI_CLK */ @@ -136,14 +134,30 @@ }
/* - * GPIO settings before entering all sleep states + * Default GPIO settings before entering non-S5 sleep states. + * Configure A12: FPMCU_RST_ODL as GPO before entering sleep. + * This guarantees that A12's native3 function is disabled. + * See https://review.coreboot.org/c/coreboot/+/32111 . */ -static const struct pad_config sleep_gpio_table[] = { +static const struct pad_config default_sleep_gpio_table[] = { PAD_CFG_GPO(GPP_A12, 1, DEEP), /* FPMCU_RST_ODL */ };
+/* + * GPIO settings before entering S5, which are same as + * default_sleep_gpio_table but also, turn off FPMCU. + */ +static const struct pad_config s5_sleep_gpio_table[] = { + PAD_CFG_GPO(GPP_A12, 0, DEEP), /* FPMCU_RST_ODL */ + PAD_CFG_GPO(GPP_C11, 0, DEEP), /* PCH_FP_PWR_EN */ +}; + const struct pad_config *variant_sleep_gpio_table(u8 slp_typ, size_t *num) { - *num = ARRAY_SIZE(sleep_gpio_table); - return sleep_gpio_table; + if (slp_typ == ACPI_S5) { + *num = ARRAY_SIZE(s5_sleep_gpio_table); + return s5_sleep_gpio_table; + } + *num = ARRAY_SIZE(default_sleep_gpio_table); + return default_sleep_gpio_table; } diff --git a/src/mainboard/google/hatch/variants/kohaku/ramstage.c b/src/mainboard/google/hatch/variants/kohaku/ramstage.c new file mode 100644 index 0000000..9b919fc --- /dev/null +++ b/src/mainboard/google/hatch/variants/kohaku/ramstage.c @@ -0,0 +1,32 @@ +/* + * This file is part of the coreboot project. + * + * Copyright 2019 Google LLC + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <delay.h> +#include <gpio.h> +#include <baseboard/variants.h> +#include <soc/gpio.h> + +void variant_ramstage_init(void) +{ + /* + * Enable power to FPMCU, wait for power rail to stabilize, + * and then deassert FPMCU reset. + * Waiting for the power rail to stabilize can take a while, + * a minimum of 400us on Kohaku. + */ + gpio_output(GPP_C11, 1); + mdelay(1); + gpio_output(GPP_A12, 1); +}
-
Craig Hesling (Code Review) -
Furquan Shaikh (Code Review)
-
Shelley Chen (Code Review)
-
Tim Wawrzynczak (Code Review)