Re: [PATCH v2 1/2] clk: mediatek: Introduce need_pm_runtime to mtk_clk_desc

[Eugen Hristev]

On 1/3/24 14:19, AngeloGioacchino Del Regno wrote:
> Il 02/01/24 09:12, Pin-yen Lin ha scritto:
>> Introduce a new need_pm_runtime variable to mtk_clk_desc to indicate
>> this clock needs a runtime PM get on the clock controller during the
>> probing stage.
>>
>> Signed-off-by: Pin-yen Lin <treapking@xxxxxxxxxxxx>
> 
> Hello Pin-yen,
> 
> We have experienced something similar, but it was really hard to reproduce after
> some changes.
> 
> In an effort to try to solve this issue (but again, reproducing is really hard),
> Eugen has sent a commit in the hope that someone else found a way to easily
> reproduce. Please look at [1].
> 
> I'm also adding Eugen to the Cc's of this email.
> 
> Cheers,
> Angelo
> 
> [1]: 
> https://patchwork.kernel.org/project/linux-pm/patch/20231225133615.78993-1-eugen.hristev@xxxxxxxxxxxxx/

Hello Pin-yen,

Can you try my patch and let me know if this changes anything for you ?

If it does not change anything, can you also try this one as well ? It's another
attempt to fix the synchronization with genpd.

https://lore.kernel.org/linux-arm-kernel/20231129113120.4907-1-eugen.hristev@xxxxxxxxxxxxx/

Thanks,
Eugen

> 
>> ---
>>
>> Changes in v2:
>> - Fix the order of error handling
>> - Update the commit message and add a comment before the runtime PM call
>>
>>   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);
> 
> 
> 
>