| Message ID | 20240108081834.408403-2-treapking@chromium.org (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [v3,1/2] clk: mediatek: Introduce need_pm_runtime to mtk_clk_desc | expand |
On Mon, Jan 8, 2024 at 4:18 PM Pin-yen Lin <treapking@chromium.org> wrote: > > 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 <treapking@chromium.org> Reviewed-by: Chen-Yu Tsai <wenst@chromium.org> Note that this compliments a patch [1] adding the power domain for the mfgcfg clock controller node, which has been floating around for almost 3 years. [1] https://lore.kernel.org/linux-mediatek/20210414073108.3899082-1-ikjn@chromium.org/ > --- > > (no changes since v1) > > 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 >
Il 23/02/24 05:43, Chen-Yu Tsai ha scritto: > On Mon, Jan 8, 2024 at 4:18 PM Pin-yen Lin <treapking@chromium.org> wrote: >> >> 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 <treapking@chromium.org> > > Reviewed-by: Chen-Yu Tsai <wenst@chromium.org> > > Note that this compliments a patch [1] adding the power domain for the mfgcfg > clock controller node, which has been floating around for almost 3 years. > ...but why does this happen *only* on MT8183 and not on any other MediaTek SoC? I understand what you're trying to solve here, but if we explore a bit more, we can maybe come to the conclusion that we don't need to add this flag, and perhaps just enable PM runtime as regular flow for all clock controllers. That would also be cleaner, to some extent. Cheers, Angelo > [1] https://lore.kernel.org/linux-mediatek/20210414073108.3899082-1-ikjn@chromium.org/ > >> --- >> >> (no changes since v1) >> >> 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 >>
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[] = {
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 <treapking@chromium.org> --- (no changes since v1) drivers/clk/mediatek/clk-mt8183-mfgcfg.c | 1 + 1 file changed, 1 insertion(+)