coreboot-gerrit January 2025

coreboot-gerrit@coreboot.org

1 participants
3269 discussions

[M] Change in coreboot[main]: drivers/option: Add CBFS file based option backend
by Subrata Banik (Code Review) 15 Jan '25

15 Jan '25

Attention is currently required from: Julius Werner, Nicholas Chin. Subrata Banik 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 9: (2 comments) Patchset: PS2: > @subratabanik@google.com If it makes things easier feel free to push new patchsets. I won't be able to get back to this until later in the day. thanks, I believe we are good now. thanks for addressing the review comments File src/drivers/option/Makefile.mk: https://review.coreboot.org/c/coreboot/+/85905/comment/950c4b63_6a4b7de2?us… : PS6, Line 6: smm-$(CONFIG_USE_CBFS_FILE_OPTION_BACKEND) += cbfs_file_option.c > Seems like the only user of the option api in SMM is the `power_on_after_fail` option, which defaults to a a value that can be set by Kconfig. So there isn't much lost by removing the ability to configure options in SMM, and keeping SMM as simple as possible and having fewer possible SMM runtime codepaths is probably not a bad idea anyway. I agree with you that pmclib.c is the only consumer where we can directly assign Kconfig value. we might not need an option there. It would eliminate the need for compiling option API for smm mode -- 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: 9 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: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Comment-Date: Wed, 15 Jan 2025 14:44:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Nicholas Chin <nic.c3.14(a)gmail.com>

1 0

[M] Change in coreboot[main]: drivers/option: Add CBFS file based option backend
by Nicholas Chin (Code Review) 15 Jan '25

15 Jan '25

Attention is currently required from: Julius Werner, Subrata Banik. Nicholas Chin 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 8: (3 comments) Patchset: PS2: > Oh, I didn't notice this by the time I pushed a new patchset. […] @subratabanik@google.com If it makes things easier feel free to push new patchsets. I won't be able to get back to this until later in the day. File src/drivers/option/Makefile.mk: https://review.coreboot.org/c/coreboot/+/85905/comment/d84eafa2_2567cf93?us… : PS6, Line 6: smm-$(CONFIG_USE_CBFS_FILE_OPTION_BACKEND) += cbfs_file_option.c > > as we don't compile vboot library in smm mode, I'm getting compilation error due to undefined refe […] Seems like the only user of the option api in SMM is the `power_on_after_fail` option, which defaults to a a value that can be set by Kconfig. So there isn't much lost by removing the ability to configure options in SMM, and keeping SMM as simple as possible and having fewer possible SMM runtime codepaths is probably not a bad idea anyway. File src/include/option.h: https://review.coreboot.org/c/coreboot/+/85905/comment/4f15264b_62273f9a?us… : PS7, Line 10: #if CONFIG(OPTION_BACKEND_NONE) > ``` […] Done -- 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: 8 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: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Comment-Date: Wed, 15 Jan 2025 14:42:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Nicholas Chin <nic.c3.14(a)gmail.com>

1 0

[L] Change in coreboot[main]: mb/asrock: Add Z87 Extreme3 (Haswell)
by Angel Pons (Code Review) 15 Jan '25

15 Jan '25

Attention is currently required from: Felix Singer, Jan Philipp Groß, Keith Hui, Máté Kukri, Nicholas Chin. Angel Pons has posted comments on this change by Jan Philipp Groß. ( https://review.coreboot.org/c/coreboot/+/85926?usp=email ) Change subject: mb/asrock: Add Z87 Extreme3 (Haswell) ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/85926?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: I3c13c068d899588eda80b9957127bcb6ccf8bab0 Gerrit-Change-Number: 85926 Gerrit-PatchSet: 4 Gerrit-Owner: Jan Philipp Groß <jeangrande(a)mailbox.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Keith Hui <buurin(a)gmail.com> Gerrit-Reviewer: Máté Kukri <km(a)mkukri.xyz> Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Keith Hui <buurin(a)gmail.com> Gerrit-Attention: Máté Kukri <km(a)mkukri.xyz> Gerrit-Attention: Jan Philipp Groß <jeangrande(a)mailbox.org> Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Comment-Date: Wed, 15 Jan 2025 14:34:22 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[L] Change in coreboot[main]: mb/asrock: Add Z87 Extreme3 (Haswell)
by Jan Philipp Groß (Code Review) 15 Jan '25

15 Jan '25

Attention is currently required from: Angel Pons, Felix Singer, Keith Hui, Máté Kukri, Nicholas Chin. Jan Philipp Groß has posted comments on this change by Jan Philipp Groß. ( https://review.coreboot.org/c/coreboot/+/85926?usp=email ) Change subject: mb/asrock: Add Z87 Extreme3 (Haswell) ...................................................................... Patch Set 3: (1 comment) File src/mainboard/asrock/z87_extreme3/devicetree.cb: https://review.coreboot.org/c/coreboot/+/85926/comment/17325e15_6035dd08?us… : PS3, Line 18: device pci 01.0 on # PEG : end > You should probably enable 01.1 for the lower PCIe x16 slot. Done -- To view, visit https://review.coreboot.org/c/coreboot/+/85926?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: I3c13c068d899588eda80b9957127bcb6ccf8bab0 Gerrit-Change-Number: 85926 Gerrit-PatchSet: 3 Gerrit-Owner: Jan Philipp Groß <jeangrande(a)mailbox.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Keith Hui <buurin(a)gmail.com> Gerrit-Reviewer: Máté Kukri <km(a)mkukri.xyz> Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Keith Hui <buurin(a)gmail.com> Gerrit-Attention: Máté Kukri <km(a)mkukri.xyz> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Comment-Date: Wed, 15 Jan 2025 14:33:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>

1 0

[L] Change in coreboot[main]: mb/asrock: Add Z87 Extreme3 (Haswell)
by Jan Philipp Groß (Code Review) 15 Jan '25

15 Jan '25

Attention is currently required from: Angel Pons, Felix Singer, Jan Philipp Groß, Keith Hui, Máté Kukri, Nicholas Chin. Hello Angel Pons, Felix Singer, Keith Hui, Máté Kukri, Nicholas Chin, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/85926?usp=email to look at the new patch set (#4). The following approvals got outdated and were removed: Code-Review+1 by Angel Pons, Verified+1 by build bot (Jenkins) Change subject: mb/asrock: Add Z87 Extreme3 (Haswell) ...................................................................... mb/asrock: Add Z87 Extreme3 (Haswell) This port was done via autoport and subsequent manual tweaking. Working: - Haswell MRC.bin - All four DDR3/DDR3L DIMM slots - S3 suspend and resume - D-Sub Port - DVI-D Port - HDMI Port - RJ-45 Gigabit LAN Port - Both rear USB 2.0 Ports - All four rear USB 3.1 Gen1 Ports - Both USB 2.0 headers - USB 3.1 Gen1 header - All six SATA3 6.0 Gb/s connectors by Intel - Both PCI Express 3.0 x16 slots (Tested with RX 550 PCIe x8 GPU) - PCI Express 2.0 x1 slots (tested with TL-WDN4800 WiFi adapter) - HD Audio Jack (Audio output tested only) - Front Audio Jack (Audio output tested only) not (yet) tested: - PCI slots - IR header - COM Port header - PS/2 Mouse/Keyboard Port - Optical SPDIF Out Port Change-Id: I3c13c068d899588eda80b9957127bcb6ccf8bab0 Signed-off-by: Jan Philipp Groß <jeangrande(a)mailbox.org> --- A src/mainboard/asrock/z87_extreme3/Kconfig A src/mainboard/asrock/z87_extreme3/Kconfig.name A src/mainboard/asrock/z87_extreme3/Makefile.mk A src/mainboard/asrock/z87_extreme3/acpi/ec.asl A src/mainboard/asrock/z87_extreme3/acpi/platform.asl A src/mainboard/asrock/z87_extreme3/acpi/superio.asl A src/mainboard/asrock/z87_extreme3/board_info.txt A src/mainboard/asrock/z87_extreme3/bootblock.c A src/mainboard/asrock/z87_extreme3/data.vbt A src/mainboard/asrock/z87_extreme3/devicetree.cb A src/mainboard/asrock/z87_extreme3/dsdt.asl A src/mainboard/asrock/z87_extreme3/gma-mainboard.ads A src/mainboard/asrock/z87_extreme3/gpio.c A src/mainboard/asrock/z87_extreme3/hda_verb.c A src/mainboard/asrock/z87_extreme3/romstage.c 15 files changed, 530 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/85926/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/85926?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: I3c13c068d899588eda80b9957127bcb6ccf8bab0 Gerrit-Change-Number: 85926 Gerrit-PatchSet: 4 Gerrit-Owner: Jan Philipp Groß <jeangrande(a)mailbox.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Keith Hui <buurin(a)gmail.com> Gerrit-Reviewer: Máté Kukri <km(a)mkukri.xyz> Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Keith Hui <buurin(a)gmail.com> Gerrit-Attention: Máté Kukri <km(a)mkukri.xyz> Gerrit-Attention: Jan Philipp Groß <jeangrande(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>

1 0

[M] Change in coreboot[main]: drivers/option: Add CBFS file based option backend
by Nicholas Chin (Code Review) 15 Jan '25

15 Jan '25

Attention is currently required from: Julius Werner, Nicholas Chin. Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/85905?usp=email to look at the new patch set (#8). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: drivers/option: Add CBFS file based option backend ...................................................................... drivers/option: Add CBFS file based option backend Add a new option backend that uses values stored in CBFS files, similar to the SeaBIOS runtime config options stored in files with the etc/ prefix. Options should be stored in cbfs with the option/ prefix. Values can be set using `cbfstool coreboot.rom add-int -i option/value -n option_name`. For simplicity, options should be stored in the COREBOOT (RO) FMAP region, which is the default for cbfstool. This backend is not available in SMM due to cbfs dependencies on vboot functions which are not added to SMM, and thus the fallback will be returned by calls to get_uint_option() in SMM. Tested with QEMU Q35 by setting various options for "sata_mode" and observing the console output for the SATA controller mode during i82801ix_sata initialization. Change-Id: Ifc0439ee42f13f49ae54d4855d1d9333c39b01f5 Signed-off-by: Nicholas Chin <nic.c3.14(a)gmail.com> --- M src/Kconfig M src/drivers/option/Makefile.mk A src/drivers/option/cbfs_file_option.c M src/include/option.h 4 files changed, 53 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/85905/8 -- 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: newpatchset Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ifc0439ee42f13f49ae54d4855d1d9333c39b01f5 Gerrit-Change-Number: 85905 Gerrit-PatchSet: 8 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: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>

1 0

[XL] Change in coreboot[main]: soc/mediatek/mt8196: Add RTC driver
by Yidi Lin (Code Review) 15 Jan '25

15 Jan '25

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/+/85978?usp=email ) Change subject: soc/mediatek/mt8196: Add RTC driver ...................................................................... Patch Set 1: (16 comments) File src/soc/mediatek/mt8196/mt6685_rtc.c: https://review.coreboot.org/c/coreboot/+/85978/comment/2d51cc77_7021d863?us… : PS1, Line 6: : #include <commonlib/bsd/bcd.h> : #include <console/console.h> : #include <delay.h> : #include <soc/rtc.h> : #include <soc/mt6685.h> : #include <soc/mt6685_rtc.h> : #include <soc/mt6685_hw.h> : #include <soc/mt6685_rtc_hw.h> : #include <soc/mt6685.h> : #include <soc/pmif.h> : #include <soc/spmi.h> : #include <stdbool.h> : #include <timer.h> sort https://review.coreboot.org/c/coreboot/+/85978/comment/4c16c7c2_04ab8c0d?us… : PS1, Line 79: RG_FQMTR_CLK_CK_PDN_SHIFT align and fix similar issues. https://review.coreboot.org/c/coreboot/+/85978/comment/e97cf240_aa1f6841?us… : PS1, Line 148: freq[2] What is the difference from freq[0] ? https://review.coreboot.org/c/coreboot/+/85978/comment/761c563e_abab95d2?us… : PS1, Line 173: u32 u16 https://review.coreboot.org/c/coreboot/+/85978/comment/a0fc5fc4_fbe50442?us… : PS1, Line 173: rtc_clr_clrset_trigger rtc_clrset_trigger https://review.coreboot.org/c/coreboot/+/85978/comment/a1d0358a_c719b02d?us… : PS1, Line 173: u32 u16 https://review.coreboot.org/c/coreboot/+/85978/comment/616b1803_047a87c5?us… : PS1, Line 233: rtc_read(RTC_AL_SEC, &rdata); : : rtc_write(RTC_AL_SEC, (rdata & (~RTC_LPD_OPT_MASK)) | RTC_LPD_OPT_EOSC_LPD); : if (!rtc_write_trigger()) rtc_clrset_trigger ? https://review.coreboot.org/c/coreboot/+/85978/comment/0efb6102_4b1098d3?us… : PS1, Line 239: rtc_read(RTC_CON, &rdata); : con = rdata | RTC_XOSC32_LPEN; : con &= ~RTC_CON_LPRST; : rtc_write(RTC_CON, con); : if (!rtc_write_trigger()) rtc_clrset_trigger ? https://review.coreboot.org/c/coreboot/+/85978/comment/89dc07da_40b45a68?us… : PS1, Line 315: RTC_SEC_DSN_REV0, rtc_sec_dsn_rev0); move to above line https://review.coreboot.org/c/coreboot/+/85978/comment/7b06e585_b2933c81?us… : PS1, Line 341: rtc_write(RTC_POWERKEY1, RTC_POWERKEY1_KEY); : rtc_write(RTC_POWERKEY2, RTC_POWERKEY2_KEY); : : if (!rtc_write_trigger()) `rtc_powerkey_init` https://review.coreboot.org/c/coreboot/+/85978/comment/3b982afe_b0031b98?us… : PS1, Line 357: rtc_write(RTC_POWERKEY1, RTC_POWERKEY1_KEY); : rtc_write(RTC_POWERKEY2, RTC_POWERKEY2_KEY); : if (!rtc_write_trigger()) rtc_powerkey_init https://review.coreboot.org/c/coreboot/+/85978/comment/fd27df3c_80c1e2c5?us… : PS1, Line 374: rtc_frequency_meter_check() move to the second and align with RETRY_MAX_TIME https://review.coreboot.org/c/coreboot/+/85978/comment/69d9a440_834ec4a3?us… : PS1, Line 404: | RTC_K_EOSC_RSV_7 | RTC_K_EOSC_RSV_6); move to above line https://review.coreboot.org/c/coreboot/+/85978/comment/55a843b0_8f229869?us… : PS1, Line 433: MT6351 Is this comment correct ? https://review.coreboot.org/c/coreboot/+/85978/comment/9b4f199d_8226c64c?us… : PS1, Line 434: rtc_write(RTC_POWERKEY1, RTC_POWERKEY1_KEY); : rtc_write(RTC_POWERKEY2, RTC_POWERKEY2_KEY); : : if (!rtc_write_trigger()) { : printk(BIOS_ERR, : "%s rtc_write_trigger after writing POWERKEY\n", __func__); : return false; : } : : if (!rtc_writeif_unlock()) { : printk(BIOS_ERR, : "%s rtc_writeif_unlock after writing POWERKEY\n", __func__); : return false; : } What is the difference between line 406~412 and here ? (additional call to `rtc_write_trigger`) https://review.coreboot.org/c/coreboot/+/85978/comment/2b05b3d0_3eb4da8a?us… : PS1, Line 585: RG_OCT1_RTC32K_1V8_0_MASK align -- To view, visit https://review.coreboot.org/c/coreboot/+/85978?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: I3dd337eaa3eed3012ddea300f7e04f2b63fb2daa Gerrit-Change-Number: 85978 Gerrit-PatchSet: 1 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: Wed, 15 Jan 2025 13:44:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[M] Change in coreboot[main]: drivers/intel/fsp2_0: Add option to control debug log level using CBFS
by Subrata Banik (Code Review) 15 Jan '25

15 Jan '25

Attention is currently required from: Andrey Petrov, Intel coreboot Reviewers, Julius Werner, Ronak Kanabar. Hello Andrey Petrov, Intel coreboot Reviewers, Julius Werner, Ronak Kanabar, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/86001?usp=email to look at the new patch set (#4). Change subject: drivers/intel/fsp2_0: Add option to control debug log level using CBFS ...................................................................... drivers/intel/fsp2_0: Add option to control debug log level using CBFS This commit introduces a new Kconfig option, FSP_DEBUG_LOG_LEVEL_USING_CBFS, which allows controlling the FSP debug log level using CBFS RAW binary files. If this option is enabled, the following files will be used to determine the log levels: - fsp_pcd_debug_level: For the overall FSP debug log level. - fsp_mrc_debug_level: For the MRC (Memory Reference Code) debug log level. The values in these files should correspond to the FSP_LOG_LEVEL_* enum values. See the Kconfig help text for more details. If this option is disabled, the log levels will be determined by calling into fsp_map_console_log_level API. This change allows for more flexibility in controlling the FSP debug log level, especially in cases of debugging silicon firmware issues with a debug AP FW binary. This capability is particularly useful when debugging issues that require examining both silicon and MRC logs simultaneously. BUG=b:227151510 TEST=Able to control the FSP debug log based on CBFS options To inject the fsp_pcd_debug_level and fsp_mrc_debug_level CBFS files with the desired log level, run: ``` cbfstool image-fatcat.serial.bin add-int -i 5 -n option/fsp_pcd_debug_level cbfstool image-fatcat.serial.bin add-int -i 5 -n option/fsp_mrc_debug_level ``` With both fsp_pcd_debug_level and fsp_mrc_debug_level present in the RO CBFS, both the silicon firmware and MRC behave as debug binaries. To verify the presence of both log-level RAW CBFS binaries in the CBFS RO slot, run: ``` sudo cbfstool fatcat/image-rex0.serial.bin print | grep fsp_ ``` This should output: ``` option/fsp_mrc_debug_level 0x88e40 raw 8 none option/fsp_pcd_debug_level 0x2a7400 raw 8 none ``` Change-Id: I2c14d26021dd0048fa24024119df857e216f18bd Signed-off-by: Subrata Banik <subratabanik(a)google.com> --- M src/drivers/intel/fsp2_0/Kconfig M src/drivers/intel/fsp2_0/debug.c M src/drivers/intel/fsp2_0/include/fsp/debug.h 3 files changed, 71 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/86001/4 -- 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: newpatchset Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I2c14d26021dd0048fa24024119df857e216f18bd Gerrit-Change-Number: 86001 Gerrit-PatchSet: 4 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-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> 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>

1 0

[M] Change in coreboot[main]: drivers/intel/fsp2_0: Add option to control debug log level using CBFS
by Subrata Banik (Code Review) 15 Jan '25

15 Jan '25

Attention is currently required from: Andrey Petrov, Intel coreboot Reviewers, Julius Werner, Ronak Kanabar. Hello Andrey Petrov, Intel coreboot Reviewers, Julius Werner, Ronak Kanabar, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/86001?usp=email to look at the new patch set (#3). Change subject: drivers/intel/fsp2_0: Add option to control debug log level using CBFS ...................................................................... drivers/intel/fsp2_0: Add option to control debug log level using CBFS This commit introduces a new Kconfig option, FSP_DEBUG_LOG_LEVEL_USING_CBFS, which allows controlling the FSP debug log level using CBFS RAW binary files. If this option is enabled, the following files will be used to determine the log levels: - fsp_pcd_debug_level: For the overall FSP debug log level. - fsp_mrc_debug_level: For the MRC (Memory Reference Code) debug log level. The values in these files should correspond to the FSP_LOG_LEVEL_* enum values. See the Kconfig help text for more details. If this option is disabled, the log levels will be determined by calling into fsp_map_console_log_level API. This change allows for more flexibility in controlling the FSP debug log level, especially in cases of debugging silicon firmware issues with a debug AP FW binary. This capability is particularly useful when debugging issues that require examining both silicon and MRC logs simultaneously. BUG=b:227151510 TEST=Able to control the FSP debug log based on CBFS options To inject the fsp_pcd_debug_level and fsp_mrc_debug_level CBFS files with the desired log level, run: ``` cbfstool image-fatcat.serial.bin add-int -i 5 -n option/fsp_pcd_debug_level cbfstool image-fatcat.serial.bin add-int -i 5 -n option/fsp_mrc_debug_level ``` With both fsp_pcd_debug_level and fsp_mrc_debug_level present in the RO CBFS, both the silicon firmware and MRC behave as debug binaries. To verify the presence of both log-level RAW CBFS binaries in the CBFS RO slot, run: ``` sudo cbfstool fatcat/image-rex0.serial.bin print | grep fsp_ ``` This should output: ``` option/fsp_mrc_debug_level 0x88e40 raw 8 none option/fsp_pcd_debug_level 0x2a7400 raw 8 none ``` Change-Id: I2c14d26021dd0048fa24024119df857e216f18bd Signed-off-by: Subrata Banik <subratabanik(a)google.com> --- M src/drivers/intel/fsp2_0/Kconfig M src/drivers/intel/fsp2_0/debug.c M src/drivers/intel/fsp2_0/include/fsp/debug.h 3 files changed, 71 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/86001/3 -- 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: newpatchset Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I2c14d26021dd0048fa24024119df857e216f18bd Gerrit-Change-Number: 86001 Gerrit-PatchSet: 3 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-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> 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>

1 0

[M] Change in coreboot[main]: drivers/intel/fsp2_0: Add option to control debug log level using CBFS
by Subrata Banik (Code Review) 15 Jan '25

15 Jan '25

Attention is currently required from: Andrey Petrov, Intel coreboot Reviewers, Julius Werner, Ronak Kanabar. Hello Andrey Petrov, Intel coreboot Reviewers, Julius Werner, Ronak Kanabar, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/86001?usp=email to look at the new patch set (#2). Change subject: drivers/intel/fsp2_0: Add option to control debug log level using CBFS ...................................................................... drivers/intel/fsp2_0: Add option to control debug log level using CBFS This commit introduces a new Kconfig option, FSP_DEBUG_LOG_LEVEL_USING_CBFS, which allows controlling the FSP debug log level using CBFS RAW binary files. If this option is enabled, the following files will be used to determine the log levels: - fsp_pcd_debug_level: For the overall FSP debug log level. - fsp_mrc_debug_level: For the MRC (Memory Reference Code) debug log level. The values in these files should correspond to the FSP_LOG_LEVEL_* enum values. See the Kconfig help text for more details. If this option is disabled, the log levels will be determined by calling into fsp_map_console_log_level API. This change allows for more flexibility in controlling the FSP debug log level, especially in cases of debugging silicon firmware issues with a debug AP FW binary. This capability is particularly useful when debugging issues that require examining both silicon and MRC logs simultaneously. BUG=b:227151510 TEST=Able to control the FSP debug log based on CBFS options To inject the fsp_pcd_debug_level and fsp_mrc_debug_level CBFS files with the desired log level, run: ``` cbfstool image-fatcat.serial.bin add-int -i 5 -n option/fsp_pcd_debug_level cbfstool image-fatcat.serial.bin add-int -i 5 -n option/fsp_mrc_debug_level ``` With both fsp_pcd_debug_level and fsp_mrc_debug_level present in the RO CBFS, both the silicon firmware and MRC behave as debug binaries. To verify the presence of both log-level RAW CBFS binaries in the CBFS RO slot, run: ``` sudo cbfstool fatcat/image-rex0.serial.bin print | grep fsp_ ``` This should output: ``` option/fsp_mrc_debug_level 0x88e40 raw 8 none option/fsp_pcd_debug_level 0x2a7400 raw 8 none ``` Change-Id: I2c14d26021dd0048fa24024119df857e216f18bd Signed-off-by: Subrata Banik <subratabanik(a)google.com> --- M src/drivers/intel/fsp2_0/Kconfig M src/drivers/intel/fsp2_0/debug.c M src/drivers/intel/fsp2_0/include/fsp/debug.h 3 files changed, 74 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/86001/2 -- 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: newpatchset Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I2c14d26021dd0048fa24024119df857e216f18bd Gerrit-Change-Number: 86001 Gerrit-PatchSet: 2 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-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> 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>

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit January 2025