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 26:
(5 comments)
Sorry, yesterday I failed to pay proper attention to the output of my git commands, so while I thought I had uploaded a new patchset before replying "Done" to all your comments, it turns out that I had not.
Please have a look at this one.
https://review.coreboot.org/c/coreboot/+/43741/21/src/drivers/spi/tpm/tpm.h File src/drivers/spi/tpm/tpm.h:
https://review.coreboot.org/c/coreboot/+/43741/21/src/drivers/spi/tpm/tpm.h@... PS21, Line 45: get_cr50_long_interrupt_pulses
How about cr50_is_long_interrupt_pulse_enabled()
Done
https://review.coreboot.org/c/coreboot/+/43741/22/src/drivers/spi/tpm/tpm.c File src/drivers/spi/tpm/tpm.c:
https://review.coreboot.org/c/coreboot/+/43741/22/src/drivers/spi/tpm/tpm.c@... PS22, Line 449: cr50
Jes - do you plan to update the name of TPM_BOARD_CFG register?
I have changed it into CR50_BOARD_CFG, to make it clear it is a Google extension, and not part of the TPM standard.
https://review.coreboot.org/c/coreboot/+/43741/22/src/drivers/spi/tpm/tpm.c@... PS22, Line 631: if (version_str[++chunk_count * chunk_size - 1])
This is not yet addressed even in the latest patchset.
Done
https://review.coreboot.org/c/coreboot/+/43741/23/src/drivers/spi/tpm/tpm.c File src/drivers/spi/tpm/tpm.c:
https://review.coreboot.org/c/coreboot/+/43741/23/src/drivers/spi/tpm/tpm.c@... PS23, Line 462: "matches desired = 0x%08x\n",
nit: No need to break any of the print strings. They can also go beyond the 96-column limit.
Done
https://review.coreboot.org/c/coreboot/+/43741/23/src/drivers/spi/tpm/tpm.c@... PS23, Line 631: if (version_str[++chunk_count * chunk_size - 1])
This is not correct as Julius pointed out on a previous patchset.
Done