Attention is currently required from: Patrick Rudolph, Christian Walter, Arthur Heymans, Kyösti Mälkki, Werner Zeh.
Angel Pons 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 1: -Code-Review
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/58240/comment/56cd2999_15f3bcd0
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.
Regarding the polling frequency issue Kyösti pointed out, do things still work properly without debug messages?
File src/drivers/pc80/tpm/tis.c:
https://review.coreboot.org/c/coreboot/+/58240/comment/7c346dd5_e34eb620
PS1, Line 257: udelay(1); /* 1 us */
> With removal of udelay() this will now poll the status as fast as it can, intentional change?
Good point. I'd simply keep the `udelay(1);` inside the loop, just to be safe.
--
To view, visit https://review.coreboot.org/c/coreboot/+/58240
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I8fd261c9d60a9a60509c847dbc4983bc05f41d48
Gerrit-Change-Number: 58240
Gerrit-PatchSet: 1
Gerrit-Owner: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Comment-Date: Tue, 12 Oct 2021 10:04:47 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Patrick Rudolph, Christian Walter, Arthur Heymans, Werner Zeh.
Kyösti Mälkki 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 1:
(3 comments)
File src/drivers/pc80/tpm/tis.c:
https://review.coreboot.org/c/coreboot/+/58240/comment/742a2c38_8f9779c5
PS1, Line 257: udelay(1); /* 1 us */
With removal of udelay() this will now poll the status as fast as it can, intentional change?
https://review.coreboot.org/c/coreboot/+/58240/comment/7369252a_3699bd9c
PS1, Line 88: #define MAX_DELAY_US (1000 * 1000)
Possibly should be replaced with USECS_PER_SEC from timer.h.
https://review.coreboot.org/c/coreboot/+/58240/comment/1a4c2c75_67a47250
PS1, Line 242: * Wait for at least a second for a status to change its state to match the
At least a second? Probably should say at most a second.
--
To view, visit https://review.coreboot.org/c/coreboot/+/58240
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I8fd261c9d60a9a60509c847dbc4983bc05f41d48
Gerrit-Change-Number: 58240
Gerrit-PatchSet: 1
Gerrit-Owner: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Comment-Date: Tue, 12 Oct 2021 10:01:17 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Matt DeVillier, Tim Wawrzynczak.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58231 )
Change subject: mb/google/fizz: use SaGv_FixedHigh
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/58231
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I0295bac619af45a0d82da2bf39985c8bdcb77d5e
Gerrit-Change-Number: 58231
Gerrit-PatchSet: 1
Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Comment-Date: Tue, 12 Oct 2021 10:00:35 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Patrick Rudolph, Christian Walter, Arthur Heymans, Werner Zeh.
Angel Pons 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 1: Code-Review+1
(2 comments)
File src/drivers/pc80/tpm/tis.c:
https://review.coreboot.org/c/coreboot/+/58240/comment/47fad25b_3b86b72e
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. It probably makes no difference in practice, but I think it's slightly more correct.
https://review.coreboot.org/c/coreboot/+/58240/comment/91d71342_9802ac28
PS1, Line 462: %d bytes of %d
For another patch: These two `%d` should be `%u`
--
To view, visit https://review.coreboot.org/c/coreboot/+/58240
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I8fd261c9d60a9a60509c847dbc4983bc05f41d48
Gerrit-Change-Number: 58240
Gerrit-PatchSet: 1
Gerrit-Owner: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Comment-Date: Tue, 12 Oct 2021 09:58:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58242 )
Change subject: libpayload: Add unit-tests framework and first test case
......................................................................
Patch Set 1:
(1 comment)
File payloads/libpayload/tests/include/tests/test.h:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-130275):
https://review.coreboot.org/c/coreboot/+/58242/comment/62363f77_08b83550
PS1, Line 36: #define TEST_REGION_UNALLOCATED(region, start, size) TEST_SYMBOL(_##region, start); \
Macros with multiple statements should be enclosed in a do - while loop
--
To view, visit https://review.coreboot.org/c/coreboot/+/58242
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iaa94ee4dcdc3f74af830113813df0e8fb0b31e4f
Gerrit-Change-Number: 58242
Gerrit-PatchSet: 1
Gerrit-Owner: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Tue, 12 Oct 2021 09:32:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Bao Zheng, Martin Roth, Zheng Bao.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57131 )
Change subject: amdfwtool: Add support for AMD's BIOS recovery feature
......................................................................
Patch Set 16:
(1 comment)
File util/amdfwtool/amdfwtool.c:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-130271):
https://review.coreboot.org/c/coreboot/+/57131/comment/80df68d6_5331fb4c
PS16, Line 1801: btl_entry = search_entry(pspdir2, AMD_FW_PSP_BOOTLOADER);
line over 96 characters
--
To view, visit https://review.coreboot.org/c/coreboot/+/57131
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I2671b95fe089aafdaf61b55bc9d2e8dcf6a66dbc
Gerrit-Change-Number: 57131
Gerrit-PatchSet: 16
Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com>
Gerrit-Reviewer: Zheng Bao
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Martin Roth <martinroth(a)google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Bao Zheng <fishbaozi(a)gmail.com>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Zheng Bao
Gerrit-Comment-Date: Tue, 12 Oct 2021 09:24:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Hsuan-ting Chen, Paul Menzel, Nick Vaccaro, Julius Werner, Andrey Petrov, Patrick Rudolph.
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57951 )
Change subject: Revert "vboot_logic: Set VB2_CONTEXT_EC_TRUSTED in verstage_main"
......................................................................
Patch Set 3: Code-Review+2
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/57951/comment/eda257f7_a14026e3
PS3, Line 11: Reason for revert: This CL did not handle Intel GPIO correctly. We need to add GPIO_EC_IN_RW into early_gpio_table for platforms using Intel SoC.
Line too long
--
To view, visit https://review.coreboot.org/c/coreboot/+/57951
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iaeb1bf598047160f01e33ad0d9d004cad59e3f75
Gerrit-Change-Number: 57951
Gerrit-PatchSet: 3
Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com>
Gerrit-CC: Daisuke Nojiri <dnojiri(a)chromium.org>
Gerrit-CC: Furquan Shaikh <furquan(a)google.com>
Gerrit-CC: Hsuan Ting Chen <roccochen(a)chromium.org>
Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Tue, 12 Oct 2021 09:23:12 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Matt DeVillier, Tim Wawrzynczak, Edward O'Callaghan.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58230 )
Change subject: mb/google/wyvern: use SaGv_FixedHigh
......................................................................
Patch Set 2: Code-Review+1
(1 comment)
Patchset:
PS2:
LGTM, but I'd like to know what Googlers think.
--
To view, visit https://review.coreboot.org/c/coreboot/+/58230
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I8ad773d1c616b746235ec67b98b83c5910464140
Gerrit-Change-Number: 58230
Gerrit-PatchSet: 2
Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Comment-Date: Tue, 12 Oct 2021 09:21:28 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Bao Zheng has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/57064 )
Change subject: amdfwtool: Remove some duplicate binaries/entries in PSP table
......................................................................
Abandoned
--
To view, visit https://review.coreboot.org/c/coreboot/+/57064
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ia81b8a85792d1565445803100bdb01c3436e5698
Gerrit-Change-Number: 57064
Gerrit-PatchSet: 8
Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com>
Gerrit-Reviewer: Zheng Bao
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Eric Peers <epeers(a)google.com>
Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-CC: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-CC: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-CC: Martin Roth <martinroth(a)google.com>
Gerrit-CC: Matt Papageorge <matthewpapa07(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Raul Rangel <rrangel(a)chromium.org>
Gerrit-MessageType: abandon