Attention is currently required from: Hung-Te Lin, Shelley Chen, Paul Menzel, Yu-Ping Wu, Jianjun Wang.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62359 )
Change subject: soc/mediatek: PCI: Assert PERST# at bootblock stage
......................................................................
Patch Set 9: Code-Review+1
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/62359/comment/4ea1b33a_dc3d1a5a
PS9, Line 14: 100ms
Is there a way to ensure this minimum delay of 100ms is still met? coreboot could be too fast in some cases (try reducing/disabling logging to UART).
--
To view, visit https://review.coreboot.org/c/coreboot/+/62359
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Id5b9369e6f8599f93415588ea585c952a41c5e7d
Gerrit-Change-Number: 62359
Gerrit-PatchSet: 9
Gerrit-Owner: Jianjun Wang <jianjun.wang(a)mediatek.corp-partner.google.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Shelley Chen <shchen(a)google.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Rex-BC Chen <rex-bc.chen(a)mediatek.com>
Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Attention: Shelley Chen <shchen(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Attention: Jianjun Wang <jianjun.wang(a)mediatek.corp-partner.google.com>
Gerrit-Comment-Date: Mon, 07 Mar 2022 15:16:04 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Hung-Te Lin, Shelley Chen, Paul Menzel, Yu-Ping Wu, Jianjun Wang.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62360 )
Change subject: mb/google/cherry: Add PCIe domain support
......................................................................
Patch Set 9:
(1 comment)
File src/mainboard/google/cherry/variants/dojo/overridetree.cb:
PS9:
Hmmm, this could cause issues. Does static.c (build/mainboard/**/static.c look sane? The main thing to check is that there's only one chip instance.
--
To view, visit https://review.coreboot.org/c/coreboot/+/62360
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ifb02960504177fe488e6784b954c16b2c8d94972
Gerrit-Change-Number: 62360
Gerrit-PatchSet: 9
Gerrit-Owner: Jianjun Wang <jianjun.wang(a)mediatek.corp-partner.google.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Shelley Chen <shchen(a)google.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Rex-BC Chen <rex-bc.chen(a)mediatek.com>
Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Attention: Shelley Chen <shchen(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Attention: Jianjun Wang <jianjun.wang(a)mediatek.corp-partner.google.com>
Gerrit-Comment-Date: Mon, 07 Mar 2022 15:12:16 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Tim Wawrzynczak, Paul Menzel, Alex Levin, Julius Werner, Jan Dabros.
Jakub Czapiga has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62474 )
Change subject: util/cbmem: Add FlameGraph-compatible timestamps output
......................................................................
Patch Set 4:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/62474/comment/b8dd86ac_ad6faf67
PS4, Line 15: -t/-T/-S options
> Can you give an example flamegraph, or list how to create them?
I will provide more info in the commit message with the new version of this patch (splitting timestamp enums/string and cbmem changes)
File src/commonlib/include/commonlib/timestamp_serialized.h:
PS4:
> nit: I think this change could be split out by itself, also, if we're going to go with this mechanis […]
I think it is a good idea. First, it will help to avoid typos in strings. Second, it will provide direct enum->string mapping.
I will prepare an appropriate patch.
--
To view, visit https://review.coreboot.org/c/coreboot/+/62474
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I3a4e20a267e9e0fbc6b3a4d6a2409b32ce8fca33
Gerrit-Change-Number: 62474
Gerrit-PatchSet: 4
Gerrit-Owner: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Reviewer: Alex Levin <levinale(a)google.com>
Gerrit-Reviewer: Jan Dabros <jsd(a)semihalf.com>
Gerrit-Reviewer: Julius Werner <jwerner(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: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Alex Levin <levinale(a)google.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Jan Dabros <jsd(a)semihalf.com>
Gerrit-Comment-Date: Mon, 07 Mar 2022 15:00:57 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Jeff Daly, Martin Roth, Mariusz Szafrański, Paul Menzel, Stefan Reinauer, Suresh Bellampalli, Vanessa Eusebio, Michael Niewöhner, Michal Motyl, Kyösti Mälkki, Patrick Rudolph.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60830 )
Change subject: util/ifdtool: Add support for Denverton SoC
......................................................................
Patch Set 15: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/60830
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I15939ce4672123f39a807d63c13ba7df98c57523
Gerrit-Change-Number: 60830
Gerrit-PatchSet: 15
Gerrit-Owner: Jeff Daly <jeffd(a)silicom-usa.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Mariusz Szafrański <mariuszx.szafranski(a)intel.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Michal Motyl <michalx.motyl(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: Suresh Bellampalli <suresh.bellampalli(a)intel.com>
Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Jeff Daly <jeffd(a)silicom-usa.com>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Mariusz Szafrański <mariuszx.szafranski(a)intel.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Suresh Bellampalli <suresh.bellampalli(a)intel.com>
Gerrit-Attention: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Attention: Michal Motyl <michalx.motyl(a)intel.com>
Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Mon, 07 Mar 2022 14:58:30 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Jeff Daly, Martin Roth, Paul Menzel, Patrick Rudolph.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60877 )
Change subject: sb/intel/common/firmware: Hook up adding 10GbE LAN firmware
......................................................................
Patch Set 16: Code-Review+1
(2 comments)
File src/southbridge/intel/common/firmware/Kconfig:
https://review.coreboot.org/c/coreboot/+/60877/comment/780e7dc2_627f529d
PS13, Line 152: HAVE_
> Done
We use the `HAVE_FOO` and `USE_FOO` scheme in other parts of the tree (for example, `MAINBOARD_HAS_LIBGFXINIT` and `MAINBOARD_USE_LIBGFXINIT`). However, looks like some options to add binary files (IFD parts and MRC.bin, to name a few) into the coreboot image don't follow this.
For the IFD parts, the `MAINBOARD_USES_IFD_*_REGION` Kconfig symbol was added much later (originally, the options to add IFD regions would all be visible), which explains why the names aren't consistent.
TL;DR: The naming scheme for the IFD Kconfig options is rather inconsistent, but the names of the options added in this change are consistent with those of the existing options.
File src/southbridge/intel/common/firmware/Kconfig:
https://review.coreboot.org/c/coreboot/+/60877/comment/ba48da6d_5cbe147e
PS16, Line 168: depends on MAINBOARD_USES_IFD_10GBE_0_REGION
I'd drop this dependency. I don't think there's any mainboard that only uses 10GbE 1 (it would be extremely odd: why not use 10GbE 0?), but I don't see the need to restrict it.
--
To view, visit https://review.coreboot.org/c/coreboot/+/60877
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Id45ab4b69a85a5f8e52c0c4b130b6d729222b4c3
Gerrit-Change-Number: 60877
Gerrit-PatchSet: 16
Gerrit-Owner: Jeff Daly <jeffd(a)silicom-usa.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <felixsinger(a)posteo.net>
Gerrit-CC: Martin Roth <martinroth(a)google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Jeff Daly <jeffd(a)silicom-usa.com>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Mon, 07 Mar 2022 14:57:17 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Jeff Daly <jeffd(a)silicom-usa.com>
Comment-In-Reply-To: Martin Roth <martinroth(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Maulik V Vaghela, Tim Wawrzynczak, Patrick Rudolph.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62645 )
Change subject: soc/intel/adl/chip.h: Convert all camel case variables to snake case
......................................................................
Patch Set 3: Code-Review+1
(5 comments)
File src/mainboard/google/brya/variants/baseboard/brask/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/62645/comment/a87834b5_abd9f670
PS3, Line 76:
can you please align this tab too ?
File src/mainboard/google/brya/variants/brya4es/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/62645/comment/4300150d_41af5d0d
PS3, Line 38: SaGv
Did you miss this ?
File src/mainboard/google/brya/variants/gimble/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/62645/comment/ae82dfbf_b25c3b2b
PS3, Line 24: SaGv
same
File src/mainboard/google/brya/variants/taniks/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/62645/comment/0ec0965a_284595c0
PS3, Line 54: SaGv
same
File src/soc/intel/alderlake/chip.h:
https://review.coreboot.org/c/coreboot/+/62645/comment/95a5d752_a31306d5
PS3, Line 230: SaGv
please fix this as well
--
To view, visit https://review.coreboot.org/c/coreboot/+/62645
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ieda567a89ec9287e3d988d489f3b3769dffcf9e0
Gerrit-Change-Number: 62645
Gerrit-PatchSet: 3
Gerrit-Owner: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Mon, 07 Mar 2022 14:52:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment