Introduce a new need_pm_runtime variable to mtk_clk_desc to indicate this
clock needs a runtime PM get during the probing stage.
Signed-off-by: Pin-yen Lin <[email protected]>
---
drivers/clk/mediatek/clk-mtk.c | 15 +++++++++++++++
drivers/clk/mediatek/clk-mtk.h | 2 ++
2 files changed, 17 insertions(+)
diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c
index 2e55368dc4d8..c31e535909c8 100644
--- a/drivers/clk/mediatek/clk-mtk.c
+++ b/drivers/clk/mediatek/clk-mtk.c
@@ -13,6 +13,7 @@
#include <linux/of.h>
#include <linux/of_address.h>
#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
#include <linux/slab.h>
#include "clk-mtk.h"
@@ -494,6 +495,14 @@ static int __mtk_clk_simple_probe(struct platform_device *pdev,
return IS_ERR(base) ? PTR_ERR(base) : -ENOMEM;
}
+
+ if (mcd->need_runtime_pm) {
+ devm_pm_runtime_enable(&pdev->dev);
+ r = pm_runtime_resume_and_get(&pdev->dev);
+ if (r)
+ return r;
+ }
+
/* Calculate how many clk_hw_onecell_data entries to allocate */
num_clks = mcd->num_clks + mcd->num_composite_clks;
num_clks += mcd->num_fixed_clks + mcd->num_factor_clks;
@@ -574,6 +583,9 @@ static int __mtk_clk_simple_probe(struct platform_device *pdev,
goto unregister_clks;
}
+ if (mcd->need_runtime_pm)
+ pm_runtime_put(&pdev->dev);
+
return r;
unregister_clks:
@@ -604,6 +616,9 @@ static int __mtk_clk_simple_probe(struct platform_device *pdev,
free_base:
if (mcd->shared_io && base)
iounmap(base);
+
+ if (mcd->need_runtime_pm)
+ pm_runtime_put(&pdev->dev);
return r;
}
diff --git a/drivers/clk/mediatek/clk-mtk.h b/drivers/clk/mediatek/clk-mtk.h
index 22096501a60a..c17fe1c2d732 100644
--- a/drivers/clk/mediatek/clk-mtk.h
+++ b/drivers/clk/mediatek/clk-mtk.h
@@ -237,6 +237,8 @@ struct mtk_clk_desc {
int (*clk_notifier_func)(struct device *dev, struct clk *clk);
unsigned int mfg_clk_idx;
+
+ bool need_runtime_pm;
};
int mtk_clk_pdev_probe(struct platform_device *pdev);
--
2.43.0.472.g3155946c3a-goog
mt8183-mfgcfg has a mutual dependency with genpd during the probing
stage, so enable need_runtim_pm to prevent a deadlock in the following
call stack:
CPU0: genpd_lock --> clk_prepare_lock
genpd_power_off_work_fn()
genpd_lock()
generic_pm_domain::power_off()
clk_unprepare()
clk_prepare_lock()
CPU1: clk_prepare_lock --> genpd_lock
clk_register()
__clk_core_init()
clk_prepare_lock()
clk_pm_runtime_get()
genpd_lock()
Do a runtime PM get at the probe function to make sure clk_register()
won't acquire the genpd lock.
Fixes: acddfc2c261b ("clk: mediatek: Add MT8183 clock support")
Signed-off-by: Pin-yen Lin <[email protected]>
---
drivers/clk/mediatek/clk-mt8183-mfgcfg.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/clk/mediatek/clk-mt8183-mfgcfg.c b/drivers/clk/mediatek/clk-mt8183-mfgcfg.c
index ba504e19d420..62d876e150e1 100644
--- a/drivers/clk/mediatek/clk-mt8183-mfgcfg.c
+++ b/drivers/clk/mediatek/clk-mt8183-mfgcfg.c
@@ -29,6 +29,7 @@ static const struct mtk_gate mfg_clks[] = {
static const struct mtk_clk_desc mfg_desc = {
.clks = mfg_clks,
.num_clks = ARRAY_SIZE(mfg_clks),
+ .need_runtime_pm = true,
};
static const struct of_device_id of_match_clk_mt8183_mfg[] = {
--
2.43.0.472.g3155946c3a-goog
On Wed, Dec 27, 2023 at 6:05 PM Pin-yen Lin <[email protected]> wrote:
>
> Introduce a new need_pm_runtime variable to mtk_clk_desc to indicate this
> clock needs a runtime PM get during the probing stage.
Actually it means (based on our discussions and your code here) that
runtime PM should be enabled for the clock controller. If runtime PM
is not enabled before the clocks are registered, the CCF subsequently
never toggles runtime PM.
The runtime PM get during the probe stage is to avoid triggering runtime
suspend/resume during each clock registration, and hopefully avoid a
deadlock. It should be mentioned separately. A comment should be added
so that folks going over the code in the future don't remove it.
> Signed-off-by: Pin-yen Lin <[email protected]>
> ---
>
> drivers/clk/mediatek/clk-mtk.c | 15 +++++++++++++++
> drivers/clk/mediatek/clk-mtk.h | 2 ++
> 2 files changed, 17 insertions(+)
>
> diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c
> index 2e55368dc4d8..c31e535909c8 100644
> --- a/drivers/clk/mediatek/clk-mtk.c
> +++ b/drivers/clk/mediatek/clk-mtk.c
> @@ -13,6 +13,7 @@
> #include <linux/of.h>
> #include <linux/of_address.h>
> #include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> #include <linux/slab.h>
>
> #include "clk-mtk.h"
> @@ -494,6 +495,14 @@ static int __mtk_clk_simple_probe(struct platform_device *pdev,
> return IS_ERR(base) ? PTR_ERR(base) : -ENOMEM;
> }
>
> +
> + if (mcd->need_runtime_pm) {
> + devm_pm_runtime_enable(&pdev->dev);
> + r = pm_runtime_resume_and_get(&pdev->dev);
A comment for the resume and get should be added. Otherwise someone looking
at this and the CCF could think that this isn't needed, since the CCF already
has similar calls.
> + if (r)
> + return r;
> + }
> +
> /* Calculate how many clk_hw_onecell_data entries to allocate */
> num_clks = mcd->num_clks + mcd->num_composite_clks;
> num_clks += mcd->num_fixed_clks + mcd->num_factor_clks;
> @@ -574,6 +583,9 @@ static int __mtk_clk_simple_probe(struct platform_device *pdev,
> goto unregister_clks;
> }
>
> + if (mcd->need_runtime_pm)
> + pm_runtime_put(&pdev->dev);
> +
> return r;
>
> unregister_clks:
> @@ -604,6 +616,9 @@ static int __mtk_clk_simple_probe(struct platform_device *pdev,
> free_base:
> if (mcd->shared_io && base)
> iounmap(base);
> +
> + if (mcd->need_runtime_pm)
> + pm_runtime_put(&pdev->dev);
Please keep the error path calls strictly in reverse order of the setup
calls. So this should go before iounmap().
ChenYu
> return r;
> }
> diff --git a/drivers/clk/mediatek/clk-mtk.h b/drivers/clk/mediatek/clk-mtk.h
> index 22096501a60a..c17fe1c2d732 100644
> --- a/drivers/clk/mediatek/clk-mtk.h
> +++ b/drivers/clk/mediatek/clk-mtk.h
> @@ -237,6 +237,8 @@ struct mtk_clk_desc {
>
> int (*clk_notifier_func)(struct device *dev, struct clk *clk);
> unsigned int mfg_clk_idx;
> +
> + bool need_runtime_pm;
> };
>
> int mtk_clk_pdev_probe(struct platform_device *pdev);
> --
> 2.43.0.472.g3155946c3a-goog
>