Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson, Karthik Ramasubramanian.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58114 )
Change subject: soc/amd/common/block/espi_util: Refactor ESPI Setup
......................................................................
Patch Set 4:
(4 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/58114/comment/012f0fb0_e73d1dcc
PS4, Line 7: ESPI
eSPI. some more instances below.
File src/soc/amd/cezanne/early_fch.c:
https://review.coreboot.org/c/coreboot/+/58114/comment/f417320c_1346f6d2
PS4, Line 43: configure_espi();
see my comments on picasso's early_fch
File src/soc/amd/picasso/early_fch.c:
https://review.coreboot.org/c/coreboot/+/58114/comment/be4e7118_f539c39a
PS4, Line 48: and SPI Fast speed overrides based on
: board version.
this isn't related to eSPI and also not what the code is doing
https://review.coreboot.org/c/coreboot/+/58114/comment/346e99f7_2fb6e8b1
PS4, Line 50: configure_espi();
lpc_early_init needs to be called before configure_espi, since lpc_early_init calls lpc_set_spibase to set up the SPI base that needs to be set up before calling for example espi_write32 in espi_setup that gets called be configure_espi. might be worth adding a comment on that here
--
To view, visit https://review.coreboot.org/c/coreboot/+/58114
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Icfeba17dae0a964c9ca73686e29c18d965589934
Gerrit-Change-Number: 58114
Gerrit-PatchSet: 4
Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Comment-Date: Tue, 12 Oct 2021 12:11:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Jakub Czapiga has uploaded a new patch set (#3). ( https://review.coreboot.org/c/coreboot/+/58242 )
Change subject: libpayload: Add unit-tests framework and first test case
......................................................................
libpayload: Add unit-tests framework and first test case
This commit adds a unit-tests framewok ported from coreboot, and test
for drivers/speaker. Usage of the unit-tests framework is same as for
the coreboot one.
Change-Id: Iaa94ee4dcdc3f74af830113813df0e8fb0b31e4f
Signed-off-by: Jakub Czapiga <jacz(a)semihalf.com>
---
M payloads/libpayload/Makefile
M payloads/libpayload/Makefile.inc
A payloads/libpayload/configs/config.unit-tests
A payloads/libpayload/tests/Makefile.inc
A payloads/libpayload/tests/drivers/Makefile.inc
A payloads/libpayload/tests/drivers/speaker-test.c
A payloads/libpayload/tests/include/mocks/x86_io.h
A payloads/libpayload/tests/include/tests/test.h
M util/testing/Makefile.inc
9 files changed, 544 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/58242/3
--
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: 3
Gerrit-Owner: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Kevin Chiu, Furquan Shaikh, Martin Roth, Eric Peers.
Rob Barnes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58161 )
Change subject: mb/google/guybrush/var/nipperkin: update fw_config field
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/58161
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Icd2c5509450e70aed158f146179f3a7fa24b547a
Gerrit-Change-Number: 58161
Gerrit-PatchSet: 1
Gerrit-Owner: Kevin Chiu <kevin.chiu.17802(a)gmail.com>
Gerrit-Reviewer: Bhanu Prakash Maiya <bhanumaiya(a)google.com>
Gerrit-Reviewer: Eric Peers <epeers(a)google.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Rob Barnes <robbarnes(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Kevin Chiu <kevin.chiu(a)quanta.corp-partner.google.com>
Gerrit-Attention: Kevin Chiu <kevin.chiu.17802(a)gmail.com>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Eric Peers <epeers(a)google.com>
Gerrit-Comment-Date: Tue, 12 Oct 2021 12:10:04 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Patrick Rudolph, Paul Menzel, Christian Walter, Arthur Heymans, Werner Zeh, Kyösti Mälkki.
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:
(1 comment)
File src/drivers/pc80/tpm/tis.c:
https://review.coreboot.org/c/coreboot/+/58240/comment/bc83b317_69687d02
PS1, Line 257: udelay(1); /* 1 us */
> I guess this is not harmfull since there is no restriction in the TPM how often the register can be […]
Polling too fast is an issue when reading the value interrupts the task one is waiting to be done. For example, when waiting for a laptop EC to finish doing something, if reading the corresponding status register triggers an interrupt handler in the EC, polling too fast can slow down the EC's normal operation and unnecessarily lengthen the wait. Something similar could happen with the ME or any other chip with its own processor. I don't know for sure, but I think this might also be the case for TPMs.
--
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: Paul Menzel <paulepanter(a)mailbox.org>
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-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Comment-Date: Tue, 12 Oct 2021 11:41:43 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Werner Zeh <werner.zeh(a)siemens.com>
Comment-In-Reply-To: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Matt DeVillier, Sumeet R Pawnikar, Arthur Heymans, Michael Niewöhner, Patrick Rudolph, Wim Vervoorn.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57988 )
Change subject: soc/intel/braswell: Set GNVS DPTE via devicetree
......................................................................
Patch Set 1:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/57988/comment/f8a39f64_423650ed
PS1, Line 10: field, as newer Intel platforms do.
> Check "status dptf" on terminal. It should show process with status running.
I'm afraid I don't have any device to test this with.
--
To view, visit https://review.coreboot.org/c/coreboot/+/57988
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I88b746c64ca57604f946eefb00a70487a2fb27c0
Gerrit-Change-Number: 57988
Gerrit-PatchSet: 1
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Wim Vervoorn <wvervoorn(a)eltan.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: Wim Vervoorn <wvervoorn(a)eltan.com>
Gerrit-Comment-Date: Tue, 12 Oct 2021 11:30:13 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
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 1:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/58240/comment/830afdbc_b36103ac
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 debu […]
Yes, I have tested both cases, with and without debug TPM messages. Both work fine.
--
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: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Comment-Date: Tue, 12 Oct 2021 11:12:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
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 1:
(5 comments)
File src/drivers/pc80/tpm/tis.c:
https://review.coreboot.org/c/coreboot/+/58240/comment/e3c74e04_9b8dbb23
PS1, Line 257: udelay(1); /* 1 us */
> Good point. I'd simply keep the `udelay(1);` inside the loop, just to be safe.
I guess this is not harmfull since there is no restriction in the TPM how often the register can be polled. On the other hand we do not need a super fast polling here so I will add back the 'udelay(1)' here.
https://review.coreboot.org/c/coreboot/+/58240/comment/35c1806d_dd7d098d
PS1, Line 88: #define MAX_DELAY_US (1000 * 1000)
> Possibly should be replaced with USECS_PER_SEC from timer.h.
Yes, sounds reasonable. Should I do it in this patch or in another one?
https://review.coreboot.org/c/coreboot/+/58240/comment/1699fdf8_39496fbc
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.
In the old code it was indeed at least a second. Will change to 'at most 1 second'.
https://review.coreboot.org/c/coreboot/+/58240/comment/effaf014_4ace9fb2
PS1, Line 457: stopwatch_init_usecs_expire(&sw, MAX_DELAY_US);
> Nit: Move it directly below the comment? (No objection if it stays here either. […]
Sure, will do.
https://review.coreboot.org/c/coreboot/+/58240/comment/9df7eb8d_a8665a68
PS1, Line 462: %d bytes of %d
> For another patch: These two `%d` should be `%u`
Will provide a follow-up patch.
--
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: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Comment-Date: Tue, 12 Oct 2021 11:04:00 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-MessageType: comment