linux-kernel - Re: [PATCH v3 2/2] clk: mediatek: mt8183: Enable need_runtime

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Mon, 26 Feb 2024 12:35:11 +0100
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
To: Chen-Yu Tsai <wenst@...omium.org>, Pin-yen Lin <treapking@...omium.org>
Cc: Michael Turquette <mturquette@...libre.com>,
 Stephen Boyd <sboyd@...nel.org>, Matthias Brugger <matthias.bgg@...il.com>,
 Weiyi Lu <weiyi.lu@...iatek.com>, linux-kernel@...r.kernel.org,
 linux-clk@...r.kernel.org, linux-mediatek@...ts.infradead.org,
 linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v3 2/2] clk: mediatek: mt8183: Enable need_runtime_pm on
 mt8183-mfgcfg

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