coreboot-gerrit December 2024

coreboot-gerrit@coreboot.org

1 participants
3018 discussions

[L] Change in coreboot[main]: soc/mediatek/mt8196: Add mtcmos init support
by Yidi Lin (Code Review) 09 Dec '24

09 Dec '24

Attention is currently required from: Elyes Haouas, Guangjie Song, 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/+/84497?usp=email ) Change subject: soc/mediatek/mt8196: Add mtcmos init support ...................................................................... Patch Set 39: Code-Review+2 (1 comment) File src/soc/mediatek/mt8196/Makefile.mk: https://review.coreboot.org/c/coreboot/+/84497/comment/9af4b6ec_71b27f21?us… : PS36, Line 8: all-y += mtcmos.c > display driver in romstage would call mtcmos_display_power_on() which is implemented in mtcmos In this case, we should add mtcmos to bootblock and ramstage respectively. -- To view, visit https://review.coreboot.org/c/coreboot/+/84497?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: I44f2bb10453377a8412e80ac0c100760ebfbaff9 Gerrit-Change-Number: 84497 Gerrit-PatchSet: 39 Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Reviewer: Guangjie Song <guangjie.song(a)mediatek.corp-partner.google.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: Elyes Haouas <ehaouas(a)noos.fr> 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: Guangjie Song <guangjie.song(a)mediatek.corp-partner.google.com> Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Comment-Date: Mon, 09 Dec 2024 12:51:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Guangjie Song <guangjie.song(a)mediatek.corp-partner.google.com> Comment-In-Reply-To: Jarried Lin <jarried.lin(a)mediatek.com> Comment-In-Reply-To: Yidi Lin <yidilin(a)google.com>

1 0

[L] Change in coreboot[main]: soc/mediatek/mt8196: Add mtcmos init support
by Jarried Lin (Code Review) 09 Dec '24

09 Dec '24

Attention is currently required from: Elyes Haouas, Guangjie Song, Hung-Te Lin, Paul Menzel, Yidi Lin, Yu-Ping Wu. Jarried Lin has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/84497?usp=email ) Change subject: soc/mediatek/mt8196: Add mtcmos init support ...................................................................... Patch Set 39: (1 comment) File src/soc/mediatek/mt8196/Makefile.mk: https://review.coreboot.org/c/coreboot/+/84497/comment/0dc3e077_f9fea479?us… : PS36, Line 8: all-y += mtcmos.c > display driver in romstage would call mtcmos_display_power_on() which is implemented in mtcmos Hi Guangjie, we will add mtcmos in display patch. thanks -- To view, visit https://review.coreboot.org/c/coreboot/+/84497?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: I44f2bb10453377a8412e80ac0c100760ebfbaff9 Gerrit-Change-Number: 84497 Gerrit-PatchSet: 39 Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Reviewer: Guangjie Song <guangjie.song(a)mediatek.corp-partner.google.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: Elyes Haouas <ehaouas(a)noos.fr> 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: Guangjie Song <guangjie.song(a)mediatek.corp-partner.google.com> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Comment-Date: Mon, 09 Dec 2024 12:28:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Guangjie Song <guangjie.song(a)mediatek.corp-partner.google.com> Comment-In-Reply-To: Yidi Lin <yidilin(a)google.com>

1 0

[L] Change in coreboot[main]: soc/mediatek/mt8196: Add mtcmos init support
by Guangjie Song (Code Review) 09 Dec '24

09 Dec '24

Attention is currently required from: Elyes Haouas, Guangjie Song, Hung-Te Lin, Paul Menzel, Yidi Lin, Yu-Ping Wu. Guangjie Song has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/84497?usp=email ) Change subject: soc/mediatek/mt8196: Add mtcmos init support ...................................................................... Patch Set 38: (8 comments) File src/soc/mediatek/mt8196/Makefile.mk: https://review.coreboot.org/c/coreboot/+/84497/comment/9926472e_c972a305?us… : PS36, Line 8: all-y += mtcmos.c > only bootblock needs this driver. display driver in romstage would call mtcmos_display_power_on() which is implemented in mtcmos File src/soc/mediatek/mt8196/mtcmos.c: https://review.coreboot.org/c/coreboot/+/84497/comment/00575bca_2b4da9b5?us… : PS36, Line 58: MMINFRA0_VOTE > VOTE_BIT Done https://review.coreboot.org/c/coreboot/+/84497/comment/29d9127b_b302a2dd?us… : PS36, Line 120: shift > bit Done https://review.coreboot.org/c/coreboot/+/84497/comment/1e6a0db1_484ae6a9?us… : PS36, Line 238: NULL, > no need. Done https://review.coreboot.org/c/coreboot/+/84497/comment/dc7a33af_d19bc326?us… : PS36, Line 353: || cb == NULL > It is no harm to register a NULL callback. Done https://review.coreboot.org/c/coreboot/+/84497/comment/c0475ccc_20804fc4?us… : PS36, Line 377: 0x%x > %#x Done https://review.coreboot.org/c/coreboot/+/84497/comment/399f2d68_626673bb?us… : PS36, Line 373: : static int mtcmos_wait_for_state(u32 *reg, u32 mask, bool onoff) : { : if (!retry(ACK_DELAY_TIMES, mtcmos_is_setclr_state(reg, mask, onoff), udelay(ACK_DELAY_US))) { : printk(BIOS_ERR, "mtcmos_is_setclr_state(%p, 0x%x, %d) reg_val=0x%x timeout\n", : reg, mask, onoff, read32(reg)); : return -1; : } : : return 0; : } > ```` […] Done https://review.coreboot.org/c/coreboot/+/84497/comment/ebacebf6_fd018a04?us… : PS36, Line 411: ret = mtcmos_bus_prot_ctrl(&bp_table[step_idx], set); : if (ret) { : printk(BIOS_ERR, "mtcmos_bus_prot_ctrl set fail: %d\n", ret); : mtcmos_bus_prot_ctrl(&bp_table[step_idx], !set); : return ret; : } > mtcmos_bus_prot_ctrl always returns zero. no mtcmos_bus_prot_ctrl would return -1 -- To view, visit https://review.coreboot.org/c/coreboot/+/84497?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: I44f2bb10453377a8412e80ac0c100760ebfbaff9 Gerrit-Change-Number: 84497 Gerrit-PatchSet: 38 Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Reviewer: Guangjie Song <guangjie.song(a)mediatek.corp-partner.google.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: Elyes Haouas <ehaouas(a)noos.fr> 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: Guangjie Song <guangjie.song(a)mediatek.corp-partner.google.com> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Comment-Date: Mon, 09 Dec 2024 12:25:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Yidi Lin <yidilin(a)google.com>

1 0

[L] Change in coreboot[main]: soc/mediatek/mt8196: Add mtcmos init support
by Jarried Lin (Code Review) 09 Dec '24

09 Dec '24

Attention is currently required from: Elyes Haouas, Guangjie Song, Hung-Te Lin, Paul Menzel, Yidi Lin, Yu-Ping Wu. Jarried Lin has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/84497?usp=email ) Change subject: soc/mediatek/mt8196: Add mtcmos init support ...................................................................... Patch Set 39: (1 comment) File src/soc/mediatek/mt8196/Makefile.mk: https://review.coreboot.org/c/coreboot/+/84497/comment/03235cf9_76e178d7?us… : PS36, Line 8: all-y += mtcmos.c > only bootblock needs this driver. Done -- To view, visit https://review.coreboot.org/c/coreboot/+/84497?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: I44f2bb10453377a8412e80ac0c100760ebfbaff9 Gerrit-Change-Number: 84497 Gerrit-PatchSet: 39 Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Reviewer: Guangjie Song <guangjie.song(a)mediatek.corp-partner.google.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: Elyes Haouas <ehaouas(a)noos.fr> 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: Guangjie Song <guangjie.song(a)mediatek.corp-partner.google.com> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Comment-Date: Mon, 09 Dec 2024 12:25:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Yidi Lin <yidilin(a)google.com>

1 0

[L] Change in coreboot[main]: soc/mediatek/mt8196: Add mtcmos init support
by Jarried Lin (Code Review) 09 Dec '24

09 Dec '24

Attention is currently required from: Elyes Haouas, Guangjie Song, Hung-Te Lin, Jarried Lin, Paul Menzel, Yu-Ping Wu. Hello Guangjie Song, Hung-Te Lin, Yidi Lin, Yu-Ping Wu, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/84497?usp=email to look at the new patch set (#39). Change subject: soc/mediatek/mt8196: Add mtcmos init support ...................................................................... soc/mediatek/mt8196: Add mtcmos init support Add mtcmos init code and APIs for controlling power domain. TEST=build pass and driver init ok BUG=b:317009620 Signed-off-by: Guangjie Song <guangjie.song(a)mediatek.corp-partner.google.com> Change-Id: I44f2bb10453377a8412e80ac0c100760ebfbaff9 --- M src/soc/mediatek/mt8196/Makefile.mk M src/soc/mediatek/mt8196/bootblock.c M src/soc/mediatek/mt8196/include/soc/memlayout.ld A src/soc/mediatek/mt8196/include/soc/spm_mtcmos.h A src/soc/mediatek/mt8196/mtcmos.c 5 files changed, 839 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/97/84497/39 -- To view, visit https://review.coreboot.org/c/coreboot/+/84497?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: I44f2bb10453377a8412e80ac0c100760ebfbaff9 Gerrit-Change-Number: 84497 Gerrit-PatchSet: 39 Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Reviewer: Guangjie Song <guangjie.song(a)mediatek.corp-partner.google.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: Elyes Haouas <ehaouas(a)noos.fr> 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: Guangjie Song <guangjie.song(a)mediatek.corp-partner.google.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr>

1 0

[M] Change in coreboot[main]: soc/intel/alderlake: Add a function to force disable memory channels
by Kapil Porwal (Code Review) 09 Dec '24

09 Dec '24

Attention is currently required from: Angel Pons, Christian Walter, Dinesh Gehlot, Eric Lai, Jamie Chen, Jayvik Desai, Jeremy Soller, Lean Sheng Tan, Michał Kopeć, Michał Żygowski, Nick Vaccaro, Piotr Król, Sean Rhodes, Subrata Banik, Tim Crawford. Hello Angel Pons, Christian Walter, Dinesh Gehlot, Eric Lai, Jamie Chen, Jayvik Desai, Jeremy Soller, Lean Sheng Tan, Michał Kopeć, Michał Żygowski, Nick Vaccaro, Piotr Król, Sean Rhodes, Subrata Banik, Tim Crawford, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/85529?usp=email to look at the new patch set (#3). The following approvals got outdated and were removed: Code-Review+1 by Eric Lai, Verified+1 by build bot (Jenkins) Change subject: soc/intel/alderlake: Add a function to force disable memory channels ...................................................................... soc/intel/alderlake: Add a function to force disable memory channels Add a function `mem_init_override_channel_mask` to disable memory channels based on given bitmap where each bit represents a memory channel. A set bit will disable corresponding memory channel. Variants can override this bitmap using `variant_mem_ch_disable_mask`. BUG=b:379311559 TEST=Make sure that channel mask is applied correctly. Lotso without any channel mask: ``` lotso-rev0 ~ # dmidecode -t 17 | grep -E "(Locator: C|Size)" Size: 2 GB Locator: Channel-0-DIMM-0 Size: 2 GB Locator: Channel-1-DIMM-0 Size: 2 GB Locator: Channel-2-DIMM-0 Size: 2 GB Locator: Channel-3-DIMM-0 Size: 2 GB Locator: Channel-0-DIMM-0 Size: 2 GB Locator: Channel-1-DIMM-0 Size: 2 GB Locator: Channel-2-DIMM-0 Size: 2 GB Locator: Channel-3-DIMM-0 lotso-rev0 ~ # dmidecode -t 17 | grep Size | wc -l 8 ``` Lotso with channel 2 & 3 masked: ``` lotso-rev0 ~ # dmidecode -t 17 | grep -E "(Locator: C|Size)" Size: 2 GB Locator: Channel-0-DIMM-0 Size: 2 GB Locator: Channel-1-DIMM-0 Size: 2 GB Locator: Channel-0-DIMM-0 Size: 2 GB Locator: Channel-1-DIMM-0 lotso-rev0 ~ # dmidecode -t 17 | grep Size | wc -l 4 ``` Change-Id: Ibfeca4509cb3d88bc1bac2ac2d480e665d895bc5 Signed-off-by: Kapil Porwal <kapilporwal(a)google.com> --- M src/mainboard/aoostar/wtr_r1/romstage_fsp_params.c M src/mainboard/cwwk/adl/romstage_fsp_params.c M src/mainboard/google/brox/romstage.c M src/mainboard/google/brox/variants/baseboard/brox/memory.c M src/mainboard/google/brox/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/brya/romstage.c M src/mainboard/google/brya/variants/baseboard/brask/memory.c M src/mainboard/google/brya/variants/baseboard/brya/memory.c M src/mainboard/google/brya/variants/baseboard/hades/memory.c M src/mainboard/google/brya/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/brya/variants/baseboard/nissa/memory.c M src/mainboard/google/brya/variants/baseboard/trulo/memory.c M src/mainboard/hardkernel/odroid-h4/romstage_fsp_params.c M src/mainboard/intel/adlrvp/romstage_fsp_params.c M src/mainboard/intel/shadowmountain/romstage.c M src/mainboard/lattepanda/mu/romstage_fsp_params.c M src/mainboard/msi/ms7d25/romstage_fsp_params.c M src/mainboard/msi/ms7e06/romstage_fsp_params.c M src/mainboard/prodrive/atlas/romstage_fsp_params.c M src/mainboard/protectli/vault_adl_p/romstage_fsp_params.c M src/mainboard/starlabs/byte_adl/variants/mk_ii/romstage.c M src/mainboard/starlabs/starbook/variants/adl/romstage.c M src/mainboard/starlabs/starbook/variants/rpl/romstage.c M src/mainboard/starlabs/starfighter/variants/rpl/romstage.c M src/mainboard/starlabs/starlite_adl/variants/mk_v/romstage.c M src/mainboard/system76/adl/variants/darp8/romstage.c M src/mainboard/system76/adl/variants/galp6/romstage.c M src/mainboard/system76/adl/variants/gaze17-3050/romstage.c M src/mainboard/system76/adl/variants/gaze17-3060-b/romstage.c M src/mainboard/system76/adl/variants/lemp11/romstage.c M src/mainboard/system76/adl/variants/oryp10/romstage.c M src/mainboard/system76/adl/variants/oryp9/romstage.c M src/mainboard/system76/rpl/variants/addw3/romstage.c M src/mainboard/system76/rpl/variants/addw4/romstage.c M src/mainboard/system76/rpl/variants/bonw15/romstage.c M src/mainboard/system76/rpl/variants/darp9/romstage.c M src/mainboard/system76/rpl/variants/galp7/romstage.c M src/mainboard/system76/rpl/variants/gaze18/romstage.c M src/mainboard/system76/rpl/variants/lemp12/romstage.c M src/mainboard/system76/rpl/variants/oryp11/romstage.c M src/mainboard/system76/rpl/variants/oryp12/romstage.c M src/mainboard/system76/rpl/variants/serw13/romstage.c M src/mainboard/topton/adl/romstage_fsp_params.c M src/soc/intel/alderlake/include/soc/meminit.h M src/soc/intel/alderlake/meminit.c 45 files changed, 182 insertions(+), 40 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/29/85529/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/85529?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: Ibfeca4509cb3d88bc1bac2ac2d480e665d895bc5 Gerrit-Change-Number: 85529 Gerrit-PatchSet: 3 Gerrit-Owner: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Jamie Chen <jamie.chen(a)intel.corp-partner.google.com> Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com> Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Crawford <tcrawford(a)system76.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Jamie Chen <jamie.chen(a)intel.corp-partner.google.com> Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Attention: Jayvik Desai <jayvik(a)google.com> Gerrit-Attention: Tim Crawford <tcrawford(a)system76.com> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>

1 0

[M] Change in coreboot[main]: soc/intel/alderlake: Add a function to force disable memory channels
by Kapil Porwal (Code Review) 09 Dec '24

09 Dec '24

Attention is currently required from: Angel Pons, Christian Walter, Dinesh Gehlot, Eric Lai, Jamie Chen, Jayvik Desai, Jeremy Soller, Lean Sheng Tan, Michał Kopeć, Michał Żygowski, Nick Vaccaro, Piotr Król, Sean Rhodes, Subrata Banik, Tim Crawford. Kapil Porwal has posted comments on this change by Kapil Porwal. ( https://review.coreboot.org/c/coreboot/+/85529?usp=email ) Change subject: soc/intel/alderlake: Add a function to force disable memory channels ...................................................................... Patch Set 2: (5 comments) File src/soc/intel/alderlake/meminit.c: https://review.coreboot.org/c/coreboot/+/85529/comment/405f6b22_e91d85c4?us… : PS1, Line 235: > can you please write a help text for easy explanation of mem_init_override_channel_mask and ch_disab […] Acknowledged https://review.coreboot.org/c/coreboot/+/85529/comment/c7c2dd62_46fe2e05?us… : PS1, Line 236: static void mem_init_override_channel_mask(FSP_M_CONFIG *mem_cfg, : uint32_t ch_disable_mask) > ``` […] Acknowledged https://review.coreboot.org/c/coreboot/+/85529/comment/4e6c32c5_3f41db65?us… : PS1, Line 252: if (ch_disable_mask == 0) { > > Do we have any invalid combination here? Like we must have Mc0Ch0 at least? Or you cann't disable […] Agreed. Added a check for Mc0Ch0. @jamie.chen@intel.corp-partner.google.com could you please let us know if there are any other constraints? https://review.coreboot.org/c/coreboot/+/85529/comment/673d828e_987a0dfd?us… : PS1, Line 264: void memcfg_init(FSPM_UPD *memupd, const struct mb_cfg *mb_cfg, : const struct mem_spd *spd_info, bool half_populated, : uint32_t ch_disable_mask) > How about: […] There are multiple ways to implement the solution - 1. patchset#1 - This way is consistent with how we pass data from mainboard code to soc APIs but it involves changes to all the ADL based mainbaords/variants. 2. Override API guarded with Kconfig option (suggested by Subrata) - It keeps the SoC layer API intact and involve changes to the concerned variant only. 3. Create multiple prototypes of memcfg_init API (suggested by Angel) - It changes the SoC layer API along with changes to the concerned mainboard & variant. 4. Export the new function and directly call it from mainboard/variant code (suggested by Angel) - It Adds one more API along with changes to the concerned mainboard & variant. I like #2 & #4, but #2 keeps the changes to minimal. Please share your thoughts. https://review.coreboot.org/c/coreboot/+/85529/comment/cd228f7d_0374a57e?us… : PS1, Line 315: mem_init_override_channel_mask(mem_cfg, ch_disable_mask); > Could this be called at the very end of the function? If so, you could export this function (make it non-static) and call it from mainboards. Yes, it is doable. Please refer the comment above. -- To view, visit https://review.coreboot.org/c/coreboot/+/85529?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: Ibfeca4509cb3d88bc1bac2ac2d480e665d895bc5 Gerrit-Change-Number: 85529 Gerrit-PatchSet: 2 Gerrit-Owner: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Jamie Chen <jamie.chen(a)intel.corp-partner.google.com> Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com> Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Crawford <tcrawford(a)system76.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Jamie Chen <jamie.chen(a)intel.corp-partner.google.com> Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Attention: Jayvik Desai <jayvik(a)google.com> Gerrit-Attention: Tim Crawford <tcrawford(a)system76.com> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Comment-Date: Mon, 09 Dec 2024 12:15:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Eric Lai <ericllai(a)google.com> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>

1 0

[L] Change in coreboot[main]: soc/mediatek/mt8196: Add mtcmos init support
by Jarried Lin (Code Review) 09 Dec '24

09 Dec '24

Attention is currently required from: Elyes Haouas, Hung-Te Lin, Jarried Lin, Paul Menzel, Yu-Ping Wu. Hello Guangjie Song, Hung-Te Lin, Yidi Lin, Yu-Ping Wu, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/84497?usp=email to look at the new patch set (#38). The following approvals got outdated and were removed: Code-Review+1 by Yu-Ping Wu, Verified+1 by build bot (Jenkins) Change subject: soc/mediatek/mt8196: Add mtcmos init support ...................................................................... soc/mediatek/mt8196: Add mtcmos init support Add mtcmos init code and APIs for controlling power domain. TEST=build pass and driver init ok BUG=b:317009620 Signed-off-by: Guangjie Song <guangjie.song(a)mediatek.corp-partner.google.com> Change-Id: I44f2bb10453377a8412e80ac0c100760ebfbaff9 --- M src/soc/mediatek/mt8196/Makefile.mk M src/soc/mediatek/mt8196/bootblock.c M src/soc/mediatek/mt8196/include/soc/memlayout.ld A src/soc/mediatek/mt8196/include/soc/spm_mtcmos.h A src/soc/mediatek/mt8196/mtcmos.c 5 files changed, 839 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/97/84497/38 -- To view, visit https://review.coreboot.org/c/coreboot/+/84497?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: I44f2bb10453377a8412e80ac0c100760ebfbaff9 Gerrit-Change-Number: 84497 Gerrit-PatchSet: 38 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: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-CC: Guangjie Song <guangjie.song(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: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr>

1 0

[L] Change in coreboot[main]: Add support for MiTAC Computing Whitestone-2 mainboard
by Mark Chang (Code Review) 09 Dec '24

09 Dec '24

Attention is currently required from: Angel Pons, Angel Pons, Arthur Heymans, Christian Walter, Jincheng Li, Lean Sheng Tan, Patrick Rudolph, Shuo Liu, yuchi.chen(a)intel.com. Mark Chang has posted comments on this change by Mark Chang. ( https://review.coreboot.org/c/coreboot/+/85532?usp=email ) Change subject: Add support for MiTAC Computing Whitestone-2 mainboard ...................................................................... Patch Set 1: (10 comments) File configs/config.mitaccomputing_ws_2: https://review.coreboot.org/c/coreboot/+/85532/comment/cd2aab08_34d6c43d?us… : PS1, Line 15: CONFIG_X2APIC_LATE_WORKAROUND=y > Is this needed for all SPR-SP mainboards? We only verified Intel 5433N on our Whitestone-2. File src/mainboard/mitaccomputing/whitestone-2/acpi/platform.asl: https://review.coreboot.org/c/coreboot/+/85532/comment/61dcd260_63f9fb2f?us… : PS1, Line 4: #include <soc/intel/common/acpi/acpi_wake_source.asl> > Not needed for this? Will remove it. https://review.coreboot.org/c/coreboot/+/85532/comment/7ec3782a_4900d4dd?us… : PS1, Line 13: } > #include <arch/x86/post. […] Add "#include <arch/x86/acpi/post.asl> will trigger build error. https://review.coreboot.org/c/coreboot/+/85532/comment/723f114e_27e9d7de?us… : PS1, Line 30: > I think these methods might be needed for ASL to compile, but I would prefer removing them if possib […] We will remove it. File src/mainboard/mitaccomputing/whitestone-2/board_info.txt: PS1: > nit: Missing `ROM package` Update ROM package: SOIC-16 File src/mainboard/mitaccomputing/whitestone-2/bootblock.c: https://review.coreboot.org/c/coreboot/+/85532/comment/10278896_4fe09c3b?us… : PS1, Line 18: static void enable_espi_lpc_io_windows(void) > I think that library is for ramstage. Also, other Xeon-SP boards do this. […] We could use below APIs to update it. - lpc_open_pmio_window(0x2e, 2); - lpc_open_pmio_window(0x3f8, 8); - lpc_io_setup_comm_a_b(); https://review.coreboot.org/c/coreboot/+/85532/comment/a71aef7a_0ae18f50?us… : PS1, Line 23: * For that end it is wired into BMC virtual port. > Is SUART2 has s port IO map as SUART1? The Whitestone-2 only use SUART1. - lpc_open_pmio_window(0x2e, 2); - lpc_open_pmio_window(0x3f8, 8); - lpc_io_setup_comm_a_b(); File src/mainboard/mitaccomputing/whitestone-2/dsdt.asl: https://review.coreboot.org/c/coreboot/+/85532/comment/f5d32fb3_43aec3a9?us… : PS1, Line 10: 0x20110725 // OEM revision > The "OEM revision" comment means this number was chosen to match that of vendor firmware (BIOS/UEFI) […] Remove it File src/mainboard/mitaccomputing/whitestone-2/mainboard.c: https://review.coreboot.org/c/coreboot/+/85532/comment/b0d3b80f_f03df174?us… : PS1, Line 80: }; > It's invoked automatically Angel, you're right, it's invoked automatically. File src/mainboard/mitaccomputing/whitestone-2/romstage.c: https://review.coreboot.org/c/coreboot/+/85532/comment/40a48d72_77615380?us… : PS1, Line 18: Protocl > typo […] resolved -- To view, visit https://review.coreboot.org/c/coreboot/+/85532?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: Icf625cf8e9c76ef08411614c15ee43d0c459b905 Gerrit-Change-Number: 85532 Gerrit-PatchSet: 1 Gerrit-Owner: Mark Chang <mark.chang(a)mitaccomputing.com> Gerrit-Reviewer: Angel Pons <angel.pons(a)9elements.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: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: yuchi.chen(a)intel.com Gerrit-CC: Wilson-MiTACComputing <wilson.chien(a)mitaccomputing.com> Gerrit-Attention: yuchi.chen(a)intel.com Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Angel Pons <angel.pons(a)9elements.com> Gerrit-Comment-Date: Mon, 09 Dec 2024 10:18:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: yuchi.chen(a)intel.com Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Shuo Liu <shuo.liu(a)intel.com>

1 0

[XS] Change in coreboot[main]: soc/intel/pantherlake: Remove hardcoded value for child nodes
by Subrata Banik (Code Review) 09 Dec '24

09 Dec '24

Attention is currently required from: Kapil Porwal, Pranava Y N, Sowmya Aralguppe. Subrata Banik has posted comments on this change by Sowmya Aralguppe. ( https://review.coreboot.org/c/coreboot/+/85531?usp=email ) Change subject: soc/intel/pantherlake: Remove hardcoded value for child nodes ...................................................................... Patch Set 4: (3 comments) File src/soc/intel/pantherlake/crashlog.c: https://review.coreboot.org/c/coreboot/+/85531/comment/bb2853f7_adbae9d1?us… : PS4, Line 319: cpu_cl_disc_tab.header.fields.count = CRASHLOG_NODES_COUNT; I'm wondering why the previously discussed items are now being ignored. Earlier, you were hardcoding the value to limit the child nodes to 2 (PUNIT & Uncore of SoC-N Die). This was done because we had seen issues where without the hardcoding we were reading some other junk nodes that were not capable of reporting the crashlog and we were seeing crashes/timeouts. ``` /* Crashlog record of PUNIT & Uncore of SoC-N Die */ ``` https://review.coreboot.org/c/coreboot/+/83106/ https://review.coreboot.org/c/coreboot/+/85531/comment/12602a16_2b08c275?us… : PS4, Line 316: printk(BIOS_DEBUG, "cpu_crashlog_discovery_table buffer count: 0x%x\n", : cpu_cl_disc_tab.header.fields.count); what is the point of showing buffer count? earlier, it was linked here due to overriding ``` cpu_cl_disc_tab.header.fields.count = CRASHLOG_NODES_COUNT; ``` I'm afraid we will go back to the previous problem which was fixed with https://review.coreboot.org/c/coreboot/+/83106/26..29. otherwise there was no need to hardcode fixed value https://b.corp.google.com/issues/346676856#comment33 https://review.coreboot.org/c/coreboot/+/85531/comment/b7c6a4ef_5ef4b22f?us… : PS4, Line 319: u32 cur_offset = 0; why we are changing this ? can still stays at line 318 -- To view, visit https://review.coreboot.org/c/coreboot/+/85531?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: I209366d324c95b7a32afdcfb792c34d927a0508e Gerrit-Change-Number: 85531 Gerrit-PatchSet: 4 Gerrit-Owner: Sowmya Aralguppe <sowmya.aralguppe(a)intel.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-CC: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-CC: Vikrant L Jadeja <vikrant.l.jadeja(a)intel.com> Gerrit-Attention: Sowmya Aralguppe <sowmya.aralguppe(a)intel.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Pranava Y N <pranavayn(a)google.com> Gerrit-Comment-Date: Mon, 09 Dec 2024 10:04:32 +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 December 2024