Hello,
This series adds clock provider support to the Qcom CPUFreq driver for
supplying the clocks to the CPU cores in Qcom SoCs.
The Qualcomm platforms making use of CPUFreq HW Engine (EPSS/OSM) supply
clocks to the CPU cores. But this is not represented clearly in devicetree.
There is no clock coming out of the CPUFreq HW node to the CPU. This created
an issue [1] with the OPP core when a recent enhancement series was submitted.
Eventhough the issue got fixed in the OPP framework in the meantime, that's
not a proper solution and this series aims to fix it properly.
There was also an attempt made by Viresh [2] to fix the issue by moving the
clocks supplied to the CPUFreq HW node to the CPU. But that was not accepted
since those clocks belong to the CPUFreq HW node only.
The proposal here is to add clock provider support to the Qcom CPUFreq HW
driver to supply clocks to the CPUs that comes out of the EPSS/OSM block.
This correctly reflects the hardware implementation.
The clock provider is a simple one that just provides the frequency of the
clocks supplied to each frequency domain in the SoC using .recalc_rate()
callback. The frequency supplied by the driver will be the actual frequency
that comes out of the EPSS/OSM block after the DCVS operation. This frequency
is not same as what the CPUFreq framework has set but it is the one that gets
supplied to the CPUs after throttling by LMh.
This series has been tested on SM8450 based dev board with the OPP hack removed
and hence there is a DTS change only for that platform. Once this series gets
accepted, rest of the platform DTS can also be modified and finally the hack on
the OPP core can be dropped.
Thanks,
Mani
[1] https://lore.kernel.org/lkml/[email protected]/
[2] https://lore.kernel.org/lkml/20220801054255.GA12039@thinkpad/t/
Changes in v7:
* Added a patch that returns the throttled frequency for cpufreq_driver->get()
callback (Sudeep & Viresh)
* Added error check for kasprintf and allocated the clk name locally
Changes in v6:
* Removed the local variable clk_name (Matthias)
* Added the clock id to the error message of devm_clk_hw_register()
Changes in v5:
* Switched to Hz unit for the CPU clocks
Changes in v4:
* Rebased on top of cpufreq/arm/linux-next branch
Changes in v3:
* Submitted the cpufreq driver cleanup patches as a separate series as
suggested by Viresh
* Removed static keyword from clk_init_data declaration
Changes in v2:
* Moved the qcom_cpufreq_data allocation to probe
* Added single clock provider with multiple clks for each freq domain
* Moved soc_data to qcom_cpufreq struct
* Added Rob's review for binding
Manivannan Sadhasivam (4):
dt-bindings: cpufreq: cpufreq-qcom-hw: Add cpufreq clock provider
arm64: dts: qcom: sm8450: Supply clock from cpufreq node to CPUs
cpufreq: qcom-hw: Add CPU clock provider support
cpufreq: qcom-hw: Fix the frequency returned by cpufreq_driver->get()
.../bindings/cpufreq/cpufreq-qcom-hw.yaml | 12 +++
arch/arm64/boot/dts/qcom/sm8450.dtsi | 9 ++
drivers/cpufreq/qcom-cpufreq-hw.c | 87 ++++++++++++++++---
3 files changed, 95 insertions(+), 13 deletions(-)
--
2.25.1
On Thu, Nov 17, 2022 at 11:01:41AM +0530, Manivannan Sadhasivam wrote:
> Hello,
>
> This series adds clock provider support to the Qcom CPUFreq driver for
> supplying the clocks to the CPU cores in Qcom SoCs.
>
> The Qualcomm platforms making use of CPUFreq HW Engine (EPSS/OSM) supply
> clocks to the CPU cores. But this is not represented clearly in devicetree.
> There is no clock coming out of the CPUFreq HW node to the CPU. This created
> an issue [1] with the OPP core when a recent enhancement series was submitted.
> Eventhough the issue got fixed in the OPP framework in the meantime, that's
> not a proper solution and this series aims to fix it properly.
>
> There was also an attempt made by Viresh [2] to fix the issue by moving the
> clocks supplied to the CPUFreq HW node to the CPU. But that was not accepted
> since those clocks belong to the CPUFreq HW node only.
>
> The proposal here is to add clock provider support to the Qcom CPUFreq HW
> driver to supply clocks to the CPUs that comes out of the EPSS/OSM block.
> This correctly reflects the hardware implementation.
>
> The clock provider is a simple one that just provides the frequency of the
> clocks supplied to each frequency domain in the SoC using .recalc_rate()
> callback. The frequency supplied by the driver will be the actual frequency
> that comes out of the EPSS/OSM block after the DCVS operation. This frequency
> is not same as what the CPUFreq framework has set but it is the one that gets
> supplied to the CPUs after throttling by LMh.
>
> This series has been tested on SM8450 based dev board with the OPP hack removed
> and hence there is a DTS change only for that platform. Once this series gets
> accepted, rest of the platform DTS can also be modified and finally the hack on
> the OPP core can be dropped.
>
> Thanks,
> Mani
>
> [1] https://lore.kernel.org/lkml/[email protected]/
> [2] https://lore.kernel.org/lkml/20220801054255.GA12039@thinkpad/t/
>
> Changes in v7:
>
> * Added a patch that returns the throttled frequency for cpufreq_driver->get()
> callback (Sudeep & Viresh)
> * Added error check for kasprintf and allocated the clk name locally
>
> Changes in v6:
>
> * Removed the local variable clk_name (Matthias)
> * Added the clock id to the error message of devm_clk_hw_register()
>
> Changes in v5:
>
> * Switched to Hz unit for the CPU clocks
>
> Changes in v4:
>
> * Rebased on top of cpufreq/arm/linux-next branch
>
> Changes in v3:
>
> * Submitted the cpufreq driver cleanup patches as a separate series as
> suggested by Viresh
> * Removed static keyword from clk_init_data declaration
>
> Changes in v2:
>
> * Moved the qcom_cpufreq_data allocation to probe
> * Added single clock provider with multiple clks for each freq domain
> * Moved soc_data to qcom_cpufreq struct
> * Added Rob's review for binding
>
> Manivannan Sadhasivam (4):
> dt-bindings: cpufreq: cpufreq-qcom-hw: Add cpufreq clock provider
> arm64: dts: qcom: sm8450: Supply clock from cpufreq node to CPUs
> cpufreq: qcom-hw: Add CPU clock provider support
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) ?
--
Regards,
Sudeep
On 17-11-22, 10:19, 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) ?
https://lore.kernel.org/lkml/[email protected]/
The basic idea (need) here was to fix the DT and let the CPU nodes have clock
related properties, which are missing currently.
The context can be seen in the above thread.
--
viresh
On Thu, Nov 17, 2022 at 10:19:03AM +0000, Sudeep Holla wrote:
> On Thu, Nov 17, 2022 at 11:01:41AM +0530, Manivannan Sadhasivam wrote:
> > Hello,
> >
> > This series adds clock provider support to the Qcom CPUFreq driver for
> > supplying the clocks to the CPU cores in Qcom SoCs.
> >
> > The Qualcomm platforms making use of CPUFreq HW Engine (EPSS/OSM) supply
> > clocks to the CPU cores. But this is not represented clearly in devicetree.
> > There is no clock coming out of the CPUFreq HW node to the CPU. This created
> > an issue [1] with the OPP core when a recent enhancement series was submitted.
> > Eventhough the issue got fixed in the OPP framework in the meantime, that's
> > not a proper solution and this series aims to fix it properly.
> >
> > There was also an attempt made by Viresh [2] to fix the issue by moving the
> > clocks supplied to the CPUFreq HW node to the CPU. But that was not accepted
> > since those clocks belong to the CPUFreq HW node only.
> >
> > The proposal here is to add clock provider support to the Qcom CPUFreq HW
> > driver to supply clocks to the CPUs that comes out of the EPSS/OSM block.
> > This correctly reflects the hardware implementation.
> >
> > The clock provider is a simple one that just provides the frequency of the
> > clocks supplied to each frequency domain in the SoC using .recalc_rate()
> > callback. The frequency supplied by the driver will be the actual frequency
> > that comes out of the EPSS/OSM block after the DCVS operation. This frequency
> > is not same as what the CPUFreq framework has set but it is the one that gets
> > supplied to the CPUs after throttling by LMh.
> >
> > This series has been tested on SM8450 based dev board with the OPP hack removed
> > and hence there is a DTS change only for that platform. Once this series gets
> > accepted, rest of the platform DTS can also be modified and finally the hack on
> > the OPP core can be dropped.
> >
> > Thanks,
> > Mani
> >
> > [1] https://lore.kernel.org/lkml/[email protected]/
> > [2] https://lore.kernel.org/lkml/20220801054255.GA12039@thinkpad/t/
> >
> > Changes in v7:
> >
> > * Added a patch that returns the throttled frequency for cpufreq_driver->get()
> > callback (Sudeep & Viresh)
> > * Added error check for kasprintf and allocated the clk name locally
> >
> > Changes in v6:
> >
> > * Removed the local variable clk_name (Matthias)
> > * Added the clock id to the error message of devm_clk_hw_register()
> >
> > Changes in v5:
> >
> > * Switched to Hz unit for the CPU clocks
> >
> > Changes in v4:
> >
> > * Rebased on top of cpufreq/arm/linux-next branch
> >
> > Changes in v3:
> >
> > * Submitted the cpufreq driver cleanup patches as a separate series as
> > suggested by Viresh
> > * Removed static keyword from clk_init_data declaration
> >
> > Changes in v2:
> >
> > * Moved the qcom_cpufreq_data allocation to probe
> > * Added single clock provider with multiple clks for each freq domain
> > * Moved soc_data to qcom_cpufreq struct
> > * Added Rob's review for binding
> >
> > Manivannan Sadhasivam (4):
> > dt-bindings: cpufreq: cpufreq-qcom-hw: Add cpufreq clock provider
> > arm64: dts: qcom: sm8450: Supply clock from cpufreq node to CPUs
> > cpufreq: qcom-hw: Add CPU clock provider support
>
> 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.
So I don't think it is applicable. But I'll defer it to Viresh.
Thanks,
Mani
> --
> Regards,
> Sudeep
--
மணிவண்ணன் சதாசிவம்
On Thu, Nov 17, 2022 at 04:54:03PM +0530, Viresh Kumar wrote:
> On 17-11-22, 10:19, 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) ?
>
> https://lore.kernel.org/lkml/[email protected]/
>
> The basic idea (need) here was to fix the DT and let the CPU nodes have clock
> related properties, which are missing currently.
>
> The context can be seen in the above thread.
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.
--
Regards,
Sudeep
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.
--
Regards,
Sudeep
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 ? Or is it
compulsory for all the devices using OPP ? Just wondering how this affects
SCMI which doesn't use or provide clocks yet.
--
Regards,
Sudeep
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).
Thanks,
Mani
> --
> Regards,
> Sudeep
--
மணிவண்ணன் சதாசிவம்
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
--
மணிவண்ணன் சதாசிவம்
On Thu, Nov 17, 2022 at 06:08:41PM +0530, Manivannan Sadhasivam wrote:
[...]
>
> 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).
>
It has been a while since I followed OPP. Thanks for all the info, helped me
get updated without looking at the code in detail.
> 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().
>
Agreed. I had assumed the qcom-cpufreq-hw as setting cpufreq directly pocking
the hardware but now I see it is using opp library to set some additional
policy.
> Is SCMI node itself has the OPP tables? Or the consumer nodes of the SCMI?
>
No, OPPs are read from the f/w and we just use OPP APIs to register them.
But we don't use OPP library to set the performance.
> 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.
>
Hmm, good to know. I prefer it is not coupled with clocks and have some
alternative mechanism that is suitable for performance domains and don't
enforce the use of clock bindings and clk framework unnecessarily.
--
Regards,
Sudeep`
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
On Fri, Nov 18, 2022 at 11:27:30AM +0530, Viresh Kumar wrote:
> 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.
>
Thanks a ton for such detailed explanation. Ulf was trying to integrate
genpd + performance domains with SCMI. That is the reason for my interest
in this topic. Sorry for all the trouble.
--
Regards,
Sudeep
On 17-11-22, 11:01, Manivannan Sadhasivam wrote:
> Hello,
>
> This series adds clock provider support to the Qcom CPUFreq driver for
> supplying the clocks to the CPU cores in Qcom SoCs.
>
> The Qualcomm platforms making use of CPUFreq HW Engine (EPSS/OSM) supply
> clocks to the CPU cores. But this is not represented clearly in devicetree.
> There is no clock coming out of the CPUFreq HW node to the CPU. This created
> an issue [1] with the OPP core when a recent enhancement series was submitted.
> Eventhough the issue got fixed in the OPP framework in the meantime, that's
> not a proper solution and this series aims to fix it properly.
>
> There was also an attempt made by Viresh [2] to fix the issue by moving the
> clocks supplied to the CPUFreq HW node to the CPU. But that was not accepted
> since those clocks belong to the CPUFreq HW node only.
>
> The proposal here is to add clock provider support to the Qcom CPUFreq HW
> driver to supply clocks to the CPUs that comes out of the EPSS/OSM block.
> This correctly reflects the hardware implementation.
>
> The clock provider is a simple one that just provides the frequency of the
> clocks supplied to each frequency domain in the SoC using .recalc_rate()
> callback. The frequency supplied by the driver will be the actual frequency
> that comes out of the EPSS/OSM block after the DCVS operation. This frequency
> is not same as what the CPUFreq framework has set but it is the one that gets
> supplied to the CPUs after throttling by LMh.
>
> This series has been tested on SM8450 based dev board with the OPP hack removed
> and hence there is a DTS change only for that platform. Once this series gets
> accepted, rest of the platform DTS can also be modified and finally the hack on
> the OPP core can be dropped.
Applied. Thanks.
If you get review comments later on, please send incremental patches
for that.
--
viresh
On Mon, Nov 21, 2022 at 10:49:59AM +0530, Viresh Kumar wrote:
> On 17-11-22, 11:01, Manivannan Sadhasivam wrote:
> > Hello,
> >
> > This series adds clock provider support to the Qcom CPUFreq driver for
> > supplying the clocks to the CPU cores in Qcom SoCs.
> >
> > The Qualcomm platforms making use of CPUFreq HW Engine (EPSS/OSM) supply
> > clocks to the CPU cores. But this is not represented clearly in devicetree.
> > There is no clock coming out of the CPUFreq HW node to the CPU. This created
> > an issue [1] with the OPP core when a recent enhancement series was submitted.
> > Eventhough the issue got fixed in the OPP framework in the meantime, that's
> > not a proper solution and this series aims to fix it properly.
> >
> > There was also an attempt made by Viresh [2] to fix the issue by moving the
> > clocks supplied to the CPUFreq HW node to the CPU. But that was not accepted
> > since those clocks belong to the CPUFreq HW node only.
> >
> > The proposal here is to add clock provider support to the Qcom CPUFreq HW
> > driver to supply clocks to the CPUs that comes out of the EPSS/OSM block.
> > This correctly reflects the hardware implementation.
> >
> > The clock provider is a simple one that just provides the frequency of the
> > clocks supplied to each frequency domain in the SoC using .recalc_rate()
> > callback. The frequency supplied by the driver will be the actual frequency
> > that comes out of the EPSS/OSM block after the DCVS operation. This frequency
> > is not same as what the CPUFreq framework has set but it is the one that gets
> > supplied to the CPUs after throttling by LMh.
> >
> > This series has been tested on SM8450 based dev board with the OPP hack removed
> > and hence there is a DTS change only for that platform. Once this series gets
> > accepted, rest of the platform DTS can also be modified and finally the hack on
> > the OPP core can be dropped.
>
> Applied. Thanks.
>
> If you get review comments later on, please send incremental patches
> for that.
>
Sure thing.
Thanks,
Mani
> --
> viresh
--
மணிவண்ணன் சதாசிவம்
On 21-11-22, 10:49, Viresh Kumar wrote:
> On 17-11-22, 11:01, Manivannan Sadhasivam wrote:
> > Hello,
> >
> > This series adds clock provider support to the Qcom CPUFreq driver for
> > supplying the clocks to the CPU cores in Qcom SoCs.
> >
> > The Qualcomm platforms making use of CPUFreq HW Engine (EPSS/OSM) supply
> > clocks to the CPU cores. But this is not represented clearly in devicetree.
> > There is no clock coming out of the CPUFreq HW node to the CPU. This created
> > an issue [1] with the OPP core when a recent enhancement series was submitted.
> > Eventhough the issue got fixed in the OPP framework in the meantime, that's
> > not a proper solution and this series aims to fix it properly.
> >
> > There was also an attempt made by Viresh [2] to fix the issue by moving the
> > clocks supplied to the CPUFreq HW node to the CPU. But that was not accepted
> > since those clocks belong to the CPUFreq HW node only.
> >
> > The proposal here is to add clock provider support to the Qcom CPUFreq HW
> > driver to supply clocks to the CPUs that comes out of the EPSS/OSM block.
> > This correctly reflects the hardware implementation.
> >
> > The clock provider is a simple one that just provides the frequency of the
> > clocks supplied to each frequency domain in the SoC using .recalc_rate()
> > callback. The frequency supplied by the driver will be the actual frequency
> > that comes out of the EPSS/OSM block after the DCVS operation. This frequency
> > is not same as what the CPUFreq framework has set but it is the one that gets
> > supplied to the CPUs after throttling by LMh.
> >
> > This series has been tested on SM8450 based dev board with the OPP hack removed
> > and hence there is a DTS change only for that platform. Once this series gets
> > accepted, rest of the platform DTS can also be modified and finally the hack on
> > the OPP core can be dropped.
>
> Applied. Thanks.
I have dropped patch 2/4 now, since Mani wants to get it merged via
SoC tree.
--
viresh
On Thu, 17 Nov 2022 11:01:41 +0530, Manivannan Sadhasivam wrote:
> This series adds clock provider support to the Qcom CPUFreq driver for
> supplying the clocks to the CPU cores in Qcom SoCs.
>
> The Qualcomm platforms making use of CPUFreq HW Engine (EPSS/OSM) supply
> clocks to the CPU cores. But this is not represented clearly in devicetree.
> There is no clock coming out of the CPUFreq HW node to the CPU. This created
> an issue [1] with the OPP core when a recent enhancement series was submitted.
> Eventhough the issue got fixed in the OPP framework in the meantime, that's
> not a proper solution and this series aims to fix it properly.
>
> [...]
Applied, thanks!
[2/4] arm64: dts: qcom: sm8450: Supply clock from cpufreq node to CPUs
commit: 8a8845e07b1164792a4dd5ad4d333d793828b366
Best regards,
--
Bjorn Andersson <[email protected]>