Hello Weiyi Lu,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/32323
to review the following change.
Change subject: WIP: mediatek/mt8183: update dcxo setting ......................................................................
WIP: mediatek/mt8183: update dcxo setting
Disable unused DCXO and power mode.
BRANCH=none TEST=Boots correctly on Kukui.
Change-Id: I2e5ce181ad327ccf852979da53baca4f249912fe Signed-off-by: Weiyi Lu weiyi.lu@mediatek.com --- M src/soc/mediatek/mt8183/include/soc/rtc.h M src/soc/mediatek/mt8183/rtc.c 2 files changed, 4 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/32323/1
diff --git a/src/soc/mediatek/mt8183/include/soc/rtc.h b/src/soc/mediatek/mt8183/include/soc/rtc.h index 3d115fe..a721ff8 100644 --- a/src/soc/mediatek/mt8183/include/soc/rtc.h +++ b/src/soc/mediatek/mt8183/include/soc/rtc.h @@ -140,6 +140,7 @@ PMIC_RG_DCXO_CW15 = 0x07AE, PMIC_RG_DCXO_CW16 = 0x07B0, PMIC_RG_DCXO_CW21 = 0x07BA, + PMIC_RG_DCXO_CW23 = 0x07BE, PMIC_RG_DCXO_ELR0 = 0x07C4 };
diff --git a/src/soc/mediatek/mt8183/rtc.c b/src/soc/mediatek/mt8183/rtc.c index 62256eb..57f26e8 100644 --- a/src/soc/mediatek/mt8183/rtc.c +++ b/src/soc/mediatek/mt8183/rtc.c @@ -281,9 +281,10 @@ rtc_write(PMIC_RG_DCXO_CW16, 0x9855);
/* 26M enable control */ - /* Enable clock buffer XO_SOC, XO_CEL */ - rtc_write(PMIC_RG_DCXO_CW00, 0x4805); + /* Enable clock buffer XO_SOC */ + rtc_write(PMIC_RG_DCXO_CW00, 0x4005); rtc_write(PMIC_RG_DCXO_CW11, 0x8000); + rtc_write(PMIC_RG_DCXO_CW23, 0x0053);
/* Load thermal coefficient */ rtc_write(PMIC_RG_TOP_TMA_KEY, 0x9CA7);
You-Cheng Syu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32323 )
Change subject: WIP: mediatek/mt8183: update dcxo setting ......................................................................
Patch Set 1:
Do we still need this CL? I saw the description in other CL (https://review.coreboot.org/c/coreboot/+/32339) mentioned that dcxo is used.
Hello Julius Werner, Ran Bi, Hung-Te Lin, build bot (Jenkins), Weiyi Lu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32323
to look at the new patch set (#2).
Change subject: mediatek/mt8183: update dcxo setting ......................................................................
mediatek/mt8183: update dcxo setting
Disable unused DCXO output buffers, only enable the output buffer for SOC. Also disable useless output buffer power mode.
BRANCH=none TEST=Boots correctly on Kukui.
Change-Id: I2e5ce181ad327ccf852979da53baca4f249912fe Signed-off-by: Weiyi Lu weiyi.lu@mediatek.com --- M src/soc/mediatek/mt8183/include/soc/rtc.h M src/soc/mediatek/mt8183/rtc.c 2 files changed, 4 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/32323/2
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32323 )
Change subject: mediatek/mt8183: update dcxo setting ......................................................................
Patch Set 2:
ping? can you reply to youcheng's question?
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32323 )
Change subject: mediatek/mt8183: update dcxo setting ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/32323/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/32323/2//COMMIT_MSG@11 PS2, Line 11: Please elaborate, *why* these changes are made.
Hello Julius Werner, Ran Bi, Hung-Te Lin, build bot (Jenkins), Weiyi Lu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32323
to look at the new patch set (#3).
Change subject: mediatek/mt8183: update dcxo output buffer setting ......................................................................
mediatek/mt8183: update dcxo output buffer setting
DCXO is consist of core that generates clock and output buffers that provide clock to other peripheral componenets. This patch mainly eliminates the extra power consumption of output buffers. We only enable the buffer for SOC and disable unused buffers for power-saving. Also disable useless buffer power mode to guarantee the lowest power state.
BRANCH=none TEST=Boots correctly on Kukui.
Change-Id: I2e5ce181ad327ccf852979da53baca4f249912fe Signed-off-by: Weiyi Lu weiyi.lu@mediatek.com --- M src/soc/mediatek/mt8183/include/soc/rtc.h M src/soc/mediatek/mt8183/rtc.c 2 files changed, 4 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/32323/3
Weiyi Lu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32323 )
Change subject: mediatek/mt8183: update dcxo output buffer setting ......................................................................
Patch Set 3:
Patch Set 2:
ping? can you reply to youcheng's question?
Hi Hung-Te and You-Cheng, I've updated more details in the commit message. Hope it can answer your questions.
You-Cheng Syu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32323 )
Change subject: mediatek/mt8183: update dcxo output buffer setting ......................................................................
Patch Set 3: Code-Review+1
(1 comment)
https://review.coreboot.org/#/c/32323/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/32323/3//COMMIT_MSG@9 PS3, Line 9: is consist of consists of
Hello Julius Werner, You-Cheng Syu, Ran Bi, Hung-Te Lin, build bot (Jenkins), Weiyi Lu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32323
to look at the new patch set (#4).
Change subject: mediatek/mt8183: update dcxo output buffer setting ......................................................................
mediatek/mt8183: update dcxo output buffer setting
DCXO consists of core that generates clock and output buffers that provide clock to other peripheral componenets. This patch mainly eliminates the extra power consumption of output buffers. We only enable the buffer for SOC and disable unused buffers for power-saving. Also disable useless buffer power mode to guarantee the lowest power state.
BRANCH=none TEST=Boots correctly on Kukui.
Change-Id: I2e5ce181ad327ccf852979da53baca4f249912fe Signed-off-by: Weiyi Lu weiyi.lu@mediatek.com --- M src/soc/mediatek/mt8183/include/soc/rtc.h M src/soc/mediatek/mt8183/rtc.c 2 files changed, 4 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/32323/4
Weiyi Lu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32323 )
Change subject: mediatek/mt8183: update dcxo output buffer setting ......................................................................
Patch Set 4:
(1 comment)
Patch Set 3: Code-Review+1
(1 comment)
https://review.coreboot.org/#/c/32323/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/32323/3//COMMIT_MSG@9 PS3, Line 9: is consist of
consists of
Done
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32323 )
Change subject: mediatek/mt8183: update dcxo output buffer setting ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/#/c/32323/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/32323/4//COMMIT_MSG@10 PS4, Line 10: componenets components
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32323 )
Change subject: mediatek/mt8183: update dcxo output buffer setting ......................................................................
Patch Set 4: Code-Review+2
(1 comment)
https://review.coreboot.org/#/c/32323/4/src/soc/mediatek/mt8183/rtc.c File src/soc/mediatek/mt8183/rtc.c:
https://review.coreboot.org/#/c/32323/4/src/soc/mediatek/mt8183/rtc.c@285 PS4, Line 285: rtc_write(PMIC_RG_DCXO_CW00, 0x4005); It would be nicer if you had named constants for every bit here and would OR them together individually
Hello Julius Werner, You-Cheng Syu, Ran Bi, Hung-Te Lin, build bot (Jenkins), Weiyi Lu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32323
to look at the new patch set (#5).
Change subject: mediatek/mt8183: update dcxo output buffer setting ......................................................................
mediatek/mt8183: update dcxo output buffer setting
DCXO consists of core that generates clock and output buffers that provide clock to other peripheral components. This patch mainly eliminates the extra power consumption of output buffers. We only enable the buffer for SOC and disable unused buffers for power-saving. Also disable useless buffer power mode to guarantee the lowest power state.
BRANCH=none TEST=Boots correctly on Kukui.
Change-Id: I2e5ce181ad327ccf852979da53baca4f249912fe Signed-off-by: Weiyi Lu weiyi.lu@mediatek.com --- M src/soc/mediatek/mt8183/include/soc/rtc.h M src/soc/mediatek/mt8183/rtc.c 2 files changed, 4 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/32323/5
Weiyi Lu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32323 )
Change subject: mediatek/mt8183: update dcxo output buffer setting ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/32323/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/32323/4//COMMIT_MSG@10 PS4, Line 10: componenets
components
Done
Martin Roth has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32323 )
Change subject: mediatek/mt8183: update dcxo output buffer setting ......................................................................
mediatek/mt8183: update dcxo output buffer setting
DCXO consists of core that generates clock and output buffers that provide clock to other peripheral components. This patch mainly eliminates the extra power consumption of output buffers. We only enable the buffer for SOC and disable unused buffers for power-saving. Also disable useless buffer power mode to guarantee the lowest power state.
BRANCH=none TEST=Boots correctly on Kukui.
Change-Id: I2e5ce181ad327ccf852979da53baca4f249912fe Signed-off-by: Weiyi Lu weiyi.lu@mediatek.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/32323 Reviewed-by: Julius Werner jwerner@chromium.org Reviewed-by: You-Cheng Syu youcheng@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/mediatek/mt8183/include/soc/rtc.h M src/soc/mediatek/mt8183/rtc.c 2 files changed, 4 insertions(+), 2 deletions(-)
Approvals: build bot (Jenkins): Verified Julius Werner: Looks good to me, approved You-Cheng Syu: Looks good to me, but someone else must approve
diff --git a/src/soc/mediatek/mt8183/include/soc/rtc.h b/src/soc/mediatek/mt8183/include/soc/rtc.h index 841a202..1f6f06a 100644 --- a/src/soc/mediatek/mt8183/include/soc/rtc.h +++ b/src/soc/mediatek/mt8183/include/soc/rtc.h @@ -147,6 +147,7 @@ PMIC_RG_DCXO_CW15 = 0x07AE, PMIC_RG_DCXO_CW16 = 0x07B0, PMIC_RG_DCXO_CW21 = 0x07BA, + PMIC_RG_DCXO_CW23 = 0x07BE, PMIC_RG_DCXO_ELR0 = 0x07C4 };
diff --git a/src/soc/mediatek/mt8183/rtc.c b/src/soc/mediatek/mt8183/rtc.c index 5879088..3bd3ab4 100644 --- a/src/soc/mediatek/mt8183/rtc.c +++ b/src/soc/mediatek/mt8183/rtc.c @@ -274,9 +274,10 @@ rtc_write(PMIC_RG_DCXO_CW16, 0x9855);
/* 26M enable control */ - /* Enable clock buffer XO_SOC, XO_CEL */ - rtc_write(PMIC_RG_DCXO_CW00, 0x4805); + /* Enable clock buffer XO_SOC */ + rtc_write(PMIC_RG_DCXO_CW00, 0x4005); rtc_write(PMIC_RG_DCXO_CW11, 0x8000); + rtc_write(PMIC_RG_DCXO_CW23, 0x0053);
/* Load thermal coefficient */ rtc_write(PMIC_RG_TOP_TMA_KEY, 0x9CA7);
-
Hung-Te Lin (Code Review) -
Julius Werner (Code Review)
-
Martin Roth (Code Review)
-
Paul Menzel (Code Review)
-
Weiyi Lu (Code Review)
-
You-Cheng Syu (Code Review)