coreboot-gerrit November 2024

coreboot-gerrit@coreboot.org

1 participants
2518 discussions

[XL] Change in coreboot[main]: soc/mediatek/mt8196: Add PLL and Clock init support
by Yidi Lin (Code Review) 04 Nov '24

04 Nov '24

Attention is currently required from: 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/+/84495?usp=email ) Change subject: soc/mediatek/mt8196: Add PLL and Clock init support ...................................................................... Patch Set 26: (2 comments) File src/soc/mediatek/mt8196/pll.c: https://review.coreboot.org/c/coreboot/+/84495/comment/ebf3f828_293d31f5?us… : PS13, Line 1196: 30 > Alternatively, most of the `mtcmos_idx` values are `PM_DUMMY1`. Can we store only the two entries […] Agree with Yu-Ping. We can default set `cg_idx` 0 ~ 49 to`PM_DUMMY1` and subsequently set above two entries. File src/soc/mediatek/mt8196/pll.c: https://review.coreboot.org/c/coreboot/+/84495/comment/8e2b1ba3_7e8d5bc8?us… : PS26, Line 1176: mmvote_mtcmos_mtcmos_table `child_idx` is the same as `parent_idx`. Is is expected ? Can we just keep either one ? -- To view, visit https://review.coreboot.org/c/coreboot/+/84495?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: Icac99fb210c87c8b7b14af627fbd2f14e4c47240 Gerrit-Change-Number: 84495 Gerrit-PatchSet: 26 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> 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-Comment-Date: Mon, 04 Nov 2024 02:48:54 +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: Yu-Ping Wu <yupingso(a)google.com> Comment-In-Reply-To: Jarried Lin <jarried.lin(a)mediatek.com>

1 0

[XL] Change in coreboot[main]: soc/mediatek/mt8196: Add PLL and Clock init support
by Yu-Ping Wu (Code Review) 04 Nov '24

04 Nov '24

Attention is currently required from: Hung-Te Lin, Jarried Lin, Paul Menzel, Yidi Lin. Yu-Ping Wu has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/84495?usp=email ) Change subject: soc/mediatek/mt8196: Add PLL and Clock init support ...................................................................... Patch Set 26: (3 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/84495/comment/30a5ac1c_abb1987a?us… : PS26, Line 7: Clock clock File src/soc/mediatek/mt8196/pll.c: https://review.coreboot.org/c/coreboot/+/84495/comment/b22855d3_da165ade?us… : PS13, Line 1196: 30 > @guangjie.song@mediatek.corp-partner.google.com […] Alternatively, most of the `mtcmos_idx` values are `PM_DUMMY1`. Can we store only the two entries ``` { CG_VDEC_SOC_GCON_1, PM_VDE_VCORE0 }, { CG_VDEC_SOC_GCON_1, PM_VDE0 }, ``` in the array, to save some space for the `.rodata` section? The complete array can be constructed in runtime in `mmvote_init()`. The same approach could be applied to `vote_cg_mtcmos_table` as well. File src/soc/mediatek/mt8196/pll.c: https://review.coreboot.org/c/coreboot/+/84495/comment/eeb204c9_36437830?us… : PS26, Line 863: ( No need for the extra `()` -- To view, visit https://review.coreboot.org/c/coreboot/+/84495?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: Icac99fb210c87c8b7b14af627fbd2f14e4c47240 Gerrit-Change-Number: 84495 Gerrit-PatchSet: 26 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> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Comment-Date: Mon, 04 Nov 2024 02:16:56 +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: Jarried Lin <jarried.lin(a)mediatek.com> Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com>

1 0

[XS] Change in coreboot[main]: mb/google/fatcat/var/francka: Generate RAM ID for MT62F2G32D4DS-020 WT:F
by Eric Lai (Code Review) 04 Nov '24

04 Nov '24

Eric Lai has submitted this change. ( https://review.coreboot.org/c/coreboot/+/84781?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: mb/google/fatcat/var/francka: Generate RAM ID for MT62F2G32D4DS-020 WT:F ...................................................................... mb/google/fatcat/var/francka: Generate RAM ID for MT62F2G32D4DS-020 WT:F Add Micron part MT62F2G32D4DS-020 WT:F only for Francka. DRAM Part Name ID to assign MT62F2G32D4DS-020 WT:F 0 (0000) BUG=b:373394046 TEST=emerge-fatcat coreboot Change-Id: I2de56c8c7a028edefbd3dc53f8b1e26dee3286f7 Signed-off-by: Amanda Huang <amanda_hwang(a)compal.corp-partner.google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/84781 Reviewed-by: Pranava Y N <pranavayn(a)google.com> Reviewed-by: Subrata Banik <subratabanik(a)google.com> Reviewed-by: Eric Lai <ericllai(a)google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/mainboard/google/fatcat/variants/francka/memory/Makefile.mk M src/mainboard/google/fatcat/variants/francka/memory/dram_id.generated.txt M src/mainboard/google/fatcat/variants/francka/memory/mem_parts_used.txt 3 files changed, 6 insertions(+), 3 deletions(-) Approvals: build bot (Jenkins): Verified Pranava Y N: Looks good to me, approved Subrata Banik: Looks good to me, approved Eric Lai: Looks good to me, approved diff --git a/src/mainboard/google/fatcat/variants/francka/memory/Makefile.mk b/src/mainboard/google/fatcat/variants/francka/memory/Makefile.mk index 5e16e25..62c45356 100644 --- a/src/mainboard/google/fatcat/variants/francka/memory/Makefile.mk +++ b/src/mainboard/google/fatcat/variants/francka/memory/Makefile.mk @@ -1,6 +1,7 @@ # SPDX-License-Identifier: GPL-2.0-or-later # This is an auto-generated file. Do not edit!! # Generated by: -# ./util/spd_tools/bin/part_id_gen PTL lp5 src/mainboard/google/fatcat/variants/fatcat/memory src/mainboard/google/fatcat/variants/fatcat/memory/mem_parts_used.txt +# util/spd_tools/bin/part_id_gen ptl lp5 src/mainboard/google/fatcat/variants/francka/memory/ src/mainboard/google/fatcat/variants/francka/memory/mem_parts_used.txt -SPD_SOURCES = placeholder +SPD_SOURCES = +SPD_SOURCES += spd/lp5/set-0/spd-10.hex # ID = 0(0b0000) Parts = MT62F2G32D4DS-020 WT:F diff --git a/src/mainboard/google/fatcat/variants/francka/memory/dram_id.generated.txt b/src/mainboard/google/fatcat/variants/francka/memory/dram_id.generated.txt index cc0bdf1..31165c4 100644 --- a/src/mainboard/google/fatcat/variants/francka/memory/dram_id.generated.txt +++ b/src/mainboard/google/fatcat/variants/francka/memory/dram_id.generated.txt @@ -1,6 +1,7 @@ # SPDX-License-Identifier: GPL-2.0-or-later # This is an auto-generated file. Do not edit!! # Generated by: -# ./util/spd_tools/bin/part_id_gen PTL lp5 src/mainboard/google/fatcat/variants/fatcat/memory src/mainboard/google/fatcat/variants/fatcat/memory/mem_parts_used.txt +# util/spd_tools/bin/part_id_gen ptl lp5 src/mainboard/google/fatcat/variants/francka/memory/ src/mainboard/google/fatcat/variants/francka/memory/mem_parts_used.txt DRAM Part Name ID to assign +MT62F2G32D4DS-020 WT:F 0 (0000) diff --git a/src/mainboard/google/fatcat/variants/francka/memory/mem_parts_used.txt b/src/mainboard/google/fatcat/variants/francka/memory/mem_parts_used.txt index 2499005..e6822f6 100644 --- a/src/mainboard/google/fatcat/variants/francka/memory/mem_parts_used.txt +++ b/src/mainboard/google/fatcat/variants/francka/memory/mem_parts_used.txt @@ -9,3 +9,4 @@ # See util/spd_tools/README.md for more details and instructions. # Part Name +MT62F2G32D4DS-020 WT:F -- To view, visit https://review.coreboot.org/c/coreboot/+/84781?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: I2de56c8c7a028edefbd3dc53f8b1e26dee3286f7 Gerrit-Change-Number: 84781 Gerrit-PatchSet: 6 Gerrit-Owner: Amanda Hwang <amanda_hwang(a)compal.corp-partner.google.com> Gerrit-Reviewer: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com> Gerrit-Reviewer: Eric Lai <ericllai(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: Ian Feng <ian_feng(a)compal.corp-partner.google.com>

1 0

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

04 Nov '24

Attention is currently required from: Hung-Te Lin, Jarried Lin, 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 26: (14 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/84497/comment/a185912a_693f90f1?us… : PS18, Line 9: Add mtcmos init code and APIs for controlling power domain > Add `.` at the end. Done File src/soc/mediatek/mt8196/mtcmos.c: https://review.coreboot.org/c/coreboot/+/84497/comment/f01d3adc_3b8f8420?us… : PS18, Line 155: {NULL, NULL} > `{ NULL, NULL }` Done https://review.coreboot.org/c/coreboot/+/84497/comment/b4855dfe_b88df54b?us… : PS18, Line 428: if (cb->id == id) { > Is it possible to have multiple `cb` with this id? yes, users call register different cb with the same id. https://review.coreboot.org/c/coreboot/+/84497/comment/723842ff_f728f051?us… : PS18, Line 508: return -ETIMEDOUT; > Print an error message? Done https://review.coreboot.org/c/coreboot/+/84497/comment/f88888e1_aa298921?us… : PS18, Line 518: if (!mask) > How's this possible? Is your concern that `shift` might be too large? Done https://review.coreboot.org/c/coreboot/+/84497/comment/ca1c96c5_478c909a?us… : PS18, Line 528: bp_sta > Doesn't need this variable. Done https://review.coreboot.org/c/coreboot/+/84497/comment/d2e41553_1de40011?us… : PS18, Line 546: end_step > Just write `bp_num - 1` here. It's clear what that means. […] Done https://review.coreboot.org/c/coreboot/+/84497/comment/293b5f1f_98fb807e?us… : PS18, Line 550: mtcmos_bus_prot_ctrl(&bp_table[step_idx], !set); > Print an error (or warning) here? Done https://review.coreboot.org/c/coreboot/+/84497/comment/7d2775c5_60109620?us… : PS18, Line 615: printk(BIOS_ERR, "mtcmos_pre_off_callback failed(%d)\n", ret); > Can we move the error messages *inside* `mtcmos_pre_off_callback` and other functions, so that we do […] Done https://review.coreboot.org/c/coreboot/+/84497/comment/20766926_6596c758?us… : PS18, Line 714: vote_mds[id] > Make it a local variable of const pointer. […] Done https://review.coreboot.org/c/coreboot/+/84497/comment/f73885b8_d95aa7ab?us… : PS18, Line 729: 20000 > Define a constant (either local const variable or macro)? Done https://review.coreboot.org/c/coreboot/+/84497/comment/5d29fe5e_7b445705?us… : PS18, Line 824: .name = "vdisp" > Add `,` Done https://review.coreboot.org/c/coreboot/+/84497/comment/6d475759_0c2b5e7a?us… : PS18, Line 850: assert(!mtcmos_onoff(id, state)); > Don't put function calls inside `assert()`. It may not be evaluated. […] Done https://review.coreboot.org/c/coreboot/+/84497/comment/dd5b412e_f9caed8a?us… : PS18, Line 921: isolationoff > Do you mean `isolation off`? 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: 26 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> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Comment-Date: Mon, 04 Nov 2024 02:01:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com>

1 0

[XS] Change in coreboot[main]: mb/google/fatcat/var/francka: Generate RAM ID for MT62F2G32D4DS-020 WT:F
by Amanda Hwang (Code Review) 04 Nov '24

04 Nov '24

Attention is currently required from: Dtrain Hsu. Amanda Hwang has posted comments on this change by Amanda Hwang. ( https://review.coreboot.org/c/coreboot/+/84781?usp=email ) Change subject: mb/google/fatcat/var/francka: Generate RAM ID for MT62F2G32D4DS-020 WT:F ...................................................................... Patch Set 5: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/84781/comment/4c9927ec_d1f75450?us… : PS4, Line 7: MT62F2G32D4DS-020 WT:F > remove this, and we added in comment should be fine. Done -- To view, visit https://review.coreboot.org/c/coreboot/+/84781?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: I2de56c8c7a028edefbd3dc53f8b1e26dee3286f7 Gerrit-Change-Number: 84781 Gerrit-PatchSet: 5 Gerrit-Owner: Amanda Hwang <amanda_hwang(a)compal.corp-partner.google.com> Gerrit-Reviewer: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com> Gerrit-Reviewer: Eric Lai <ericllai(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: Ian Feng <ian_feng(a)compal.corp-partner.google.com> Gerrit-Attention: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com> Gerrit-Comment-Date: Mon, 04 Nov 2024 02:00:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Eric Lai <ericllai(a)google.com>

1 0

[XS] Change in coreboot[main]: mb/google/fatcat/var/francka: Generate RAM ID for MT62F2G32D4DS-020 WT:F
by Amanda Hwang (Code Review) 04 Nov '24

04 Nov '24

Attention is currently required from: Amanda Hwang, Dtrain Hsu. Hello Dtrain Hsu, Eric Lai, Pranava Y N, Subrata Banik, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/84781?usp=email to look at the new patch set (#5). Change subject: mb/google/fatcat/var/francka: Generate RAM ID for MT62F2G32D4DS-020 WT:F ...................................................................... mb/google/fatcat/var/francka: Generate RAM ID for MT62F2G32D4DS-020 WT:F Add Micron part MT62F2G32D4DS-020 WT:F only for Francka. DRAM Part Name ID to assign MT62F2G32D4DS-020 WT:F 0 (0000) BUG=b:373394046 TEST=emerge-fatcat coreboot Change-Id: I2de56c8c7a028edefbd3dc53f8b1e26dee3286f7 Signed-off-by: Amanda Huang <amanda_hwang(a)compal.corp-partner.google.com> --- M src/mainboard/google/fatcat/variants/francka/memory/Makefile.mk M src/mainboard/google/fatcat/variants/francka/memory/dram_id.generated.txt M src/mainboard/google/fatcat/variants/francka/memory/mem_parts_used.txt 3 files changed, 6 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/81/84781/5 -- To view, visit https://review.coreboot.org/c/coreboot/+/84781?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: I2de56c8c7a028edefbd3dc53f8b1e26dee3286f7 Gerrit-Change-Number: 84781 Gerrit-PatchSet: 5 Gerrit-Owner: Amanda Hwang <amanda_hwang(a)compal.corp-partner.google.com> Gerrit-Reviewer: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com> Gerrit-Reviewer: Eric Lai <ericllai(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: Ian Feng <ian_feng(a)compal.corp-partner.google.com> Gerrit-Attention: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com> Gerrit-Attention: Amanda Hwang <amanda_hwang(a)compal.corp-partner.google.com>

1 0

[XL] Change in coreboot[main]: soc/mediatek/mt8196: Add PLL and Clock init support
by Guangjie Song (Code Review) 04 Nov '24

04 Nov '24

Attention is currently required from: Hung-Te Lin, Jarried 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/+/84495?usp=email ) Change subject: soc/mediatek/mt8196: Add PLL and Clock init support ...................................................................... Patch Set 26: (1 comment) File src/soc/mediatek/mt8196/pll.c: https://review.coreboot.org/c/coreboot/+/84495/comment/85bf3d2d_47c3f29e?us… : PS13, Line 1196: 30 > @guangjie.song@mediatek.corp-partner.google.com […] "cg_idx" is pre-assigned and has a 1:1 mapping relationship with the clock control register. Values of cg_idx beyond 30 are not mapped to specific clock control registers, so they have no practical significance and therefore no macros are defined to match them. -- To view, visit https://review.coreboot.org/c/coreboot/+/84495?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: Icac99fb210c87c8b7b14af627fbd2f14e4c47240 Gerrit-Change-Number: 84495 Gerrit-PatchSet: 26 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> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Comment-Date: Mon, 04 Nov 2024 01:58:23 +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: Jarried Lin <jarried.lin(a)mediatek.com> Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com>

1 0

[XS] Change in coreboot[main]: mb/google/fatcat/var/francka: Generate RAM ID for Micron MT62F2G32D4D...
by Eric Lai (Code Review) 04 Nov '24

04 Nov '24

Attention is currently required from: Amanda Hwang, Dtrain Hsu. Eric Lai has posted comments on this change by Amanda Hwang. ( https://review.coreboot.org/c/coreboot/+/84781?usp=email ) The change is no longer submittable: All-Comments-Resolved is unsatisfied now. Change subject: mb/google/fatcat/var/francka: Generate RAM ID for Micron MT62F2G32D4DS-020 WT:F ...................................................................... Patch Set 4: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/84781/comment/7c1fbc9d_6375c74e?us… : PS4, Line 7: MT62F2G32D4DS-020 WT:F remove this, and we added in comment should be fine. -- To view, visit https://review.coreboot.org/c/coreboot/+/84781?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: I2de56c8c7a028edefbd3dc53f8b1e26dee3286f7 Gerrit-Change-Number: 84781 Gerrit-PatchSet: 4 Gerrit-Owner: Amanda Hwang <amanda_hwang(a)compal.corp-partner.google.com> Gerrit-Reviewer: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com> Gerrit-Reviewer: Eric Lai <ericllai(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: Ian Feng <ian_feng(a)compal.corp-partner.google.com> Gerrit-Attention: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com> Gerrit-Attention: Amanda Hwang <amanda_hwang(a)compal.corp-partner.google.com> Gerrit-Comment-Date: Mon, 04 Nov 2024 01:58:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[XS] Change in coreboot[main]: mb/google/fatcat/var/francka: Generate RAM ID for Micron MT62F2G32D4D...
by Eric Lai (Code Review) 04 Nov '24

04 Nov '24

Attention is currently required from: Amanda Hwang, Dtrain Hsu. Eric Lai has posted comments on this change by Amanda Hwang. ( https://review.coreboot.org/c/coreboot/+/84781?usp=email ) Change subject: mb/google/fatcat/var/francka: Generate RAM ID for Micron MT62F2G32D4DS-020 WT:F ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/84781?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: I2de56c8c7a028edefbd3dc53f8b1e26dee3286f7 Gerrit-Change-Number: 84781 Gerrit-PatchSet: 4 Gerrit-Owner: Amanda Hwang <amanda_hwang(a)compal.corp-partner.google.com> Gerrit-Reviewer: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com> Gerrit-Reviewer: Eric Lai <ericllai(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: Ian Feng <ian_feng(a)compal.corp-partner.google.com> Gerrit-Attention: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com> Gerrit-Attention: Amanda Hwang <amanda_hwang(a)compal.corp-partner.google.com> Gerrit-Comment-Date: Mon, 04 Nov 2024 01:57:05 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[XS] Change in coreboot[main]: soc/mediatek/mt8188/spi: Fix out-of-bound array access for pad_funcs
by Yu-Ping Wu (Code Review) 04 Nov '24

04 Nov '24

Yu-Ping Wu has submitted this change. ( https://review.coreboot.org/c/coreboot/+/84950?usp=email ) Change subject: soc/mediatek/mt8188/spi: Fix out-of-bound array access for pad_funcs ...................................................................... soc/mediatek/mt8188/spi: Fix out-of-bound array access for pad_funcs The size of the inner array of the 2-dimensional array pad_funcs should be 4 instead of SPI_BUS_NUMBER (6). This bug leads to two extra gpio_set_mode() calls with unexpected GPIOs. Inspecting spi.o, the data immediately after the .rodata.pad_funcs section is .rodata.spi_ctrlr_bus_map, with the following data: 00000428 00 00 00 00 00 00 00 00 00 00 00 00 05 00 00 00 00000438 00 00 00 00 00 00 00 00 ... This is equivalent to the following calls: gpio_set_mode(GPIO(GPIO05), 0); gpio_set_mode(GPIO(GPIO00), 0); The second call is already included in the pad_funcs array, so the first call is the only practical impact of this bug. Change-Id: I9c44f09b3cdadbbf039b95efca7144f213672092 Signed-off-by: Yu-Ping Wu <yupingso(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/coreboot/+/84950 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Yidi Lin <yidilin(a)google.com> Reviewed-by: Hung-Te Lin <hungte(a)chromium.org> Reviewed-by: Paul Menzel <paulepanter(a)mailbox.org> --- M src/soc/mediatek/mt8188/spi.c 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Paul Menzel: Looks good to me, but someone else must approve Hung-Te Lin: Looks good to me, approved Yidi Lin: Looks good to me, approved build bot (Jenkins): Verified diff --git a/src/soc/mediatek/mt8188/spi.c b/src/soc/mediatek/mt8188/spi.c index 994663d..66fbf0f 100644 --- a/src/soc/mediatek/mt8188/spi.c +++ b/src/soc/mediatek/mt8188/spi.c @@ -114,7 +114,7 @@ ptr = pad_funcs[bus]; - for (unsigned int i = 0; i < SPI_BUS_NUMBER; i++) + for (unsigned int i = 0; i < ARRAY_SIZE(pad_funcs[0]); i++) gpio_set_mode(ptr[i].gpio, ptr[i].func); } -- To view, visit https://review.coreboot.org/c/coreboot/+/84950?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: I9c44f09b3cdadbbf039b95efca7144f213672092 Gerrit-Change-Number: 84950 Gerrit-PatchSet: 2 Gerrit-Owner: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.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>

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2024