Re: [PATCH v5 3/3] cpufreq: qcom-hw: Add CPU clock provider support

Manivannan Sadhasivam
2022-11-09T12:35:41Z

On Wed, Nov 09, 2022 at 11:08:31AM +0000, Sudeep Holla wrote:
> On Wed, Nov 09, 2022 at 01:19:08PM +0530, Manivannan Sadhasivam wrote:
>
> [...]
>
> > qcom_cpufreq_hw_get() returns the frequency that got programmed by the cpufreq
> > core. But that frequency is not necessarily the one that gets delivered to the
> > CPU cores because the EPSS/OSM hardware block may vary the frequency after the
> > DCVS operation.
> >
> > So this frequency is the final one that gets delivered to the CPU cores.
> >
>
> OK, thanks for the info. Just wondering if there is any issue making
> qcom_cpufreq_hw_get() return this value instead of all these complexity.
> I think the DT binding is too confusing as cpufreq-dt uses that to manage
> DVFS which this one uses it.
>
> If possible we should just make cpufreq_get(cpu) return the value you need
> and use the same where ever you need. Sorry if I am missing something obvious
> but I am struggling to see that.
>

I don't think using the final DCVS frequency would be applicable for cpufreq
core.

cpufreq core sets the desired frequency in the form of index using the
target_index() callback and the qcom-cpufreq-hw driver uses that index directly
to select the specific entry in the hardware LUT (Look Up Table).

Then with get() callback, the frequency will be returned based on the LUT index
read from the hardware. In this case, the frequency is going to be static
(i.e, what gets set by the cpufreq core will be the same). I believe this is
what the API also expects.

In the case of qcom_lmh_get_throttle_freq(), the frequency is going to be
dynamic (i.e changes with every internal DCVS operation). But this is exactly
what the OPP core expects with clk_get_rate() of CPU clock, so using
qcom_lmh_get_throttle_freq() makes sense there.

Thanks,
Mani

> --
> Regards,
> Sudeep

--
மணிவண்ணன் சதாசிவம்