Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39693 )
Change subject: drivers/spi/tpm: Add support for non CR50 SPI TPM2 ......................................................................
Patch Set 1:
(7 comments)
https://review.coreboot.org/c/coreboot/+/39693/1/src/drivers/spi/tpm/Kconfig File src/drivers/spi/tpm/Kconfig:
https://review.coreboot.org/c/coreboot/+/39693/1/src/drivers/spi/tpm/Kconfig... PS1, Line 19: select SPI_TPM
I think this should select MAINBOARD_HAS_SPI_TPM, and then things in the other Kconfig file might be […]
Done
https://review.coreboot.org/c/coreboot/+/39693/1/src/drivers/spi/tpm/tpm.c File src/drivers/spi/tpm/tpm.c:
https://review.coreboot.org/c/coreboot/+/39693/1/src/drivers/spi/tpm/tpm.c@1... PS1, Line 186: * last bit after shifting in the address bytes.
The problem is that many of our SPI drivers don't support full-duplex communication. […]
Done
https://review.coreboot.org/c/coreboot/+/39693/1/src/drivers/spi/tpm/tpm.c@1... PS1, Line 189: spi_xfer(&spi_slave, header.body, sizeof(header.body), &header_resp, sizeof(header_resp.body));
Could we fix this somehow?
Done
https://review.coreboot.org/c/coreboot/+/39693/1/src/drivers/spi/tpm/tpm.c@1... PS1, Line 190: (header_resp.body[3] & 1)
I haven't read the spec closely, but are you sure that the ready bit can only be set in the last byt […]
that's what the spec says. The last byte of the address has the ready bit set.
https://review.coreboot.org/c/coreboot/+/39693/1/src/drivers/spi/tpm/tpm.c@1... PS1, Line 194: point.
nit: fits on the previous line
Done
https://review.coreboot.org/c/coreboot/+/39693/1/src/security/tpm/Kconfig File src/security/tpm/Kconfig:
https://review.coreboot.org/c/coreboot/+/39693/1/src/security/tpm/Kconfig@29 PS1, Line 29: || MAINBOARD_HAS_SPI_TPM_CR50 || MAINBOARD_HAS_CRB_TPM \
Uh, these lines are getting a bit too long
there's no limit for Kconfig, is there?
https://review.coreboot.org/c/coreboot/+/39693/1/src/security/tpm/Kconfig@31 PS1, Line 31: config TPM2_CR50
We already have TPM_CR50 in tss/vendor/cr50/Kconfig
Done