Re: [PATCH v4 2/8] clk: qcom: ipq5332: enable few nssnoc clocks in driver probe

From: Kathiravan Thirumoorthy
Date: Wed Feb 14 2024 - 04:20:37 EST




On 1/26/2024 1:35 AM, Andrew Lunn wrote:
On Mon, Jan 22, 2024 at 11:26:58AM +0530, Kathiravan Thirumoorthy wrote:
gcc_snoc_nssnoc_clk, gcc_snoc_nssnoc_1_clk, gcc_nssnoc_nsscc_clk are
enabled by default and it's RCG is properly configured by bootloader.

Which bootloader? Mainline barebox?


Thanks for taking time to review the patches. I couldn't get time to respond back, sorry for the delay.

I was referring to the U-boot which is delivered as part of the QSDK. I will call it out explicitly in the next patch.


Some of the NSS clocks needs these clocks to be enabled. To avoid
these clocks being disabled by clock framework, drop these entries
from the clock table and enable it in the driver probe itself.

If they are critical clocks, i would expect a device to reference
them. The CCF only disabled unused clocks in late_initcall_sync(),
which means all drivers should of probed and taken a reference on any
clocks they require.


Some of the NSSCC clocks are enabled by bootloaders and CCF disables the same (because currently there are no consumers for these clocks available in the tree. These clocks are consumed by the Networking drivers which are being upstreamed). To access the NSSCC clocks, gcc_snoc_nssnoc_clk, gcc_snoc_nssnoc_1_clk, gcc_nssnoc_nsscc_clk clocks needs to be enabled, else system is going to reboot. To prevent this, I enabled it in probe.

However looking back, gcc_snoc_nssnoc_clk, gcc_snoc_nssnoc_1_clk, gcc_nssnoc_nsscc_clk are consumed by the networking drivers only. So is it okay to drop these clocks from the GCC driver and add it back once the actual consumer needs it? So that we don't have to enable it in probe.

Please let me know your thoughts.



Please correctly describe the clock tree in device tree, not hide
clocks because your DT description is not complete.

Andrew

---
pw-bot: cr