Attention is currently required from: Patrick Rudolph, Paul Menzel, Christian Walter, Angel Pons, Arthur Heymans, Kyösti Mälkki. Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58240 )
Change subject: drivers/pc80/tpm: Use stopwatch for timeout-loops ......................................................................
Patch Set 2:
(6 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/58240/comment/8202bdad_674d7024 PS1, Line 20: TEST=Enable TPM debug messages on a board where the TPM hits a timeout : by failure and make sure that the debug messages occur in the log : just in the timeout period.
Yes, I have tested both cases, with and without debug TPM messages. Both work fine.
Ack
File src/drivers/pc80/tpm/tis.c:
https://review.coreboot.org/c/coreboot/+/58240/comment/f490b4a3_f778dc79 PS1, Line 257: udelay(1); /* 1 us */
OK, will do!
Ack
https://review.coreboot.org/c/coreboot/+/58240/comment/7d4df269_4a7171bb PS1, Line 88: #define MAX_DELAY_US (1000 * 1000)
Yes, sounds reasonable. […]
Ack
https://review.coreboot.org/c/coreboot/+/58240/comment/36d30066_8889f344 PS1, Line 242: * Wait for at least a second for a status to change its state to match the
In the old code it was indeed at least a second. Will change to 'at most 1 second'.
Ack
https://review.coreboot.org/c/coreboot/+/58240/comment/6c1c30c9_dcabaf44 PS1, Line 256: while (!stopwatch_expired(&sw)) { : u8 value = tpm_read_status(locality); : if ((value & mask) == expected) : return 0; : }
nit: I'd use a do-while loop to ensure the loop body is executed at least once. […]
Ack
https://review.coreboot.org/c/coreboot/+/58240/comment/3ffcc18c_54ea0f69 PS1, Line 457: stopwatch_init_usecs_expire(&sw, MAX_DELAY_US);
Sure, will do.
Ack