Jes Klinke has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43741 )
Change subject: Enable long cr50 ready pulses for Tigerlake systems ......................................................................
Patch Set 11:
(7 comments)
PTAL
https://review.coreboot.org/c/coreboot/+/43741/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43741/2//COMMIT_MSG@15 PS2, Line 15: to FSP. For Volteer (and future Tigerlake boards) we enable mode S0i3.4
The version is always loaded the first time the TPM interface is initialized in each stage, so if yo […]
Done
https://review.coreboot.org/c/coreboot/+/43741/7/src/drivers/spi/tpm/tpm.c File src/drivers/spi/tpm/tpm.c:
https://review.coreboot.org/c/coreboot/+/43741/7/src/drivers/spi/tpm/tpm.c@3... PS7, Line 357: int cr50_read_board_cfg(uint32_t *status)
Maybe check the version number right in here and return a -1 or something if we know the board doesn […]
I have deleted these access methods, as they do not make sense, now that the logic which previously used them has been moved into this same file.
https://review.coreboot.org/c/coreboot/+/43741/7/src/drivers/spi/tpm/tpm.c@5... PS7, Line 571: set_cr50_board_cfg();
Twice?
Done
https://review.coreboot.org/c/coreboot/+/43741/7/src/drivers/spi/tpm/tpm.c@6... PS7, Line 600: static
Why is this static?
Deleted.
https://review.coreboot.org/c/coreboot/+/43741/7/src/drivers/spi/tpm/tpm.c@6... PS7, Line 650: index = 3;
This is super complicated. Can't we just do something like […]
Done
https://review.coreboot.org/c/coreboot/+/43741/7/src/drivers/spi/tpm/tpm.c@6... PS7, Line 661: #if CONFIG(TPM_CR50) && CONFIG_TPM_BOARD_CFG
Please always use […]
I had put some of the helper methods inside an #if block, so using if() (as opposed to #if) would have resulted in compilation errors on non-Google targets. Now I have made the helper methods static, and removed the #if around them. I assume that the compiler will silently drop these methods from the output, in cases where all their invocations are from dead code in if statements with compile-time constant conditions.
https://review.coreboot.org/c/coreboot/+/43741/7/src/drivers/spi/tpm/tpm.c@6... PS7, Line 662: ENV_SEPARATE_VERSTAGE || ENV_BOOTBLOCK || !CONFIG(VBOOT)
Let's factor this check out into a separate function (e.g. […]
Done