Attention is currently required from: Andrey Petrov, Intel coreboot Reviewers, Julius Werner, Jérémy Compostella, Paul Menzel, Ronak Kanabar.
Subrata Banik has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/86001?usp=email )
Change subject: drivers/intel/fsp2_0: Add option to control debug log level using CBFS
......................................................................
Patch Set 7:
(1 comment)
File src/drivers/intel/fsp2_0/debug.c:
https://review.coreboot.org/c/coreboot/+/86001/comment/3569011f_f4ed063c?us… :
PS6, Line 185: if (!CONFIG(USE_CBFS_FILE_OPTION_BACKEND))
> Why? Why shouldn't this just work for all option backends equally?
>
> To be clear, I think this function should just be:
> ```
> enum fsp_log_level fsp_get_pcd_debug_log_level(void)
> {
> return get_uint_option("fsp_pcd_debug_level", fsp_map_console_log_level());
> }
> ```
>
> [Sorry, looks like I forgot to publish this comment yesterday.]
Looks like you missed to read my comments previously (?) as I have mentioned why we can't call fsp_map_console_log_level as fallback because we are now stitching debug fsp by default with ap fw that means when we are not injecting options to control debug level (like default ap fw won't have any option included), we shouldn't see any debug log from fsp. Calling fsp_map_console_log_level api as fallback would mean that we are getting chatty console by default which is not intention of this CL.
One should see more chatty console when they inject options into cbfs. If they don't select cbfs backend kconfig then this CL doesn't change the behavior of console level.
--
To view, visit https://review.coreboot.org/c/coreboot/+/86001?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I2c14d26021dd0048fa24024119df857e216f18bd
Gerrit-Change-Number: 86001
Gerrit-PatchSet: 7
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Comment-Date: Sat, 18 Jan 2025 04:19:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Attention is currently required from: Bill XIE.
Keith Hui has posted comments on this change by Keith Hui. ( https://review.coreboot.org/c/coreboot/+/85413?usp=email )
Change subject: mb/asus/p8z77-v: Attempt to correctly route PCIe lanes
......................................................................
Patch Set 2:
(1 comment)
File src/mainboard/asus/p8x7x-series/variants/p8z77-v/early_init.c:
https://review.coreboot.org/c/coreboot/+/85413/comment/6a5e7d4a_f0a10cd9?us… :
PS1, Line 67: gpio5 |= 0x20;
> I'm investigating. […]
I think I have all GPIO signals figured out, and wrote what I found into early_init.c. Have at it and let me know how it goes.
ps. A physical breadboard is still king for small simulations.
--
To view, visit https://review.coreboot.org/c/coreboot/+/85413?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: If41197a1f817a48c209d25fc1ae461ec97ccf16c
Gerrit-Change-Number: 85413
Gerrit-PatchSet: 2
Gerrit-Owner: Keith Hui <buurin(a)gmail.com>
Gerrit-Reviewer: Bill XIE <persmule(a)hardenedlinux.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Bill XIE <persmule(a)hardenedlinux.org>
Gerrit-Comment-Date: Sat, 18 Jan 2025 02:53:04 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Bill XIE <persmule(a)hardenedlinux.org>
Comment-In-Reply-To: Keith Hui <buurin(a)gmail.com>
Attention is currently required from: Bill XIE.
Hello Bill XIE, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/85413?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: mb/asus/p8z77-v: Attempt to correctly route PCIe lanes
......................................................................
mb/asus/p8z77-v: Attempt to correctly route PCIe lanes
Bill Xie documented in his initial code drop that he was unsuccessful
in reproducing all the PCIe configurations possible with vendor
firmware. I obtained a boardview to this board and have identified the
PCIe lane topology and the required control signals.
There are PCIe slot presence signals wired to GPIOs 34,20,7 for
PCIEX1_1,PCIEX1_2,PCIEX16_3 respectively, the last one only sense the
presence of a PCIe x4 or larger card. PCIe lanes 1-4 are routed by way
of three ASM1440 2-way switches controlled by GP54-GP56 on NCT6779D
super I/O chip. PCIe lanes 5-8 are fixed.
With these details, it is now possible to attempt to reproduce all the
vendor PCIe configurations.
1. Change GPIO20 of PCH to GPIO input so coreboot can detect a
card inserted into PCIEX1_2.
2. Add an nvram option to force PCIe lane 4 to serve ASM1061 and its
two SATA 6Gbps ports. Another one needs to be added later to enable
users to allocate all lanes to PCIEX16_3 and make it x4.
3. Add code into bootblock to check the PCHSTRP9 soft strap
and whether (1) is true. There is a sanity check to warn of a
PCIe configuration that is not valid on this board.
4. Based on (1) and (2), program SIO GPIO5 as appropriate.
Changing PCIEX16_3 from x1 to x4 requires changing PCHSTRP9 in
the SPI flash descriptor. How coreboot can manage this is TBD.
This is based on boardview only, and is untested because I have
no hardware.
Change-Id: If41197a1f817a48c209d25fc1ae461ec97ccf16c
Signed-off-by: Keith Hui <buurin(a)gmail.com>
---
M src/mainboard/asus/p8x7x-series/variants/p8z77-v/cmos.default
M src/mainboard/asus/p8x7x-series/variants/p8z77-v/cmos.layout
M src/mainboard/asus/p8x7x-series/variants/p8z77-v/early_init.c
M src/mainboard/asus/p8x7x-series/variants/p8z77-v/gpio.c
M src/mainboard/asus/p8x7x-series/variants/p8z77-v/overridetree.cb
5 files changed, 114 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/85413/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/85413?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: If41197a1f817a48c209d25fc1ae461ec97ccf16c
Gerrit-Change-Number: 85413
Gerrit-PatchSet: 2
Gerrit-Owner: Keith Hui <buurin(a)gmail.com>
Gerrit-Reviewer: Bill XIE <persmule(a)hardenedlinux.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Bill XIE <persmule(a)hardenedlinux.org>
Attention is currently required from: Andrey Petrov, Intel coreboot Reviewers, Jérémy Compostella, Paul Menzel, Ronak Kanabar, Subrata Banik.
Julius Werner has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/86001?usp=email )
Change subject: drivers/intel/fsp2_0: Add option to control debug log level using CBFS
......................................................................
Patch Set 7:
(1 comment)
File src/drivers/intel/fsp2_0/debug.c:
https://review.coreboot.org/c/coreboot/+/86001/comment/e7da1cbf_cf4fc8f7?us… :
PS6, Line 185: if (!CONFIG(USE_CBFS_FILE_OPTION_BACKEND))
Why? Why shouldn't this just work for all option backends equally?
To be clear, I think this function should just be:
```
enum fsp_log_level fsp_get_pcd_debug_log_level(void)
{
return get_uint_option("fsp_pcd_debug_level", fsp_map_console_log_level());
}
```
[Sorry, looks like I forgot to publish this comment yesterday.]
--
To view, visit https://review.coreboot.org/c/coreboot/+/86001?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I2c14d26021dd0048fa24024119df857e216f18bd
Gerrit-Change-Number: 86001
Gerrit-PatchSet: 7
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Comment-Date: Sat, 18 Jan 2025 02:27:37 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Jérémy Compostella, Nicholas Chin, Subrata Banik.
Julius Werner has posted comments on this change by Nicholas Chin. ( https://review.coreboot.org/c/coreboot/+/85905?usp=email )
Change subject: drivers/option: Add CBFS file based option backend
......................................................................
Patch Set 14:
(1 comment)
Patchset:
PS4:
> > > Acknowledged […]
Hmm... yeah, okay, this points out an important issue of this backend in combination with CONFIG_VBOOT_RETURN_FROM_VERSTAGE. The whole point of RETURN_FROM_VERSTAGE is that we can save SRAM and flash space by avoiding to link the CBFS and SPI driver code into verstage. Using this backend breaks all that.
I would say this patch should add `depends on !VBOOT_RETURN_FROM_VERSTAGE` to the USE_CBFS_FILE_OPTION_BACKEND option to prevent people from accidentally making that mistake. That means that it won't be usable on most Arm platforms, but I guess that's fine for now until someone starts finding a real need for it.
For the Intel boards, maybe just make it `default USE_CBFS_FILE_OPTION_BACKEND if CHROMEOS && PLATFORM_USES_FSP2_0` for now? Or select it in ChromeOS' downstream ebuilds for the appropriate boards instead.
--
To view, visit https://review.coreboot.org/c/coreboot/+/85905?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ifc0439ee42f13f49ae54d4855d1d9333c39b01f5
Gerrit-Change-Number: 85905
Gerrit-PatchSet: 14
Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-CC: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Comment-Date: Sat, 18 Jan 2025 02:26:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Comment-In-Reply-To: Nicholas Chin <nic.c3.14(a)gmail.com>
Attention is currently required from: Hung-Te Lin, Jarried Lin, Paul Menzel, Yu-Ping Wu.
Yidi Lin has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/85950?usp=email )
Change subject: soc/mediatek/mt8196: Add DDP driver
......................................................................
Patch Set 11: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/85950?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I006911e83d940c1eec7135a6a0c36fbfa2aad466
Gerrit-Change-Number: 85950
Gerrit-PatchSet: 11
Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nancy Lin <nancy.lin(a)mediatek.corp-partner.google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Comment-Date: Sat, 18 Jan 2025 00:21:30 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Hung-Te Lin, Jarried Lin, Paul Menzel, Yu-Ping Wu, agogo.
Yidi Lin has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/85888?usp=email )
Change subject: soc/mediatek/mt8196: Initialize MCUPM
......................................................................
Patch Set 10: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/85888?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I223f245d384f32d54f6170a28b29573638f77296
Gerrit-Change-Number: 85888
Gerrit-PatchSet: 10
Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: agogo <agogo.huang(a)mediatek.corp-partner.google.com>
Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Attention: agogo <agogo.huang(a)mediatek.corp-partner.google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Comment-Date: Sat, 18 Jan 2025 00:20:45 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Hung-Te Lin, Jarried Lin, Paul Menzel.
Yidi Lin has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/85979?usp=email )
Change subject: mb/google/rauru: Enable RTC
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/85979?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I95822fc7646d41dbbc61258741a2a42988fc31d7
Gerrit-Change-Number: 85979
Gerrit-PatchSet: 3
Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com>
Gerrit-Comment-Date: Sat, 18 Jan 2025 00:20:11 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Hung-Te Lin, Jarried Lin.
Yidi Lin has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/86014?usp=email )
Change subject: soc/mediatek/mt8196: Add mtk-fsp loader in romstage
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/86014?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I73710227e6d9e3f0c717e17db0cc798265eb1f72
Gerrit-Change-Number: 86014
Gerrit-PatchSet: 3
Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com>
Gerrit-Comment-Date: Sat, 18 Jan 2025 00:19:31 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes