Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43741 )
Change subject: drivers/spi/tpm: Enable long cr50 ready pulses for Tiger Lake systems ......................................................................
Patch Set 18:
(3 comments)
https://review.coreboot.org/c/coreboot/+/43741/18/src/drivers/spi/tpm/tpm.c File src/drivers/spi/tpm/tpm.c:
https://review.coreboot.org/c/coreboot/+/43741/18/src/drivers/spi/tpm/tpm.c@... PS18, Line 485: CONFIG_TPM_BOARD_CFG I think you dropped the part where this gets defined in Kconfig? Or was that already in another patch?
https://review.coreboot.org/c/coreboot/+/43741/18/src/drivers/spi/tpm/tpm.c@... PS18, Line 638: padded nit: zero-terminated. zero-padded implies you're filling the whole rest of the array with zeroes.
https://review.coreboot.org/c/coreboot/+/43741/18/src/security/tpm/tss/vendo... File src/security/tpm/tss/vendor/cr50/Kconfig:
https://review.coreboot.org/c/coreboot/+/43741/18/src/security/tpm/tss/vendo... PS18, Line 19: "Whether to request longer pulses using Cr50 BOARD_CFG register" Would probably be good to clarify that just selecting this option doesn't guarantee you're gonna get the long pulses. (Also maybe calling it CR50_REQUEST_LONG_INTERRUPT_PULSES could help make that clearer.)