RE: [EXT] Re: [PATCH] clk: add sys node to disable unused clk

[Zhipeng Wang]

Hi Stephen,

First of all thank you very much for your reply. 

Our clk providers do not just open the clk used as you said, but rely on late_initcall_sync(clk_disable_unused); close the clks that are not used. But in order to support Android gki, I modularized clk providers and some clk consumers. This change makes late_initcall_sync(clk_disable_unused); unable to achieve our expected function. I am sorry for what may not be expressed in the commit.

I also hope you can rip out the late initcall to disable unused clks, because this can promote clk providers to be more reasonable. My main job is Android, so I need to make suggestions to our linux BSP team to improve clk providers. Because of late_initcall_sync(clk_disable_unused);, my suggestion seems to be customized.

Because I call clk_disable_unused in the user space and meet my expectations , it close the clks that are not used. That's why I uploaded this patch to you.

I hope you can make a decision whether to rip out the late initcall to disable unused clks after careful consideration. Thank you very much. 


BRs
Zhipeng
-----Original Message-----
From: Stephen Boyd <sboyd@xxxxxxxxxx> 
Sent: 2021年8月29日 13:21
To: Zhipeng Wang <zhipeng.wang_1@xxxxxxx>; mturquette@xxxxxxxxxxxx
Cc: linux-clk@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
Subject: [EXT] Re: [PATCH] clk: add sys node to disable unused clk

Caution: EXT Email

Quoting Zhipeng Wang (2021-08-13 02:11:18)
> The normal sequence is that the clock provider registers clk to the 
> clock framework, and the clock framework manages the clock resources 
> of the system. Clk consumers obtain clk through the clock framework 
> API, enable and disable clk.
>
> Not all clk registered through the clock provider will be used by the 
> clock consumer, so the clock framework has a function 
> late_initcall_sync(clk_disable_unused); disables the unused clk.
>
> Now we modularize the clock provider and some consumers, which will 
> cause late_initcall_sync(clk_disable_unused); cannot work properly, so 
> increase the sys node.

Sorry, I honestly don't know what's going on here. The commit text is describing the clk disable unused logic, and then we're adding a sysfs attribute without documenting it in Documentation/ABI? And there's one sentence about modular clks not working properly, but I don't know what is wrong about it besides it is "wrong".

I've been considering ripping out the late initcall to disable unused clks. It pretty much only leads to problems. If we want to save power, then maybe we should have clk providers tell us what clks aren't ever going to be used unless they're referenced in DT or via some clkdev lookup and only turn the ones off that are in that list if they're on.