Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34380 )
Change subject: src/drivers/intel/ptt: Add PTT Support ......................................................................
Patch Set 13:
(14 comments)
https://review.coreboot.org/c/coreboot/+/34380/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34380/3//COMMIT_MSG@10 PS3, Line 10: integrated TPM is enabled/active.
Corrected spelling: Add function which checks if … […]
Done
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/Kconfig File src/drivers/ptt/Kconfig:
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/Kconfig@1 PS3, Line 1: config INTEL_PTT
Rephrase: HAVE_INTEL_PTT? ACTIVATE_INTEL_PTT?
Done
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/Kconfig@5 PS3, Line 5: Intel Platform Trust Technology like Intel iTPM
Please elaborate.
Done
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/ptt.h File src/drivers/ptt/ptt.h:
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/ptt.h@7 PS3, Line 7: PTT_ENABLE_BIT
PTT_ENABLED? No idea, what is common in coreboot.
Done
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/ptt.h@7 PS3, Line 7: <<
Please add spaces around the operator. […]
Done
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/ptt.h@8 PS3, Line 8:
Please document the function.
Done
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/ptt.c File src/drivers/ptt/ptt.c:
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/ptt.c@12 PS3, Line 12: Register
register
Done
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/ptt.c@13 PS3, Line 13: dump_status
That’s quite generic and nothing is dumped.
Done
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/ptt.c@13 PS3, Line 13: static uint32_t dump_status(int index, int reg_addr)
Can this be moved to some common code?
Done
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/ptt.c@17 PS3, Line 17: return reg;
Just one line? […]
Done
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/ptt.c@25 PS3, Line 25: * Return 0 if active, -1 otherwise.
return true if active, false if not
Done
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/ptt.c@27 PS3, Line 27: int ptt_active(void)
bool
Done
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/ptt.c@29 PS3, Line 29: // Check if PTT establishment bit is valid
Please indent the comment line correctly.
Done
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/ptt.c@32 PS3, Line 32: Bit
bit
Done