coreboot-gerrit February 2025

coreboot-gerrit@coreboot.org

1 participants
2886 discussions

[XS] Change in coreboot[main]: amdfwtool: Set address mode of BIOS binary as context defines
by Bao Zheng (Code Review) 12 Feb '25

12 Feb '25

Attention is currently required from: Felix Held, Julius Werner, Marshall Dawson, Maximilian Brune, Zheng Bao, ritul guru. Bao Zheng has posted comments on this change by Bao Zheng. ( https://review.coreboot.org/c/coreboot/+/85466?usp=email ) Change subject: amdfwtool: Set address mode of BIOS binary as context defines ...................................................................... Patch Set 9: (1 comment) Patchset: PS9: > I tested on birman-plus: The binary changed as expected, but the size of all CBFS files is the same […] Do you use the default setting to build birman-plus. Based on my test, only majolica and onyx_poc(genoa) have change the final image. -- To view, visit https://review.coreboot.org/c/coreboot/+/85466?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: I9c2f2983d03c913b28fbd87aa0925a32a4649d62 Gerrit-Change-Number: 85466 Gerrit-PatchSet: 9 Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Zheng Bao Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: ritul guru <ritul.bits(a)gmail.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: ritul guru <ritul.bits(a)gmail.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Zheng Bao Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Wed, 12 Feb 2025 09:28:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Maximilian Brune <maximilian.brune(a)9elements.com>

1 0

[XS] Change in coreboot[main]: soc/mediatek/mt8196: Fix RTC recovery by disabling external XTAL
by Yidi Lin (Code Review) 12 Feb '25

12 Feb '25

Yidi Lin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/86353?usp=email ) Change subject: soc/mediatek/mt8196: Fix RTC recovery by disabling external XTAL ...................................................................... soc/mediatek/mt8196: Fix RTC recovery by disabling external XTAL Configure PMIC register 0x50c bit0 which decides whether to use external xtal. This bit of mt6685 should be set to 1, to disable external xtal. BRANCH=rauru BUG=b:395485005 TEST=emerge-rauru coreboot chromeos-bootimage, remove battery and charger, then insert battery and charge, RTC boots normally. Signed-off-by: Shunxi Zhang <ot_shunxi.zhang(a)mediatek.com> Change-Id: Iea44f13af030f24c02993dd43a35a9d8b4f72179 Reviewed-on: https://review.coreboot.org/c/coreboot/+/86353 Reviewed-by: Shunxi Zhang <ot_shunxi.zhang(a)mediatek.corp-partner.google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Yu-Ping Wu <yupingso(a)google.com> Reviewed-by: Yidi Lin <yidilin(a)google.com> --- M src/soc/mediatek/mt8196/mt6685_rtc.c 1 file changed, 4 insertions(+), 0 deletions(-) Approvals: Yu-Ping Wu: Looks good to me, approved Yidi Lin: Looks good to me, approved build bot (Jenkins): Verified Shunxi Zhang: Looks good to me, but someone else must approve diff --git a/src/soc/mediatek/mt8196/mt6685_rtc.c b/src/soc/mediatek/mt8196/mt6685_rtc.c index 4564a31..4e63f10 100644 --- a/src/soc/mediatek/mt8196/mt6685_rtc.c +++ b/src/soc/mediatek/mt8196/mt6685_rtc.c @@ -345,6 +345,10 @@ { printk(BIOS_INFO, "%s: enter\n", __func__); + config_interface(SCK_TOP_XTAL_SEL_ADDR, 1, SCK_TOP_XTAL_SEL_MASK, + SCK_TOP_XTAL_SEL_SHIFT); + udelay(100); + if (!retry(RECOVERY_RETRY_COUNT, rtc_frequency_meter_check() && rtc_init_after_recovery())) { -- To view, visit https://review.coreboot.org/c/coreboot/+/86353?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Iea44f13af030f24c02993dd43a35a9d8b4f72179 Gerrit-Change-Number: 86353 Gerrit-PatchSet: 3 Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Shunxi Zhang <ot_shunxi.zhang(a)mediatek.corp-partner.google.com> 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>

1 0

[XS] Change in coreboot[main]: soc/mediatek/mt8196: Remove tvdpll3 disable/enable
by Yidi Lin (Code Review) 12 Feb '25

12 Feb '25

Yidi Lin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/86343?usp=email ) ( 4 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: soc/mediatek/mt8196: Remove tvdpll3 disable/enable ...................................................................... soc/mediatek/mt8196: Remove tvdpll3 disable/enable The tvdpll3 cannot be disabled during suspend because of the enable operation, so we remove the enable operation. Hardware can now automatically enable and disable tvdpll3 based on the clock demand of its downstream. BRANCH=rauru BUG=b:377628718 TEST=Bootup OK, Suspend/Resume OK and FW screen shown OK, with MMinfra kernel/vcp patch, mminfra can be turned off to reduce power consumption. Signed-off-by: Guangjie Song <guangjie.song(a)mediatek.com> Change-Id: Ib9c72a1602c1f76dc94cca5c4a61a542a853560b Reviewed-on: https://review.coreboot.org/c/coreboot/+/86343 Reviewed-by: Yidi Lin <yidilin(a)google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Yu-Ping Wu <yupingso(a)google.com> --- M src/soc/mediatek/mt8196/pll.c 1 file changed, 0 insertions(+), 4 deletions(-) Approvals: Yu-Ping Wu: Looks good to me, approved Yidi Lin: Looks good to me, approved build bot (Jenkins): Verified diff --git a/src/soc/mediatek/mt8196/pll.c b/src/soc/mediatek/mt8196/pll.c index 59a9b09..2007197 100644 --- a/src/soc/mediatek/mt8196/pll.c +++ b/src/soc/mediatek/mt8196/pll.c @@ -1572,11 +1572,7 @@ { const struct pll *pll = &plls[CLK_APMIXED2_TVDPLL3]; - clrbits32(pll->reg, MT8196_PLL_EN); pll_set_rate(pll, freq); - setbits32(pll->reg, MT8196_PLL_EN); - - udelay(PLL_EN_DELAY); } void mt_pll_edp_mux_set_sel(u32 sel) -- To view, visit https://review.coreboot.org/c/coreboot/+/86343?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ib9c72a1602c1f76dc94cca5c4a61a542a853560b Gerrit-Change-Number: 86343 Gerrit-PatchSet: 6 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: Guangjie Song <guangjie.song(a)mediatek.corp-partner.google.com>

1 0

[XS] Change in coreboot[main]: soc/mediatek/mt8196: Require mtk_fsp_*.elf to exist
by Yidi Lin (Code Review) 12 Feb '25

12 Feb '25

Yidi Lin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/86355?usp=email ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: soc/mediatek/mt8196: Require mtk_fsp_*.elf to exist ...................................................................... soc/mediatek/mt8196: Require mtk_fsp_*.elf to exist As MT8196 won't be able to boot up without mtk_fsp_romstage.elf and mtk_fsp_ramstage.elf, ensure their presence in build time. Change-Id: I668319ae1f63818e324002e7ae4d888479edb9cf Signed-off-by: Yu-Ping Wu <yupingso(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/coreboot/+/86355 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Yidi Lin <yidilin(a)google.com> --- M src/soc/mediatek/mt8196/Makefile.mk 1 file changed, 0 insertions(+), 4 deletions(-) Approvals: build bot (Jenkins): Verified Yidi Lin: Looks good to me, approved diff --git a/src/soc/mediatek/mt8196/Makefile.mk b/src/soc/mediatek/mt8196/Makefile.mk index 4dd2d2d..85b670a 100644 --- a/src/soc/mediatek/mt8196/Makefile.mk +++ b/src/soc/mediatek/mt8196/Makefile.mk @@ -125,17 +125,13 @@ $(FSP_ROMSTAGE_CBFS)-file := $(MT8196_BLOB_DIR)/mtk_fsp_romstage.elf $(FSP_ROMSTAGE_CBFS)-type := stage $(FSP_ROMSTAGE_CBFS)-compression := $(CBFS_PRERAM_COMPRESS_FLAG) -ifneq ($(wildcard $($(FSP_ROMSTAGE_CBFS)-file)),) cbfs-files-y += $(FSP_ROMSTAGE_CBFS) -endif FSP_RAMSTAGE_CBFS := $(CONFIG_CBFS_PREFIX)/mtk_fsp_ramstage $(FSP_RAMSTAGE_CBFS)-file := $(MT8196_BLOB_DIR)/mtk_fsp_ramstage.elf $(FSP_RAMSTAGE_CBFS)-type := stage $(FSP_RAMSTAGE_CBFS)-compression := $(CBFS_COMPRESS_FLAG) -ifneq ($(wildcard $($(FSP_RAMSTAGE_CBFS)-file)),) cbfs-files-y += $(FSP_RAMSTAGE_CBFS) -endif $(objcbfs)/bootblock.bin: $(objcbfs)/bootblock.raw.bin ./util/mtkheader/gen-bl-img.py mt8196 sf $< $@ -- To view, visit https://review.coreboot.org/c/coreboot/+/86355?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I668319ae1f63818e324002e7ae4d888479edb9cf Gerrit-Change-Number: 86355 Gerrit-PatchSet: 4 Gerrit-Owner: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[XS] Change in coreboot[main]: amdfwtool: Set address mode of BIOS binary as context defines
by Bao Zheng (Code Review) 12 Feb '25

12 Feb '25

Attention is currently required from: Felix Held, Julius Werner, Marshall Dawson, Zheng Bao, ritul guru. Bao Zheng has posted comments on this change by Bao Zheng. ( https://review.coreboot.org/c/coreboot/+/85466?usp=email ) The change is no longer submittable: All-Comments-Resolved is unsatisfied now. Change subject: amdfwtool: Set address mode of BIOS binary as context defines ...................................................................... Patch Set 9: (1 comment) Patchset: PS9: Hold. -- To view, visit https://review.coreboot.org/c/coreboot/+/85466?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: I9c2f2983d03c913b28fbd87aa0925a32a4649d62 Gerrit-Change-Number: 85466 Gerrit-PatchSet: 9 Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Zheng Bao Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: ritul guru <ritul.bits(a)gmail.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: ritul guru <ritul.bits(a)gmail.com> Gerrit-Attention: Zheng Bao Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Wed, 12 Feb 2025 08:55:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[S] Change in coreboot[main]: util/crossgcc: Update LLVM from 18.1.8 to 19.1.7
by Elyes Haouas (Code Review) 12 Feb '25

12 Feb '25

Attention is currently required from: Elyes Haouas, Martin L Roth. Elyes Haouas has uploaded a new patch set (#11) to the change originally created by Felix Singer. ( https://review.coreboot.org/c/coreboot/+/85169?usp=email ) Change subject: util/crossgcc: Update LLVM from 18.1.8 to 19.1.7 ...................................................................... util/crossgcc: Update LLVM from 18.1.8 to 19.1.7 Change-Id: I4a392b472b7450685343e1343c7b84c78b0cad3d Signed-off-by: Felix Singer <felixsinger(a)posteo.net> --- M util/crossgcc/buildgcc R util/crossgcc/patches/clang-19.1.7.src_x86_baremetal.patch D util/crossgcc/sum/clang-18.1.8.src.tar.xz.cksum A util/crossgcc/sum/clang-19.1.7.src.tar.xz.cksum D util/crossgcc/sum/clang-tools-extra-18.1.8.src.tar.xz.cksum A util/crossgcc/sum/clang-tools-extra-19.1.7.src.tar.xz.cksum D util/crossgcc/sum/cmake-18.1.8.src.tar.xz.cksum A util/crossgcc/sum/cmake-19.1.7.src.tar.xz.cksum D util/crossgcc/sum/compiler-rt-18.1.8.src.tar.xz.cksum A util/crossgcc/sum/compiler-rt-19.1.7.src.tar.xz.cksum D util/crossgcc/sum/libunwind-18.1.8.src.tar.xz.cksum A util/crossgcc/sum/libunwind-19.1.7.src.tar.xz.cksum D util/crossgcc/sum/lld-18.1.8.src.tar.xz.cksum A util/crossgcc/sum/lld-19.1.7.src.tar.xz.cksum D util/crossgcc/sum/llvm-18.1.8.src.tar.xz.cksum A util/crossgcc/sum/llvm-19.1.7.src.tar.xz.cksum 16 files changed, 8 insertions(+), 8 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/85169/11 -- To view, visit https://review.coreboot.org/c/coreboot/+/85169?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: I4a392b472b7450685343e1343c7b84c78b0cad3d Gerrit-Change-Number: 85169 Gerrit-PatchSet: 11 Gerrit-Owner: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr>

1 0

[S] Change in coreboot[main]: mb/trulo/var/uldrenite: Support x32 memory configuration
by Eric Lai (Code Review) 12 Feb '25

12 Feb '25

Attention is currently required from: Dinesh Gehlot, Jayvik Desai, John Su, Kapil Porwal, Nick Vaccaro. Eric Lai has posted comments on this change by John Su. ( https://review.coreboot.org/c/coreboot/+/86364?usp=email ) Change subject: mb/trulo/var/uldrenite: Support x32 memory configuration ...................................................................... Patch Set 5: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/86364?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: Idd3534bba0379a7bb06f8fbbeb9469e938e5a629 Gerrit-Change-Number: 86364 Gerrit-PatchSet: 5 Gerrit-Owner: John Su <john_su(a)compal.corp-partner.google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jamie Chen <jamie_chen(a)compal.corp-partner.google.com> Gerrit-CC: Van Chen <van_chen(a)compal.corp-partner.google.com> Gerrit-Attention: Jayvik Desai <jayvik(a)google.com> Gerrit-Attention: John Su <john_su(a)compal.corp-partner.google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Comment-Date: Wed, 12 Feb 2025 08:17:48 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[L] Change in coreboot[main]: haswell NRI: Add DDR3 JEDEC reset and init
by Angel Pons (Code Review) 12 Feb '25

12 Feb '25

Attention is currently required from: Alicja Michalska, Arthur Heymans, Nico Huber. Angel Pons has posted comments on this change by Angel Pons. ( https://review.coreboot.org/c/coreboot/+/64189?usp=email ) Change subject: haswell NRI: Add DDR3 JEDEC reset and init ...................................................................... Patch Set 8: (3 comments) File src/northbridge/intel/haswell/native_raminit/raminit_native.h: https://review.coreboot.org/c/coreboot/+/64189/comment/eaeeecc9_e6c3281e?us… : PS8, Line 79: RAMINIT_STATUS_UNSPECIFIED_ERROR, /** TODO: Deprecated in favor of specific values **/ > Assuming error code reporting isn't ready yet? This comment is because some of the code I haven't upstreamed yet still uses this, and I'd rather get rid of this one line after the thousands of lines I've still got around are at least upstreamed. I actually don't use error code values for anything yet (0 = success, anything else = failure), but I felt it'd be nicer than hardcoding magic numbers all over the place (which was the case before). I will eventually use them to try raminit again after a failure, using more relaxed parameters or disabling a failing channel. https://review.coreboot.org/c/coreboot/+/64189/comment/d4e30546_219bf759?us… : PS8, Line 176: int8_t rxvref[NUM_CHANNELS][NUM_SLOTRANKS][NUM_LANES]; > uint? No, this is a signed offset. Zero offset corresponds to VDDQ/2 (half of the I/O voltage), and the offset can be positive or negative. The timing offsets (tx_dq, txdqs, rcven, rxdqs, clk/ctl/cke pi codes...) are delays measured in "PI ticks" (1/64th of a QCLK (quadrature clock) clock cycle, or 1/128th of a DCLK (DRAM clock) clock cycle). These are unsigned since you cannot go back in time. I could add comments to these but I think it'd be better to do so in a follow-up as some of the patches in this train add more stuff in here (so adding comments now would cause a bunch of merge conflicts). File src/southbridge/intel/lynxpoint/pch.h: https://review.coreboot.org/c/coreboot/+/64189/comment/943cb361_4841a832?us… : PS8, Line 590: #define PM_CFG2_DRAM_RESET_CTL (1 << 26) /* ULT only */ > NIT: Remove the space to make it look a bit nicer? The space is there to indicate this is a bitfield within the register. Looks like this file is inconsistent with space usage but I prefer having the space to make it easier to follow -- To view, visit https://review.coreboot.org/c/coreboot/+/64189?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: I2a0c066537021b587599228086727cb1e041bff5 Gerrit-Change-Number: 64189 Gerrit-PatchSet: 8 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Alicja Michalska <ahplka19(a)gmail.com> Gerrit-CC: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Alicja Michalska <ahplka19(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Wed, 12 Feb 2025 08:07:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Alicja Michalska <ahplka19(a)gmail.com>

1 0

[XL] Change in coreboot[main]: haswell NRI: Configure initial MC settings
by Angel Pons (Code Review) 11 Feb '25

11 Feb '25

Attention is currently required from: Alicja Michalska, Arthur Heymans, Eric Lai, Felix Held, Felix Singer, Nico Huber, Patrick Rudolph. Angel Pons has posted comments on this change by Angel Pons. ( https://review.coreboot.org/c/coreboot/+/64186?usp=email ) Change subject: haswell NRI: Configure initial MC settings ...................................................................... Patch Set 7: (5 comments) File src/northbridge/intel/haswell/native_raminit/configure_mc.c: https://review.coreboot.org/c/coreboot/+/64186/comment/b7281944_0c8bb070?us… : PS7, Line 16: if (!is_hsw_ult()) > What's the purpose of this check? Does NRI currently support ULT CPUs? On Haswell Trad (not ULT), we don't program this register. Trad doesn't support configurable channel interleave (it's forced on), and it doesn't support LPDDR (3) either. I know this code works on Haswell Trad and Haswell ULT with DDR3, but I don't have any Haswell device with LPDDR3. Should I add any clarification? I use this pattern quite a lot. https://review.coreboot.org/c/coreboot/+/64186/comment/be653885_09458948?us… : PS7, Line 641: if (!(reg32 & BIT(31))) { /** TODO: What to do if this is locked? **/ > If it's locked, then reading the register will fail, right? I think it would be nice to put a debug […] This checks if the lock bit is unset before programming. I believe that *writing* the register will fail, but I'm not sure how to handle it (proceed anyway since it shouldn't affect booting? that's the current approach). I also haven't checked if the register remains locked after a reset. I do know that the register can be locked if we are to retry raminit for some reason (e.g. failed warm/fast boot). I'll try to refactor this to log an error if the register is locked *and* its value differs from the desired value. https://review.coreboot.org/c/coreboot/+/64186/comment/58b24c65_80ac7afa?us… : PS7, Line 660: /** TODO: Remember why this is only done on cold boots **/ > Stored in cache? I'm pretty sure it's that, yes File src/northbridge/intel/haswell/native_raminit/raminit_main.c: https://review.coreboot.org/c/coreboot/+/64186/comment/e2b65ce1_9d31a8dc?us… : PS7, Line 58: ctrl->vdd_mv = is_hsw_ult() ? 1350 : 1500; /** FIXME: Hardcoded, does it matter? **/ > Probably not, DDR3U isn't very common :) Yeah, for now it'll do. I know this will need to change once NRI supports voltage switching (which I want to implement someday), but I should first finish the rest of the training steps (I've got a lot to clean up and upstream) File src/northbridge/intel/haswell/native_raminit/reg_structs.h: https://review.coreboot.org/c/coreboot/+/64186/comment/34eec391_ac4a0244?us… : PS7, Line 12: int32_t vref : 6; // Bits 31:26 > s/int/uint, or is there a good reason for it? […] This is very much intentional - this 6-bit bitfield is actually a signed two's complement value. Conveniently enough, using bitfields for register definitions makes reading and writing these signed bitfields painless. -- To view, visit https://review.coreboot.org/c/coreboot/+/64186?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: If33846b51cb1bab5d0458fe626e13afb1bdc900e Gerrit-Change-Number: 64186 Gerrit-PatchSet: 7 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Alicja Michalska <ahplka19(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Lean Sheng Tan <tanleansheng(a)outlook.com> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Alicja Michalska <ahplka19(a)gmail.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Wed, 12 Feb 2025 07:47:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Alicja Michalska <ahplka19(a)gmail.com>

1 0

[S] Change in coreboot[main]: soc/mediatek/mt8196: Set MT6316 deglitch time from 2ns to 4ns
by Yu-Ping Wu (Code Review) 11 Feb '25

11 Feb '25

Attention is currently required from: Hung-Te Lin, Jarried Lin. Yu-Ping Wu has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/86371?usp=email ) Change subject: soc/mediatek/mt8196: Set MT6316 deglitch time from 2ns to 4ns ...................................................................... Patch Set 1: (2 comments) File src/soc/mediatek/common/include/soc/mt6316.h: https://review.coreboot.org/c/coreboot/+/86371/comment/ab12b970_86c85f63?us… : PS1, Line 32: RG Remove `RG`. Other values here are also registers, but without `RG` in the names. Also remove `_ADDR`? File src/soc/mediatek/common/mt6316.c: https://review.coreboot.org/c/coreboot/+/86371/comment/074ddc26_a505e9e7?us… : PS1, Line 177: 0xA 0x0A -- To view, visit https://review.coreboot.org/c/coreboot/+/86371?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: I77bd50cc6c25d6dcded57d9d65d92a0dd19c3c86 Gerrit-Change-Number: 86371 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-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Comment-Date: Wed, 12 Feb 2025 07:33:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit February 2025