Change in coreboot[master]: cr50 i2c: add error message reporting TPM IRQ timeout

List overview All Threads
Download

newer

older

Change in coreboot[master]:...

Vadim Bendebury (Code Review)

26 Nov 2019 26 Nov '19

11:38 p.m.

Vadim Bendebury has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37255 )

Change subject: cr50 i2c: add error message reporting TPM IRQ timeout ......................................................................

cr50 i2c: add error message reporting TPM IRQ timeout

Various recent x86 SOCs have trouble registering short pulses generated by the H1 to indicate that it is ready for the next transaction.

This patch adds an error message to report this condition, which would greatly reduce the amount of guesswork when troubleshooting new platforms.

BUG=b:144002424

TEST=tried this code on the Drallion device exhibiting the problem, observed error messages in the Coreboot log;

$ grep IRQ ap.log Cr50 i2c TPM IRQ timeout! Cr50 i2c TPM IRQ timeout! Cr50 i2c TPM IRQ timeout! Cr50 i2c TPM IRQ timeout! ...

Change-Id: I5f6ee3986bed58e12fd0ec8cecbf35f46c9263c2 --- M src/drivers/i2c/tpm/cr50.c 1 file changed, 3 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/37255/1

diff --git a/src/drivers/i2c/tpm/cr50.c b/src/drivers/i2c/tpm/cr50.c index 6714bd4..f9a2862 100644 --- a/src/drivers/i2c/tpm/cr50.c +++ b/src/drivers/i2c/tpm/cr50.c @@ -78,9 +78,10 @@ stopwatch_init_msecs_expire(&sw, CR50_TIMEOUT_IRQ_MS);

while (!tis_plat_irq_status()) - if (stopwatch_expired(&sw)) + if (stopwatch_expired(&sw)) { + printk(BIOS_ERR, "Cr50 i2c TPM IRQ timeout!\n"); return -1; - + } return 0; }

Attachments:

attachment.htm (text/html — 3.1 KB)

Show replies by date

Mathew King (Code Review)

26 Nov 26 Nov

11:47 p.m.

Mathew King has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37255 )

Change subject: cr50 i2c: add error message reporting TPM IRQ timeout ......................................................................

Patch Set 1: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/37255 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5f6ee3986bed58e12fd0ec8cecbf35f46c9263c2 Gerrit-Change-Number: 37255 Gerrit-PatchSet: 1 Gerrit-Owner: Vadim Bendebury vbendeb@chromium.org Gerrit-Reviewer: Mathew King mathewk@chromium.org Gerrit-Reviewer: Philipp Deppenwiese zaolin.daisuki@gmail.com Gerrit-Reviewer: Vadim Bendebury vbendeb@chromium.org Gerrit-Comment-Date: Tue, 26 Nov 2019 22:47:15 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Vadim Bendebury (Code Review)

27 Nov 27 Nov

12:16 a.m.

Hello Mathew King, Philipp Deppenwiese, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/37255

to look at the new patch set (#2).

Change subject: cr50 i2c: add error message reporting TPM IRQ timeout ......................................................................

cr50 i2c: add error message reporting TPM IRQ timeout

Various recent x86 SOCs have trouble registering short pulses generated by the H1 to indicate that it is ready for the next transaction.

This patch adds an error message to report this condition, which would greatly reduce the amount of guesswork when troubleshooting new platforms.

BUG=b:144002424

TEST=tried this code on the Drallion device exhibiting the problem, observed error messages in the coreboot log;

$ grep IRQ ap.log Cr50 i2c TPM IRQ timeout! Cr50 i2c TPM IRQ timeout! Cr50 i2c TPM IRQ timeout! Cr50 i2c TPM IRQ timeout! ...

Change-Id: I5f6ee3986bed58e12fd0ec8cecbf35f46c9263c2 --- M src/drivers/i2c/tpm/cr50.c 1 file changed, 3 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/37255/2

-- To view, visit https://review.coreboot.org/c/coreboot/+/37255 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5f6ee3986bed58e12fd0ec8cecbf35f46c9263c2 Gerrit-Change-Number: 37255 Gerrit-PatchSet: 2 Gerrit-Owner: Vadim Bendebury vbendeb@chromium.org Gerrit-Reviewer: Mathew King mathewk@chromium.org Gerrit-Reviewer: Philipp Deppenwiese zaolin.daisuki@gmail.com Gerrit-Reviewer: Vadim Bendebury vbendeb@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: newpatchset

Vadim Bendebury (Code Review)

12:16 a.m.

Vadim Bendebury has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37255 )

Change subject: cr50 i2c: add error message reporting TPM IRQ timeout ......................................................................

Patch Set 2:

modified commit message to make Jenkins happy.

-- To view, visit https://review.coreboot.org/c/coreboot/+/37255 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5f6ee3986bed58e12fd0ec8cecbf35f46c9263c2 Gerrit-Change-Number: 37255 Gerrit-PatchSet: 2 Gerrit-Owner: Vadim Bendebury vbendeb@chromium.org Gerrit-Reviewer: Mathew King mathewk@chromium.org Gerrit-Reviewer: Philipp Deppenwiese zaolin.daisuki@gmail.com Gerrit-Reviewer: Vadim Bendebury vbendeb@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Tue, 26 Nov 2019 23:16:36 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Vadim Bendebury (Code Review)

5:42 p.m.

Hello Mathew King, Philipp Deppenwiese, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/37255

to look at the new patch set (#3).

Change subject: cr50 i2c: add error message reporting TPM IRQ timeout ......................................................................

cr50 i2c: add error message reporting TPM IRQ timeout

Various recent x86 SOCs have trouble registering short pulses generated by the H1 to indicate that it is ready for the next transaction.

This patch adds an error message to report this condition, which would greatly reduce the amount of guesswork when troubleshooting new platforms.

BUG=b:144002424

TEST=tried this code on the Drallion device exhibiting the problem, observed error messages in the coreboot log;

$ grep IRQ ap.log Cr50 i2c TPM IRQ timeout! Cr50 i2c TPM IRQ timeout! Cr50 i2c TPM IRQ timeout! Cr50 i2c TPM IRQ timeout! ...

Change-Id: I5f6ee3986bed58e12fd0ec8cecbf35f46c9263c2 Signed-off-by: Vadim Bendebury vbendeb@chromium.org --- M src/drivers/i2c/tpm/cr50.c 1 file changed, 3 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/37255/3

-- To view, visit https://review.coreboot.org/c/coreboot/+/37255 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5f6ee3986bed58e12fd0ec8cecbf35f46c9263c2 Gerrit-Change-Number: 37255 Gerrit-PatchSet: 3 Gerrit-Owner: Vadim Bendebury vbendeb@chromium.org Gerrit-Reviewer: Mathew King mathewk@chromium.org Gerrit-Reviewer: Philipp Deppenwiese zaolin.daisuki@gmail.com Gerrit-Reviewer: Vadim Bendebury vbendeb@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: newpatchset

Vadim Bendebury (Code Review)

5:43 p.m.

Vadim Bendebury has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37255 )

Change subject: cr50 i2c: add error message reporting TPM IRQ timeout ......................................................................

Patch Set 3:

Shouldn't Jenkins be showing all problems with the patch in one pass.

-- To view, visit https://review.coreboot.org/c/coreboot/+/37255 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5f6ee3986bed58e12fd0ec8cecbf35f46c9263c2 Gerrit-Change-Number: 37255 Gerrit-PatchSet: 3 Gerrit-Owner: Vadim Bendebury vbendeb@chromium.org Gerrit-Reviewer: Mathew King mathewk@chromium.org Gerrit-Reviewer: Philipp Deppenwiese zaolin.daisuki@gmail.com Gerrit-Reviewer: Vadim Bendebury vbendeb@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Wed, 27 Nov 2019 16:43:28 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Vadim Bendebury (Code Review)

6:29 p.m.

Vadim Bendebury has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37255 )

Change subject: cr50 i2c: add error message reporting TPM IRQ timeout ......................................................................

cr50 i2c: add error message reporting TPM IRQ timeout

Various recent x86 SOCs have trouble registering short pulses generated by the H1 to indicate that it is ready for the next transaction.

This patch adds an error message to report this condition, which would greatly reduce the amount of guesswork when troubleshooting new platforms.

BUG=b:144002424

TEST=tried this code on the Drallion device exhibiting the problem, observed error messages in the coreboot log;

$ grep IRQ ap.log Cr50 i2c TPM IRQ timeout! Cr50 i2c TPM IRQ timeout! Cr50 i2c TPM IRQ timeout! Cr50 i2c TPM IRQ timeout! ...

Change-Id: I5f6ee3986bed58e12fd0ec8cecbf35f46c9263c2 Signed-off-by: Vadim Bendebury vbendeb@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/37255 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Mathew King mathewk@chromium.org --- M src/drivers/i2c/tpm/cr50.c 1 file changed, 3 insertions(+), 2 deletions(-)

Approvals: build bot (Jenkins): Verified Mathew King: Looks good to me, approved

while (!tis_plat_irq_status()) - if (stopwatch_expired(&sw)) + if (stopwatch_expired(&sw)) { + printk(BIOS_ERR, "Cr50 i2c TPM IRQ timeout!\n"); return -1; - + } return 0; }

-- To view, visit https://review.coreboot.org/c/coreboot/+/37255 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5f6ee3986bed58e12fd0ec8cecbf35f46c9263c2 Gerrit-Change-Number: 37255 Gerrit-PatchSet: 4 Gerrit-Owner: Vadim Bendebury vbendeb@chromium.org Gerrit-Reviewer: Mathew King mathewk@chromium.org Gerrit-Reviewer: Philipp Deppenwiese zaolin.daisuki@gmail.com Gerrit-Reviewer: Vadim Bendebury vbendeb@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: merged

Patrick Georgi (Code Review)

7:25 p.m.

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37255 )

Change subject: cr50 i2c: add error message reporting TPM IRQ timeout ......................................................................

Patch Set 4:

...

Patch Set 3:

Shouldn't Jenkins be showing all problems with the patch in one pass.

Good point. Will look into that (and you gave me a good test case to work against :-) )

-- To view, visit https://review.coreboot.org/c/coreboot/+/37255 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5f6ee3986bed58e12fd0ec8cecbf35f46c9263c2 Gerrit-Change-Number: 37255 Gerrit-PatchSet: 4 Gerrit-Owner: Vadim Bendebury vbendeb@chromium.org Gerrit-Reviewer: Mathew King mathewk@chromium.org Gerrit-Reviewer: Philipp Deppenwiese zaolin.daisuki@gmail.com Gerrit-Reviewer: Vadim Bendebury vbendeb@chromium.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Patrick Georgi pgeorgi@google.com Gerrit-Comment-Date: Wed, 27 Nov 2019 18:25:51 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1816

days inactive

1817

days old

coreboot-gerrit@coreboot.org

7 comments

3 participants

tags (0)

participants (3)

Mathew King (Code Review)
Patrick Georgi (Code Review)
Vadim Bendebury (Code Review)