Jes Klinke 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 19:
(2 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 patc […]
Bummer, it should have been CR50_BOARD_CFG_VALUE, which is now computed based on Kconfig setting CR50_USE_LONG_INTERRUPT_PULSES (and in the future other separate Kconfig settings could set other bits in CR50_BOARD_CFG_VALUE.)
I am hindered by my Volteer sources running on the Google coreboot mirror, so initially I developed the patch there, and then copied it to my "pure" coreboot repository in order to send it for review. This means that it is not easy to try to compile again, after making changes in the coreboot repository.
There must be a way of cross compiling (or plain compiling) coreboot itself, not connected to a ChromeOS checkout, but I am not familiar with how to do so.
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.
I do believe that Cr50 will truly zero-pad the sequence of bytes transmitted over SPI, even though this code does not depend on more than a single zero terminator. (It would be silly to send random bytes, and even worse to send parts of the Cr50 heap/stack.) As I have not actually verified that it is zero padded, I guess I should just stick to what this code actually requires.