[RFC] cpufreq: Add ->get_rate() driver callback

From: Viresh Kumar
Date: Wed Jul 08 2015 - 06:38:07 EST


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@xxxxxxxxxx>
---
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(-)

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);
--
2.4.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/