Change in coreboot[master]: mediatek/mt8183: keep XO_SOC always in Full Power Mode

List overview All Threads
Download

newer

older

Change in coreboot[4.11_branch]:...

Change in coreboot[master]:...

Weiyi Lu (Code Review)

9 Oct 2019 9 Oct '19

8:55 a.m.

Hello Weiyi Lu,

I'd like you to do a code review. Please visit

https://review.coreboot.org/c/coreboot/+/35905

to review the following change.

Change subject: mediatek/mt8183: keep XO_SOC always in Full Power Mode ......................................................................

mediatek/mt8183: keep XO_SOC always in Full Power Mode

We could have XO_SOC output 26MHz to SOC by keeping it always in Full Power Mode even when the system suspended as a workaround for the random failure of suspend resume test.

BRANCH=none TEST=suspend resume stress test by 2500 times passed on Krane.

Change-Id: Id6cf8a9968cbdd2a5865aa2fd158a1ba0beb90f5 Signed-off-by: Weiyi Lu weiyi.lu@mediatek.com --- M src/soc/mediatek/mt8183/rtc.c 1 file changed, 5 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/35905/1

diff --git a/src/soc/mediatek/mt8183/rtc.c b/src/soc/mediatek/mt8183/rtc.c index 19b717c..c2d6c80 100644 --- a/src/soc/mediatek/mt8183/rtc.c +++ b/src/soc/mediatek/mt8183/rtc.c @@ -411,8 +411,11 @@ 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 and XO_CEL. + * Also keep XO_SOC always in Full Power Mode. + */ + rtc_write(PMIC_RG_DCXO_CW00, 0x6805); rtc_write(PMIC_RG_DCXO_CW11, 0x8000);

/* Load thermal coefficient */

Attachments:

attachment.htm (text/html — 3.1 KB)

Show replies by date

Paul Menzel (Code Review)

10 Oct 10 Oct

3:21 p.m.

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35905 )

Change subject: mediatek/mt8183: keep XO_SOC always in Full Power Mode ......................................................................

Patch Set 1:

(2 comments)

https://review.coreboot.org/c/coreboot/+/35905/1//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/35905/1//COMMIT_MSG@11 PS1, Line 11: the random failure of suspend resume test. Please add more details about that failure (summary of some bug report). Also, how much more power is now used in suspend?

https://review.coreboot.org/c/coreboot/+/35905/1/src/soc/mediatek/mt8183/rtc... File src/soc/mediatek/mt8183/rtc.c:

https://review.coreboot.org/c/coreboot/+/35905/1/src/soc/mediatek/mt8183/rtc... PS1, Line 416: * Also keep XO_SOC always in Full Power Mode. Mark this as FIXME? Reference some bug URL?

-- To view, visit https://review.coreboot.org/c/coreboot/+/35905 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id6cf8a9968cbdd2a5865aa2fd158a1ba0beb90f5 Gerrit-Change-Number: 35905 Gerrit-PatchSet: 1 Gerrit-Owner: Weiyi Lu weiyi.lu@mediatek.corp-partner.google.com Gerrit-Reviewer: Hung-Te Lin hungte@chromium.org Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: Weiyi Lu weiyi.lu@mediatek.com Gerrit-Reviewer: Weiyi Lu weiyi.lu@mediatek.corp-partner.google.com Gerrit-Reviewer: You-Cheng Syu youcheng@google.com Gerrit-Reviewer: Yu-Ping Wu yupingso@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: SJ Huang sj.huang@mediatek.corp-partner.google.com Gerrit-Comment-Date: Thu, 10 Oct 2019 15:21:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Hung-Te Lin (Code Review)

5:49 p.m.

Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35905 )

Change subject: mediatek/mt8183: keep XO_SOC always in Full Power Mode ......................................................................

Patch Set 1:

(2 comments)

https://review.coreboot.org/c/coreboot/+/35905/1//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/35905/1//COMMIT_MSG@10 PS1, Line 10: as a workaround Like Paul said, if this is a workaround please state the pros and cons of it.

https://review.coreboot.org/c/coreboot/+/35905/1//COMMIT_MSG@13 PS1, Line 13: none kukui

-- To view, visit https://review.coreboot.org/c/coreboot/+/35905 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id6cf8a9968cbdd2a5865aa2fd158a1ba0beb90f5 Gerrit-Change-Number: 35905 Gerrit-PatchSet: 1 Gerrit-Owner: Weiyi Lu weiyi.lu@mediatek.corp-partner.google.com Gerrit-Reviewer: Hung-Te Lin hungte@chromium.org Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: Weiyi Lu weiyi.lu@mediatek.com Gerrit-Reviewer: Weiyi Lu weiyi.lu@mediatek.corp-partner.google.com Gerrit-Reviewer: You-Cheng Syu youcheng@google.com Gerrit-Reviewer: Yu-Ping Wu yupingso@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: SJ Huang sj.huang@mediatek.corp-partner.google.com Gerrit-Comment-Date: Thu, 10 Oct 2019 17:49:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Weiyi Lu (Code Review)

15 Oct 15 Oct

5:11 a.m.

Hello Yu-Ping Wu, Julius Werner, You-Cheng Syu, Hung-Te Lin, build bot (Jenkins), Weiyi Lu,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/35905

to look at the new patch set (#2).

Change subject: mediatek/mt8183: keep XO_SOC always in Full Power Mode ......................................................................

mediatek/mt8183: keep XO_SOC always in Full Power Mode

We could have XO_SOC output 26MHz to SOC by keeping it always in Full Power Mode even when the system is suspended as a workaround for the random failure of suspend resume test[1]. After applying this patch, the power consumption will increase about 9.5mW at maximum during system suspended.

[1] https://issuetracker.google.com/issues/136980838

BRANCH=kukui TEST=suspend resume stress test by 2500 times passed on Krane.

Change-Id: Id6cf8a9968cbdd2a5865aa2fd158a1ba0beb90f5 Signed-off-by: Weiyi Lu weiyi.lu@mediatek.com --- M src/soc/mediatek/mt8183/rtc.c 1 file changed, 5 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/35905/2

-- To view, visit https://review.coreboot.org/c/coreboot/+/35905 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id6cf8a9968cbdd2a5865aa2fd158a1ba0beb90f5 Gerrit-Change-Number: 35905 Gerrit-PatchSet: 2 Gerrit-Owner: Weiyi Lu weiyi.lu@mediatek.corp-partner.google.com Gerrit-Reviewer: Hung-Te Lin hungte@chromium.org Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: Weiyi Lu weiyi.lu@mediatek.com Gerrit-Reviewer: Weiyi Lu weiyi.lu@mediatek.corp-partner.google.com Gerrit-Reviewer: You-Cheng Syu youcheng@google.com Gerrit-Reviewer: Yu-Ping Wu yupingso@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: SJ Huang sj.huang@mediatek.corp-partner.google.com Gerrit-MessageType: newpatchset

Weiyi Lu (Code Review)

5:15 a.m.

Weiyi Lu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35905 )

Change subject: mediatek/mt8183: keep XO_SOC always in Full Power Mode ......................................................................

Patch Set 2:

(2 comments)

https://review.coreboot.org/c/coreboot/+/35905/1//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/35905/1//COMMIT_MSG@13 PS1, Line 13: none

...

kukui

Done

https://review.coreboot.org/c/coreboot/+/35905/1/src/soc/mediatek/mt8183/rtc... File src/soc/mediatek/mt8183/rtc.c:

https://review.coreboot.org/c/coreboot/+/35905/1/src/soc/mediatek/mt8183/rtc... PS1, Line 416: * Also keep XO_SOC always in Full Power Mode.

...

Mark this as FIXME? Reference some bug URL?

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/35905 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id6cf8a9968cbdd2a5865aa2fd158a1ba0beb90f5 Gerrit-Change-Number: 35905 Gerrit-PatchSet: 2 Gerrit-Owner: Weiyi Lu weiyi.lu@mediatek.corp-partner.google.com Gerrit-Reviewer: Hung-Te Lin hungte@chromium.org Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: Weiyi Lu weiyi.lu@mediatek.com Gerrit-Reviewer: Weiyi Lu weiyi.lu@mediatek.corp-partner.google.com Gerrit-Reviewer: You-Cheng Syu youcheng@google.com Gerrit-Reviewer: Yu-Ping Wu yupingso@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: SJ Huang sj.huang@mediatek.corp-partner.google.com Gerrit-Comment-Date: Tue, 15 Oct 2019 05:15:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel paulepanter@users.sourceforge.net Comment-In-Reply-To: Hung-Te Lin hungte@chromium.org Gerrit-MessageType: comment

Paul Menzel (Code Review)

2:55 p.m.

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35905 )

Change subject: mediatek/mt8183: keep XO_SOC always in Full Power Mode ......................................................................

Patch Set 2:

(4 comments)

https://review.coreboot.org/c/coreboot/+/35905/2//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/35905/2//COMMIT_MSG@11 PS2, Line 11: test[1] Please add a space before [.

https://review.coreboot.org/c/coreboot/+/35905/2//COMMIT_MSG@11 PS2, Line 11: for the random failure of suspend resume test[1]. I’d still prefer more details, as I am unable to access the bug report.

https://review.coreboot.org/c/coreboot/+/35905/2//COMMIT_MSG@15 PS2, Line 15: [1] https://issuetracker.google.com/issues/136980838 I believe there is a tag for issues in the Google bug tracker.

https://review.coreboot.org/c/coreboot/+/35905/2/src/soc/mediatek/mt8183/rtc... File src/soc/mediatek/mt8183/rtc.c:

https://review.coreboot.org/c/coreboot/+/35905/2/src/soc/mediatek/mt8183/rtc... PS2, Line 416: * FIXME: Also keep XO_SOC always in Full Power Mode.

...

… to fix resume issues from suspend.

Also reference bug URL?

-- To view, visit https://review.coreboot.org/c/coreboot/+/35905 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id6cf8a9968cbdd2a5865aa2fd158a1ba0beb90f5 Gerrit-Change-Number: 35905 Gerrit-PatchSet: 2 Gerrit-Owner: Weiyi Lu weiyi.lu@mediatek.corp-partner.google.com Gerrit-Reviewer: Hung-Te Lin hungte@chromium.org Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: Weiyi Lu weiyi.lu@mediatek.com Gerrit-Reviewer: Weiyi Lu weiyi.lu@mediatek.corp-partner.google.com Gerrit-Reviewer: You-Cheng Syu youcheng@google.com Gerrit-Reviewer: Yu-Ping Wu yupingso@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: SJ Huang sj.huang@mediatek.corp-partner.google.com Gerrit-Comment-Date: Tue, 15 Oct 2019 14:55:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Yu-Ping Wu (Code Review)

16 Oct 16 Oct

2:29 a.m.

Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35905 )

Change subject: mediatek/mt8183: keep XO_SOC always in Full Power Mode ......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/35905/2//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/35905/2//COMMIT_MSG@16 PS2, Line 16: BUG=b:136980838

-- To view, visit https://review.coreboot.org/c/coreboot/+/35905 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id6cf8a9968cbdd2a5865aa2fd158a1ba0beb90f5 Gerrit-Change-Number: 35905 Gerrit-PatchSet: 2 Gerrit-Owner: Weiyi Lu weiyi.lu@mediatek.corp-partner.google.com Gerrit-Reviewer: Hung-Te Lin hungte@chromium.org Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: Weiyi Lu weiyi.lu@mediatek.com Gerrit-Reviewer: Weiyi Lu weiyi.lu@mediatek.corp-partner.google.com Gerrit-Reviewer: You-Cheng Syu youcheng@google.com Gerrit-Reviewer: Yu-Ping Wu yupingso@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: SJ Huang sj.huang@mediatek.corp-partner.google.com Gerrit-Comment-Date: Wed, 16 Oct 2019 02:29:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Yu-Ping Wu (Code Review)

21 Oct 21 Oct

6:32 a.m.

Yu-Ping Wu has uploaded a new patch set (#3) to the change originally created by Weiyi Lu. ( https://review.coreboot.org/c/coreboot/+/35905 )

Change subject: mediatek/mt8183: keep XO_SOC always in Full Power Mode ......................................................................

mediatek/mt8183: keep XO_SOC always in Full Power Mode

We could have XO_SOC output 26MHz to SOC by keeping it always in Full Power Mode even when the system is suspended as a workaround for the random failure of suspend resume test [1]. After applying this patch, the power consumption will increase about 9.5mW at maximum during system suspended.

[1] https://issuetracker.google.com/issues/136980838

BRANCH=kukui BUG=b:136980838 TEST=suspend resume stress test by 2500 times passed on Krane.

Change-Id: Id6cf8a9968cbdd2a5865aa2fd158a1ba0beb90f5 Signed-off-by: Weiyi Lu weiyi.lu@mediatek.com --- M src/soc/mediatek/mt8183/rtc.c 1 file changed, 6 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/35905/3

-- To view, visit https://review.coreboot.org/c/coreboot/+/35905 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id6cf8a9968cbdd2a5865aa2fd158a1ba0beb90f5 Gerrit-Change-Number: 35905 Gerrit-PatchSet: 3 Gerrit-Owner: Weiyi Lu weiyi.lu@mediatek.corp-partner.google.com Gerrit-Reviewer: Hung-Te Lin hungte@chromium.org Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: Weiyi Lu weiyi.lu@mediatek.com Gerrit-Reviewer: Weiyi Lu weiyi.lu@mediatek.corp-partner.google.com Gerrit-Reviewer: You-Cheng Syu youcheng@google.com Gerrit-Reviewer: Yu-Ping Wu yupingso@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: SJ Huang sj.huang@mediatek.corp-partner.google.com Gerrit-MessageType: newpatchset

Yu-Ping Wu (Code Review)

6:32 a.m.

Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35905 )

Change subject: mediatek/mt8183: keep XO_SOC always in Full Power Mode ......................................................................

Patch Set 3:

(5 comments)

https://review.coreboot.org/c/coreboot/+/35905/1//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/35905/1//COMMIT_MSG@10 PS1, Line 10: as a workaround

...

Like Paul said, if this is a workaround please state the pros and cons of it.

Done

https://review.coreboot.org/c/coreboot/+/35905/2//COMMIT_MSG Commit Message:

https://review.coreboot.org/c/coreboot/+/35905/2//COMMIT_MSG@11 PS2, Line 11: test[1]

...

Please add a space before [.

Done

https://review.coreboot.org/c/coreboot/+/35905/2//COMMIT_MSG@15 PS2, Line 15: [1] https://issuetracker.google.com/issues/136980838

...

I believe there is a tag for issues in the Google bug tracker.

Done

https://review.coreboot.org/c/coreboot/+/35905/2//COMMIT_MSG@16 PS2, Line 16:

...

BUG=b:136980838

Done

https://review.coreboot.org/c/coreboot/+/35905/2/src/soc/mediatek/mt8183/rtc... File src/soc/mediatek/mt8183/rtc.c:

https://review.coreboot.org/c/coreboot/+/35905/2/src/soc/mediatek/mt8183/rtc... PS2, Line 416: * FIXME: Also keep XO_SOC always in Full Power Mode.

...

...
… to fix resume issues from suspend. […]

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/35905 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id6cf8a9968cbdd2a5865aa2fd158a1ba0beb90f5 Gerrit-Change-Number: 35905 Gerrit-PatchSet: 3 Gerrit-Owner: Weiyi Lu weiyi.lu@mediatek.corp-partner.google.com Gerrit-Reviewer: Hung-Te Lin hungte@chromium.org Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: Weiyi Lu weiyi.lu@mediatek.com Gerrit-Reviewer: Weiyi Lu weiyi.lu@mediatek.corp-partner.google.com Gerrit-Reviewer: You-Cheng Syu youcheng@google.com Gerrit-Reviewer: Yu-Ping Wu yupingso@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: SJ Huang sj.huang@mediatek.corp-partner.google.com Gerrit-Comment-Date: Mon, 21 Oct 2019 06:32:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Yu-Ping Wu yupingso@google.com Comment-In-Reply-To: Paul Menzel paulepanter@users.sourceforge.net Comment-In-Reply-To: Hung-Te Lin hungte@chromium.org Gerrit-MessageType: comment

Weiyi Lu (Code Review)

26 Nov 26 Nov

5:28 a.m.

Weiyi Lu has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/35905 )

Change subject: mediatek/mt8183: keep XO_SOC always in Full Power Mode ......................................................................

Abandoned

abandon this workaround patch

-- To view, visit https://review.coreboot.org/c/coreboot/+/35905 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id6cf8a9968cbdd2a5865aa2fd158a1ba0beb90f5 Gerrit-Change-Number: 35905 Gerrit-PatchSet: 3 Gerrit-Owner: Weiyi Lu weiyi.lu@mediatek.corp-partner.google.com Gerrit-Reviewer: Hung-Te Lin hungte@chromium.org Gerrit-Reviewer: Julius Werner jwerner@chromium.org Gerrit-Reviewer: Weiyi Lu weiyi.lu@mediatek.com Gerrit-Reviewer: Weiyi Lu weiyi.lu@mediatek.corp-partner.google.com Gerrit-Reviewer: You-Cheng Syu youcheng@google.com Gerrit-Reviewer: Yu-Ping Wu yupingso@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-CC: SJ Huang sj.huang@mediatek.corp-partner.google.com Gerrit-MessageType: abandon

1769

days inactive

1817

days old

coreboot-gerrit@coreboot.org

9 comments

4 participants

tags (0)

participants (4)

Hung-Te Lin (Code Review)
Paul Menzel (Code Review)
Weiyi Lu (Code Review)
Yu-Ping Wu (Code Review)