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.