On 17-11-22, 12:01, Sudeep Holla wrote: > Thanks for the link. Sorry I still don't get the complete picture. Who are > the consumers of these clock nodes if not cpufreq itself. > > I am going to guess, so other device(like inter-connect) with phandle into > CPU device perhaps ? Also I assume it will have phandle to non-CPU device > and hence we need generic device clock solution. Sorry for the noise, but > I still find having both clocks and qcom,freq-domain property is quite > confusing but I am fine as I understand it bit better now. Lemme try to explain what the initial problem was, because of which I suggested the DT to be fixed, even if no one is going to use it as a client. The OPP core provides two features: - Parsing of the OPP table and provide the data to the client. - Ability to switch the OPPs, i.e. configuring all resources. qcom-cpufreq-hw driver uses both of these, but in a tricky way (like Tegra30). It used the OPP core to parse the data, along with "opp-hz" property and switch the OPPs by calling dev_pm_opp_set_opp(). But it doesn't want dev_pm_opp_set_opp() to change the clock rate, but configure everything else. Now the OPP core needs to distinguish platforms for valid and invalid configurations, to make sure something isn't broken. For example a developer wants to change the OPP along with frequency and passes a valid OPP table. But forgets to set the clock entry in device's node. This is an error and the OPP core needs to report it. There can be more of such issues with different configurations. Also, as Mani explained, if the OPP core is required to switch the OPPs, then it needs to know the initial frequency of the device to see if we are going up or down the frequency graph. And so it will do a clk_get_rate() if there is "opp-hz" available. What we did in case of Tegra30 (commit 1b195626) is provide a .config_clks helper, which does nothing. So the OPP core doesn't need to know if frequency is programmed or not. The same can not be done for Qcom right now as the CPU node doesn't have the clk property though it has "opp-hz". Weather we have a user in kernel (OS) or not, shouldn't decide how the DT looks like. The DT should clearly define what the hardware looks like, irrespective of the users. The CPU has a clock and it should be mentioned. If the OPP core chooses to use that information, then it is a fine expectation to have. And so we are here. Most likely no one will ever do clk_set_rate() on this new clock, which is fine, though OPP core will likely do clk_get_rate() here. Hope I was able to clarify few things here. -- viresh