Change in coreboot[master]: mb/google/asurada: Implement enable_requlator and regulator_is_enabled
Yidi Lin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46687 )
Change subject: mb/google/asurada: Implement enable_requlator and regulator_is_enabled ......................................................................
mb/google/asurada: Implement enable_requlator and regulator_is_enabled
SD Card driver needs to access two regulators - MT6360_LDO5 and MT6360_LDO3. These two regulators are disabled by default.
Two APIs are implemented, mainboard_enable_requlator - Configure the regulator as enabled / disabled. mainboard_regulator_is_enabled - Query if the regulator is enabled.
BUG=b:168863056,b:147789962 BRANCH=none TEST=emerge-asurada coreboot
Change-Id: I391f908fcb33ffdcccc53063644482eabc863ac4 Signed-off-by: Yidi Lin yidi.lin@mediatek.com --- M src/mainboard/google/asurada/regulator.c M src/soc/mediatek/common/include/soc/regulator.h 2 files changed, 38 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/46687/1
diff --git a/src/mainboard/google/asurada/regulator.c b/src/mainboard/google/asurada/regulator.c index 740d98b..ecfe4ef 100644 --- a/src/mainboard/google/asurada/regulator.c +++ b/src/mainboard/google/asurada/regulator.c @@ -25,6 +25,10 @@ return MT6360_LDO7; case MTK_REGULATOR_VMDDR: return MT6360_LDO6; + case MTK_REGULATOR_VCC: + return MT6360_LDO5; + case MTK_REGULATOR_VCCQ: + return MT6360_LDO3; default: break; } @@ -96,3 +100,32 @@
return 0; } + +void mainboard_enable_requlator(enum mtk_regulator regulator, uint8_t enable) +{ + int id; + + id = get_mt6360_regulator_id(regulator); + if (id >= 0) { + google_chromeec_regulator_enable(id, enable); + return; + } + + printk(BIOS_WARNING, "Invalid regualtor ID: %d\n", regulator); +} + +uint8_t mainboard_regulator_is_enabled(enum mtk_regulator regulator) +{ + int id; + + id = get_mt6360_regulator_id(regulator); + if (id >= 0) { + uint8_t enabled = 0; + google_chromeec_regulator_is_enabled(id, &enabled); + return enabled; + } + + printk(BIOS_WARNING, "Invalid regualtor ID: %d\n", regulator); + + return 0; +} diff --git a/src/soc/mediatek/common/include/soc/regulator.h b/src/soc/mediatek/common/include/soc/regulator.h index 6d9ff4e..35ce1ac 100644 --- a/src/soc/mediatek/common/include/soc/regulator.h +++ b/src/soc/mediatek/common/include/soc/regulator.h @@ -11,10 +11,15 @@ MTK_REGULATOR_VDDQ, MTK_REGULATOR_VMDDR, MTK_REGULATOR_VCORE, + MTK_REGULATOR_VCC, + MTK_REGULATOR_VCCQ, };
void mainboard_set_regulator_vol(enum mtk_regulator regulator, uint32_t voltage_uv); uint32_t mainboard_get_regulator_vol(enum mtk_regulator regulator);
+void mainboard_enable_requlator(enum mtk_regulator regulator, uint8_t enable); +uint8_t mainboard_regulator_is_enabled(enum mtk_regulator regulator); + #endif /* SOC_MEDIATEK_COMMON_REGULATOR_H */
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46687 )
Change subject: mb/google/asurada: Implement enable_requlator and regulator_is_enabled ......................................................................
Patch Set 1:
(4 comments)
https://review.coreboot.org/c/coreboot/+/46687/1/src/mainboard/google/asurad... File src/mainboard/google/asurada/regulator.c:
https://review.coreboot.org/c/coreboot/+/46687/1/src/mainboard/google/asurad... PS1, Line 114: regualtor regulator
https://review.coreboot.org/c/coreboot/+/46687/1/src/mainboard/google/asurad... PS1, Line 109: if (id >= 0) { : google_chromeec_regulator_enable(id, enable); : return; : } : : printk(BIOS_WARNING, "Invalid regualtor ID: %d\n", regulator); Reverse the order:
if (id < 0) { printk(...); return; }
google_chromeec_regulator_enable(id, enable);
https://review.coreboot.org/c/coreboot/+/46687/1/src/mainboard/google/asurad... PS1, Line 128: Invalid regualtor ID: %d Invalid regulator ID: %d; assuming disabled
https://review.coreboot.org/c/coreboot/+/46687/1/src/mainboard/google/asurad... PS1, Line 122: if (id >= 0) { : uint8_t enabled = 0; : google_chromeec_regulator_is_enabled(id, &enabled); : return enabled; : } : : printk(BIOS_WARNING, "Invalid regualtor ID: %d\n", regulator); : : return 0; Same.
Hello Xi Chen, Hung-Te Lin, build bot (Jenkins), Yu-Ping Wu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46687
to look at the new patch set (#2).
Change subject: mb/google/asurada: Implement enable_requlator and regulator_is_enabled ......................................................................
mb/google/asurada: Implement enable_requlator and regulator_is_enabled
SD Card driver needs to access two regulators - MT6360_LDO5 and MT6360_LDO3. These two regulators are disabled by default.
Two APIs are implemented, mainboard_enable_requlator - Configure the regulator as enabled / disabled. mainboard_regulator_is_enabled - Query if the regulator is enabled.
BUG=b:168863056,b:147789962 BRANCH=none TEST=emerge-asurada coreboot
Change-Id: I391f908fcb33ffdcccc53063644482eabc863ac4 Signed-off-by: Yidi Lin yidi.lin@mediatek.com --- M src/mainboard/google/asurada/Makefile.inc M src/mainboard/google/asurada/regulator.c M src/soc/mediatek/common/include/soc/regulator.h 3 files changed, 40 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/46687/2
Yidi Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46687 )
Change subject: mb/google/asurada: Implement enable_requlator and regulator_is_enabled ......................................................................
Patch Set 2:
(4 comments)
https://review.coreboot.org/c/coreboot/+/46687/1/src/mainboard/google/asurad... File src/mainboard/google/asurada/regulator.c:
https://review.coreboot.org/c/coreboot/+/46687/1/src/mainboard/google/asurad... PS1, Line 109: if (id >= 0) { : google_chromeec_regulator_enable(id, enable); : return; : } : : printk(BIOS_WARNING, "Invalid regualtor ID: %d\n", regulator);
Reverse the order: […]
Ack
https://review.coreboot.org/c/coreboot/+/46687/1/src/mainboard/google/asurad... PS1, Line 114: regualtor
regulator
Ack
https://review.coreboot.org/c/coreboot/+/46687/1/src/mainboard/google/asurad... PS1, Line 128: Invalid regualtor ID: %d
Invalid regulator ID: %d; assuming disabled
Ack
https://review.coreboot.org/c/coreboot/+/46687/1/src/mainboard/google/asurad... PS1, Line 122: if (id >= 0) { : uint8_t enabled = 0; : google_chromeec_regulator_is_enabled(id, &enabled); : return enabled; : } : : printk(BIOS_WARNING, "Invalid regualtor ID: %d\n", regulator); : : return 0;
Same.
Ack
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46687 )
Change subject: mb/google/asurada: Implement enable_requlator and regulator_is_enabled ......................................................................
Patch Set 2:
(3 comments)
https://review.coreboot.org/c/coreboot/+/46687/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46687/2//COMMIT_MSG@13 PS2, Line 13: mainboard_enable_requlator - Configure the regulator as enabled / disabled. : mainboard_regulator_is_enabled - Query if the regulator is enabled. Consider using a list:
- mainboard_enable_requlator: Configure the regulator as enabled/disabled. - mainboard_regulator_is_enabled: Query if the regulator is enabled.
https://review.coreboot.org/c/coreboot/+/46687/2/src/mainboard/google/asurad... File src/mainboard/google/asurada/regulator.c:
https://review.coreboot.org/c/coreboot/+/46687/2/src/mainboard/google/asurad... PS2, Line 114: google_chromeec_regulator_enable This might return -1
https://review.coreboot.org/c/coreboot/+/46687/2/src/mainboard/google/asurad... PS2, Line 129: google_chromeec_regulator_is_enabled This might return -1. Should we check the return value?
Hello Xi Chen, Hung-Te Lin, build bot (Jenkins), Patrick Georgi, Martin Roth, Yu-Ping Wu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46687
to look at the new patch set (#3).
Change subject: mb/google/asurada: Implement enable_requlator and regulator_is_enabled ......................................................................
mb/google/asurada: Implement enable_requlator and regulator_is_enabled
SD Card driver needs to access two regulators - MT6360_LDO5 and MT6360_LDO3. These two regulators are disabled by default.
Two APIs are implemented: - mainboard_enable_requlator: Configure the regulator as enabled/disabled. - mainboard_regulator_is_enabled: Query if the regulator is enabled.
BUG=b:168863056,b:147789962 BRANCH=none TEST=emerge-asurada coreboot
Change-Id: I391f908fcb33ffdcccc53063644482eabc863ac4 Signed-off-by: Yidi Lin yidi.lin@mediatek.com --- M src/mainboard/google/asurada/Makefile.inc M src/mainboard/google/asurada/regulator.c M src/soc/mediatek/common/include/soc/regulator.h 3 files changed, 45 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/46687/3
Yidi Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46687 )
Change subject: mb/google/asurada: Implement enable_requlator and regulator_is_enabled ......................................................................
Patch Set 4:
(3 comments)
https://review.coreboot.org/c/coreboot/+/46687/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46687/2//COMMIT_MSG@13 PS2, Line 13: mainboard_enable_requlator - Configure the regulator as enabled / disabled. : mainboard_regulator_is_enabled - Query if the regulator is enabled.
Consider using a list: […]
Ack
https://review.coreboot.org/c/coreboot/+/46687/2/src/mainboard/google/asurad... File src/mainboard/google/asurada/regulator.c:
https://review.coreboot.org/c/coreboot/+/46687/2/src/mainboard/google/asurad... PS2, Line 114: google_chromeec_regulator_enable
This might return -1
Ack. Return the result to the caller.
https://review.coreboot.org/c/coreboot/+/46687/2/src/mainboard/google/asurad... PS2, Line 129: google_chromeec_regulator_is_enabled
This might return -1. […]
Ack. Print the warning message and assume that the regulator is disabled.
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46687 )
Change subject: mb/google/asurada: Implement enable_requlator and regulator_is_enabled ......................................................................
Patch Set 4:
(4 comments)
https://review.coreboot.org/c/coreboot/+/46687/4/src/mainboard/google/asurad... File src/mainboard/google/asurada/regulator.c:
https://review.coreboot.org/c/coreboot/+/46687/4/src/mainboard/google/asurad... PS4, Line 104: regulator id? It's fine if you wan to print regulator instead of id.
https://review.coreboot.org/c/coreboot/+/46687/4/src/mainboard/google/asurad... PS4, Line 118: regulator id?
https://review.coreboot.org/c/coreboot/+/46687/4/src/mainboard/google/asurad... PS4, Line 122: = 0 No need for initialization.
https://review.coreboot.org/c/coreboot/+/46687/4/src/mainboard/google/asurad... PS4, Line 126: regulator id?
Hello Xi Chen, Hung-Te Lin, build bot (Jenkins), Patrick Georgi, Martin Roth, Yu-Ping Wu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46687
to look at the new patch set (#5).
Change subject: mb/google/asurada: Implement enable_requlator and regulator_is_enabled ......................................................................
mb/google/asurada: Implement enable_requlator and regulator_is_enabled
SD Card driver needs to access two regulators - MT6360_LDO5 and MT6360_LDO3. These two regulators are disabled by default.
Two APIs are implemented: - mainboard_enable_requlator: Configure the regulator as enabled/disabled. - mainboard_regulator_is_enabled: Query if the regulator is enabled.
BUG=b:168863056,b:147789962 BRANCH=none TEST=emerge-asurada coreboot
Change-Id: I391f908fcb33ffdcccc53063644482eabc863ac4 Signed-off-by: Yidi Lin yidi.lin@mediatek.com --- M src/mainboard/google/asurada/Makefile.inc M src/mainboard/google/asurada/regulator.c M src/soc/mediatek/common/include/soc/regulator.h 3 files changed, 45 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/46687/5
Yidi Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46687 )
Change subject: mb/google/asurada: Implement enable_requlator and regulator_is_enabled ......................................................................
Patch Set 5:
(4 comments)
https://review.coreboot.org/c/coreboot/+/46687/4/src/mainboard/google/asurad... File src/mainboard/google/asurada/regulator.c:
https://review.coreboot.org/c/coreboot/+/46687/4/src/mainboard/google/asurad... PS4, Line 104: regulator
id? It's fine if you wan to print regulator instead of id.
Ack. It is easier to know what input value causes the invalid ID.
https://review.coreboot.org/c/coreboot/+/46687/4/src/mainboard/google/asurad... PS4, Line 118: regulator
id?
Ack
https://review.coreboot.org/c/coreboot/+/46687/4/src/mainboard/google/asurad... PS4, Line 122: = 0
No need for initialization.
Ack
https://review.coreboot.org/c/coreboot/+/46687/4/src/mainboard/google/asurad... PS4, Line 126: regulator
id?
Ack
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46687 )
Change subject: mb/google/asurada: Implement enable_requlator and regulator_is_enabled ......................................................................
Patch Set 5: Code-Review+2
Hello Xi Chen, Hung-Te Lin, build bot (Jenkins), Patrick Georgi, Martin Roth, Yu-Ping Wu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46687
to look at the new patch set (#8).
Change subject: mb/google/asurada: Implement enable_requlator and regulator_is_enabled ......................................................................
mb/google/asurada: Implement enable_requlator and regulator_is_enabled
SD Card driver needs to access two regulators - MT6360_LDO5 and MT6360_LDO3. These two regulators are disabled by default.
Two APIs are implemented: - mainboard_enable_requlator: Configure the regulator as enabled/disabled. - mainboard_regulator_is_enabled: Query if the regulator is enabled.
BUG=b:168863056,b:147789962 BRANCH=none TEST=emerge-asurada coreboot
Change-Id: I391f908fcb33ffdcccc53063644482eabc863ac4 Signed-off-by: Yidi Lin yidi.lin@mediatek.com --- M src/mainboard/google/asurada/Makefile.inc M src/mainboard/google/asurada/regulator.c M src/soc/mediatek/common/include/soc/regulator.h 3 files changed, 49 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/46687/8
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46687 )
Change subject: mb/google/asurada: Implement enable_requlator and regulator_is_enabled ......................................................................
Patch Set 8:
(2 comments)
https://review.coreboot.org/c/coreboot/+/46687/8//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46687/8//COMMIT_MSG@7 PS8, Line 7: q g
https://review.coreboot.org/c/coreboot/+/46687/8//COMMIT_MSG@13 PS8, Line 13: q g
Hello Xi Chen, Hung-Te Lin, build bot (Jenkins), Patrick Georgi, Martin Roth, Yu-Ping Wu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46687
to look at the new patch set (#10).
Change subject: mb/google/asurada: Implement enable_regulator and regulator_is_enabled ......................................................................
mb/google/asurada: Implement enable_regulator and regulator_is_enabled
SD Card driver needs to access two regulators - MT6360_LDO5 and MT6360_LDO3. These two regulators are disabled by default.
Two APIs are implemented: - mainboard_enable_regulator: Configure the regulator as enabled/disabled. - mainboard_regulator_is_enabled: Query if the regulator is enabled.
BUG=b:168863056,b:147789962 BRANCH=none TEST=emerge-asurada coreboot
Change-Id: I391f908fcb33ffdcccc53063644482eabc863ac4 Signed-off-by: Yidi Lin yidi.lin@mediatek.com --- M src/mainboard/google/asurada/Makefile.inc M src/mainboard/google/asurada/regulator.c M src/soc/mediatek/common/include/soc/regulator.h 3 files changed, 49 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/46687/10
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46687 )
Change subject: mb/google/asurada: Implement enable_regulator and regulator_is_enabled ......................................................................
Patch Set 10: Code-Review+1
(2 comments)
https://review.coreboot.org/c/coreboot/+/46687/8//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46687/8//COMMIT_MSG@7 PS8, Line 7: q
g
Done
https://review.coreboot.org/c/coreboot/+/46687/8//COMMIT_MSG@13 PS8, Line 13: q
g
Done
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46687 )
Change subject: mb/google/asurada: Implement enable_regulator and regulator_is_enabled ......................................................................
Patch Set 11: Code-Review+2
Hung-Te Lin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/46687 )
Change subject: mb/google/asurada: Implement enable_regulator and regulator_is_enabled ......................................................................
mb/google/asurada: Implement enable_regulator and regulator_is_enabled
SD Card driver needs to access two regulators - MT6360_LDO5 and MT6360_LDO3. These two regulators are disabled by default.
Two APIs are implemented: - mainboard_enable_regulator: Configure the regulator as enabled/disabled. - mainboard_regulator_is_enabled: Query if the regulator is enabled.
BUG=b:168863056,b:147789962 BRANCH=none TEST=emerge-asurada coreboot
Change-Id: I391f908fcb33ffdcccc53063644482eabc863ac4 Signed-off-by: Yidi Lin yidi.lin@mediatek.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/46687 Reviewed-by: Hung-Te Lin hungte@chromium.org Reviewed-by: Yu-Ping Wu yupingso@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/google/asurada/Makefile.inc M src/mainboard/google/asurada/regulator.c M src/soc/mediatek/common/include/soc/regulator.h 3 files changed, 49 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Hung-Te Lin: Looks good to me, approved Yu-Ping Wu: Looks good to me, but someone else must approve
diff --git a/src/mainboard/google/asurada/Makefile.inc b/src/mainboard/google/asurada/Makefile.inc index 87582b7..601b485 100644 --- a/src/mainboard/google/asurada/Makefile.inc +++ b/src/mainboard/google/asurada/Makefile.inc @@ -20,3 +20,4 @@ ramstage-y += chromeos.c ramstage-y += mainboard.c ramstage-y += reset.c +ramstage-y += regulator.c diff --git a/src/mainboard/google/asurada/regulator.c b/src/mainboard/google/asurada/regulator.c index b06388d..d57df01 100644 --- a/src/mainboard/google/asurada/regulator.c +++ b/src/mainboard/google/asurada/regulator.c @@ -15,6 +15,10 @@ return MT6360_LDO7; case MTK_REGULATOR_VMDDR: return MT6360_LDO6; + case MTK_REGULATOR_VCC: + return MT6360_LDO5; + case MTK_REGULATOR_VCCQ: + return MT6360_LDO3; default: break; } @@ -90,3 +94,42 @@
return 0; } + +int mainboard_enable_regulator(enum mtk_regulator regulator, uint8_t enable) +{ + /* Return 0 if the regulator is already enabled or disabled. */ + if (mainboard_regulator_is_enabled(regulator) == enable) + return 0; + + int id; + + id = get_mt6360_regulator_id(regulator); + if (id < 0) { + printk(BIOS_WARNING, "Invalid regulator ID: %d\n", regulator); + return -1; + } + + return google_chromeec_regulator_enable(id, enable); +} + +uint8_t mainboard_regulator_is_enabled(enum mtk_regulator regulator) +{ + int id; + + id = get_mt6360_regulator_id(regulator); + if (id < 0) { + printk(BIOS_WARNING, "Invalid regulator ID: %d\n; assuming disabled", + regulator); + return 0; + } + + uint8_t enabled; + if (google_chromeec_regulator_is_enabled(id, &enabled) < 0) { + printk(BIOS_WARNING, + "Failed to query regulator ID: %d\n; assuming disabled", + regulator); + return 0; + } + + return enabled; +} diff --git a/src/soc/mediatek/common/include/soc/regulator.h b/src/soc/mediatek/common/include/soc/regulator.h index 6d9ff4e..0cd0f1e 100644 --- a/src/soc/mediatek/common/include/soc/regulator.h +++ b/src/soc/mediatek/common/include/soc/regulator.h @@ -11,10 +11,15 @@ MTK_REGULATOR_VDDQ, MTK_REGULATOR_VMDDR, MTK_REGULATOR_VCORE, + MTK_REGULATOR_VCC, + MTK_REGULATOR_VCCQ, };
void mainboard_set_regulator_vol(enum mtk_regulator regulator, uint32_t voltage_uv); uint32_t mainboard_get_regulator_vol(enum mtk_regulator regulator);
+int mainboard_enable_regulator(enum mtk_regulator regulator, uint8_t enable); +uint8_t mainboard_regulator_is_enabled(enum mtk_regulator regulator); + #endif /* SOC_MEDIATEK_COMMON_REGULATOR_H */
-
Hung-Te Lin (Code Review) -
Yidi Lin (Code Review)
-
Yu-Ping Wu (Code Review)