Re: [PATCH v3 15/16] soc: mediatek: pm-domains: Add default power off flag

From: Matthias Brugger
Date: Thu Oct 29 2020 - 10:52:06 EST




On 27/10/2020 12:18, Weiyi Lu wrote:
On Tue, 2020-10-27 at 11:53 +0100, Matthias Brugger wrote:

On 26/10/2020 18:55, Enric Balletbo i Serra wrote:
From: Weiyi Lu <weiyi.lu@xxxxxxxxxxxx>

For some power domain, like conn on MT8192, it should be default OFF.
Because the power on/off control relies the function of connectivity chip
and its firmware. And if project choose other chip vendor solution,
those necessary connectivity functions will not provided.

Signed-off-by: Weiyi Lu <weiyi.lu@xxxxxxxxxxxx>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@xxxxxxxxxxxxx>
---

Changes in v3: None
Changes in v2: None

drivers/soc/mediatek/mtk-pm-domains.c | 23 +++++++++++++++++------
drivers/soc/mediatek/mtk-pm-domains.h | 1 +
2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
index 63993076a544..fe0e955076a0 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.c
+++ b/drivers/soc/mediatek/mtk-pm-domains.c
@@ -378,10 +378,16 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys *scpsys, struct device_no
* software. The unused domains will be switched off during
* late_init time.
*/
- ret = scpsys_power_on(&pd->genpd);
- if (ret < 0) {
- dev_err(scpsys->dev, "%pOF: failed to power on domain: %d\n", node, ret);
- goto err_unprepare_clocks;
+ if (MTK_SCPD_CAPS(pd, MTK_SCPD_KEEP_DEFAULT_OFF)) {
+ if (scpsys_domain_is_on(pd))
+ dev_warn(scpsys->dev,
+ "%pOF: A default off power domain has been ON\n", node);
+ } else {
+ ret = scpsys_power_on(&pd->genpd);
+ if (ret < 0) {
+ dev_err(scpsys->dev, "%pOF: failed to power on domain: %d\n", node, ret);
+ goto err_unprepare_clocks;
+ }
}
if (scpsys->domains[id]) {
@@ -395,7 +401,11 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys *scpsys, struct device_no
pd->genpd.power_off = scpsys_power_off;
pd->genpd.power_on = scpsys_power_on;
- pm_genpd_init(&pd->genpd, NULL, false);
+ if (MTK_SCPD_CAPS(pd, MTK_SCPD_KEEP_DEFAULT_OFF))
+ pm_genpd_init(&pd->genpd, NULL, true);
+ else
+ pm_genpd_init(&pd->genpd, NULL, false);
+
scpsys->domains[id] = &pd->genpd;
return scpsys->pd_data.domains[id];
@@ -478,7 +488,8 @@ static void scpsys_remove_one_domain(struct scpsys_domain *pd)
"failed to remove domain '%s' : %d - state may be inconsistent\n",
pd->genpd.name, ret);
- scpsys_power_off(&pd->genpd);
+ if (!MTK_SCPD_CAPS(pd, MTK_SCPD_KEEP_DEFAULT_OFF))
+ scpsys_power_off(&pd->genpd);

OK, so you merged Weiyi's patches in this series :)

So same comment here: Does it really hurt if we turn-off a already turned-off
power domain? Or can we get rid of this check?


We do need this check here. If you try to turn-off this power domain,
you might make the clock or regulator reference count unbalanced.


Correct. Could we check via scpsys_domain_is_on() instead? I'd like to avoid MTK_SCPD_KEEP_DEFAULT_OFF use here.

Regards,
Matthias

Regards,
Matthias

clk_bulk_unprepare(pd->num_clks, pd->clks);
clk_bulk_put(pd->num_clks, pd->clks);
diff --git a/drivers/soc/mediatek/mtk-pm-domains.h b/drivers/soc/mediatek/mtk-pm-domains.h
index 2ad213be84a5..0fa6a938b40c 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.h
+++ b/drivers/soc/mediatek/mtk-pm-domains.h
@@ -6,6 +6,7 @@
#define MTK_SCPD_ACTIVE_WAKEUP BIT(0)
#define MTK_SCPD_FWAIT_SRAM BIT(1)
#define MTK_SCPD_SRAM_ISO BIT(2)
+#define MTK_SCPD_KEEP_DEFAULT_OFF BIT(3)
#define MTK_SCPD_CAPS(_scpd, _x) ((_scpd)->data->caps & (_x))
#define SPM_VDE_PWR_CON 0x0210