Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44277 )
Change subject: drivers/spi/tpm: Add support to cache the TPM firmware version ......................................................................
Patch Set 2:
(15 comments)
https://review.coreboot.org/c/coreboot/+/44277/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44277/2//COMMIT_MSG@15 PS2, Line 15: DUT Which one?
https://review.coreboot.org/c/coreboot/+/44277/2/src/drivers/spi/tpm/tpm.h File src/drivers/spi/tpm/tpm.h:
https://review.coreboot.org/c/coreboot/+/44277/2/src/drivers/spi/tpm/tpm.h@5... PS2, Line 58: /** /*
https://review.coreboot.org/c/coreboot/+/44277/2/src/drivers/spi/tpm/tpm.h@5... PS2, Line 59: a firmware version Maybe:
… a specific firmware version.
https://review.coreboot.org/c/coreboot/+/44277/2/src/drivers/spi/tpm/tpm.h@6... PS2, Line 60: * @match : Firmware version to be matched against. Remove tab?
https://review.coreboot.org/c/coreboot/+/44277/2/src/drivers/spi/tpm/tpm.h@6... PS2, Line 64: is_tpm2_fw_version
is_same_tpm2_fw_version
https://review.coreboot.org/c/coreboot/+/44277/2/src/drivers/spi/tpm/tpm.c File src/drivers/spi/tpm/tpm.c:
https://review.coreboot.org/c/coreboot/+/44277/2/src/drivers/spi/tpm/tpm.c@4... PS2, Line 430: /** /*
https://review.coreboot.org/c/coreboot/+/44277/2/src/drivers/spi/tpm/tpm.c@4... PS2, Line 432: * @fw_ver : Tokenized firmware version string. I’d use exactly one space before the colon, and maybe the tab afterward.
https://review.coreboot.org/c/coreboot/+/44277/2/src/drivers/spi/tpm/tpm.c@4... PS2, Line 435: * @return: 0 on success, -1 on error. Please use `CB_SUCCESS` and friends.
https://review.coreboot.org/c/coreboot/+/44277/2/src/drivers/spi/tpm/tpm.c@4... PS2, Line 445: */ Remove?
https://review.coreboot.org/c/coreboot/+/44277/2/src/drivers/spi/tpm/tpm.c@4... PS2, Line 447: enum tpm2_fw_ver_type type = TPM2_FW_VER_TYPE_INIT; Why not use null?
https://review.coreboot.org/c/coreboot/+/44277/2/src/drivers/spi/tpm/tpm.c@4... PS2, Line 454: if (type == TPM2_FW_VER_TYPE_INIT) Could also be just an `else` branch above.
https://review.coreboot.org/c/coreboot/+/44277/2/src/drivers/spi/tpm/tpm.c@4... PS2, Line 459: printk(BIOS_ERR, "Error parsing after epoch\n"); Prefix with TPM, and also note the effect: TPM firmware can’t be cached.
https://review.coreboot.org/c/coreboot/+/44277/2/src/drivers/spi/tpm/tpm.c@4... PS2, Line 491: uint32_t size_t
https://review.coreboot.org/c/coreboot/+/44277/2/src/drivers/spi/tpm/tpm.c@8... PS2, Line 847: Setup Set up
https://review.coreboot.org/c/coreboot/+/44277/2/src/drivers/spi/tpm/tpm.c@8... PS2, Line 855: setup set up