2021-05-31 04:36:06

by Hsin-Yi Wang

[permalink] [raw]
Subject: [PATCH 1/3] soc: mtk-pm-domains: Fix the clock prepared issue

From: Weiyi Lu <[email protected]>

In this new power domain driver, when adding one power domain
it will prepare the depenedent clocks at the same.
So we only do clk_bulk_enable/disable control during power ON/OFF.
When system suspend, the pm runtime framework will forcely power off
power domains. However, the dependent clocks are disabled but kept
preapred.

In MediaTek clock drivers, PLL would be turned ON when we do
clk_bulk_prepare control.

Clock hierarchy:
PLL -->
DIV_CK -->
CLK_MUX
(may be dependent clocks)
-->
SUBSYS_CG
(may be dependent clocks)

It will lead some unexpected clock states during system suspend.
This patch will fix by doing prepare_enable/disable_unprepare on
dependent clocks at the same time while we are going to power on/off
any power domain.

Signed-off-by: Weiyi Lu <[email protected]>
Signed-off-by: Hsin-Yi Wang <[email protected]>
---
drivers/soc/mediatek/mtk-pm-domains.c | 31 +++++++--------------------
1 file changed, 8 insertions(+), 23 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
index 0af00efa0ef8..536d8c64b2b4 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.c
+++ b/drivers/soc/mediatek/mtk-pm-domains.c
@@ -211,7 +211,7 @@ static int scpsys_power_on(struct generic_pm_domain *genpd)
if (ret)
return ret;

- ret = clk_bulk_enable(pd->num_clks, pd->clks);
+ ret = clk_bulk_prepare_enable(pd->num_clks, pd->clks);
if (ret)
goto err_reg;

@@ -229,7 +229,7 @@ static int scpsys_power_on(struct generic_pm_domain *genpd)
regmap_clear_bits(scpsys->base, pd->data->ctl_offs, PWR_ISO_BIT);
regmap_set_bits(scpsys->base, pd->data->ctl_offs, PWR_RST_B_BIT);

- ret = clk_bulk_enable(pd->num_subsys_clks, pd->subsys_clks);
+ ret = clk_bulk_prepare_enable(pd->num_subsys_clks, pd->subsys_clks);
if (ret)
goto err_pwr_ack;

@@ -246,9 +246,9 @@ static int scpsys_power_on(struct generic_pm_domain *genpd)
err_disable_sram:
scpsys_sram_disable(pd);
err_disable_subsys_clks:
- clk_bulk_disable(pd->num_subsys_clks, pd->subsys_clks);
+ clk_bulk_disable_unprepare(pd->num_subsys_clks, pd->subsys_clks);
err_pwr_ack:
- clk_bulk_disable(pd->num_clks, pd->clks);
+ clk_bulk_disable_unprepare(pd->num_clks, pd->clks);
err_reg:
scpsys_regulator_disable(pd->supply);
return ret;
@@ -269,7 +269,7 @@ static int scpsys_power_off(struct generic_pm_domain *genpd)
if (ret < 0)
return ret;

- clk_bulk_disable(pd->num_subsys_clks, pd->subsys_clks);
+ clk_bulk_disable_unprepare(pd->num_subsys_clks, pd->subsys_clks);

/* subsys power off */
regmap_clear_bits(scpsys->base, pd->data->ctl_offs, PWR_RST_B_BIT);
@@ -284,7 +284,7 @@ static int scpsys_power_off(struct generic_pm_domain *genpd)
if (ret < 0)
return ret;

- clk_bulk_disable(pd->num_clks, pd->clks);
+ clk_bulk_disable_unprepare(pd->num_clks, pd->clks);

scpsys_regulator_disable(pd->supply);

@@ -405,14 +405,6 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys *scpsys, struct device_no
pd->subsys_clks[i].clk = clk;
}

- ret = clk_bulk_prepare(pd->num_clks, pd->clks);
- if (ret)
- goto err_put_subsys_clocks;
-
- ret = clk_bulk_prepare(pd->num_subsys_clks, pd->subsys_clks);
- if (ret)
- goto err_unprepare_clocks;
-
/*
* Initially turn on all domains to make the domains usable
* with !CONFIG_PM and to get the hardware in sync with the
@@ -427,7 +419,7 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys *scpsys, struct device_no
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;
+ goto err_put_subsys_clocks;
}
}

@@ -435,7 +427,7 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys *scpsys, struct device_no
ret = -EINVAL;
dev_err(scpsys->dev,
"power domain with id %d already exists, check your device-tree\n", id);
- goto err_unprepare_subsys_clocks;
+ goto err_put_subsys_clocks;
}

if (!pd->data->name)
@@ -455,10 +447,6 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys *scpsys, struct device_no

return scpsys->pd_data.domains[id];

-err_unprepare_subsys_clocks:
- clk_bulk_unprepare(pd->num_subsys_clks, pd->subsys_clks);
-err_unprepare_clocks:
- clk_bulk_unprepare(pd->num_clks, pd->clks);
err_put_subsys_clocks:
clk_bulk_put(pd->num_subsys_clks, pd->subsys_clks);
err_put_clocks:
@@ -537,10 +525,7 @@ 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);

- clk_bulk_unprepare(pd->num_clks, pd->clks);
clk_bulk_put(pd->num_clks, pd->clks);
-
- clk_bulk_unprepare(pd->num_subsys_clks, pd->subsys_clks);
clk_bulk_put(pd->num_subsys_clks, pd->subsys_clks);
}

--
2.32.0.rc0.204.g9fa02ecfa5-goog


2021-05-31 04:37:02

by Hsin-Yi Wang

[permalink] [raw]
Subject: [PATCH 2/3] soc: mtk-pm-domains: do not register smi node as syscon

Mediatek requires mmsys clocks to be unprepared during suspend,
otherwise system has chances to hang.

syscon_regmap_lookup_by_phandle_optional() will attach and prepare the
first clock in smi node, leading to additional prepare to the clock
which is not balanced with the prepare/unprepare pair in resume/suspend
callbacks.

If a power domain node requests an smi node and the smi node's first
clock is an mmsys clock, it will results in an unstabke suspend resume.

Signed-off-by: Hsin-Yi Wang <[email protected]>
---
drivers/soc/mediatek/mtk-pm-domains.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
index 536d8c64b2b4..a9ba71eee4bb 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.c
+++ b/drivers/soc/mediatek/mtk-pm-domains.c
@@ -296,7 +296,7 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys *scpsys, struct device_no
{
const struct scpsys_domain_data *domain_data;
struct scpsys_domain *pd;
- struct device_node *root_node = scpsys->dev->of_node;
+ struct device_node *root_node = scpsys->dev->of_node, *smi_node;
struct property *prop;
const char *clk_name;
int i, ret, num_clks;
@@ -352,9 +352,13 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys *scpsys, struct device_no
if (IS_ERR(pd->infracfg))
return ERR_CAST(pd->infracfg);

- pd->smi = syscon_regmap_lookup_by_phandle_optional(node, "mediatek,smi");
- if (IS_ERR(pd->smi))
- return ERR_CAST(pd->smi);
+ smi_node = of_parse_phandle(node, "mediatek,smi", 0);
+ if (smi_node) {
+ pd->smi = device_node_to_regmap(smi_node);
+ of_node_put(smi_node);
+ if (IS_ERR(pd->smi))
+ return ERR_CAST(pd->smi);
+ }

num_clks = of_clk_get_parent_count(node);
if (num_clks > 0) {
--
2.32.0.rc0.204.g9fa02ecfa5-goog

2021-05-31 04:37:39

by Hsin-Yi Wang

[permalink] [raw]
Subject: [PATCH 3/3] arm64: dts: mt8183: remove syscon from smi_common node

We don't need to register smi_common as syscon. Also add required
property power-domains for this node.

Signed-off-by: Hsin-Yi Wang <[email protected]>
---
arch/arm64/boot/dts/mediatek/mt8183.dtsi | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index c5e822b6b77a..e074c0d402ff 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -1263,13 +1263,14 @@ larb0: larb@14017000 {
};

smi_common: smi@14019000 {
- compatible = "mediatek,mt8183-smi-common", "syscon";
+ compatible = "mediatek,mt8183-smi-common";
reg = <0 0x14019000 0 0x1000>;
clocks = <&mmsys CLK_MM_SMI_COMMON>,
<&mmsys CLK_MM_SMI_COMMON>,
<&mmsys CLK_MM_GALS_COMM0>,
<&mmsys CLK_MM_GALS_COMM1>;
clock-names = "apb", "smi", "gals0", "gals1";
+ power-domains = <&spm MT8183_POWER_DOMAIN_DISP>;
};

imgsys: syscon@15020000 {
--
2.32.0.rc0.204.g9fa02ecfa5-goog

2021-05-31 10:44:09

by Chun-Jie Chen

[permalink] [raw]
Subject: Re: [PATCH 1/3] soc: mtk-pm-domains: Fix the clock prepared issue

On Mon, 2021-05-31 at 12:35 +0800, Hsin-Yi Wang wrote:
> From: Weiyi Lu <[email protected]>
>
> In this new power domain driver, when adding one power domain
> it will prepare the depenedent clocks at the same.
> So we only do clk_bulk_enable/disable control during power ON/OFF.
> When system suspend, the pm runtime framework will forcely power off
> power domains. However, the dependent clocks are disabled but kept
> preapred.
>
> In MediaTek clock drivers, PLL would be turned ON when we do
> clk_bulk_prepare control.
>
> Clock hierarchy:
> PLL -->
> DIV_CK -->
> CLK_MUX
> (may be dependent clocks)
> -->
> SUBSYS_CG
> (may be dependent clocks)
>
> It will lead some unexpected clock states during system suspend.
> This patch will fix by doing prepare_enable/disable_unprepare on
> dependent clocks at the same time while we are going to power on/off
> any power domain.
>
> Signed-off-by: Weiyi Lu <[email protected]>
> Signed-off-by: Hsin-Yi Wang <[email protected]>

Reviewed-by: chun-jie.chen <[email protected]>

> ---
> drivers/soc/mediatek/mtk-pm-domains.c | 31 +++++++----------------
> ----
> 1 file changed, 8 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/soc/mediatek/mtk-pm-domains.c
> b/drivers/soc/mediatek/mtk-pm-domains.c
> index 0af00efa0ef8..536d8c64b2b4 100644
> --- a/drivers/soc/mediatek/mtk-pm-domains.c
> +++ b/drivers/soc/mediatek/mtk-pm-domains.c
> @@ -211,7 +211,7 @@ static int scpsys_power_on(struct
> generic_pm_domain *genpd)
> if (ret)
> return ret;
>
> - ret = clk_bulk_enable(pd->num_clks, pd->clks);
> + ret = clk_bulk_prepare_enable(pd->num_clks, pd->clks);
> if (ret)
> goto err_reg;
>
> @@ -229,7 +229,7 @@ static int scpsys_power_on(struct
> generic_pm_domain *genpd)
> regmap_clear_bits(scpsys->base, pd->data->ctl_offs,
> PWR_ISO_BIT);
> regmap_set_bits(scpsys->base, pd->data->ctl_offs,
> PWR_RST_B_BIT);
>
> - ret = clk_bulk_enable(pd->num_subsys_clks, pd->subsys_clks);
> + ret = clk_bulk_prepare_enable(pd->num_subsys_clks, pd-
> >subsys_clks);
> if (ret)
> goto err_pwr_ack;
>
> @@ -246,9 +246,9 @@ static int scpsys_power_on(struct
> generic_pm_domain *genpd)
> err_disable_sram:
> scpsys_sram_disable(pd);
> err_disable_subsys_clks:
> - clk_bulk_disable(pd->num_subsys_clks, pd->subsys_clks);
> + clk_bulk_disable_unprepare(pd->num_subsys_clks, pd-
> >subsys_clks);
> err_pwr_ack:
> - clk_bulk_disable(pd->num_clks, pd->clks);
> + clk_bulk_disable_unprepare(pd->num_clks, pd->clks);
> err_reg:
> scpsys_regulator_disable(pd->supply);
> return ret;
> @@ -269,7 +269,7 @@ static int scpsys_power_off(struct
> generic_pm_domain *genpd)
> if (ret < 0)
> return ret;
>
> - clk_bulk_disable(pd->num_subsys_clks, pd->subsys_clks);
> + clk_bulk_disable_unprepare(pd->num_subsys_clks, pd-
> >subsys_clks);
>
> /* subsys power off */
> regmap_clear_bits(scpsys->base, pd->data->ctl_offs,
> PWR_RST_B_BIT);
> @@ -284,7 +284,7 @@ static int scpsys_power_off(struct
> generic_pm_domain *genpd)
> if (ret < 0)
> return ret;
>
> - clk_bulk_disable(pd->num_clks, pd->clks);
> + clk_bulk_disable_unprepare(pd->num_clks, pd->clks);
>
> scpsys_regulator_disable(pd->supply);
>
> @@ -405,14 +405,6 @@ generic_pm_domain *scpsys_add_one_domain(struct
> scpsys *scpsys, struct device_no
> pd->subsys_clks[i].clk = clk;
> }
>
> - ret = clk_bulk_prepare(pd->num_clks, pd->clks);
> - if (ret)
> - goto err_put_subsys_clocks;
> -
> - ret = clk_bulk_prepare(pd->num_subsys_clks, pd->subsys_clks);
> - if (ret)
> - goto err_unprepare_clocks;
> -
> /*
> * Initially turn on all domains to make the domains usable
> * with !CONFIG_PM and to get the hardware in sync with the
> @@ -427,7 +419,7 @@ generic_pm_domain *scpsys_add_one_domain(struct
> scpsys *scpsys, struct device_no
> 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;
> + goto err_put_subsys_clocks;
> }
> }
>
> @@ -435,7 +427,7 @@ generic_pm_domain *scpsys_add_one_domain(struct
> scpsys *scpsys, struct device_no
> ret = -EINVAL;
> dev_err(scpsys->dev,
> "power domain with id %d already exists, check
> your device-tree\n", id);
> - goto err_unprepare_subsys_clocks;
> + goto err_put_subsys_clocks;
> }
>
> if (!pd->data->name)
> @@ -455,10 +447,6 @@ generic_pm_domain *scpsys_add_one_domain(struct
> scpsys *scpsys, struct device_no
>
> return scpsys->pd_data.domains[id];
>
> -err_unprepare_subsys_clocks:
> - clk_bulk_unprepare(pd->num_subsys_clks, pd->subsys_clks);
> -err_unprepare_clocks:
> - clk_bulk_unprepare(pd->num_clks, pd->clks);
> err_put_subsys_clocks:
> clk_bulk_put(pd->num_subsys_clks, pd->subsys_clks);
> err_put_clocks:
> @@ -537,10 +525,7 @@ 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);
>
> - clk_bulk_unprepare(pd->num_clks, pd->clks);
> clk_bulk_put(pd->num_clks, pd->clks);
> -
> - clk_bulk_unprepare(pd->num_subsys_clks, pd->subsys_clks);
> clk_bulk_put(pd->num_subsys_clks, pd->subsys_clks);
> }
>

2021-05-31 11:38:48

by Chun-Jie Chen

[permalink] [raw]
Subject: Re: [PATCH 2/3] soc: mtk-pm-domains: do not register smi node as syscon

On Mon, 2021-05-31 at 12:35 +0800, Hsin-Yi Wang wrote:
> Mediatek requires mmsys clocks to be unprepared during suspend,
> otherwise system has chances to hang.
>
> syscon_regmap_lookup_by_phandle_optional() will attach and prepare
> the
> first clock in smi node, leading to additional prepare to the clock
> which is not balanced with the prepare/unprepare pair in
> resume/suspend
> callbacks.
>
> If a power domain node requests an smi node and the smi node's first
> clock is an mmsys clock, it will results in an unstabke suspend
> resume.
>
> Signed-off-by: Hsin-Yi Wang <[email protected]>

Reviewed-by: chun-jie.chen <[email protected]>

> ---
> drivers/soc/mediatek/mtk-pm-domains.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/soc/mediatek/mtk-pm-domains.c
> b/drivers/soc/mediatek/mtk-pm-domains.c
> index 536d8c64b2b4..a9ba71eee4bb 100644
> --- a/drivers/soc/mediatek/mtk-pm-domains.c
> +++ b/drivers/soc/mediatek/mtk-pm-domains.c
> @@ -296,7 +296,7 @@ generic_pm_domain *scpsys_add_one_domain(struct
> scpsys *scpsys, struct device_no
> {
> const struct scpsys_domain_data *domain_data;
> struct scpsys_domain *pd;
> - struct device_node *root_node = scpsys->dev->of_node;
> + struct device_node *root_node = scpsys->dev->of_node,
> *smi_node;
> struct property *prop;
> const char *clk_name;
> int i, ret, num_clks;
> @@ -352,9 +352,13 @@ generic_pm_domain *scpsys_add_one_domain(struct
> scpsys *scpsys, struct device_no
> if (IS_ERR(pd->infracfg))
> return ERR_CAST(pd->infracfg);
>
> - pd->smi = syscon_regmap_lookup_by_phandle_optional(node,
> "mediatek,smi");
> - if (IS_ERR(pd->smi))
> - return ERR_CAST(pd->smi);
> + smi_node = of_parse_phandle(node, "mediatek,smi", 0);
> + if (smi_node) {
> + pd->smi = device_node_to_regmap(smi_node);
> + of_node_put(smi_node);
> + if (IS_ERR(pd->smi))
> + return ERR_CAST(pd->smi);
> + }
>
> num_clks = of_clk_get_parent_count(node);
> if (num_clks > 0) {

2021-05-31 21:39:15

by Enric Balletbo Serra

[permalink] [raw]
Subject: Re: [PATCH 2/3] soc: mtk-pm-domains: do not register smi node as syscon

Hi Hsin-Yi,

(again without html, sorry for the noise)

Thank you for the patch.

Missatge de Hsin-Yi Wang <[email protected]> del dia dl., 31 de maig
2021 a les 6:35:
>
> Mediatek requires mmsys clocks to be unprepared during suspend,
> otherwise system has chances to hang.
>
> syscon_regmap_lookup_by_phandle_optional() will attach and prepare the
> first clock in smi node, leading to additional prepare to the clock
> which is not balanced with the prepare/unprepare pair in resume/suspend
> callbacks.
>
> If a power domain node requests an smi node and the smi node's first
> clock is an mmsys clock, it will results in an unstabke suspend resume.

Typo s/unstabke/unstable/

I think it would be nice to have a Fixes tag for that patch. So can be
picked for the stable kernel more easily.

>
> Signed-off-by: Hsin-Yi Wang <[email protected]>

I have a nit below but the patch looks good to me. So

Reviewed-by: Enric Balletbo i Serra <[email protected]>

> ---
> drivers/soc/mediatek/mtk-pm-domains.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
> index 536d8c64b2b4..a9ba71eee4bb 100644
> --- a/drivers/soc/mediatek/mtk-pm-domains.c
> +++ b/drivers/soc/mediatek/mtk-pm-domains.c
> @@ -296,7 +296,7 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys *scpsys, struct device_no
> {
> const struct scpsys_domain_data *domain_data;
> struct scpsys_domain *pd;
> - struct device_node *root_node = scpsys->dev->of_node;
> + struct device_node *root_node = scpsys->dev->of_node, *smi_node;

nit: Personal preference, but I'd prefer to add the smi_node in a new
line, so it's really clear that the only thing you are doing here is
adding a new variable.

> struct property *prop;
> const char *clk_name;
> int i, ret, num_clks;
> @@ -352,9 +352,13 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys *scpsys, struct device_no
> if (IS_ERR(pd->infracfg))
> return ERR_CAST(pd->infracfg);
>
> - pd->smi = syscon_regmap_lookup_by_phandle_optional(node, "mediatek,smi");
> - if (IS_ERR(pd->smi))
> - return ERR_CAST(pd->smi);
> + smi_node = of_parse_phandle(node, "mediatek,smi", 0);
> + if (smi_node) {
> + pd->smi = device_node_to_regmap(smi_node);
> + of_node_put(smi_node);
> + if (IS_ERR(pd->smi))
> + return ERR_CAST(pd->smi);
> + }
>
> num_clks = of_clk_get_parent_count(node);
> if (num_clks > 0) {
> --
> 2.32.0.rc0.204.g9fa02ecfa5-goog
>
>
> _______________________________________________
> Linux-mediatek mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

2021-05-31 21:39:49

by Enric Balletbo Serra

[permalink] [raw]
Subject: Re: [PATCH 1/3] soc: mtk-pm-domains: Fix the clock prepared issue

Hi Hsin-Yi,

Thank you for the patch.

Missatge de Hsin-Yi Wang <[email protected]> del dia dl., 31 de maig
2021 a les 6:35:
>
> From: Weiyi Lu <[email protected]>
>
> In this new power domain driver, when adding one power domain
> it will prepare the depenedent clocks at the same.

Typo: s/depenedent/dependent/

> So we only do clk_bulk_enable/disable control during power ON/OFF.
> When system suspend, the pm runtime framework will forcely power off
> power domains. However, the dependent clocks are disabled but kept
> preapred.

Typo: s/preapred/prepared

>
> In MediaTek clock drivers, PLL would be turned ON when we do
> clk_bulk_prepare control.
>
> Clock hierarchy:
> PLL -->
> DIV_CK -->
> CLK_MUX
> (may be dependent clocks)
> -->
> SUBSYS_CG
> (may be dependent clocks)
>
> It will lead some unexpected clock states during system suspend.
> This patch will fix by doing prepare_enable/disable_unprepare on
> dependent clocks at the same time while we are going to power on/off
> any power domain.
>

I think it would be nice to have a Fixes tag here, so this can be
backported more easily.

> Signed-off-by: Weiyi Lu <[email protected]>
> Signed-off-by: Hsin-Yi Wang <[email protected]>

Reviewed-by: Enric Balletbo i Serra <[email protected]>

> ---
> drivers/soc/mediatek/mtk-pm-domains.c | 31 +++++++--------------------
> 1 file changed, 8 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
> index 0af00efa0ef8..536d8c64b2b4 100644
> --- a/drivers/soc/mediatek/mtk-pm-domains.c
> +++ b/drivers/soc/mediatek/mtk-pm-domains.c
> @@ -211,7 +211,7 @@ static int scpsys_power_on(struct generic_pm_domain *genpd)
> if (ret)
> return ret;
>
> - ret = clk_bulk_enable(pd->num_clks, pd->clks);
> + ret = clk_bulk_prepare_enable(pd->num_clks, pd->clks);
> if (ret)
> goto err_reg;
>
> @@ -229,7 +229,7 @@ static int scpsys_power_on(struct generic_pm_domain *genpd)
> regmap_clear_bits(scpsys->base, pd->data->ctl_offs, PWR_ISO_BIT);
> regmap_set_bits(scpsys->base, pd->data->ctl_offs, PWR_RST_B_BIT);
>
> - ret = clk_bulk_enable(pd->num_subsys_clks, pd->subsys_clks);
> + ret = clk_bulk_prepare_enable(pd->num_subsys_clks, pd->subsys_clks);
> if (ret)
> goto err_pwr_ack;
>
> @@ -246,9 +246,9 @@ static int scpsys_power_on(struct generic_pm_domain *genpd)
> err_disable_sram:
> scpsys_sram_disable(pd);
> err_disable_subsys_clks:
> - clk_bulk_disable(pd->num_subsys_clks, pd->subsys_clks);
> + clk_bulk_disable_unprepare(pd->num_subsys_clks, pd->subsys_clks);
> err_pwr_ack:
> - clk_bulk_disable(pd->num_clks, pd->clks);
> + clk_bulk_disable_unprepare(pd->num_clks, pd->clks);
> err_reg:
> scpsys_regulator_disable(pd->supply);
> return ret;
> @@ -269,7 +269,7 @@ static int scpsys_power_off(struct generic_pm_domain *genpd)
> if (ret < 0)
> return ret;
>
> - clk_bulk_disable(pd->num_subsys_clks, pd->subsys_clks);
> + clk_bulk_disable_unprepare(pd->num_subsys_clks, pd->subsys_clks);
>
> /* subsys power off */
> regmap_clear_bits(scpsys->base, pd->data->ctl_offs, PWR_RST_B_BIT);
> @@ -284,7 +284,7 @@ static int scpsys_power_off(struct generic_pm_domain *genpd)
> if (ret < 0)
> return ret;
>
> - clk_bulk_disable(pd->num_clks, pd->clks);
> + clk_bulk_disable_unprepare(pd->num_clks, pd->clks);
>
> scpsys_regulator_disable(pd->supply);
>
> @@ -405,14 +405,6 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys *scpsys, struct device_no
> pd->subsys_clks[i].clk = clk;
> }
>
> - ret = clk_bulk_prepare(pd->num_clks, pd->clks);
> - if (ret)
> - goto err_put_subsys_clocks;
> -
> - ret = clk_bulk_prepare(pd->num_subsys_clks, pd->subsys_clks);
> - if (ret)
> - goto err_unprepare_clocks;
> -
> /*
> * Initially turn on all domains to make the domains usable
> * with !CONFIG_PM and to get the hardware in sync with the
> @@ -427,7 +419,7 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys *scpsys, struct device_no
> 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;
> + goto err_put_subsys_clocks;
> }
> }
>
> @@ -435,7 +427,7 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys *scpsys, struct device_no
> ret = -EINVAL;
> dev_err(scpsys->dev,
> "power domain with id %d already exists, check your device-tree\n", id);
> - goto err_unprepare_subsys_clocks;
> + goto err_put_subsys_clocks;
> }
>
> if (!pd->data->name)
> @@ -455,10 +447,6 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys *scpsys, struct device_no
>
> return scpsys->pd_data.domains[id];
>
> -err_unprepare_subsys_clocks:
> - clk_bulk_unprepare(pd->num_subsys_clks, pd->subsys_clks);
> -err_unprepare_clocks:
> - clk_bulk_unprepare(pd->num_clks, pd->clks);
> err_put_subsys_clocks:
> clk_bulk_put(pd->num_subsys_clks, pd->subsys_clks);
> err_put_clocks:
> @@ -537,10 +525,7 @@ 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);
>
> - clk_bulk_unprepare(pd->num_clks, pd->clks);
> clk_bulk_put(pd->num_clks, pd->clks);
> -
> - clk_bulk_unprepare(pd->num_subsys_clks, pd->subsys_clks);
> clk_bulk_put(pd->num_subsys_clks, pd->subsys_clks);
> }
>
> --
> 2.32.0.rc0.204.g9fa02ecfa5-goog
>
>
> _______________________________________________
> Linux-mediatek mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

2021-05-31 21:42:56

by Enric Balletbo Serra

[permalink] [raw]
Subject: Re: [PATCH 3/3] arm64: dts: mt8183: remove syscon from smi_common node

Hi Hsin-Yi,

(again without html, sorry for the noise)

Thank you for the patch.

Missatge de Hsin-Yi Wang <[email protected]> del dia dl., 31 de maig
2021 a les 6:36:
>
> We don't need to register smi_common as syscon. Also add required
> property power-domains for this node.
>
> Signed-off-by: Hsin-Yi Wang <[email protected]>

Reviewed-by: Enric Balletbo i Serra <[email protected]>

> ---
> arch/arm64/boot/dts/mediatek/mt8183.dtsi | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> index c5e822b6b77a..e074c0d402ff 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> @@ -1263,13 +1263,14 @@ larb0: larb@14017000 {
> };
>
> smi_common: smi@14019000 {
> - compatible = "mediatek,mt8183-smi-common", "syscon";
> + compatible = "mediatek,mt8183-smi-common";
> reg = <0 0x14019000 0 0x1000>;
> clocks = <&mmsys CLK_MM_SMI_COMMON>,
> <&mmsys CLK_MM_SMI_COMMON>,
> <&mmsys CLK_MM_GALS_COMM0>,
> <&mmsys CLK_MM_GALS_COMM1>;
> clock-names = "apb", "smi", "gals0", "gals1";
> + power-domains = <&spm MT8183_POWER_DOMAIN_DISP>;
> };
>
> imgsys: syscon@15020000 {
> --
> 2.32.0.rc0.204.g9fa02ecfa5-goog
>
>
> _______________________________________________
> Linux-mediatek mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-mediatek