Re: [PATCH v3 2/2] clk: mediatek: mt8183: Enable need_runtime_pm on mt8183-mfgcfg

[AngeloGioacchino Del Regno]

Il 23/02/24 05:43, Chen-Yu Tsai ha scritto:
> On Mon, Jan 8, 2024 at 4:18 PM Pin-yen Lin <treapking@xxxxxxxxxxxx> 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@xxxxxxxxxxxx>
>
> Reviewed-by: Chen-Yu Tsai <wenst@xxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxx/
>
> > ---
> >
> > (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