Martin Roth has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47308 )
Change subject: mb/google/zork: Init fingerprint GPIOs for boot vs resume ......................................................................
mb/google/zork: Init fingerprint GPIOs for boot vs resume
Add a function that initializes GPIOs based on the sleep type that the system is coming back from. This allows initialization of the fingerprint GPIOs which need to be handled differently between wake from S3 and boot from S5.
If fingerprint is disabled on the trembyle board, set the pins to no-connect. Dalboz doesn't have fingerprint and the GPIOS are configured differently due to the FT5 chip having fewer GPIOS than FP5, so nothing needs to be initialized there.
There were also a couple of trivial comment clean ups regarding the FPMCU GPIOS.
BUG=b:171837716 TEST=Boot & Check GPIO states. BRANCH=Zork
Signed-off-by: Martin Roth martinroth@chromium.org Change-Id: Id40bc952b4f6b87493a637e0bf12a5579c736b87
Change-Id: I16a2e621145782e0a908bb3e49478586c09a0e0a --- M src/mainboard/google/zork/bootblock.c M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c M src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/zork/variants/ezkinil/gpio.c M src/mainboard/google/zork/variants/woomax/gpio.c 6 files changed, 47 insertions(+), 18 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/47308/1
diff --git a/src/mainboard/google/zork/bootblock.c b/src/mainboard/google/zork/bootblock.c index a7636de..af071b2 100644 --- a/src/mainboard/google/zork/bootblock.c +++ b/src/mainboard/google/zork/bootblock.c @@ -2,12 +2,16 @@
#include <bootblock_common.h> #include <baseboard/variants.h> +#include <acpi/acpi.h>
void bootblock_mainboard_early_init(void) { size_t num_gpios; const struct soc_amd_gpio *gpios;
+ gpios=variant_poweron_gpio_table(&num_gpios, acpi_get_sleep_type()); + program_gpios(gpios, num_gpios); + if (!CONFIG(VBOOT_STARTS_BEFORE_BOOTBLOCK)) { gpios = variant_early_gpio_table(&num_gpios); program_gpios(gpios, num_gpios); diff --git a/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c b/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c index 2398d07..e538aef 100644 --- a/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c +++ b/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c @@ -30,7 +30,7 @@ PAD_NF(GPIO_8, ACP_I2S_LRCLK, PULL_NONE), /* TOUCHPAD_INT_ODL */ PAD_SCI(GPIO_9, PULL_NONE, EDGE_LOW), - /* S0iX SLP - (unused - goes to EC & FPMCU */ + /* S0iX SLP - (unused - goes to EC */ PAD_NC(GPIO_10), /* EC_IN_RW_OD */ PAD_GPI(GPIO_11, PULL_NONE), @@ -291,6 +291,12 @@ wifi_power_reset_configure_pre_v3(); }
+const __weak struct soc_amd_gpio *variant_poweron_gpio_table(size_t *size, int slp_typ) +{ + *size = 0; + return NULL; +} + static const struct soc_amd_gpio gpio_sleep_table[] = { /* PCIE_RST1_L */ PAD_GPO(GPIO_27, LOW), diff --git a/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c b/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c index a436d1c..74e7ed2 100644 --- a/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c +++ b/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c @@ -1,5 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-or-later */
+#include <acpi/acpi.h> #include <baseboard/variants.h> #include <delay.h> #include <gpio.h> @@ -32,8 +33,6 @@ PAD_SCI(GPIO_9, PULL_NONE, EDGE_LOW), /* S0iX SLP - (unused - goes to EC & FPMCU */ PAD_NC(GPIO_10), - /* FPMCU_RST_L */ - PAD_GPO(GPIO_11, HIGH), /* USI_INT_ODL */ PAD_GPI(GPIO_12, PULL_NONE), /* EN_PWR_TOUCHPAD_PS2 */ @@ -71,8 +70,6 @@ PAD_NF(GPIO_30, ESPI_CS_L, PULL_NONE), /* EC_AP_INT_ODL (Sensor Framesync) */ PAD_GPI(GPIO_31, PULL_NONE), - /* EN_PWR_FP */ - PAD_GPO(GPIO_32, HIGH), /* GPIO_33 - GPIO_39: Not available */ /* NVME_AUX_RESET_L */ PAD_GPO(GPIO_40, HIGH), @@ -87,7 +84,7 @@ PAD_GPO(GPIO_67, LOW), // Select Camera 1 Dmic /* EMMC_RESET_L */ PAD_GPO(GPIO_68, HIGH), - /* FPMCU_BOOT0 - TODO: Check this */ + /* FPMCU_BOOT0 */ PAD_GPO(GPIO_69, LOW), /* EMMC_CLK */ PAD_NF(GPIO_70, EMMC_CLK, PULL_NONE), @@ -300,6 +297,34 @@ wifi_power_reset_configure_pre_v3(); }
+static const struct soc_amd_gpio gpio_fingerprint_poweron_table[] = { + /* FPMCU_RST_L */ + PAD_GPO(GPIO_11, LOW), + /* EN_PWR_FP */ + PAD_GPO(GPIO_32, LOW), +}; + +static const struct soc_amd_gpio gpio_no_fingerprint_poweron_table[] = { + /* FPMCU_RST_L */ + PAD_NC(GPIO_11), + /* EN_PWR_FP */ + PAD_NC(GPIO_32), +}; + +const __weak struct soc_amd_gpio *variant_poweron_gpio_table(size_t *size, int slp_typ) +{ + if (variant_init_fingerprint()) { + if (slp_typ != ACPI_S0) { + return NULL; + } + *size = ARRAY_SIZE(gpio_fingerprint_poweron_table); + return gpio_fingerprint_poweron_table; + } + + *size = ARRAY_SIZE(gpio_no_fingerprint_poweron_table); + return gpio_no_fingerprint_poweron_table; +} + static const struct soc_amd_gpio gpio_sleep_table[] = { /* NVME_AUX_RESET_L */ PAD_GPO(GPIO_40, LOW), diff --git a/src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h b/src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h index 338b918..5c040d3 100644 --- a/src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h +++ b/src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h @@ -25,6 +25,12 @@ const struct soc_amd_gpio *variant_override_gpio_table(size_t *size);
/* + * This function provides GPIO table for the pads that need to be configured when + * powering on because of reset, powering on, or resuming from S3. + */ +const struct soc_amd_gpio *variant_poweron_gpio_table(size_t *size, int slp_typ); + +/* * This function provides GPIO table for the pads that need to be configured when entering * sleep. */ diff --git a/src/mainboard/google/zork/variants/ezkinil/gpio.c b/src/mainboard/google/zork/variants/ezkinil/gpio.c index d2dd104..f86d926 100644 --- a/src/mainboard/google/zork/variants/ezkinil/gpio.c +++ b/src/mainboard/google/zork/variants/ezkinil/gpio.c @@ -37,8 +37,6 @@ PAD_NC(GPIO_4), /* PEN_POWER_EN - Not connected */ PAD_NC(GPIO_5), - /* FPMCU_RST_L Change NC */ - PAD_NC(GPIO_11), /* DMIC_SEL */ PAD_GPO(GPIO_13, LOW), // Select Camera 1 Dmic /* EN_PWR_WIFI */ @@ -62,8 +60,6 @@ PAD_NC(GPIO_4), /* PEN_POWER_EN - Not connected */ PAD_NC(GPIO_5), - /* FPMCU_RST_L Change NC */ - PAD_NC(GPIO_11), /* FPMCU_BOOT0 Change NC */ PAD_NC(GPIO_69), /* EN_DEV_BEEP_L */ diff --git a/src/mainboard/google/zork/variants/woomax/gpio.c b/src/mainboard/google/zork/variants/woomax/gpio.c index cb98df7..329e7dd 100644 --- a/src/mainboard/google/zork/variants/woomax/gpio.c +++ b/src/mainboard/google/zork/variants/woomax/gpio.c @@ -10,10 +10,6 @@ PAD_NC(GPIO_5), /* GPIO_6 NC */ PAD_NC(GPIO_6), - /* GPIO_11 NC */ - PAD_NC(GPIO_11), - /* GPIO_32 NC */ - PAD_NC(GPIO_32), /* GPIO_69 NC */ PAD_NC(GPIO_69), /* RAM_ID_4 */ @@ -37,10 +33,6 @@ PAD_NC(GPIO_5), /* GPIO_6 NC */ PAD_NC(GPIO_6), - /* GPIO_11 NC */ - PAD_NC(GPIO_11), - /* GPIO_32 NC */ - PAD_NC(GPIO_32), /* GPIO_69 NC */ PAD_NC(GPIO_69), /* RAM_ID_4 */
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47308 )
Change subject: mb/google/zork: Init fingerprint GPIOs for boot vs resume ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/47308/1/src/mainboard/google/zork/b... File src/mainboard/google/zork/bootblock.c:
https://review.coreboot.org/c/coreboot/+/47308/1/src/mainboard/google/zork/b... PS1, Line 12: gpios=variant_poweron_gpio_table(&num_gpios, acpi_get_sleep_type()); spaces required around that '=' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/47308/1/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c:
https://review.coreboot.org/c/coreboot/+/47308/1/src/mainboard/google/zork/v... PS1, Line 317: if (slp_typ != ACPI_S0) { braces {} are not necessary for single statement blocks
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47308 )
Change subject: mb/google/zork: Init fingerprint GPIOs for boot vs resume ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/47308/2/src/mainboard/google/zork/b... File src/mainboard/google/zork/bootblock.c:
https://review.coreboot.org/c/coreboot/+/47308/2/src/mainboard/google/zork/b... PS2, Line 12: gpios=variant_poweron_gpio_table(&num_gpios, acpi_get_sleep_type()); spaces required around that '=' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/47308/2/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c:
https://review.coreboot.org/c/coreboot/+/47308/2/src/mainboard/google/zork/v... PS2, Line 317: if (slp_typ != ACPI_S0) { braces {} are not necessary for single statement blocks
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47308
to look at the new patch set (#3).
Change subject: mb/google/zork: Init fingerprint GPIOs for boot vs resume ......................................................................
mb/google/zork: Init fingerprint GPIOs for boot vs resume
Add a function that initializes GPIOs based on the sleep type that the system is coming back from. This allows initialization of the fingerprint GPIOs which need to be handled differently between wake from S3 and boot from S5.
If fingerprint is disabled on the trembyle board, set the pins to no-connect. Dalboz doesn't have fingerprint and the GPIOS are configured differently due to the FT5 chip having fewer GPIOS than FP5, so nothing needs to be initialized there.
There were also a couple of trivial comment clean ups regarding the FPMCU GPIOS.
BUG=b:171837716 TEST=Boot & Check GPIO states. BRANCH=Zork
Signed-off-by: Martin Roth martinroth@chromium.org Change-Id: I16a2e621145782e0a908bb3e49478586c09a0e0a --- M src/mainboard/google/zork/bootblock.c M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c M src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/zork/variants/ezkinil/gpio.c M src/mainboard/google/zork/variants/woomax/gpio.c 6 files changed, 47 insertions(+), 18 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/47308/3
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47308
to look at the new patch set (#4).
Change subject: mb/google/zork: Init fingerprint GPIOs for boot vs resume ......................................................................
mb/google/zork: Init fingerprint GPIOs for boot vs resume
Add a function that initializes GPIOs based on the sleep type that the system is coming back from. This allows initialization of the fingerprint GPIOs which need to be handled differently between wake from S3 and boot from S5.
If fingerprint is disabled on the trembyle board, set the pins to no-connect. Dalboz doesn't have fingerprint and the GPIOS are configured differently due to the FT5 chip having fewer GPIOS than FP5, so nothing needs to be initialized there.
There were also a couple of trivial comment clean ups regarding the FPMCU GPIOS.
BUG=b:171837716 TEST=Boot & Check GPIO states. BRANCH=Zork
Signed-off-by: Martin Roth martinroth@chromium.org Change-Id: I16a2e621145782e0a908bb3e49478586c09a0e0a --- M src/mainboard/google/zork/bootblock.c M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c M src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/zork/variants/ezkinil/gpio.c M src/mainboard/google/zork/variants/woomax/gpio.c 6 files changed, 47 insertions(+), 18 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/47308/4
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47308 )
Change subject: mb/google/zork: Init fingerprint GPIOs for boot vs resume ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/47308/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/47308/4//COMMIT_MSG@10 PS4, Line 10: This allows initialization of the : fingerprint GPIOs which need to be handled differently between wake : from S3 and boot from S5. What are these requirements? Can you please capture those in the commit message?
https://review.coreboot.org/c/coreboot/+/47308/4/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c:
https://review.coreboot.org/c/coreboot/+/47308/4/src/mainboard/google/zork/v... PS4, Line 300: gpio_fingerprint_poweron_table From this change, it looks like the fingerprint power is kept off and reset is asserted on boot-up. What enables the power to fingerprint? I don't see that happening in this CL.
Also, it is not clear what the requirements are. Do the GPIOs need to be driven low and then driven high in case on non-S3 boot whereas keep them driven high in case of S3 resume?
Peichao Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47308 )
Change subject: mb/google/zork: Init fingerprint GPIOs for boot vs resume ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47308/4/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c:
https://review.coreboot.org/c/coreboot/+/47308/4/src/mainboard/google/zork/v... PS4, Line 300: gpio_fingerprint_poweron_table
From this change, it looks like the fingerprint power is kept off and reset is asserted on boot-up. […]
Dear Furquan,
As far as I know, power_en and RST pin need keep low when boot from S5 and S3 respectively, like ticket: https://partnerissuetracker.corp.google.com/issues/171004327#comment24
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47308 )
Change subject: mb/google/zork: Init fingerprint GPIOs for boot vs resume ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/47308/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/47308/4//COMMIT_MSG@10 PS4, Line 10: This allows initialization of the : fingerprint GPIOs which need to be handled differently between wake : from S3 and boot from S5.
What are these requirements? Can you please capture those in the commit message?
Sure, I'll add it.
On initial boot, the state of the FP sensor could be either enabled or disabled. Because of this, on boot, we power off the sensor for >200ms, to reset its state, then power it back on.
In suspend/resume, the fingerprint sensor should remain powered the entire time.
https://review.coreboot.org/c/coreboot/+/47308/4/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c:
https://review.coreboot.org/c/coreboot/+/47308/4/src/mainboard/google/zork/v... PS4, Line 300: gpio_fingerprint_poweron_table
Dear Furquan, […]
The power enable and reset get enabled at the end of the boot process. See cb:47310
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47308 )
Change subject: mb/google/zork: Init fingerprint GPIOs for boot vs resume ......................................................................
Patch Set 4:
(4 comments)
https://review.coreboot.org/c/coreboot/+/47308/4/src/mainboard/google/zork/b... File src/mainboard/google/zork/bootblock.c:
https://review.coreboot.org/c/coreboot/+/47308/4/src/mainboard/google/zork/b... PS4, Line 15: if (!CONFIG(VBOOT_STARTS_BEFORE_BOOTBLOCK)) { : gpios = variant_early_gpio_table(&num_gpios); : program_gpios(gpios, num_gpios); : } Not for this change, but I think we should drop this call in bootblock now that all Zork variants are using VBOOT_STARTS_BEFORE_BOOTBLOCK. Similarly, all other blocks under Zork can be dropped which are now dead.
https://review.coreboot.org/c/coreboot/+/47308/4/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c:
https://review.coreboot.org/c/coreboot/+/47308/4/src/mainboard/google/zork/v... PS4, Line 300: gpio_fingerprint_poweron_table
The power enable and reset get enabled at the end of the boot process. […]
I think it would be better to squash the changes in CB:47310 into this CL. Both are dealing with power sequencing the FPMCU correctly. It would be good to get a complete picture about it i.e. bootblock powers off and then it gets sequenced correctly in ramstage.
https://review.coreboot.org/c/coreboot/+/47308/4/src/mainboard/google/zork/v... PS4, Line 309: PAD_NC nit: Should we add a `PAD_RETAIN_CONFIG` macro that can be used in ramstage to retain the pad configuration done previously. It will act as placeholder to ensure that all pads are present in `gpio_set_stage_ram` and can be used by variants not using FPMCU to configure as they like i.e. PAD_NC or anything else.
https://review.coreboot.org/c/coreboot/+/47308/4/src/mainboard/google/zork/v... PS4, Line 317: slp_typ != ACPI_S0 Shouldn't this be only checking for ACPI_S3? In case of ACPI_S5, we would want to assert reset and disable power.
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47308 )
Change subject: mb/google/zork: Init fingerprint GPIOs for boot vs resume ......................................................................
Patch Set 4:
(4 comments)
https://review.coreboot.org/c/coreboot/+/47308/4/src/mainboard/google/zork/b... File src/mainboard/google/zork/bootblock.c:
https://review.coreboot.org/c/coreboot/+/47308/4/src/mainboard/google/zork/b... PS4, Line 15: if (!CONFIG(VBOOT_STARTS_BEFORE_BOOTBLOCK)) { : gpios = variant_early_gpio_table(&num_gpios); : program_gpios(gpios, num_gpios); : }
Not for this change, but I think we should drop this call in bootblock now that all Zork variants ar […]
Ok, I filed b:172848137 Zork: Assume that VBOOT_STARTS_BEFORE_BOOTBLOCK is enabled, drop checks
https://review.coreboot.org/c/coreboot/+/47308/4/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c:
https://review.coreboot.org/c/coreboot/+/47308/4/src/mainboard/google/zork/v... PS4, Line 300: gpio_fingerprint_poweron_table
I think it would be better to squash the changes in CB:47310 into this CL. […]
Sounds good.
https://review.coreboot.org/c/coreboot/+/47308/4/src/mainboard/google/zork/v... PS4, Line 309: PAD_NC
nit: Should we add a `PAD_RETAIN_CONFIG` macro that can be used in ramstage to retain the pad config […]
created b:172848722 - Zork: Add "placeholder" gpio macro that leaves the GPIO init unchanged
https://review.coreboot.org/c/coreboot/+/47308/4/src/mainboard/google/zork/v... PS4, Line 317: slp_typ != ACPI_S0
Shouldn't this be only checking for ACPI_S3? In case of ACPI_S5, we would want to assert reset and d […]
In my testing, I either got an S3 or an S0 wake. I never got an S5, although that was my first assumption too. I'll change it to be == ACPI_S3, but in effect it's the same check.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47308 )
Change subject: mb/google/zork: Init fingerprint GPIOs for boot vs resume ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47308/4/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c:
https://review.coreboot.org/c/coreboot/+/47308/4/src/mainboard/google/zork/v... PS4, Line 317: slp_typ != ACPI_S0
In my testing, I either got an S3 or an S0 wake. […]
If you poweroff and power back on within 10 seconds, I think it should be waking up from S5. Does that not happen?
Hello build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47308
to look at the new patch set (#5).
Change subject: mb/google/zork: Init fingerprint GPIOs for boot vs resume ......................................................................
mb/google/zork: Init fingerprint GPIOs for boot vs resume
Add a function that initializes GPIOs based on the sleep type that the system is coming back from. This allows initialization of the fingerprint GPIOs which need to be handled differently between wake from S3 and boot from S5.
If fingerprint is disabled on the trembyle board, set the pins to no-connect. Dalboz doesn't have fingerprint and the GPIOS are configured differently due to the FT5 chip having fewer GPIOS than FP5, so nothing needs to be initialized there.
There were also a couple of trivial comment clean ups regarding the FPMCU GPIOS.
BUG=b:171837716 TEST=Boot & Check GPIO states. BRANCH=Zork
Signed-off-by: Martin Roth martinroth@chromium.org Change-Id: I16a2e621145782e0a908bb3e49478586c09a0e0a --- M src/mainboard/google/zork/bootblock.c M src/mainboard/google/zork/mainboard.c M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c M src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/zork/variants/ezkinil/gpio.c M src/mainboard/google/zork/variants/woomax/gpio.c 7 files changed, 84 insertions(+), 19 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/47308/5
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47308 )
Change subject: mb/google/zork: Init fingerprint GPIOs for boot vs resume ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47308/5/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c:
https://review.coreboot.org/c/coreboot/+/47308/5/src/mainboard/google/zork/v... PS5, Line 314: if (fpmcu_needs_delay()) { braces {} are not necessary for single statement blocks
Hello build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47308
to look at the new patch set (#6).
Change subject: mb/google/zork: Init fingerprint GPIOs for boot vs resume ......................................................................
mb/google/zork: Init fingerprint GPIOs for boot vs resume
Add a function that initializes GPIOs based on the sleep type that the system is coming back from. This allows initialization of the fingerprint GPIOs which need to be handled differently between wake from S3 and boot from S5.
If fingerprint is disabled on the trembyle board, set the pins to no-connect. Dalboz doesn't have fingerprint and the GPIOS are configured differently due to the FT5 chip having fewer GPIOS than FP5, so nothing needs to be initialized there.
There were also a couple of trivial comment clean ups regarding the FPMCU GPIOS.
BUG=b:171837716 TEST=Boot & Check GPIO states. BRANCH=Zork
Signed-off-by: Martin Roth martinroth@chromium.org Change-Id: I16a2e621145782e0a908bb3e49478586c09a0e0a --- M src/mainboard/google/zork/bootblock.c M src/mainboard/google/zork/mainboard.c M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c M src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/zork/variants/ezkinil/gpio.c M src/mainboard/google/zork/variants/woomax/gpio.c 7 files changed, 84 insertions(+), 19 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/47308/6
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47308 )
Change subject: mb/google/zork: Init fingerprint GPIOs for boot vs resume ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47308/6/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c:
https://review.coreboot.org/c/coreboot/+/47308/6/src/mainboard/google/zork/v... PS6, Line 314: if (fpmcu_needs_delay()) { braces {} are not necessary for single statement blocks
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47308 )
Change subject: mb/google/zork: Init fingerprint GPIOs for boot vs resume ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47308/6/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c:
https://review.coreboot.org/c/coreboot/+/47308/6/src/mainboard/google/zork/v... PS6, Line 304: if (google_chromeec_cbi_get_board_version(&board_version)) : board_version = 1; : This reads board version but doesn't do anything with it. Not sure if you were intending to add check for board version here.
Hello build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47308
to look at the new patch set (#7).
Change subject: mb/google/zork: Init fingerprint GPIOs for boot vs resume ......................................................................
mb/google/zork: Init fingerprint GPIOs for boot vs resume
Add a function that initializes GPIOs based on the sleep type that the system is coming back from. This allows initialization of the fingerprint GPIOs which need to be handled differently between wake from S3 and boot from S5.
If fingerprint is disabled on the trembyle board, set the pins to no-connect. Dalboz doesn't have fingerprint and the GPIOS are configured differently due to the FT5 chip having fewer GPIOS than FP5, so nothing needs to be initialized there.
There were also a couple of trivial comment clean ups regarding the FPMCU GPIOS.
BUG=b:171837716 TEST=Boot & Check GPIO states. BRANCH=Zork
Signed-off-by: Martin Roth martinroth@chromium.org Change-Id: I16a2e621145782e0a908bb3e49478586c09a0e0a --- M src/mainboard/google/zork/bootblock.c M src/mainboard/google/zork/mainboard.c M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c M src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/zork/variants/ezkinil/gpio.c M src/mainboard/google/zork/variants/woomax/gpio.c 7 files changed, 81 insertions(+), 19 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/47308/7
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47308 )
Change subject: mb/google/zork: Init fingerprint GPIOs for boot vs resume ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47308/7/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c:
https://review.coreboot.org/c/coreboot/+/47308/7/src/mainboard/google/zork/v... PS7, Line 294: const Why const?
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47308 )
Change subject: mb/google/zork: Init fingerprint GPIOs for boot vs resume ......................................................................
Patch Set 7:
(2 comments)
https://review.coreboot.org/c/coreboot/+/47308/7/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c:
https://review.coreboot.org/c/coreboot/+/47308/7/src/mainboard/google/zork/v... PS7, Line 302: ) I think this should be done only when slp_typ is not ACPI_S3?
https://review.coreboot.org/c/coreboot/+/47308/7/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h:
https://review.coreboot.org/c/coreboot/+/47308/7/src/mainboard/google/zork/v... PS7, Line 29: powering on because of reset, powering on, or resuming from S3 That is the case for all GPIOs, right? Probably add GPIOs that need to be configured early on in bootblock?
Hello build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47308
to look at the new patch set (#8).
Change subject: mb/google/zork: Init fingerprint GPIOs for boot vs resume ......................................................................
mb/google/zork: Init fingerprint GPIOs for boot vs resume
Add a function that initializes GPIOs based on the sleep type that the system is coming back from. This allows initialization of the fingerprint GPIOs which need to be handled differently between wake from S3 and boot from S5.
If fingerprint is disabled on the trembyle-based board, set the pins to no-connect. Dalboz doesn't have fingerprint and the GPIOS are configured differently due to the FT5 chip having fewer GPIOS than FP5, so nothing needs to be initialized there.
There were also a couple of trivial comment clean ups regarding the FPMCU GPIOS.
BUG=b:171837716 TEST=Boot & Check GPIO states. BRANCH=Zork
Signed-off-by: Martin Roth martinroth@chromium.org Change-Id: I16a2e621145782e0a908bb3e49478586c09a0e0a --- M src/mainboard/google/zork/Kconfig M src/mainboard/google/zork/bootblock.c M src/mainboard/google/zork/mainboard.c M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c M src/mainboard/google/zork/variants/baseboard/helpers.c M src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/zork/variants/ezkinil/gpio.c M src/mainboard/google/zork/variants/woomax/gpio.c 9 files changed, 119 insertions(+), 20 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/47308/8
Hello build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47308
to look at the new patch set (#9).
Change subject: mb/google/zork: Init fingerprint GPIOs for boot vs resume ......................................................................
mb/google/zork: Init fingerprint GPIOs for boot vs resume
Add a function that initializes GPIOs based on the sleep type that the system is coming back from. This allows initialization of the fingerprint GPIOs which need to be handled differently between wake from S3 and boot from S5.
If fingerprint is disabled on the trembyle-based board, set the pins to no-connect. Dalboz doesn't have fingerprint and the GPIOS are configured differently due to the FT5 chip having fewer GPIOS than FP5, so nothing needs to be initialized there.
There were also a couple of trivial comment clean ups regarding the FPMCU GPIOS.
BUG=b:171837716 TEST=Boot & Check GPIO states. BRANCH=Zork
Signed-off-by: Martin Roth martinroth@chromium.org Change-Id: I16a2e621145782e0a908bb3e49478586c09a0e0a --- M src/mainboard/google/zork/Kconfig M src/mainboard/google/zork/bootblock.c M src/mainboard/google/zork/mainboard.c M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c M src/mainboard/google/zork/variants/baseboard/helpers.c M src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/zork/variants/ezkinil/gpio.c M src/mainboard/google/zork/variants/woomax/gpio.c 9 files changed, 119 insertions(+), 20 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/47308/9
Hello build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47308
to look at the new patch set (#10).
Change subject: mb/google/zork: Init fingerprint GPIOs for boot vs resume ......................................................................
mb/google/zork: Init fingerprint GPIOs for boot vs resume
Add a function that initializes GPIOs based on the sleep type that the system is coming back from. This allows initialization of the fingerprint GPIOs which need to be handled differently between wake from S3 and boot from S5.
If fingerprint is disabled on the trembyle-based board, set the pins to no-connect. Dalboz doesn't have fingerprint and the GPIOS are configured differently due to the FT5 chip having fewer GPIOS than FP5, so nothing needs to be initialized there.
There were also a couple of trivial comment clean ups regarding the FPMCU GPIOS.
BUG=b:171837716 TEST=Boot & Check GPIO states. BRANCH=Zork
Signed-off-by: Martin Roth martinroth@chromium.org Change-Id: I16a2e621145782e0a908bb3e49478586c09a0e0a --- M src/mainboard/google/zork/Kconfig M src/mainboard/google/zork/bootblock.c M src/mainboard/google/zork/mainboard.c M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c M src/mainboard/google/zork/variants/baseboard/helpers.c M src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/zork/variants/ezkinil/gpio.c M src/mainboard/google/zork/variants/woomax/gpio.c 9 files changed, 119 insertions(+), 20 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/47308/10
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47308 )
Change subject: mb/google/zork: Init fingerprint GPIOs for boot vs resume ......................................................................
Patch Set 10: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/47308/10/src/mainboard/google/zork/... File src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h:
https://review.coreboot.org/c/coreboot/+/47308/10/src/mainboard/google/zork/... PS10, Line 83: of if
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47308 )
Change subject: mb/google/zork: Init fingerprint GPIOs for boot vs resume ......................................................................
Patch Set 10:
(5 comments)
https://review.coreboot.org/c/coreboot/+/47308/7/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c:
https://review.coreboot.org/c/coreboot/+/47308/7/src/mainboard/google/zork/v... PS7, Line 294: const
Why const?
Left over from an earlier version of the patch. Removed.
https://review.coreboot.org/c/coreboot/+/47308/6/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c:
https://review.coreboot.org/c/coreboot/+/47308/6/src/mainboard/google/zork/v... PS6, Line 304: if (google_chromeec_cbi_get_board_version(&board_version)) : board_version = 1; :
This reads board version but doesn't do anything with it. […]
No, this one should have been removed.
https://review.coreboot.org/c/coreboot/+/47308/7/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c:
https://review.coreboot.org/c/coreboot/+/47308/7/src/mainboard/google/zork/v... PS7, Line 302: )
I think this should be done only when slp_typ is not ACPI_S3?
Done
https://review.coreboot.org/c/coreboot/+/47308/10/src/mainboard/google/zork/... File src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h:
https://review.coreboot.org/c/coreboot/+/47308/10/src/mainboard/google/zork/... PS10, Line 83: of
if
Done
https://review.coreboot.org/c/coreboot/+/47308/7/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h:
https://review.coreboot.org/c/coreboot/+/47308/7/src/mainboard/google/zork/v... PS7, Line 29: powering on because of reset, powering on, or resuming from S3
That is the case for all GPIOs, right? Probably add GPIOs that need to be configured early on in boo […]
Ack
Hello build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47308
to look at the new patch set (#11).
Change subject: mb/google/zork: Init fingerprint GPIOs for boot vs resume ......................................................................
mb/google/zork: Init fingerprint GPIOs for boot vs resume
Add a function that initializes GPIOs based on the sleep type that the system is coming back from. This allows initialization of the fingerprint GPIOs which need to be handled differently between wake from S3 and boot from S5.
If fingerprint is disabled on the trembyle-based board, set the pins to no-connect. Dalboz doesn't have fingerprint and the GPIOS are configured differently due to the FT5 chip having fewer GPIOS than FP5, so nothing needs to be initialized there.
There were also a couple of trivial comment clean ups regarding the FPMCU GPIOS.
BUG=b:171837716 TEST=Boot & Check GPIO states. BRANCH=Zork
Signed-off-by: Martin Roth martinroth@chromium.org Change-Id: I16a2e621145782e0a908bb3e49478586c09a0e0a --- M src/mainboard/google/zork/Kconfig M src/mainboard/google/zork/bootblock.c M src/mainboard/google/zork/mainboard.c M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c M src/mainboard/google/zork/variants/baseboard/helpers.c M src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/zork/variants/ezkinil/gpio.c M src/mainboard/google/zork/variants/woomax/gpio.c 9 files changed, 119 insertions(+), 20 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/47308/11
Hello build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47308
to look at the new patch set (#12).
Change subject: mb/google/zork: Init fingerprint GPIOs for boot vs resume ......................................................................
mb/google/zork: Init fingerprint GPIOs for boot vs resume
Add a function that initializes GPIOs based on the sleep type that the system is coming back from. This allows initialization of the fingerprint GPIOs which need to be handled differently between wake from S3 and boot from S5.
On initial boot, the state of the FP sensor could be either enabled or disabled. Because of this, on boot, we power off the sensor for >200ms, to reset its state, then power it back on.
In suspend/resume, the fingerprint sensor should remain powered the entire time.
If fingerprint is disabled on the trembyle-based board, set the pins to no-connect. Dalboz doesn't have fingerprint and the GPIOS are configured differently due to the FT5 chip having fewer GPIOS than FP5, so nothing needs to be initialized there.
There were also a couple of trivial comment clean ups regarding the FPMCU GPIOS.
BUG=b:171837716 TEST=Boot & Check GPIO states. BRANCH=Zork
Signed-off-by: Martin Roth martinroth@chromium.org Change-Id: I16a2e621145782e0a908bb3e49478586c09a0e0a --- M src/mainboard/google/zork/Kconfig M src/mainboard/google/zork/bootblock.c M src/mainboard/google/zork/mainboard.c M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c M src/mainboard/google/zork/variants/baseboard/helpers.c M src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/zork/variants/ezkinil/gpio.c M src/mainboard/google/zork/variants/woomax/gpio.c 9 files changed, 119 insertions(+), 20 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/47308/12
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47308 )
Change subject: mb/google/zork: Init fingerprint GPIOs for boot vs resume ......................................................................
Patch Set 12:
(2 comments)
https://review.coreboot.org/c/coreboot/+/47308/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/47308/4//COMMIT_MSG@10 PS4, Line 10: This allows initialization of the : fingerprint GPIOs which need to be handled differently between wake : from S3 and boot from S5.
Sure, I'll add it. […]
Done
https://review.coreboot.org/c/coreboot/+/47308/4/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c:
https://review.coreboot.org/c/coreboot/+/47308/4/src/mainboard/google/zork/v... PS4, Line 317: slp_typ != ACPI_S0
If you poweroff and power back on within 10 seconds, I think it should be waking up from S5. […]
Good point. I didn't test that, but I updated the code to just match ACPI_S3.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47308 )
Change subject: mb/google/zork: Init fingerprint GPIOs for boot vs resume ......................................................................
Patch Set 12: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/47308 )
Change subject: mb/google/zork: Init fingerprint GPIOs for boot vs resume ......................................................................
mb/google/zork: Init fingerprint GPIOs for boot vs resume
Add a function that initializes GPIOs based on the sleep type that the system is coming back from. This allows initialization of the fingerprint GPIOs which need to be handled differently between wake from S3 and boot from S5.
On initial boot, the state of the FP sensor could be either enabled or disabled. Because of this, on boot, we power off the sensor for >200ms, to reset its state, then power it back on.
In suspend/resume, the fingerprint sensor should remain powered the entire time.
If fingerprint is disabled on the trembyle-based board, set the pins to no-connect. Dalboz doesn't have fingerprint and the GPIOS are configured differently due to the FT5 chip having fewer GPIOS than FP5, so nothing needs to be initialized there.
There were also a couple of trivial comment clean ups regarding the FPMCU GPIOS.
BUG=b:171837716 TEST=Boot & Check GPIO states. BRANCH=Zork
Signed-off-by: Martin Roth martinroth@chromium.org Change-Id: I16a2e621145782e0a908bb3e49478586c09a0e0a Reviewed-on: https://review.coreboot.org/c/coreboot/+/47308 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com --- M src/mainboard/google/zork/Kconfig M src/mainboard/google/zork/bootblock.c M src/mainboard/google/zork/mainboard.c M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c M src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c M src/mainboard/google/zork/variants/baseboard/helpers.c M src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/zork/variants/ezkinil/gpio.c M src/mainboard/google/zork/variants/woomax/gpio.c 9 files changed, 119 insertions(+), 20 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/mainboard/google/zork/Kconfig b/src/mainboard/google/zork/Kconfig index 02181fb..320f668 100644 --- a/src/mainboard/google/zork/Kconfig +++ b/src/mainboard/google/zork/Kconfig @@ -225,6 +225,22 @@ Minimum board version where the variant starts supporting active low power enable for WiFi.
+config VARIANT_HAS_FPMCU + bool + default y if BOARD_GOOGLE_BERKNIP + default y if BOARD_GOOGLE_MORPHIUS + default n + help + Select y if any SKU of the board has a fingerprint sensor + +config VARIANT_MAX_BOARD_ID_BROKEN_FMPCU_POWER + int + default 4 if BOARD_GOOGLE_MORPHIUS + default 3 if BOARD_GOOGLE_BERKNIP + default 0 + help + Last board version that needs the extra delay for FPMCU init. + config VBOOT_STARTS_BEFORE_BOOTBLOCK bool "PSP verstage" default y if VBOOT diff --git a/src/mainboard/google/zork/bootblock.c b/src/mainboard/google/zork/bootblock.c index a7636de..ed05888 100644 --- a/src/mainboard/google/zork/bootblock.c +++ b/src/mainboard/google/zork/bootblock.c @@ -2,12 +2,16 @@
#include <bootblock_common.h> #include <baseboard/variants.h> +#include <acpi/acpi.h>
void bootblock_mainboard_early_init(void) { size_t num_gpios; const struct soc_amd_gpio *gpios;
+ gpios = variant_bootblock_gpio_table(&num_gpios, acpi_get_sleep_type()); + program_gpios(gpios, num_gpios); + if (!CONFIG(VBOOT_STARTS_BEFORE_BOOTBLOCK)) { gpios = variant_early_gpio_table(&num_gpios); program_gpios(gpios, num_gpios); diff --git a/src/mainboard/google/zork/mainboard.c b/src/mainboard/google/zork/mainboard.c index 9503d37..6d930d0 100644 --- a/src/mainboard/google/zork/mainboard.c +++ b/src/mainboard/google/zork/mainboard.c @@ -247,12 +247,13 @@
gnvs = acpi_get_gnvs();
- if (gnvs) { gnvs->tmps = CTL_TDP_SENSOR_ID; gnvs->tcrt = CRITICAL_TEMPERATURE; gnvs->tpsv = PASSIVE_TEMPERATURE; } + + finalize_gpios(acpi_get_sleep_type()); }
struct chip_operations mainboard_ops = { diff --git a/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c b/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c index 2398d07..e43ccdd 100644 --- a/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c +++ b/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_dalboz.c @@ -30,7 +30,7 @@ PAD_NF(GPIO_8, ACP_I2S_LRCLK, PULL_NONE), /* TOUCHPAD_INT_ODL */ PAD_SCI(GPIO_9, PULL_NONE, EDGE_LOW), - /* S0iX SLP - (unused - goes to EC & FPMCU */ + /* S0iX SLP - (unused - goes to EC */ PAD_NC(GPIO_10), /* EC_IN_RW_OD */ PAD_GPI(GPIO_11, PULL_NONE), @@ -291,6 +291,16 @@ wifi_power_reset_configure_pre_v3(); }
+__weak void finalize_gpios(int slp_typ) +{ +} + +const __weak struct soc_amd_gpio *variant_bootblock_gpio_table(size_t *size, int slp_typ) +{ + *size = 0; + return NULL; +} + static const struct soc_amd_gpio gpio_sleep_table[] = { /* PCIE_RST1_L */ PAD_GPO(GPIO_27, LOW), diff --git a/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c b/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c index a436d1c..9e980db 100644 --- a/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c +++ b/src/mainboard/google/zork/variants/baseboard/gpio_baseboard_trembyle.c @@ -1,7 +1,9 @@ /* SPDX-License-Identifier: GPL-2.0-or-later */
+#include <acpi/acpi.h> #include <baseboard/variants.h> #include <delay.h> +#include <ec/google/chromeec/ec.h> #include <gpio.h> #include <soc/gpio.h> #include <soc/smi.h> @@ -32,8 +34,6 @@ PAD_SCI(GPIO_9, PULL_NONE, EDGE_LOW), /* S0iX SLP - (unused - goes to EC & FPMCU */ PAD_NC(GPIO_10), - /* FPMCU_RST_L */ - PAD_GPO(GPIO_11, HIGH), /* USI_INT_ODL */ PAD_GPI(GPIO_12, PULL_NONE), /* EN_PWR_TOUCHPAD_PS2 */ @@ -71,8 +71,6 @@ PAD_NF(GPIO_30, ESPI_CS_L, PULL_NONE), /* EC_AP_INT_ODL (Sensor Framesync) */ PAD_GPI(GPIO_31, PULL_NONE), - /* EN_PWR_FP */ - PAD_GPO(GPIO_32, HIGH), /* GPIO_33 - GPIO_39: Not available */ /* NVME_AUX_RESET_L */ PAD_GPO(GPIO_40, HIGH), @@ -87,7 +85,7 @@ PAD_GPO(GPIO_67, LOW), // Select Camera 1 Dmic /* EMMC_RESET_L */ PAD_GPO(GPIO_68, HIGH), - /* FPMCU_BOOT0 - TODO: Check this */ + /* FPMCU_BOOT0 */ PAD_GPO(GPIO_69, LOW), /* EMMC_CLK */ PAD_NF(GPIO_70, EMMC_CLK, PULL_NONE), @@ -300,6 +298,51 @@ wifi_power_reset_configure_pre_v3(); }
+__weak void finalize_gpios(int slp_typ) +{ + if (variant_has_fingerprint() && slp_typ != ACPI_S3) { + + if (fpmcu_needs_delay()) + mdelay(550); + + /* + * Enable the FPMCU by enabling EN_PWR_FP, then bringing it out + * of reset by setting FPMCU_RST_L high 3ms later. + */ + gpio_set(GPIO_32, 1); + mdelay(3); + gpio_set(GPIO_11, 1); + } +} + +static const struct soc_amd_gpio gpio_fingerprint_bootblock_table[] = { + /* FPMCU_RST_L */ + PAD_GPO(GPIO_11, LOW), + /* EN_PWR_FP */ + PAD_GPO(GPIO_32, LOW), +}; + +static const struct soc_amd_gpio gpio_no_fingerprint_bootblock_table[] = { + /* FPMCU_RST_L */ + PAD_NC(GPIO_11), + /* EN_PWR_FP */ + PAD_NC(GPIO_32), +}; + +const __weak struct soc_amd_gpio *variant_bootblock_gpio_table(size_t *size, int slp_typ) +{ + if (variant_has_fingerprint()) { + if (slp_typ == ACPI_S3) + return NULL; + + *size = ARRAY_SIZE(gpio_fingerprint_bootblock_table); + return gpio_fingerprint_bootblock_table; + } + + *size = ARRAY_SIZE(gpio_no_fingerprint_bootblock_table); + return gpio_no_fingerprint_bootblock_table; +} + static const struct soc_amd_gpio gpio_sleep_table[] = { /* NVME_AUX_RESET_L */ PAD_GPO(GPIO_40, LOW), diff --git a/src/mainboard/google/zork/variants/baseboard/helpers.c b/src/mainboard/google/zork/variants/baseboard/helpers.c index 7071035..d12a8ed4 100644 --- a/src/mainboard/google/zork/variants/baseboard/helpers.c +++ b/src/mainboard/google/zork/variants/baseboard/helpers.c @@ -149,3 +149,31 @@ { return extract_field(FW_CONFIG_MASK_DB_INDEX, FW_CONFIG_DB_INDEX_SHIFT); } + +bool variant_has_fingerprint(void) +{ + if (CONFIG(VARIANT_HAS_FPMCU)) + return true; + + return false; +} + +bool fpmcu_needs_delay(void) +{ + /* + * Older board versions need an extra delay here to finish resetting + * the FPMCU. The resistor value in the glitch prevention circuit was + * sized so that the FPMCU doesn't turn of for ~1 second. On newer + * boards, that's been updated to ~30ms, which allows the FPMCU's + * reset to be completed in the time between bootblock and finalize. + */ + uint32_t board_version; + + if (google_chromeec_cbi_get_board_version(&board_version)) + board_version = 1; + + if (board_version <= CONFIG_VARIANT_MAX_BOARD_ID_BROKEN_FMPCU_POWER) + return true; + + return false; +} diff --git a/src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h b/src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h index 338b918..4ec6add 100644 --- a/src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h +++ b/src/mainboard/google/zork/variants/baseboard/include/baseboard/variants.h @@ -24,12 +24,17 @@ */ const struct soc_amd_gpio *variant_override_gpio_table(size_t *size);
+/* This function provides GPIO init in bootblock. */ +const struct soc_amd_gpio *variant_bootblock_gpio_table(size_t *size, int slp_typ); + /* * This function provides GPIO table for the pads that need to be configured when entering * sleep. */ const struct soc_amd_gpio *variant_sleep_gpio_table(size_t *size, int slp_typ);
+/* Program any required GPIOs at the finalize phase */ +void finalize_gpios(int slp_typ); /* Modify devictree settings during ramstage. */ void variant_devtree_update(void); /* Update audio configuration in devicetree during ramstage. */ @@ -69,9 +74,13 @@ bool variant_uses_v3_6_schematics(void); /* Return true if variant uses CODEC_GPI pin for headphone jack interrupt. */ bool variant_uses_codec_gpi(void); -/* Return true if variant has active low power enable fow WiFi. */ +/* Return true if variant has active low power enable for WiFi. */ bool variant_has_active_low_wifi_power(void); /* Return value of daughterboard ID */ int variant_get_daughterboard_id(void); +/* Return true if the board has a fingerprint sensor. */ +bool variant_has_fingerprint(void); +/* Return true if the board needs an extra fpmcu delay. */ +bool fpmcu_needs_delay(void);
#endif /* __BASEBOARD_VARIANTS_H__ */ diff --git a/src/mainboard/google/zork/variants/ezkinil/gpio.c b/src/mainboard/google/zork/variants/ezkinil/gpio.c index d2dd104..f86d926 100644 --- a/src/mainboard/google/zork/variants/ezkinil/gpio.c +++ b/src/mainboard/google/zork/variants/ezkinil/gpio.c @@ -37,8 +37,6 @@ PAD_NC(GPIO_4), /* PEN_POWER_EN - Not connected */ PAD_NC(GPIO_5), - /* FPMCU_RST_L Change NC */ - PAD_NC(GPIO_11), /* DMIC_SEL */ PAD_GPO(GPIO_13, LOW), // Select Camera 1 Dmic /* EN_PWR_WIFI */ @@ -62,8 +60,6 @@ PAD_NC(GPIO_4), /* PEN_POWER_EN - Not connected */ PAD_NC(GPIO_5), - /* FPMCU_RST_L Change NC */ - PAD_NC(GPIO_11), /* FPMCU_BOOT0 Change NC */ PAD_NC(GPIO_69), /* EN_DEV_BEEP_L */ diff --git a/src/mainboard/google/zork/variants/woomax/gpio.c b/src/mainboard/google/zork/variants/woomax/gpio.c index cb98df7..329e7dd 100644 --- a/src/mainboard/google/zork/variants/woomax/gpio.c +++ b/src/mainboard/google/zork/variants/woomax/gpio.c @@ -10,10 +10,6 @@ PAD_NC(GPIO_5), /* GPIO_6 NC */ PAD_NC(GPIO_6), - /* GPIO_11 NC */ - PAD_NC(GPIO_11), - /* GPIO_32 NC */ - PAD_NC(GPIO_32), /* GPIO_69 NC */ PAD_NC(GPIO_69), /* RAM_ID_4 */ @@ -37,10 +33,6 @@ PAD_NC(GPIO_5), /* GPIO_6 NC */ PAD_NC(GPIO_6), - /* GPIO_11 NC */ - PAD_NC(GPIO_11), - /* GPIO_32 NC */ - PAD_NC(GPIO_32), /* GPIO_69 NC */ PAD_NC(GPIO_69), /* RAM_ID_4 */
-
build bot (Jenkins) (Code Review) -
Furquan Shaikh (Code Review)
-
Martin Roth (Code Review)
-
Patrick Georgi (Code Review)
-
Peichao Wang (Code Review)