coreboot-gerrit January 2025

coreboot-gerrit@coreboot.org

1 participants
3269 discussions

[XS] Change in coreboot[main]: amdfwtool: Set address mode as relative table
by Bao Zheng (Code Review) 19 Jan '25

19 Jan '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/+/84530?usp=email ) Change subject: amdfwtool: Set address mode as relative table ...................................................................... Patch Set 9: (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/84530/comment/f7057ad9_0c9da306?us… : PS8, Line 12: the entry address mode is not set. That is not correct. > maybe i misread the code, but isn't the entry address mode set accordingly if the table address mode […] Address mode 1, which is relative to BIOS. https://review.coreboot.org/c/coreboot/+/84530/comment/f1f308c1_23e31f3f?us… : PS8, Line 12: the entry address mode is not set. That is not correct. > maybe i misread the code, but isn't the entry address mode set accordingly if the table address mode […] This attribute is ignored if the directory address mode is 1 (not 2 or 3). But we can use this "ignored" bit field. We can transfer the information for table header to entries. It doesn't affect any old and later family. -- To view, visit https://review.coreboot.org/c/coreboot/+/84530?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: I156b315d350d9e7217afc7442ca80277bb7f9095 Gerrit-Change-Number: 84530 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: 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: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Zheng Bao Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Mon, 20 Jan 2025 06:22:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>

1 0

[S] Change in coreboot[main]: soc/mediatek/mt8196: Rename mtk_pwrsel.* to pwrsel.*
by Yidi Lin (Code Review) 19 Jan '25

19 Jan '25

Attention is currently required from: Hung-Te Lin, Yu-Ping Wu. Yidi Lin has posted comments on this change by Yu-Ping Wu. ( https://review.coreboot.org/c/coreboot/+/86059?usp=email ) Change subject: soc/mediatek/mt8196: Rename mtk_pwrsel.* to pwrsel.* ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/86059?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: I28131d44067c33b5d8682a85cc8a73fc42604de3 Gerrit-Change-Number: 86059 Gerrit-PatchSet: 2 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> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Comment-Date: Mon, 20 Jan 2025 05:11:24 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[XS] Change in coreboot[main]: mb/google/rauru: Add DVFS support in romstage
by Jarried Lin (Code Review) 19 Jan '25

19 Jan '25

Attention is currently required from: Yu-Ping Wu. Jarried Lin has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/86042?usp=email ) Change subject: mb/google/rauru: Add DVFS support in romstage ...................................................................... Patch Set 4: (1 comment) This change is ready for review. Commit Message: https://review.coreboot.org/c/coreboot/+/86042/comment/10a27e7c_0df0f4ed?us… : PS3, Line 7: dvfs > DVFS Done -- To view, visit https://review.coreboot.org/c/coreboot/+/86042?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: I6f290946365b4c5a650651ebea30ffc76583d2b2 Gerrit-Change-Number: 86042 Gerrit-PatchSet: 4 Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Comment-Date: Mon, 20 Jan 2025 04:22:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com>

1 0

[S] Change in coreboot[main]: soc/mediatek/mt8196: Add DVFS driver
by Jarried Lin (Code Review) 19 Jan '25

19 Jan '25

Attention is currently required from: Hung-Te Lin, Yidi Lin, Yu-Ping Wu. Jarried Lin has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/86041?usp=email ) Change subject: soc/mediatek/mt8196: Add DVFS driver ...................................................................... Patch Set 3: (6 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/86041/comment/8cb84e45_6e05c234?us… : PS2, Line 7: dvfs > DVFS Done File src/soc/mediatek/mt8196/Makefile.mk: https://review.coreboot.org/c/coreboot/+/86041/comment/1bf29376_0a1d083e?us… : PS2, Line 43: romstage-y += mtk_dvfs.c > Rename and sort this. Done File src/soc/mediatek/mt8196/include/soc/mtk_dvfs.h: PS2: > Rename to dvfs. […] Done https://review.coreboot.org/c/coreboot/+/86041/comment/ba06055d_0ef63d57?us… : PS2, Line 3: MTK_DVFS > DVFS Done File src/soc/mediatek/mt8196/mtk_dvfs.c: PS2: > Rename dvfs. […] Done https://review.coreboot.org/c/coreboot/+/86041/comment/fad2825a_db76d474?us… : PS2, Line 11: 0x01 > `BIT(0)` Done -- To view, visit https://review.coreboot.org/c/coreboot/+/86041?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: I92b7c57ad8c3d9e9954f02a08954939f45c5e2c2 Gerrit-Change-Number: 86041 Gerrit-PatchSet: 3 Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-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-Comment-Date: Mon, 20 Jan 2025 04:22:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com>

1 0

[S] Change in coreboot[main]: soc/mediatek/mt8196: Add DVFS driver
by Jarried Lin (Code Review) 19 Jan '25

19 Jan '25

Attention is currently required from: Hung-Te Lin, Yidi Lin. Hello 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/+/86041?usp=email to look at the new patch set (#3). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: soc/mediatek/mt8196: Add DVFS driver ...................................................................... soc/mediatek/mt8196: Add DVFS driver TEST=Build pass. BUG=b:317009620 Change-Id: I92b7c57ad8c3d9e9954f02a08954939f45c5e2c2 Signed-off-by: Jarried Lin <jarried.lin(a)mediatek.corp-partner.google.com> --- M src/soc/mediatek/mt8196/Makefile.mk A src/soc/mediatek/mt8196/dvfs.c M src/soc/mediatek/mt8196/include/soc/addressmap.h A src/soc/mediatek/mt8196/include/soc/dvfs.h 4 files changed, 33 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/86041/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/86041?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: I92b7c57ad8c3d9e9954f02a08954939f45c5e2c2 Gerrit-Change-Number: 86041 Gerrit-PatchSet: 3 Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Yidi Lin <yidilin(a)google.com>

1 0

[XL] Change in coreboot[main]: soc/mediatek/mt8196: Add eDP driver
by Jitao Shi (Code Review) 19 Jan '25

19 Jan '25

Attention is currently required from: Bincai Liu, Hung-Te Lin, Jarried Lin. Jitao Shi has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/85949?usp=email ) Change subject: soc/mediatek/mt8196: Add eDP driver ...................................................................... Patch Set 6: (42 comments) File src/soc/mediatek/mt8196/dp_intf.c: https://review.coreboot.org/c/coreboot/+/85949/comment/ecd2cefc_e8834210?us… : PS2, Line 91: 0x20 > not done will fix next patchset https://review.coreboot.org/c/coreboot/+/85949/comment/a6033db7_0b07e1c4?us… : PS2, Line 162: 0x8000 > not done will fix next patchset https://review.coreboot.org/c/coreboot/+/85949/comment/17c926ac_6a10e898?us… : PS2, Line 198: mtk_dvo_power_on > Then can we rename this to `mtk_dpintf_power_on` for consistency? will fix next patchset File src/soc/mediatek/mt8196/dp_intf.c: https://review.coreboot.org/c/coreboot/+/85949/comment/516d0683_23cb8f1b?us… : PS4, Line 102: mtk_dvo_gs[MTK_DVO_8K_30FPS] > Why is `struct mtk_dvo_gs_info *gs_info` removed? If the index is fixed, let's add the local variabl […] will fix next patchset https://review.coreboot.org/c/coreboot/+/85949/comment/b182c460_cd4eef17?us… : PS4, Line 106: 0xFFFFFFF > 0xFFFFFFFF will fix next patchset https://review.coreboot.org/c/coreboot/+/85949/comment/740c5522_cc27c6fb?us… : PS4, Line 108: 0xFFFFFFF > 0xFFFFFFFF will fix next patchset https://review.coreboot.org/c/coreboot/+/85949/comment/4150c64c_7fa60eae?us… : PS4, Line 133: [dvo] > How about `[eDPTX] dvo`? will fix next patchset File src/soc/mediatek/mt8196/dp_intf.c: https://review.coreboot.org/c/coreboot/+/85949/comment/2a99f151_0a3ad32e?us… : PS6, Line 32: const struct mtk_dvo_sync_param *sync) > exceed 96 characters will fix next patchset File src/soc/mediatek/mt8196/dptx.c: https://review.coreboot.org/c/coreboot/+/85949/comment/10f80c46_1018c7a7?us… : PS2, Line 372: write32p(MMSYS1_CONFIG_BASE + MMSYS1_CG_CLR0, 0xFFFFFFFF); : write32p(MMSYS1_CONFIG_BASE + MMSYS1_CG_CLR1, 0x00040000); > So, could you answer my question? Do we need to set the same register twice? this clock is already configured in mainboard.c,we will remove in dptx.c and mtk_dpintf.c mainboard_init()->mtk_display_init()->mtk_ddp_init() File src/soc/mediatek/mt8196/dptx.c: https://review.coreboot.org/c/coreboot/+/85949/comment/3adc373c_6f8ba858?us… : PS4, Line 41: case DPTX_PATTERN_UNKNOWN: : printk(BIOS_ERR, "Pattern unknown\n"); : mtk_dp_mask(mtk_dp, REG_3400_DP_TRANS_P0, 0x0, PATTERN_EN_DP_TRANS_4P_MASK); : return; : default: : mtk_dp_mask(mtk_dp, REG_3400_DP_TRANS_P0, 0x0, PATTERN_EN_DP_TRANS_4P_MASK); : return; > Merge these 2 cases: […] will fix next patchset https://review.coreboot.org/c/coreboot/+/85949/comment/df538bcd_b60e8af3?us… : PS4, Line 54: u8 dpcd_adjust_req[DP_LANSE_ADJUST_SIZE]) > Align. […] will fix next patchset https://review.coreboot.org/c/coreboot/+/85949/comment/067d80af_a0ccce17?us… : PS4, Line 92: lanerate > linkrate will fix next patchset https://review.coreboot.org/c/coreboot/+/85949/comment/3bd4ef2e_8200a532?us… : PS4, Line 94: lane_count > Rename to `lanecount_enhanced_frame`. will fix next patchset https://review.coreboot.org/c/coreboot/+/85949/comment/c4a0fff6_3b32e655?us… : PS4, Line 96: u8 target_link_rate = lanerate; : u8 target_lane_count = lanecount; > No need for these `target_` variables. Directly use `linkrate` and `lanecount`. will fix next patchset https://review.coreboot.org/c/coreboot/+/85949/comment/2547a7ee_6941c260?us… : PS4, Line 140: DPTX_PLUG_OUT > You're changing the behavior in PS4. […] will fix next patchset https://review.coreboot.org/c/coreboot/+/85949/comment/428ed1aa_d9ec36fa?us… : PS4, Line 406: " > Add `%s: ` for `__func__` will fix next patchset File src/soc/mediatek/mt8196/dptx.c: https://review.coreboot.org/c/coreboot/+/85949/comment/027f3c9d_bdcd4c2a?us… : PS6, Line 384: mdelay > are you sure this delay it correct ? udelay(50) is ok. will fix next patchset File src/soc/mediatek/mt8196/dptx_hal.c: https://review.coreboot.org/c/coreboot/+/85949/comment/f98f8c5d_077a12ab?us… : PS2, Line 52: switch (out_format) { > Not done. Please define a local variable `u32 val`. will fix next patchset https://review.coreboot.org/c/coreboot/+/85949/comment/50203c06_a1a10870?us… : PS2, Line 146: switch (value) { > Not done. Please define a local variable `u32 val`. will fix next patchset https://review.coreboot.org/c/coreboot/+/85949/comment/53609ed4_cef796e4?us… : PS2, Line 307: mtk_edp_hal_reset_swing_pre_emphasis > Yes, I know these 2 functions are different. I'm asking if we can name the functions better. […] will fix next patchset File src/soc/mediatek/mt8196/dptx_hal.c: https://review.coreboot.org/c/coreboot/+/85949/comment/e62b134c_547ad0a3?us… : PS4, Line 53: switch (out_format) { > > `switch and case should be at the same indent` […] will fix next patchset https://review.coreboot.org/c/coreboot/+/85949/comment/f2aab74a_05497a83?us… : PS4, Line 86: u8 *swing_val, u8 *pre_emphasis) > align […] will fix next patchset https://review.coreboot.org/c/coreboot/+/85949/comment/e4aca7c6_4bfee723?us… : PS4, Line 135: GENMASK(2, 0)); > move to previous line will fix next patchset https://review.coreboot.org/c/coreboot/+/85949/comment/5e36429d_a0daa501?us… : PS4, Line 142: switch (value) { > > `switch and case should be at the same indent` […] will fix next patchset https://review.coreboot.org/c/coreboot/+/85949/comment/690ebf7e_1efedf72?us… : PS4, Line 337: (mtk_dp_phy_read > Align with `WAIT_AUX_READY_RETRY_TIMES` will fix next patchset https://review.coreboot.org/c/coreboot/+/85949/comment/3e266d9a_c42a5a12?us… : PS4, Line 337: 0x146c > upper case will fix next patchset https://review.coreboot.org/c/coreboot/+/85949/comment/44db0c4c_6a907dd5?us… : PS4, Line 362: pre_emphasis > preemphasis will fix next patchset https://review.coreboot.org/c/coreboot/+/85949/comment/69d9864b_58b733d0?us… : PS4, Line 362: mtk_dp_set_swing_pre_emphasis > Now that this function contains only a single line, we can write the content of `mtk_edp_phy_set_swi […] will fix next patchset https://review.coreboot.org/c/coreboot/+/85949/comment/b66e8ed5_ab5a0242?us… : PS4, Line 363: pre_emphasis > preemphasis will fix next patchset https://review.coreboot.org/c/coreboot/+/85949/comment/ff1393e5_abcec3b1?us… : PS4, Line 388: , > remove `,` will fix next patchset https://review.coreboot.org/c/coreboot/+/85949/comment/1b8491bf_1aa660f6?us… : PS4, Line 389: break > return will fix next patchset File src/soc/mediatek/mt8196/dptx_hal.c: https://review.coreboot.org/c/coreboot/+/85949/comment/f95aa360_5892fc72?us… : PS6, Line 316: mdelay > why does it take a long time ? will fix next Patchset, No need this delay，udelay(10) is ok. https://review.coreboot.org/c/coreboot/+/85949/comment/8d182643_d7024ddd?us… : PS6, Line 338: mdelay > why does it take a long time ? Here is polling PHY ready, Retry 5 times with a delay of 2ms each time。 we will fix next patchset File src/soc/mediatek/mt8196/include/soc/dp_intf.h: https://review.coreboot.org/c/coreboot/+/85949/comment/2be2ce4c_0f358c1a?us… : PS6, Line 137: 0x6c > 0x6C will fix next patchset https://review.coreboot.org/c/coreboot/+/85949/comment/13abe559_3ee7a584?us… : PS6, Line 141: c > C will fix next patchset File src/soc/mediatek/mt8196/include/soc/dp_intf.h: https://review.coreboot.org/c/coreboot/+/85949/comment/ae4608d9_516ce462?us… : PS4, Line 124: #define VACT_SHIFT 16 > align will fix next patchset File src/soc/mediatek/mt8196/include/soc/dp_intf.h: https://review.coreboot.org/c/coreboot/+/85949/comment/24fe96fc_fa01fc3e?us… : PS2, Line 199: gs_level > not done will be used in mt8196/dp_intf.c next patchset File src/soc/mediatek/mt8196/include/soc/dptx_hal.h: https://review.coreboot.org/c/coreboot/+/85949/comment/947b1459_be0d5bba?us… : PS4, Line 26: pre_emphasis > preemphasis will be fix next patch https://review.coreboot.org/c/coreboot/+/85949/comment/9c207345_9af51a85?us… : PS4, Line 31: pre_emphasis > preemphasis will be fix next patch. https://review.coreboot.org/c/coreboot/+/85949/comment/fa5e1616_9cb8463e?us… : PS4, Line 32: pre_emphasis > preemphasis will be fix next patch File src/soc/mediatek/mt8196/include/soc/dptx_hal.h: https://review.coreboot.org/c/coreboot/+/85949/comment/956405e8_5c1f3a20?us… : PS2, Line 38: void mtk_edp_phy_wait_aux_ldo_ready(struct mtk_dp *mtk_dp); : void mtk_edp_hal_phy_set_idle_pattern(struct mtk_dp *mtk_dp, u8 lane_count, bool enable); : void mtk_edp_set_swing_pre_emphasis > So, you think `mtk_dp_` makes better sense than `dptx_hal_`? Why? will be change to dptx_hal_ next patchset File src/soc/mediatek/mt8196/include/soc/dptx_reg.h: https://review.coreboot.org/c/coreboot/+/85949/comment/b27ebdfb_5f70bc97?us… : PS4, Line 204: #define DP_TRAIN_VOLTAGE_PREEMPHASIS_LEVEL_3 0x3 > Duplicate of `DPTX_PREEMPHASIS3` defined in src/soc/mediatek/common/dp/include/soc/dptx_common.h. will fix next patchset -- To view, visit https://review.coreboot.org/c/coreboot/+/85949?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: Iccba53f6c6181ca84624c216f9641a2ae9041671 Gerrit-Change-Number: 85949 Gerrit-PatchSet: 6 Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Reviewer: Bincai Liu <bincai.liu(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: Jitao Shi <jitao.shi(a)mediatek.corp-partner.google.com> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Bincai Liu <bincai.liu(a)mediatek.corp-partner.google.com> Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Comment-Date: Mon, 20 Jan 2025 04:10:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Bincai Liu <bincai.liu(a)mediatek.corp-partner.google.com> Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com> Comment-In-Reply-To: Yidi Lin <yidilin(a)google.com>

1 0

[XL] Change in coreboot[main]: soc/mediatek/mt8196: Add RTC driver
by Shunxi Zhang (Code Review) 19 Jan '25

19 Jan '25

Attention is currently required from: Hung-Te Lin, Jarried Lin, Paul Menzel, Yidi Lin, Yu-Ping Wu. Shunxi Zhang 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 5: Code-Review+1 (24 comments) File src/soc/mediatek/mt8196/include/soc/mt6685_rtc.h: https://review.coreboot.org/c/coreboot/+/85978/comment/b5131e66_7fdcc18d?us… : PS2, Line 84: /* : * Complete the RTC initialization process, : * setting of related registers : */ > `/* Complete the RTC initialization process and register settings. […] will update File src/soc/mediatek/mt8196/include/soc/mt6685_rtc_hw.h: PS1: > keep comment unresolved The new version(patchset 3) of header file mt6685_rtc_hw.h deletes the same definition as rtc_common.h File src/soc/mediatek/mt8196/include/soc/rtc.h: https://review.coreboot.org/c/coreboot/+/85978/comment/2982039b_0855560c?us… : PS2, Line 58: WATCH_INIT > If this means "stopwatch_init", please use a more descriptive name. […] will update https://review.coreboot.org/c/coreboot/+/85978/comment/19f29831_673884ed?us… : PS2, Line 59: WAIT_EOSC_CHECK_CLK_RDY_TIME_US > `EOSC_CHECK_CLK_TIMEOUT_US` will update https://review.coreboot.org/c/coreboot/+/85978/comment/25535ee9_f1de19fa?us… : PS2, Line 60: RETRY_MAX_TIME > `RECOVERY_RETRY_COUNT` will update File src/soc/mediatek/mt8196/include/soc/rtc.h: https://review.coreboot.org/c/coreboot/+/85978/comment/13f562fb_26ff0c07?us… : PS1, Line 18: RTC_BBPU_PWREN = BIT(0), > Keep this open until resolved. The new version of the header file mt6685_RTC_HW.H deletes the same macro definition as rtc_common.h File src/soc/mediatek/mt8196/mt6685_rtc.c: https://review.coreboot.org/c/coreboot/+/85978/comment/37a8dd61_e5f424c4?us… : PS1, Line 68: RG_BANK_FQMTR_RST_SHIFT); > keep comment unresolved will update https://review.coreboot.org/c/coreboot/+/85978/comment/acd91690_0fafed9b?us… : PS1, Line 148: freq[2] > Please answer Yidi's question here in the Gerrit comment. freq[0] is: select 26M as fixed clock freq[2] is: select 26M as target clock https://review.coreboot.org/c/coreboot/+/85978/comment/59b95a92_c99ce384?us… : PS1, Line 167: printk(BIOS_INFO, "EOSC cali val = %#x\n", val); > Then use log level debug? will update https://review.coreboot.org/c/coreboot/+/85978/comment/0e3ee24a_ce1661ab?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()) > keep comment unresolved will update https://review.coreboot.org/c/coreboot/+/85978/comment/b637d771_fe300474?us… : PS1, Line 417: > not done will update File src/soc/mediatek/mt8196/mt6685_rtc.c: https://review.coreboot.org/c/coreboot/+/85978/comment/a847d39e_065a781a?us… : PS2, Line 101: ERROR: get frequency > `FQMTR read` will update https://review.coreboot.org/c/coreboot/+/85978/comment/efa2907d_1d142248?us… : PS2, Line 105: 26M/32K = 794 > `26MHz / 23K = (26 * 10^6) / (23 * 1024) ~= 794` will update https://review.coreboot.org/c/coreboot/+/85978/comment/9222fee6_089a3903?us… : PS2, Line 139: > You removed the wrong blank line. […] will update https://review.coreboot.org/c/coreboot/+/85978/comment/fe951a16_308452f4?us… : PS2, Line 193: RTC_CON_GPU > align will update https://review.coreboot.org/c/coreboot/+/85978/comment/f72a7bc7_fdb2f984?us… : PS2, Line 325: rtc_powerkey_init(); : : if (!rtc_write_trigger()) : return false; > `rtc_powerkey_init` already contains `rtc_write_trigger`. Change this to […] will update https://review.coreboot.org/c/coreboot/+/85978/comment/250e71ff_5844372c?us… : PS2, Line 340: rtc_powerkey_init(); : if (!rtc_write_trigger()) : return false; > ``` […] will update https://review.coreboot.org/c/coreboot/+/85978/comment/d98a441f_e27041c3?us… : PS2, Line 375: wrritten > written will update https://review.coreboot.org/c/coreboot/+/85978/comment/89f8b190_1b91193f?us… : PS2, Line 388: rtc_powerkey_init(); > By changing the 2 `rtc_write` calls to `rtc_powerkey_init`, there will be an extra `rtc_write_trigge […] Done https://review.coreboot.org/c/coreboot/+/85978/comment/fa2bc204_2c9cab59?us… : PS2, Line 414: need > Done done https://review.coreboot.org/c/coreboot/+/85978/comment/41684237_ff0cada8?us… : PS2, Line 415: rtc_powerkey_init(); : : if (!rtc_write_trigger()) { > `if (!rtc_powerkey_init()) {` will update https://review.coreboot.org/c/coreboot/+/85978/comment/462e7b0b_df97311b?us… : PS2, Line 419: rtc_write_trigger > `rtc_powerkey_init failed` will update https://review.coreboot.org/c/coreboot/+/85978/comment/89c0765e_491ed506?us… : PS2, Line 425: > failed will update https://review.coreboot.org/c/coreboot/+/85978/comment/40c726de_a903eceb?us… : PS2, Line 504: Check clock source are match with 32K exist > `Check if clock sources match existing 32K` […] will update -- 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: 5 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> 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-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Comment-Date: Mon, 20 Jan 2025 04:05:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Shunxi Zhang <ot_shunxi.zhang(a)mediatek.corp-partner.google.com> Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com> Comment-In-Reply-To: Yidi Lin <yidilin(a)google.com>

1 0

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

19 Jan '25

Attention is currently required from: Hung-Te Lin, Paul Menzel, Shunxi Zhang, Yidi Lin, Yu-Ping Wu. Jarried 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 5: (1 comment) File src/soc/mediatek/mt8196/mt6685_rtc.c: https://review.coreboot.org/c/coreboot/+/85978/comment/9e899686_846661bf?us… : PS1, Line 167: printk(BIOS_INFO, "EOSC cali val = %#x\n", val); > Then use log level debug? Done -- 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: 5 Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Shunxi Zhang <ot_shunxi.zhang(a)mediatek.corp-partner.google.com> Gerrit-Attention: Shunxi Zhang <ot_shunxi.zhang(a)mediatek.corp-partner.google.com> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Comment-Date: Mon, 20 Jan 2025 03:52:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Shunxi Zhang <ot_shunxi.zhang(a)mediatek.corp-partner.google.com> Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>

1 0

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

19 Jan '25

Attention is currently required from: Hung-Te Lin, Shunxi Zhang, Yidi Lin, Yu-Ping Wu. Jarried 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 5: (1 comment) File src/soc/mediatek/mt8196/mt6685_rtc.c: https://review.coreboot.org/c/coreboot/+/85978/comment/1e12bb27_adac89b7?us… : PS2, Line 193: RTC_CON_GPU > You are right. I did not see there is `)` on above. […] Done -- 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: 5 Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Shunxi Zhang <ot_shunxi.zhang(a)mediatek.corp-partner.google.com> Gerrit-Attention: Shunxi Zhang <ot_shunxi.zhang(a)mediatek.corp-partner.google.com> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Comment-Date: Mon, 20 Jan 2025 03:49:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Yidi Lin <yidilin(a)google.com> Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com>

1 0

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

19 Jan '25

Attention is currently required from: Hung-Te Lin, Jarried Lin, Shunxi Zhang, Yu-Ping Wu. Hello 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/+/85978?usp=email to look at the new patch set (#5). Change subject: soc/mediatek/mt8196: Add RTC driver ...................................................................... soc/mediatek/mt8196: Add RTC driver Add RTC drivers for MT6685. TEST=build pass. BUG=b:317009620 Change-Id: I3dd337eaa3eed3012ddea300f7e04f2b63fb2daa Signed-off-by: Shunxi Zhang <ot_shunxi.zhang(a)mediatek.com> --- M src/soc/mediatek/common/include/soc/rtc_common.h M src/soc/mediatek/common/rtc_osc_init.c M src/soc/mediatek/mt8196/Makefile.mk A src/soc/mediatek/mt8196/include/soc/mt6685_hw.h A src/soc/mediatek/mt8196/include/soc/mt6685_rtc.h A src/soc/mediatek/mt8196/include/soc/mt6685_rtc_hw.h A src/soc/mediatek/mt8196/include/soc/rtc.h A src/soc/mediatek/mt8196/mt6685_rtc.c 8 files changed, 1,976 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/78/85978/5 -- 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: newpatchset Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I3dd337eaa3eed3012ddea300f7e04f2b63fb2daa Gerrit-Change-Number: 85978 Gerrit-PatchSet: 5 Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Shunxi Zhang <ot_shunxi.zhang(a)mediatek.corp-partner.google.com> Gerrit-Attention: Shunxi Zhang <ot_shunxi.zhang(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: Jarried Lin <jarried.lin(a)mediatek.com>

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit January 2025