Re: [PATCH v7 0/4] qcom-cpufreq-hw: Add CPU clock provider support

From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
To: Sudeep Holla <sudeep.holla@arm.com>
Cc: andersson@kernel.org, viresh.kumar@linaro.org,
	krzysztof.kozlowski+dt@linaro.org, rafael@kernel.org,
	robh+dt@kernel.org, johan@kernel.org, devicetree@vger.kernel.org,
	linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-pm@vger.kernel.org
Subject: Re: [PATCH v7 0/4] qcom-cpufreq-hw: Add CPU clock provider support
Date: Thu, 17 Nov 2022 18:08:41 +0530	[thread overview]
Message-ID: <20221117123841.GI93179@thinkpad> (raw)
In-Reply-To: <20221117120846.yhmilsndw2bmmvnv@bogus>

On Thu, Nov 17, 2022 at 12:08:46PM +0000, Sudeep Holla wrote:
> On Thu, Nov 17, 2022 at 05:28:07PM +0530, Manivannan Sadhasivam wrote:
> > On Thu, Nov 17, 2022 at 11:52:03AM +0000, Sudeep Holla wrote:
> > > On Thu, Nov 17, 2022 at 04:42:07PM +0530, Manivannan Sadhasivam wrote:
> > > > On Thu, Nov 17, 2022 at 10:19:03AM +0000, Sudeep Holla wrote:
> > > > > 
> > > > > Why do you need the above 3 changes if the below(4/4) will ensure
> > > > > cpufreq_get(cpu) returns the clock frequency. I was expecting to drop the
> > > > > whole "confusing" clock bindings and the unnecessary clock provider.
> > > > > 
> > > > > Can't we just use cpufreq_get(cpu) ?
> > > > > 
> > > > 
> > > > This can be possible for OPP implementations for the CPUs but not for other
> > > > peripherals making use of OPP framework like GPU etc... Moreover this may end
> > > > up with different code path for CPUs and other peripherals inside OPP framework.
> > > > 
> > > 
> > > Fair enough, you can use this for non-CPU devices. But you are adding this for
> > > CPUs here. Is the consumer unaware that this is a CPU or non-CPU device ?
> > > If so, make sense. Otherwise, it is unnecessary to go through the clk
> > > framework to get CPU frequency.
> > > 
> > 
> > The consumer here is the OPP framework and yes it doesn't have the knowledge of
> > the device it is dealing with (for this context).
> 
> Ah OK, I thought it is something else. Does this mean OPP is tied with clk
> framework or clock bindings ? Is this for some specific feature ?

AFAIK, OPP framework needs to know the current frequency of the device it is
dealing with for setting the device's OPP. So it uses clk_get_rate() of the
first clock of the device. If the clock is not available, then it uses the
frequency in the first entry of the OPP table (since it is going to be the
minimum freq of the device).

As you can see, the clk_get_rate() is eminent for switching the OPPs and since
OPP framework doesn't know what device it is dealing with, it cannot use
cpufreq_get().

> Or is it
> compulsory for all the devices using OPP ? Just wondering how this affects
> SCMI which doesn't use or provide clocks yet.

Is SCMI node itself has the OPP tables? Or the consumer nodes of the SCMI?

TLDR; If you tell OPP framework to set a new OPP for a device, it needs to the
know the current frequency of the device. And it is not manadatory now, but in
the future maybe.

Thanks,
Mani

> -- 
> Regards,
> Sudeep

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