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.