Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47309 )
Change subject: mb/google/zork: Add func to determine fingerprint gpio init ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/47309/2/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/berknip/variant.c:
https://review.coreboot.org/c/coreboot/+/47309/2/src/mainboard/google/zork/v... PS2, Line 10: if (ENV_RAMSTAGE) { : cfg = (struct soc_amd_picasso_config *)config_of_soc();
It's not strictly required. I'll be glad to remove it if you want.
I think we should let the linker optimize things out rather than adding ENV_RAMSTAGE checks.
https://review.coreboot.org/c/coreboot/+/47309/2/src/mainboard/google/zork/v... PS2, Line 21: BERKNIP_SKU0_ID 0x5A030020
Yes, I totally agree. […]
Okay. Taking a step back: Do we really need to differentiate between SKUs with and without fingerprint. The only thing this information is used for in firmware is to configure FPMCU enable and reset GPIOs differently. On the SKUs where there is no fingerprint, I think it is okay to still configure the enable and reset lines in a similar way. Those SKUs will have the lines not connected to anything, so there should not be any harm.
I think we can simply add a Kconfig VARIANT_HAS_FINGERPRINT and use that to configure pads either as NC if the variant doesn't set the Kconfig. Else, configure the pads as per the sequencing required.
The reason why I am asking not to check SKU ID here is because if there is a new SKU for the variant that has fingerprint then it will require change to coreboot which means a firmware re-qualification, which is unnecessary. Instead we can have the variant always assume that pads for fingerprint need to be configured if any SKU requires it.