Jes Klinke has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44359 )
Change subject: mainboard/google/volteer: Enable long cr50 ready pulses ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/44359/4/src/mainboard/google/voltee... File src/mainboard/google/volteer/Kconfig:
https://review.coreboot.org/c/coreboot/+/44359/4/src/mainboard/google/voltee... PS4, Line 61: config CR50_USE_LONG_INTERRUPT_PULSES : bool : default y
You can just do a "select CR50_USE_LONG_INTERRUPT_PULSES" under BOARD_GOOGLE_BASEBOARD_VOLTEER
Actually, I am starting to think that this Kconfig setting belongs in soc/tigerlake. That is, if the SoC is Tiger Lake, and TPM_CR50 is selected, then we should autumatically also select CR50_USE_LONG_INTERRUPT_PULSES. I have seen config clauses declaring that the default should depend on some other setting, but I do not know if such a default in the soc directory would properly override the default "n" from drivers.
You may say that it is odd to have the Kconfig in soc/tigerlake, but the other logic in mainboard, but I see it as this version-detection logic is only for a "transition" phase, until our Cr50 from the factory have new enough firmware, new Tiger Lake designs coming out after that will only need CR50_USE_LONG_INTERRUPT_PULSES to be selected, no additional code in mainboard. (It may well be that we stop making Tiger Lake designs before we update the factory image of Cr50, but even then, we will have new similar "lakes" in the future, which could benefit from copying a CR50_USE_LONG_INTERRUPT_PULSES setting from soc/tigerlake.)
https://review.coreboot.org/c/coreboot/+/44359/4/src/mainboard/google/voltee... File src/mainboard/google/volteer/chromeos.c:
https://review.coreboot.org/c/coreboot/+/44359/4/src/mainboard/google/voltee... PS4, Line 48: S03.4
S0i3. […]
Done