| Message ID | fb44111fa7a762924661ad48a70985d597229bc4.1436351759.git.viresh.kumar@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 09-07-15, 01:41, Rafael J. Wysocki wrote: > On Wednesday, July 08, 2015 04:07:32 PM Viresh Kumar wrote: > > CPUFreq drivers today support a ->get(cpu) callback, which returns > > current rate of a CPU. The problem with ->get() is that it takes a cpu > > number as parameter and this unnecessarily makes things complex. > > > > Firstly the core gets the cpu number by doing operation 'policy->cpu' on > > the policy and then many drivers need to get the policy back and so do > > cpufreq_cpu_get(cpu) on the passed cpu. > > > > As cpufreq core works on policies, it would be better if we pass them > > 'policy' directly and drivers can use policy->cpu if that's all they > > need. > > > > Hence, this patch adds in another callback, ->get_rate() which does > > exactly the same work as ->get(), just that we pass 'policy' as > > parameter instead of 'cpu'. > > > > The plan is to migrate all drivers to this new callback and remove > > ->get() after that. > > > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > > --- > > Hi Rafael, > > > > I hope you are fine with this stuff :), once you approve I will get > > other patches to migrate existing drivers to this interface. > > I'm generally fine with it, but please target it at 4.4 at the earliest. Sure, but I was a bit curious on why 4.4 and not 4.3 ? as we are still at 4.2-rc1 today, and these patches can be done fairly quickly.
On 10 July 2015 at 04:56, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > *You* can do them faily quickly. Say if all of the outstanding cpufreq > patches are queued up for 4.3, then it will be the time to submit more. > Otherwise I'll just have an ever growing queue of patches to process > which won't speed up things at all. Ack. -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10-07-15, 01:26, Rafael J. Wysocki wrote: > *You* can do them faily quickly. Say if all of the outstanding cpufreq > patches are queued up for 4.3, then it will be the time to submit more. > Otherwise I'll just have an ever growing queue of patches to process > which won't speed up things at all. Lots of pending stuff is already committed. Would you like me to send these now? Also, how should I send them? I mean, you want separate patches for drivers? Or all of them in a single patch. This is how stats may look: drivers/cpufreq/acpi-cpufreq.c | 7 ++++--- drivers/cpufreq/arm_big_little.c | 11 ++++++++--- drivers/cpufreq/at32ap-cpufreq.c | 2 +- drivers/cpufreq/blackfin-cpufreq.c | 6 +++--- drivers/cpufreq/cpufreq-dt.c | 2 +- drivers/cpufreq/cpufreq-nforce2.c | 8 ++++---- drivers/cpufreq/cris-artpec3-cpufreq.c | 4 ++-- drivers/cpufreq/cris-etraxfs-cpufreq.c | 4 ++-- drivers/cpufreq/davinci-cpufreq.c | 2 +- drivers/cpufreq/dbx500-cpufreq.c | 2 +- drivers/cpufreq/e_powersaver.c | 5 +++-- drivers/cpufreq/elanfreq.c | 6 +++--- drivers/cpufreq/exynos-cpufreq.c | 2 +- drivers/cpufreq/exynos5440-cpufreq.c | 2 +- drivers/cpufreq/gx-suspmod.c | 6 +++--- drivers/cpufreq/ia64-acpi-cpufreq.c | 5 +++-- drivers/cpufreq/imx6q-cpufreq.c | 2 +- drivers/cpufreq/integrator-cpufreq.c | 5 +++-- drivers/cpufreq/intel_pstate.c | 6 +++--- drivers/cpufreq/kirkwood-cpufreq.c | 5 +++-- drivers/cpufreq/longhaul.c | 6 +++--- drivers/cpufreq/longrun.c | 6 +++--- drivers/cpufreq/loongson2_cpufreq.c | 2 +- drivers/cpufreq/ls1x-cpufreq.c | 2 +- drivers/cpufreq/maple-cpufreq.c | 4 ++-- drivers/cpufreq/omap-cpufreq.c | 2 +- drivers/cpufreq/p4-clockmod.c | 7 +++---- drivers/cpufreq/pcc-cpufreq.c | 5 +++-- drivers/cpufreq/pmac32-cpufreq.c | 4 ++-- drivers/cpufreq/pmac64-cpufreq.c | 4 ++-- drivers/cpufreq/powernow-k6.c | 4 ++-- drivers/cpufreq/powernow-k7.c | 6 +++--- drivers/cpufreq/powernow-k8.c | 5 +++-- drivers/cpufreq/powernv-cpufreq.c | 8 ++++---- drivers/cpufreq/pxa2xx-cpufreq.c | 4 ++-- drivers/cpufreq/pxa3xx-cpufreq.c | 4 ++-- drivers/cpufreq/qoriq-cpufreq.c | 2 +- drivers/cpufreq/s3c2416-cpufreq.c | 6 +++--- drivers/cpufreq/s3c24xx-cpufreq.c | 2 +- drivers/cpufreq/s3c64xx-cpufreq.c | 2 +- drivers/cpufreq/s5pv210-cpufreq.c | 2 +- drivers/cpufreq/sa1100-cpufreq.c | 7 ++++++- drivers/cpufreq/sa1110-cpufreq.c | 7 ++++++- drivers/cpufreq/sc520_freq.c | 4 ++-- drivers/cpufreq/sh-cpufreq.c | 8 ++++---- drivers/cpufreq/sparc-us2e-cpufreq.c | 5 +++-- drivers/cpufreq/sparc-us3-cpufreq.c | 5 +++-- drivers/cpufreq/spear-cpufreq.c | 2 +- drivers/cpufreq/speedstep-centrino.c | 5 +++-- drivers/cpufreq/speedstep-ich.c | 7 ++++--- drivers/cpufreq/speedstep-smi.c | 6 +++--- drivers/cpufreq/tegra-cpufreq.c | 2 +- drivers/cpufreq/unicore2-cpufreq.c | 2 +- 53 files changed, 132 insertions(+), 107 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index b612411655f9..b66e169601e8 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -154,6 +154,20 @@ void disable_cpufreq(void) } static DEFINE_MUTEX(cpufreq_governor_mutex); +/* Operations for ->get() and ->get_rate() operations */ +static inline bool driver_supports_get_rate(void) +{ + return cpufreq_driver->get || cpufreq_driver->get_rate; +} + +static inline unsigned int policy_get_rate(struct cpufreq_policy *policy) +{ + if (cpufreq_driver->get) + return cpufreq_driver->get(policy->cpu); + else + return cpufreq_driver->get_rate(policy); +} + bool have_governor_per_policy(void) { return !!(cpufreq_driver->flags & CPUFREQ_HAVE_GOVERNOR_PER_POLICY); @@ -596,8 +610,9 @@ static ssize_t show_scaling_cur_freq(struct cpufreq_policy *policy, char *buf) { ssize_t ret; - if (cpufreq_driver && cpufreq_driver->setpolicy && cpufreq_driver->get) - ret = sprintf(buf, "%u\n", cpufreq_driver->get(policy->cpu)); + if (cpufreq_driver && cpufreq_driver->setpolicy && + driver_supports_get_rate()) + ret = sprintf(buf, "%u\n", policy_get_rate(policy)); else ret = sprintf(buf, "%u\n", policy->cur); return ret; @@ -1032,7 +1047,7 @@ static int cpufreq_add_dev_interface(struct cpufreq_policy *policy, return ret; drv_attr++; } - if (cpufreq_driver->get) { + if (driver_supports_get_rate()) { ret = sysfs_create_file(&policy->kobj, &cpuinfo_cur_freq.attr); if (ret) return ret; @@ -1313,10 +1328,10 @@ static int cpufreq_add_dev(struct device *dev, struct subsys_interface *sif) write_unlock_irqrestore(&cpufreq_driver_lock, flags); } - if (cpufreq_driver->get && !cpufreq_driver->setpolicy) { - policy->cur = cpufreq_driver->get(policy->cpu); + if (driver_supports_get_rate() && !cpufreq_driver->setpolicy) { + policy->cur = policy_get_rate(policy); if (!policy->cur) { - pr_err("%s: ->get() failed\n", __func__); + pr_err("%s: ->get_rate() failed\n", __func__); goto err_get_freq; } } @@ -1594,14 +1609,17 @@ unsigned int cpufreq_quick_get(unsigned int cpu) struct cpufreq_policy *policy; unsigned int ret_freq = 0; - if (cpufreq_driver && cpufreq_driver->setpolicy && cpufreq_driver->get) - return cpufreq_driver->get(cpu); - policy = cpufreq_cpu_get(cpu); - if (policy) { + if (!policy) + return 0; + + if (cpufreq_driver && cpufreq_driver->setpolicy && + driver_supports_get_rate()) + ret_freq = policy_get_rate(policy); + else ret_freq = policy->cur; - cpufreq_cpu_put(policy); - } + + cpufreq_cpu_put(policy); return ret_freq; } @@ -1631,10 +1649,10 @@ static unsigned int __cpufreq_get(struct cpufreq_policy *policy) { unsigned int ret_freq = 0; - if (!cpufreq_driver->get) + if (!driver_supports_get_rate()) return ret_freq; - ret_freq = cpufreq_driver->get(policy->cpu); + ret_freq = policy_get_rate(policy); /* Updating inactive policies is invalid, so avoid doing that. */ if (unlikely(policy_is_inactive(policy))) @@ -2345,8 +2363,8 @@ int cpufreq_update_policy(unsigned int cpu) * BIOS might change freq behind our back * -> ask driver for current freq and notify governors about a change */ - if (cpufreq_driver->get && !cpufreq_driver->setpolicy) { - new_policy.cur = cpufreq_driver->get(cpu); + if (driver_supports_get_rate() && !cpufreq_driver->setpolicy) { + new_policy.cur = policy_get_rate(policy); if (WARN_ON(!new_policy.cur)) { ret = -EIO; goto unlock; diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index 29ad97c34fd5..a82049683016 100644 --- a/include/linux/cpufreq.h +++ b/include/linux/cpufreq.h @@ -263,6 +263,7 @@ struct cpufreq_driver { /* should be defined, if possible */ unsigned int (*get)(unsigned int cpu); + unsigned int (*get_rate)(struct cpufreq_policy *policy); /* optional */ int (*bios_limit)(int cpu, unsigned int *limit);
CPUFreq drivers today support a ->get(cpu) callback, which returns current rate of a CPU. The problem with ->get() is that it takes a cpu number as parameter and this unnecessarily makes things complex. Firstly the core gets the cpu number by doing operation 'policy->cpu' on the policy and then many drivers need to get the policy back and so do cpufreq_cpu_get(cpu) on the passed cpu. As cpufreq core works on policies, it would be better if we pass them 'policy' directly and drivers can use policy->cpu if that's all they need. Hence, this patch adds in another callback, ->get_rate() which does exactly the same work as ->get(), just that we pass 'policy' as parameter instead of 'cpu'. The plan is to migrate all drivers to this new callback and remove ->get() after that. Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- Hi Rafael, I hope you are fine with this stuff :), once you approve I will get other patches to migrate existing drivers to this interface. drivers/cpufreq/cpufreq.c | 50 ++++++++++++++++++++++++++++++++--------------- include/linux/cpufreq.h | 1 + 2 files changed, 35 insertions(+), 16 deletions(-)