* [PATCH] cpufreq: Return 0 from ->fast_switch() on errors
@ 2017-08-09 4:51 Viresh Kumar
2017-08-10 23:13 ` Rafael J. Wysocki
0 siblings, 1 reply; 2+ messages in thread
From: Viresh Kumar @ 2017-08-09 4:51 UTC (permalink / raw)
To: Rafael Wysocki, Ingo Molnar, Peter Zijlstra
Cc: Viresh Kumar, linux-pm, Vincent Guittot, sudeep.holla,
Dietmar.Eggemann, linux-kernel
CPUFREQ_ENTRY_INVALID is a special symbol which is used to specify that
an entry in the cpufreq table is invalid. But using it outside of the
scope of the cpufreq table looks a bit incorrect.
We can represent an invalid frequency by writing it as 0 instead if we
need. Note that it is already done that way for the return value of the
->get() callback.
Lets do the same for ->fast_switch() and not use CPUFREQ_ENTRY_INVALID
outside of the scope of cpufreq table.
Also update the comment over cpufreq_driver_fast_switch() to clearly
mention what this returns.
None of the drivers return CPUFREQ_ENTRY_INVALID as of now from
->fast_switch() callback and so we don't need to update any of those.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/cpufreq.c | 7 ++++---
kernel/sched/cpufreq_schedutil.c | 2 +-
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index b2cc98551fc3..9588e15da26b 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1832,9 +1832,10 @@ EXPORT_SYMBOL(cpufreq_unregister_notifier);
* twice in parallel for the same policy and that it will never be called in
* parallel with either ->target() or ->target_index() for the same policy.
*
- * If CPUFREQ_ENTRY_INVALID is returned by the driver's ->fast_switch()
- * callback to indicate an error condition, the hardware configuration must be
- * preserved.
+ * Returns the actual frequency set for the CPU.
+ *
+ * If 0 is returned by the driver's ->fast_switch() callback to indicate an
+ * error condition, the hardware configuration must be preserved.
*/
unsigned int cpufreq_driver_fast_switch(struct cpufreq_policy *policy,
unsigned int target_freq)
diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
index 5465bf221e8f..2e74c49776be 100644
--- a/kernel/sched/cpufreq_schedutil.c
+++ b/kernel/sched/cpufreq_schedutil.c
@@ -123,7 +123,7 @@ static void sugov_update_commit(struct sugov_policy *sg_policy, u64 time,
if (policy->fast_switch_enabled) {
next_freq = cpufreq_driver_fast_switch(policy, next_freq);
- if (next_freq == CPUFREQ_ENTRY_INVALID)
+ if (!next_freq)
return;
policy->cur = next_freq;
--
2.13.0.71.gd7076ec9c9cb
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] cpufreq: Return 0 from ->fast_switch() on errors
2017-08-09 4:51 [PATCH] cpufreq: Return 0 from ->fast_switch() on errors Viresh Kumar
@ 2017-08-10 23:13 ` Rafael J. Wysocki
0 siblings, 0 replies; 2+ messages in thread
From: Rafael J. Wysocki @ 2017-08-10 23:13 UTC (permalink / raw)
To: Viresh Kumar
Cc: Ingo Molnar, Peter Zijlstra, linux-pm, Vincent Guittot,
sudeep.holla, Dietmar.Eggemann, linux-kernel
On Wednesday, August 9, 2017 6:51:46 AM CEST Viresh Kumar wrote:
> CPUFREQ_ENTRY_INVALID is a special symbol which is used to specify that
> an entry in the cpufreq table is invalid. But using it outside of the
> scope of the cpufreq table looks a bit incorrect.
>
> We can represent an invalid frequency by writing it as 0 instead if we
> need. Note that it is already done that way for the return value of the
> ->get() callback.
>
> Lets do the same for ->fast_switch() and not use CPUFREQ_ENTRY_INVALID
> outside of the scope of cpufreq table.
>
> Also update the comment over cpufreq_driver_fast_switch() to clearly
> mention what this returns.
>
> None of the drivers return CPUFREQ_ENTRY_INVALID as of now from
> ->fast_switch() callback and so we don't need to update any of those.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> drivers/cpufreq/cpufreq.c | 7 ++++---
> kernel/sched/cpufreq_schedutil.c | 2 +-
> 2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index b2cc98551fc3..9588e15da26b 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1832,9 +1832,10 @@ EXPORT_SYMBOL(cpufreq_unregister_notifier);
> * twice in parallel for the same policy and that it will never be called in
> * parallel with either ->target() or ->target_index() for the same policy.
> *
> - * If CPUFREQ_ENTRY_INVALID is returned by the driver's ->fast_switch()
> - * callback to indicate an error condition, the hardware configuration must be
> - * preserved.
> + * Returns the actual frequency set for the CPU.
> + *
> + * If 0 is returned by the driver's ->fast_switch() callback to indicate an
> + * error condition, the hardware configuration must be preserved.
> */
> unsigned int cpufreq_driver_fast_switch(struct cpufreq_policy *policy,
> unsigned int target_freq)
> diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
> index 5465bf221e8f..2e74c49776be 100644
> --- a/kernel/sched/cpufreq_schedutil.c
> +++ b/kernel/sched/cpufreq_schedutil.c
> @@ -123,7 +123,7 @@ static void sugov_update_commit(struct sugov_policy *sg_policy, u64 time,
>
> if (policy->fast_switch_enabled) {
> next_freq = cpufreq_driver_fast_switch(policy, next_freq);
> - if (next_freq == CPUFREQ_ENTRY_INVALID)
> + if (!next_freq)
> return;
>
> policy->cur = next_freq;
>
Applied, thanks!
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-08-10 23:21 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-09 4:51 [PATCH] cpufreq: Return 0 from ->fast_switch() on errors Viresh Kumar
2017-08-10 23:13 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).