* [PATCH 1/4] cpufreq: Add kerneldoc comments for two core functions
2019-02-19 23:21 [PATCH 0/4] cpufreq: Assorted cleanups related to cpufreq_set_policy() Rafael J. Wysocki
@ 2019-02-19 23:22 ` Rafael J. Wysocki
2019-02-19 23:24 ` [PATCH 2/4] cpufreq: Reorder and simplify cpufreq_update_policy() Rafael J. Wysocki
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2019-02-19 23:22 UTC (permalink / raw)
To: Linux PM; +Cc: LKML, Viresh Kumar
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Add kerneldoc comments describing cpufreq_set_policy() and
cpufreq_update_policy() as they have not been properly documented
so far and they really need to be documented.
While at it, fix white space around the cpufreq_set_policy() header.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/cpufreq/cpufreq.c | 32 ++++++++++++++++++++++++--------
1 file changed, 24 insertions(+), 8 deletions(-)
Index: linux-pm/drivers/cpufreq/cpufreq.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq.c
+++ linux-pm/drivers/cpufreq/cpufreq.c
@@ -2218,12 +2218,25 @@ int cpufreq_get_policy(struct cpufreq_po
}
EXPORT_SYMBOL(cpufreq_get_policy);
-/*
- * policy : current policy.
- * new_policy: policy to be set.
+/**
+ * cpufreq_set_policy - Modify cpufreq policy parameters.
+ * @policy: Policy object to modify.
+ * @new_policy: New policy data.
+ *
+ * Pass @new_policy to the cpufreq driver's ->verify() callback, run the
+ * installed policy notifiers for it with the CPUFREQ_ADJUST value, pass it to
+ * the driver's ->verify() callback again and run the notifiers for it again
+ * with the CPUFREQ_NOTIFY value. Next, copy the min and max parameters
+ * of @new_policy to @policy and either invoke the driver's ->setpolicy()
+ * callback (if present) or carry out a governor update for @policy. That is,
+ * run the current governor's ->limits() callback (if the governor field in
+ * @new_policy points to the same object as the one in @policy) or replace the
+ * governor for @policy with the new one stored in @new_policy.
+ *
+ * The cpuinfo part of @policy is not updated by this function.
*/
static int cpufreq_set_policy(struct cpufreq_policy *policy,
- struct cpufreq_policy *new_policy)
+ struct cpufreq_policy *new_policy)
{
struct cpufreq_governor *old_gov;
int ret;
@@ -2319,11 +2332,14 @@ static int cpufreq_set_policy(struct cpu
}
/**
- * cpufreq_update_policy - re-evaluate an existing cpufreq policy
- * @cpu: CPU which shall be re-evaluated
+ * cpufreq_update_policy - Re-evaluate an existing cpufreq policy.
+ * @cpu: CPU to re-evaluate the policy for.
*
- * Useful for policy notifiers which have different necessities
- * at different times.
+ * Update the current frequency for the cpufreq policy of @cpu and use
+ * cpufreq_set_policy() to re-apply the min and max limits saved in the
+ * user_policy sub-structure of that policy, which triggers the evaluation
+ * of policy notifiers and the cpufreq driver's ->verify() callback for the
+ * policy in question, among other things.
*/
void cpufreq_update_policy(unsigned int cpu)
{
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 2/4] cpufreq: Reorder and simplify cpufreq_update_policy()
2019-02-19 23:21 [PATCH 0/4] cpufreq: Assorted cleanups related to cpufreq_set_policy() Rafael J. Wysocki
2019-02-19 23:22 ` [PATCH 1/4] cpufreq: Add kerneldoc comments for two core functions Rafael J. Wysocki
@ 2019-02-19 23:24 ` Rafael J. Wysocki
2019-02-19 23:25 ` [PATCH 3/4] cpufreq: Fix two debug messages in cpufreq_set_policy() Rafael J. Wysocki
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2019-02-19 23:24 UTC (permalink / raw)
To: Linux PM; +Cc: LKML, Viresh Kumar
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
In cpufreq_update_policy(), instead of updating new_policy.cur
separately, which is kind of confusing, because cpufreq_set_policy()
doesn't take that value into account directly anyway, make the copy
of the existing policy after calling cpufreq_update_current_freq().
No intentional changes of behavior.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/cpufreq/cpufreq.c | 19 +++++++------------
1 file changed, 7 insertions(+), 12 deletions(-)
Index: linux-pm/drivers/cpufreq/cpufreq.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq.c
+++ linux-pm/drivers/cpufreq/cpufreq.c
@@ -2354,23 +2354,18 @@ void cpufreq_update_policy(unsigned int
if (policy_is_inactive(policy))
goto unlock;
- pr_debug("updating policy for CPU %u\n", cpu);
- memcpy(&new_policy, policy, sizeof(*policy));
- new_policy.min = policy->user_policy.min;
- new_policy.max = policy->user_policy.max;
-
/*
* 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) {
- if (cpufreq_suspended)
- goto unlock;
+ if (cpufreq_driver->get && !cpufreq_driver->setpolicy &&
+ (cpufreq_suspended || WARN_ON(!cpufreq_update_current_freq(policy))))
+ goto unlock;
- new_policy.cur = cpufreq_update_current_freq(policy);
- if (WARN_ON(!new_policy.cur))
- goto unlock;
- }
+ pr_debug("updating policy for CPU %u\n", cpu);
+ memcpy(&new_policy, policy, sizeof(*policy));
+ new_policy.min = policy->user_policy.min;
+ new_policy.max = policy->user_policy.max;
cpufreq_set_policy(policy, &new_policy);
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 3/4] cpufreq: Fix two debug messages in cpufreq_set_policy()
2019-02-19 23:21 [PATCH 0/4] cpufreq: Assorted cleanups related to cpufreq_set_policy() Rafael J. Wysocki
2019-02-19 23:22 ` [PATCH 1/4] cpufreq: Add kerneldoc comments for two core functions Rafael J. Wysocki
2019-02-19 23:24 ` [PATCH 2/4] cpufreq: Reorder and simplify cpufreq_update_policy() Rafael J. Wysocki
@ 2019-02-19 23:25 ` Rafael J. Wysocki
2019-02-19 23:26 ` [PATCH 4/4] cpufreq: Pass updated policy to driver ->setpolicy() callback Rafael J. Wysocki
2019-02-20 5:05 ` [PATCH 0/4] cpufreq: Assorted cleanups related to cpufreq_set_policy() Viresh Kumar
4 siblings, 0 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2019-02-19 23:25 UTC (permalink / raw)
To: Linux PM; +Cc: LKML, Viresh Kumar
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Remove the redundant "cpufreq:" prefix from two debug messages in
cpufreq_set_policy().
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/cpufreq/cpufreq.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Index: linux-pm/drivers/cpufreq/cpufreq.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq.c
+++ linux-pm/drivers/cpufreq/cpufreq.c
@@ -2290,7 +2290,7 @@ static int cpufreq_set_policy(struct cpu
}
if (new_policy->governor == policy->governor) {
- pr_debug("cpufreq: governor limits update\n");
+ pr_debug("governor limits update\n");
cpufreq_governor_limits(policy);
return 0;
}
@@ -2311,7 +2311,7 @@ static int cpufreq_set_policy(struct cpu
if (!ret) {
ret = cpufreq_start_governor(policy);
if (!ret) {
- pr_debug("cpufreq: governor change\n");
+ pr_debug("governor change\n");
sched_cpufreq_governor_change(policy, old_gov);
return 0;
}
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 4/4] cpufreq: Pass updated policy to driver ->setpolicy() callback
2019-02-19 23:21 [PATCH 0/4] cpufreq: Assorted cleanups related to cpufreq_set_policy() Rafael J. Wysocki
` (2 preceding siblings ...)
2019-02-19 23:25 ` [PATCH 3/4] cpufreq: Fix two debug messages in cpufreq_set_policy() Rafael J. Wysocki
@ 2019-02-19 23:26 ` Rafael J. Wysocki
2019-02-20 5:05 ` [PATCH 0/4] cpufreq: Assorted cleanups related to cpufreq_set_policy() Viresh Kumar
4 siblings, 0 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2019-02-19 23:26 UTC (permalink / raw)
To: Linux PM; +Cc: LKML, Viresh Kumar
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
The invocation of the ->setpolicy() cpufreq driver callback should
be equivalent to calling cpufreq_governor_limits(policy) for drivers
with internal governors, but in fact it isn't so, because the
temporary new_policy object is passed to it instead of the updated
policy.
That is a bit confusing, so make cpufreq_set_policy() pass the
updated policy to the driver ->setpolicy() callback.
No intentional changes of behavior.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/cpufreq/cpufreq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux-pm/drivers/cpufreq/cpufreq.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq.c
+++ linux-pm/drivers/cpufreq/cpufreq.c
@@ -2286,7 +2286,7 @@ static int cpufreq_set_policy(struct cpu
if (cpufreq_driver->setpolicy) {
policy->policy = new_policy->policy;
pr_debug("setting range\n");
- return cpufreq_driver->setpolicy(new_policy);
+ return cpufreq_driver->setpolicy(policy);
}
if (new_policy->governor == policy->governor) {
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 0/4] cpufreq: Assorted cleanups related to cpufreq_set_policy()
2019-02-19 23:21 [PATCH 0/4] cpufreq: Assorted cleanups related to cpufreq_set_policy() Rafael J. Wysocki
` (3 preceding siblings ...)
2019-02-19 23:26 ` [PATCH 4/4] cpufreq: Pass updated policy to driver ->setpolicy() callback Rafael J. Wysocki
@ 2019-02-20 5:05 ` Viresh Kumar
4 siblings, 0 replies; 6+ messages in thread
From: Viresh Kumar @ 2019-02-20 5:05 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Linux PM, LKML
On 20-02-19, 00:21, Rafael J. Wysocki wrote:
> Hi,
>
> There are a few things related to cpufreq_set_policy() and cpufreq_update_policy()
> that increase the level of confusion thereof quite unnecessarily, so this series
> attempts to clean them up. Please refer to the patch changelogs for details.
>
> Please let me know if you see any problems with these patches.
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
--
viresh
^ permalink raw reply [flat|nested] 6+ messages in thread