Messages in this thread | | | Date | Thu, 20 Oct 2022 08:39:50 +0300 | Subject | Re: [PATCH 3/4] cpufreq: qcom-hw: Add CPU clock provider support | From | Dmitry Baryshkov <> |
| |
On 19/10/2022 16:59, Manivannan Sadhasivam wrote: > Qcom CPUFreq hardware (EPSS/OSM) controls clock and voltage to the CPU > cores. But this relationship is not represented with the clk framework > so far. > > So, let's make the qcom-cpufreq-hw driver a clock provider. This makes the > clock producer/consumer relationship cleaner and is also useful for CPU > related frameworks like OPP to know the frequency at which the CPUs are > running. > > The clock frequency provided by the driver is for each CPU policy. We > cannot get the frequency of each CPU core because, not all platforms > support per-core DCVS feature. > > Also the frequency supplied by the driver is 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. > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > --- > drivers/cpufreq/qcom-cpufreq-hw.c | 67 +++++++++++++++++++++++++++++-- > 1 file changed, 63 insertions(+), 4 deletions(-) > > diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c > index a5b3b8d0e164..4dd710f9fb69 100644 > --- a/drivers/cpufreq/qcom-cpufreq-hw.c > +++ b/drivers/cpufreq/qcom-cpufreq-hw.c > @@ -4,6 +4,7 @@ > */ > > #include <linux/bitfield.h> > +#include <linux/clk-provider.h> > #include <linux/cpufreq.h> > #include <linux/init.h> > #include <linux/interconnect.h> > @@ -54,6 +55,7 @@ struct qcom_cpufreq_data { > bool cancel_throttle; > struct delayed_work throttle_work; > struct cpufreq_policy *policy; > + struct clk_hw cpu_clk; > > bool per_core_dcvs; > }; > @@ -482,6 +484,54 @@ static void qcom_cpufreq_hw_lmh_exit(struct qcom_cpufreq_data *data) > free_irq(data->throttle_irq, data); > } > > +static unsigned long qcom_cpufreq_hw_recalc_rate(struct clk_hw *hw, unsigned long parent_rate) > +{ > + struct qcom_cpufreq_data *data = container_of(hw, struct qcom_cpufreq_data, cpu_clk); > + > + return qcom_lmh_get_throttle_freq(data) / HZ_PER_KHZ; > +} > + > +static const struct clk_ops qcom_cpufreq_hw_clk_ops = { > + .recalc_rate = qcom_cpufreq_hw_recalc_rate, > +}; > + > +static int qcom_cpufreq_hw_clk_add(struct qcom_cpufreq_data *data, u32 index) > +{ > + struct platform_device *pdev = cpufreq_get_driver_data(); > + struct device *dev = &pdev->dev; > + char *clk_name = devm_kasprintf(dev, GFP_KERNEL, "qcom_cpufreq%d", index); > + static struct clk_init_data init = {}; > + int ret; > + > + init.name = clk_name; > + init.flags = CLK_GET_RATE_NOCACHE; > + init.ops = &qcom_cpufreq_hw_clk_ops; > + data->cpu_clk.init = &init; > + > + ret = clk_hw_register(dev, &data->cpu_clk); > + if (ret < 0) { > + dev_err(dev, "Failed to register Qcom CPUFreq clock\n"); > + return ret; > + } > + > + ret = of_clk_add_hw_provider(dev->of_node, of_clk_hw_simple_get, &data->cpu_clk);
This doesn't look corresponding to the DT bindings you are adding. of_clk_hw_simple_get() would return a single clock per dt node, whichever arguments were passed, while you are adding clocks correspoding to CPU clusters.
From what I see according to the bindings, you should register a single provider using the of_clk_hw_onecell_get() function.
> + if (ret < 0) { > + dev_err(dev, "Failed to add Qcom CPUFreq clock provider\n"); > + clk_hw_unregister(&data->cpu_clk); > + } > + > + return ret; > +} > + > +static void qcom_cpufreq_hw_clk_remove(struct qcom_cpufreq_data *data) > +{ > + struct platform_device *pdev = cpufreq_get_driver_data(); > + struct device *dev = &pdev->dev; > + > + of_clk_del_provider(dev->of_node); > + clk_hw_unregister(&data->cpu_clk); > +} > + > static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy) > { > struct platform_device *pdev = cpufreq_get_driver_data(); > @@ -556,19 +606,24 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy) > policy->driver_data = data; > policy->dvfs_possible_from_any_cpu = true; > > + ret = qcom_cpufreq_hw_clk_add(data, index); > + if (ret) { > + dev_err(dev, "Domain-%d failed to add CPU clock\n", index); > + goto error; > + } > + > ret = qcom_cpufreq_hw_read_lut(cpu_dev, policy); > if (ret) { > dev_err(dev, "Domain-%d failed to read LUT\n", index); > - goto error; > + goto clk_remove; > } > > ret = dev_pm_opp_get_opp_count(cpu_dev); > if (ret <= 0) { > dev_err(cpu_dev, "Failed to add OPPs\n"); > ret = -ENODEV; > - goto error; > + goto clk_remove; > } > - > if (policy_has_boost_freq(policy)) { > ret = cpufreq_enable_boost_support(); > if (ret) > @@ -577,9 +632,12 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy) > > ret = qcom_cpufreq_hw_lmh_init(policy, index); > if (ret) > - goto error; > + goto clk_remove; > > return 0; > + > +clk_remove: > + qcom_cpufreq_hw_clk_remove(data); > error: > kfree(data); > unmap_base: > @@ -599,6 +657,7 @@ static int qcom_cpufreq_hw_cpu_exit(struct cpufreq_policy *policy) > dev_pm_opp_remove_all_dynamic(cpu_dev); > dev_pm_opp_of_cpumask_remove_table(policy->related_cpus); > qcom_cpufreq_hw_lmh_exit(data); > + qcom_cpufreq_hw_clk_remove(data); > kfree(policy->freq_table); > kfree(data); > iounmap(base);
-- With best wishes Dmitry
| |