Furquan Shaikh 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 21: Code-Review+2
(3 comments)
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 nit: Since this is returning true/false, I think we should name this something like `cr50_supports_long_interrupt_pulses()`
https://review.coreboot.org/c/coreboot/+/43741/20/src/drivers/spi/tpm/tpm.c File src/drivers/spi/tpm/tpm.c:
https://review.coreboot.org/c/coreboot/+/43741/20/src/drivers/spi/tpm/tpm.c@... PS20, Line 622: ;
The logic does not depend on the buffer being pre-initialized to zero. […]
We rely on version_str[**chunk_count * chunk_size - 1] to be zero when checking in line 637 below if we have to exit early i.e. before the entire 300 bytes are written. This means that the assumption here is that tpm2_read_reg will fill it with 0s. So, ideally, we can drop the explicit zero-termination after while loop if pre-initialized to 0 here. But, I think it's fine if you want to continue without this.
https://review.coreboot.org/c/coreboot/+/43741/21/src/drivers/spi/tpm/tpm.c File src/drivers/spi/tpm/tpm.c:
https://review.coreboot.org/c/coreboot/+/43741/21/src/drivers/spi/tpm/tpm.c@... PS21, Line 439: { brace can be dropped.